Bug 204255 - Block layout invalidation logic triggers excessive layout on height percentage descendants
Summary: Block layout invalidation logic triggers excessive layout on height percentag...
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: zalan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-11-15 15:23 PST by zalan
Modified: 2019-11-18 13:30 PST (History)
9 users (show)

See Also:


Attachments
Patch (5.98 KB, patch)
2019-11-18 10:47 PST, zalan
no flags Details | Formatted Diff | Diff
Patch (6.05 KB, patch)
2019-11-18 11:32 PST, zalan
no flags Details | Formatted Diff | Diff
Patch (6.05 KB, patch)
2019-11-18 12:57 PST, zalan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description zalan 2019-11-15 15:23:13 PST
rdar://problem/57236490
Comment 1 zalan 2019-11-18 10:47:39 PST
Created attachment 383762 [details]
Patch
Comment 2 Simon Fraser (smfr) 2019-11-18 11:09:15 PST
Comment on attachment 383762 [details]
Patch

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

> Source/WebCore/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=204255

Radar

> Source/WebCore/ChangeLog:14
> +        This patch attempts to optimize the height percentage invalidation logic by skipping boxes with out-of-flow ancestors.
> +        The normal box invalidation logic works on parent-child level (a.k.a when the parent is dirty it walks its direct children list and marks them dirty if needed).
> +        However percent height values require containing block-child type of invalidation which atm we manage using a "percentHeightDescendantsMap".
> +        percentHeightDescendantsMap contains all the height percentage descendants and as part of the box invalidation the block container always marks all of these special descendants dirty.
> +        This leads to excessive and unnecessary layouts in certain cases when the "walk the ancestor chain and mark boxes dirty all the way to the RenderView" code ends up descending back
> +        to these height percentage boxes.
> +        We could optimize it slightly by checking if there's a height percentage containing block in the ancestor chain. In such cases, we know that the descendant's height used value computation will eventually stop at this out-of-flow containing block.

Tidy this up a bit!

> Source/WebCore/rendering/RenderBlock.cpp:825
> +        auto descendantNeedsLayout = true;

Nooooo

> LayoutTests/fast/block/height-percentage-descendants-with-absolute-pos-containingblock.html:11
> +    <img style="background-color: black; filter: blur(20px); height: 100%;" src="broken.jpg">

Don't need the blur.
Comment 3 zalan 2019-11-18 11:32:22 PST
Created attachment 383766 [details]
Patch
Comment 4 WebKit Commit Bot 2019-11-18 12:55:33 PST
Comment on attachment 383766 [details]
Patch

Rejecting attachment 383766 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 383766, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in LayoutTests/ChangeLog contains OOPS!.

Full output: https://webkit-queues.webkit.org/results/13264835
Comment 5 zalan 2019-11-18 12:57:10 PST
Created attachment 383779 [details]
Patch
Comment 6 WebKit Commit Bot 2019-11-18 13:30:14 PST
Comment on attachment 383779 [details]
Patch

Clearing flags on attachment: 383779

Committed r252562: <https://trac.webkit.org/changeset/252562>
Comment 7 WebKit Commit Bot 2019-11-18 13:30:16 PST
All reviewed patches have been landed.  Closing bug.