[Webkit-unassigned] [Bug 235420] Position:fixed layers shouldn't allocate a backing buffer if all children are offscreen

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=235420

Simon Fraser (smfr) <simon.fraser at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #449611|1                           |0
        is obsolete|                            |

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

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

> Source/WebCore/rendering/RenderLayerBacking.cpp:2793
> +static bool intersectsWithAncestor(const RenderLayer& ancestor, const LayoutRect& ancestorCompositedBounds, const RenderLayer& child)

I would put child as the first argument.

> Source/WebCore/rendering/RenderLayerBacking.cpp:2798
> +            return true;

It would be more truthful to have the function return std::optional<bool> and return nullopt here.

> Source/WebCore/rendering/RenderLayerBacking.cpp:2802
> +    LayoutPoint delta = child.convertToLayerCoords(&ancestor, LayoutPoint(), RenderLayer::AdjustForColumns);

auto delta. Maybe also "{ }" instead of LayoutPoint().

It's unfortunate that this function is called inside a descendant layer tree walk, then your loop above, and child.convertToLayerCoords() both do ancestor tree walks, which can be a perf issue (we've seen this in the past). Maybe traverseVisibleNonCompositedDescendantLayers() can accumulate the "offset from ancestor" as it's doing the descendant traversal.

Probably fine in this case (we don't expect a lot of ancestor walking) but generally to be avoided.

> Source/WebCore/rendering/RenderLayerBacking.cpp:2803
> +    LayoutRect overlap = child.overlapBounds();

auto

> Source/WebCore/rendering/RenderLayerBacking.cpp:2804
> +    overlap.move(delta.x(), delta.y());

overlap.moveBy(delta);

> LayoutTests/compositing/backing/no-backing-for-offscreen-children-of-position-fixed.html:19
> +  <div style="position: fixed;z-index: -1;left: 0;top: 0;right: 0;bottom: 0;opacity: 0.5;">

I don't think the z-index: -1 is relevant. Also this layer doesn't need to be full height or width (and making it so makes it harder to identify in the layer output). Also the opacity is not required.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20220121/00ad3a4b/attachment.htm>