layout: Preserve cached intrinsic inline sizes in more cases#41160
layout: Preserve cached intrinsic inline sizes in more cases#41160Loirooriol merged 1 commit intoservo:mainfrom
Conversation
|
🔨 Triggering try run (#20070885797) for Linux (WPT) |
|
Test results for linux-wpt from try job (#20070885797): Flaky unexpected result (33)
Stable unexpected results that are known to be intermittent (18)
|
|
✨ Try run (#20070885797) succeeded. |
fc1aa02 to
3d3475b
Compare
|
Example: <!DOCTYPE html>
<style>:root { width: max-content; border: solid; }</style>
<section>
<main style="width: 100px; background: cyan">
<article>
<div id="target" style="height: 100px"></div>
</article>
</main>
</section>
<script>requestAnimationFrame(() => target.style.width = "200px");</script>We will no longer clear the cached intrinsic size for |
3d3475b to
0c6fc1b
Compare
mrobinson
left a comment
There was a problem hiding this comment.
Looks good, but the new logic is pretty hairy so I suggested some comments. Feel free to make them clearer / more correct as you like.
| let outer_inline_content_sizes_depend_on_content = Cell::new(false); | ||
| node.with_each_layout_box_base_including_pseudos(|base| { | ||
| base.clear_fragments(); | ||
| if original_element_damage.contains(RestyleDamage::RELAYOUT) { |
There was a problem hiding this comment.
| if original_element_damage.contains(RestyleDamage::RELAYOUT) { | |
| // If the node itself has damage, we must clear both cached inline sizes and also | |
| // any cached layout results. | |
| if original_element_damage.contains(RestyleDamage::RELAYOUT) { |
| *base.cached_layout_result.borrow_mut() = None; | ||
| *base.cached_inline_content_size.borrow_mut() = None; | ||
| } else if damage_from_children.contains(RestyleDamage::RELAYOUT) { | ||
| *base.cached_layout_result.borrow_mut() = None; |
There was a problem hiding this comment.
| *base.cached_layout_result.borrow_mut() = None; | |
| // If any children had layout damage, we never reuse the cached layout result | |
| // as the descendants will need to change. | |
| *base.cached_layout_result.borrow_mut() = None; |
| *base.cached_inline_content_size.borrow_mut() = None; | ||
| } else if damage_from_children.contains(RestyleDamage::RELAYOUT) { | ||
| *base.cached_layout_result.borrow_mut() = None; | ||
| if !damage_from_children.contains(LayoutDamage::recompute_inline_content_sizes()) { |
There was a problem hiding this comment.
| if !damage_from_children.contains(LayoutDamage::recompute_inline_content_sizes()) { | |
| // This can happen when a descendent doesn't have layout damage and doesn't depend | |
| // on content size -- for instance when it is has a fixed inline size. | |
| if !damage_from_children.contains(LayoutDamage::recompute_inline_content_sizes()) { |
| } | ||
| *base.cached_inline_content_size.borrow_mut() = None; | ||
| } | ||
| if !base.base_fragment_info.is_anonymous() { |
There was a problem hiding this comment.
This could probably also use an explanatory comment.
| if outer_inline_content_sizes_depend_on_content.get() { | ||
| damage_for_parent.insert(LayoutDamage::recompute_inline_content_sizes()) | ||
| } |
There was a problem hiding this comment.
| if outer_inline_content_sizes_depend_on_content.get() { | |
| damage_for_parent.insert(LayoutDamage::recompute_inline_content_sizes()) | |
| } | |
| // Only clear the cached inline content sizes on ancestors, when this node's inline content | |
| // sizes actually depend on content, otherwise the ancestors don't on the descendant's | |
| // content either. | |
| if outer_inline_content_sizes_depend_on_content.get() { | |
| damage_for_parent.insert(LayoutDamage::recompute_inline_content_sizes()) | |
| } |
| /// Recollect the box children for this element, because some of the them will be | ||
| /// rebuilt. | ||
| const RECOLLECT_BOX_TREE_CHILDREN = 0b0111_1111_1111 << 4; | ||
| /// Clear the cached inline content sizes. |
There was a problem hiding this comment.
| /// Clear the cached inline content sizes. | |
| /// Clear the cached inline content sizes and recompute them during the next layout. |
If a box has layout damage, but the intrinsic contribution of some ancestor doesn't depend on its contents, then we don't need to clear the cached intrinsic sizes of further ancestors. Signed-off-by: Oriol Brufau <[email protected]> Co-authored-by: Martin Robinson <[email protected]>
0c6fc1b to
4300a5d
Compare
If a box has layout damage, but the intrinsic contribution of some ancestor doesn't depend on its contents, then we don't need to clear the cached intrinsic sizes of further ancestors.
Testing: No behavior change, just an optimization. It would be good to have unit tests that check we don't do unnecessary work, but leaving this for the future.