[webkit-reviews] review denied: [Bug 217596] [GPU Process][Resource caching 6/7]: Cache the BitmapImage frame in GPU Process and allow referencing it with its RemoteResourceIdentifier : [Attachment 412586] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 217596: [GPU Process][Resource caching 6/7]: Cache the BitmapImage frame in
GPU Process and allow referencing it with its RemoteResourceIdentifier
https://bugs.webkit.org/show_bug.cgi?id=217596

Attachment 412586: Patch

https://bugs.webkit.org/attachment.cgi?id=412586&action=review




--- Comment #9 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 412586
  --> https://bugs.webkit.org/attachment.cgi?id=412586
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=412586&action=review

> Source/WebCore/platform/graphics/BitmapImage.cpp:214
> +static inline void drawNativeImage(BitmapImage& image, GraphicsContext&
context, const NativeImagePtr& nativeImage, const FloatRect& destRect, const
FloatRect& srcRect, const IntSize& srcSize, const ImagePaintingOptions&
options)

It's really confusing for this to take both a BitmapImage and a NativeImage.
Can the latter be derived from the former?

> Source/WebCore/platform/graphics/GraphicsContext.cpp:734
> +void GraphicsContext::drawNativeImage(const NativeImagePtr& image, const
FloatSize& imageSize, const FloatRect& destRect, const FloatRect& srcRect,
const ImagePaintingOptions& options, CachedImage* cachedImage)

This is a layering violation. You can't use CachedImage in platform code.

> Source/WebCore/platform/graphics/displaylists/DisplayListItems.h:1736
> +    WEBCORE_EXPORT DrawNativeImage(const NativeImageType&, const FloatSize&
imageSize, const FloatRect& destRect, const FloatRect& srcRect, const
ImagePaintingOptions&);

I don't think it makes sense to encode draw native image. If we're drawing an
image with GPUP, can't it always be represented as an ImageBuffer?

> Source/WebCore/platform/graphics/displaylists/DisplayListRecorder.cpp:190
> +void Recorder::drawNativeImage(const NativeImagePtr& image, const FloatSize&
imageSize, const FloatRect& destRect, const FloatRect& srcRect, const
ImagePaintingOptions& options, CachedImage* cachedImage)

layering violation.