Bug 231832 - Make sure child layers of top layer elements are rendered and correctly z-ordered (top-layer-stacking.html fails)
Summary: Make sure child layers of top layer elements are rendered and correctly z-ord...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks: dialog-element
  Show dependency treegraph
 
Reported: 2021-10-15 14:21 PDT by Tim Nguyen (:ntim)
Modified: 2021-10-16 03:35 PDT (History)
12 users (show)

See Also:


Attachments
Patch (6.57 KB, patch)
2021-10-15 17:20 PDT, Tim Nguyen (:ntim)
no flags Details | Formatted Diff | Diff
Patch (5.62 KB, patch)
2021-10-15 18:35 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Nguyen (:ntim) 2021-10-15 14:21:35 PDT
I suspect something is wrong in RenderLayer::rebuildZOrderLists
Comment 1 Radar WebKit Bug Importer 2021-10-15 14:21:44 PDT
<rdar://problem/84316561>
Comment 2 Tim Nguyen (:ntim) 2021-10-15 17:20:03 PDT
Created attachment 441461 [details]
Patch
Comment 3 Simon Fraser (smfr) 2021-10-15 17:30:10 PDT
Comment on attachment 441461 [details]
Patch

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

I think the real bug here is that RenderLayer::collectLayers() just early returns when establishesTopLayer() is true, so top layer layers never get any children.

> Source/WebCore/rendering/RenderLayer.cpp:768
> +    collectLayersForChildren(includeHiddenLayers, false, posZOrderList, negZOrderList, accumulatedDirtyFlags);

This is called the "boolean trap": a true/false argument that is hard to understand here at the call site. We generally avoid this by using enums.

> Source/WebCore/rendering/RenderLayer.cpp:803
> +                auto startOfSorting = posZOrderList->end();
> +                layer->collectLayersForChildren(includeHiddenLayers, true, posZOrderList, negZOrderList, accumulatedDirtyFlags);
> +                std::stable_sort(startOfSorting, posZOrderList->end(), compareZIndex);

I don't think this is right. This will append all descendant layers to 'posZOrderList' but what needs to happen is that 'layer' here, which is a stacking context, builds its own positive and negative z-order lists.

> Source/WebCore/rendering/RenderLayer.cpp:827
> +        auto& layerList = (zIndex() >= 0 || hasTopLayerAncestor) ? positiveZOrderList : negativeZOrderList;

Here all descendants of top layer layers append to the positiveZOrderList but this is wrong.
Comment 4 Simon Fraser (smfr) 2021-10-15 18:35:06 PDT
Created attachment 441466 [details]
Patch
Comment 5 EWS 2021-10-16 03:35:27 PDT
Committed r284314 (243110@main): <https://commits.webkit.org/243110@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 441466 [details].