[webkit-reviews] review denied: [Bug 93174] RenderLayerBacking must call GraphicsLayer::setContentsToImage when the image is changed. : [Attachment 156509] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied Huang Dongsung
<luxtella at company100.net>'s request for review:
Bug 93174: RenderLayerBacking must call GraphicsLayer::setContentsToImage when
the image is changed.
https://bugs.webkit.org/show_bug.cgi?id=93174

Attachment 156509: Patch
https://bugs.webkit.org/attachment.cgi?id=156509&action=review

------- Additional Comments from Simon Fraser (smfr) <simon.fraser at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=156509&action=review


> Source/WebCore/ChangeLog:13
> +	   Currently, layout indirectly call
RenderLayerBacking::updateGraphicsLayerConfiguration()
> +	   and updateGraphicsLayerConfiguration() indirectly call
GraphicsLayer::setContentsToImage().
> +	   It burdens all accelerated compositing implementations because each
implementation
> +	   should check if the image is changed.

This comment is confusing. updateGraphicsLayerConfiguration() isn't the place
where animated GIFs get updated on compositing layers; that happens via
RenderBoxModelObject::contentChanged() which ends up in
RenderLayerBacking::contentChanged().

> Source/WebCore/rendering/RenderLayerBacking.cpp:1089
> +    NativeImagePtr newNativeImagePtr = image->nativeImageForCurrentFrame();
> +    if (newNativeImagePtr != m_compositedNativeImagePtr) {
> +	   m_compositedNativeImagePtr = newNativeImagePtr;
> +
> +	   // This is a no-op if the layer doesn't have an inner layer for the
image.
> +	   m_graphicsLayer->setContentsToImage(image);

It seems odd to inspect the NativeImagePtr here, but still send the Image down
to GraphicsLayer.

This code also makes the assumption that pointer equality on a NativeImagePtr
is a valid way to tell if the image is unchanged. That may not hold for all
platforms. I think the platform code should continue to make the decision about
whether the image has changed.