[webkit-reviews] review denied: [Bug 230885] [Performance] Optimize RenderLayer::clipCrossesPaintingBoundary : [Attachment 447935] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has denied cathiechen
<cathiechen at igalia.com>'s request for review:
Bug 230885: [Performance] Optimize RenderLayer::clipCrossesPaintingBoundary
https://bugs.webkit.org/show_bug.cgi?id=230885

Attachment 447935: Patch

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




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

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

> Source/WebCore/rendering/RenderLayer.cpp:1971
> +    if (compositedWithOwnBackingStore(*layer))
> +	   return false;
> +
> +    if (!layer->paintsIntoProvidedBacking()) {

The way this code uses `layer` makes it look like it's going to loop, but it
doesn't. It would be clearer to use `this` in place of `layer`.

> Source/WebCore/rendering/RenderLayer.cpp:1975
> +	   if (layer == parentLayer)

How is this ever possible?

> Source/WebCore/rendering/RenderLayer.cpp:1979
> +	   if (compositedWithOwnBackingStore(*parentLayer))
> +	       return false;

But this layer can paint into parentLayer's backing, right? This will be pretty
common.