From b317620486952494a3883129e936f1bb25d29bdc Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Thu, 18 Jan 2024 14:03:30 +0100 Subject: [PATCH] LibWeb: Resolve relpos fragment offsets only for inline paintables Prior to this change, we iterated through all fragments within each PaintableWithLines to resolve the relative position offset. This happened before transferring the fragments to their corresponding inline paintables. With this change, we significantly reduce amount of work by attempting to resolve relative position offsets only for those contained within inline paintables. Performance improvement on https://html.spec.whatwg.org/ --- .../Libraries/LibWeb/Layout/LayoutState.cpp | 80 +++++++++++-------- .../Libraries/LibWeb/Layout/LayoutState.h | 2 +- 2 files changed, 46 insertions(+), 36 deletions(-) diff --git a/Userland/Libraries/LibWeb/Layout/LayoutState.cpp b/Userland/Libraries/LibWeb/Layout/LayoutState.cpp index 868e56978f6..af231dab73b 100644 --- a/Userland/Libraries/LibWeb/Layout/LayoutState.cpp +++ b/Userland/Libraries/LibWeb/Layout/LayoutState.cpp @@ -139,48 +139,22 @@ static CSSPixelRect measure_scrollable_overflow(Box const& box) return scrollable_overflow_rect; } -void LayoutState::resolve_relative_positions(Vector const& paintables_with_lines) +void LayoutState::resolve_relative_positions() { - // This function resolves relative position offsets of all the boxes & fragments in the paint tree. + // This function resolves relative position offsets of fragments that belong to inline paintables. // It runs *after* the paint tree has been constructed, so it modifies paintable node & fragment offsets directly. - - // Regular boxes (not line box fragments): for (auto& it : used_values_per_layout_node) { auto& used_values = *it.value; auto& node = const_cast(used_values.node()); - if (!node.is_box()) + auto* paintable = node.paintable(); + if (!paintable) + continue; + if (!is(*paintable)) continue; - auto& paintable = static_cast(*node.paintable()); - CSSPixelPoint offset; - - if (used_values.containing_line_box_fragment.has_value()) { - // Atomic inline case: - // We know that `node` is an atomic inline because `containing_line_box_fragments` refers to the - // line box fragment in the parent block container that contains it. - auto const& containing_line_box_fragment = used_values.containing_line_box_fragment.value(); - auto const& containing_block = *node.containing_block(); - auto const& containing_block_used_values = get(containing_block); - auto const& fragment = containing_block_used_values.line_boxes[containing_line_box_fragment.line_box_index].fragments()[containing_line_box_fragment.fragment_index]; - - // The fragment has the final offset for the atomic inline, so we just need to copy it from there. - offset = fragment.offset(); - } else { - // Not an atomic inline, much simpler case. - offset = used_values.offset; - } - // Apply relative position inset if appropriate. - if (node.computed_values().position() == CSS::Positioning::Relative && is(node)) { - auto& inset = static_cast(node).box_model().inset; - offset.translate_by(inset.left, inset.top); - } - paintable.set_offset(offset); - } - - // Line box fragments: - for (auto const& paintable_with_lines : paintables_with_lines) { - for (auto& fragment : paintable_with_lines.fragments()) { + auto const& inline_paintable = static_cast(*paintable); + for (auto& fragment : inline_paintable.fragments()) { auto const& fragment_node = fragment.layout_node(); if (!is(*fragment_node.parent())) continue; @@ -489,7 +463,41 @@ void LayoutState::commit(Box& root) } } - resolve_relative_positions(paintables_with_lines); + // Resolve relative positions for regular boxes (not line box fragments): + // NOTE: This needs to occur before fragments are transferred into the corresponding inline paintables, because + // after this transfer, the containing_line_box_fragment will no longer be valid. + for (auto& it : used_values_per_layout_node) { + auto& used_values = *it.value; + auto& node = const_cast(used_values.node()); + + if (!node.is_box()) + continue; + + auto& paintable = static_cast(*node.paintable()); + CSSPixelPoint offset; + + if (used_values.containing_line_box_fragment.has_value()) { + // Atomic inline case: + // We know that `node` is an atomic inline because `containing_line_box_fragments` refers to the + // line box fragment in the parent block container that contains it. + auto const& containing_line_box_fragment = used_values.containing_line_box_fragment.value(); + auto const& containing_block = *node.containing_block(); + auto const& containing_block_used_values = get(containing_block); + auto const& fragment = containing_block_used_values.line_boxes[containing_line_box_fragment.line_box_index].fragments()[containing_line_box_fragment.fragment_index]; + + // The fragment has the final offset for the atomic inline, so we just need to copy it from there. + offset = fragment.offset(); + } else { + // Not an atomic inline, much simpler case. + offset = used_values.offset; + } + // Apply relative position inset if appropriate. + if (node.computed_values().position() == CSS::Positioning::Relative && is(node)) { + auto const& inset = static_cast(node).box_model().inset; + offset.translate_by(inset.left, inset.top); + } + paintable.set_offset(offset); + } // Make a pass over all the line boxes to: // - Collect all text nodes, so we can create paintables for them later. @@ -522,6 +530,8 @@ void LayoutState::commit(Box& root) build_paint_tree(root); + resolve_relative_positions(); + // Measure overflow in scroll containers. for (auto& it : used_values_per_layout_node) { auto& used_values = *it.value; diff --git a/Userland/Libraries/LibWeb/Layout/LayoutState.h b/Userland/Libraries/LibWeb/Layout/LayoutState.h index 7c8454d0614..074f2a4cb2f 100644 --- a/Userland/Libraries/LibWeb/Layout/LayoutState.h +++ b/Userland/Libraries/LibWeb/Layout/LayoutState.h @@ -188,7 +188,7 @@ struct LayoutState { LayoutState const& m_root; private: - void resolve_relative_positions(Vector const&); + void resolve_relative_positions(); void resolve_border_radii(); void resolve_box_shadow_data(); void resolve_text_shadows(Vector const& paintables_with_lines);