From 195ef5e26fc5c38b6a37ec8d0b28ee6fad142fab Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 24 Mar 2022 22:45:51 +0100 Subject: [PATCH] LibWeb: Bring CSS line-height implementation closer to spec We now distribute the line-height evenly between the space above and below inline-level boxes. This noticeably improves our baseline alignment in many cases. Note that the "vertical-align: " case is quite awkward, as the extra height added by the offset baseline must count towards the line box height. There's a lot of room for improvement here, but this makes the buckets container on Acid3 show up in the right place, with the right size. --- Userland/Libraries/LibWeb/Dump.cpp | 3 +- Userland/Libraries/LibWeb/Layout/LineBox.h | 2 + .../LibWeb/Layout/LineBoxFragment.cpp | 45 +++++++++++++++++++ .../Libraries/LibWeb/Layout/LineBoxFragment.h | 4 ++ .../Libraries/LibWeb/Layout/LineBuilder.cpp | 33 ++++++++++---- Userland/Services/WebContent/main.cpp | 1 + 6 files changed, 79 insertions(+), 9 deletions(-) diff --git a/Userland/Libraries/LibWeb/Dump.cpp b/Userland/Libraries/LibWeb/Dump.cpp index e65226e467..ceb342e912 100644 --- a/Userland/Libraries/LibWeb/Dump.cpp +++ b/Userland/Libraries/LibWeb/Dump.cpp @@ -232,11 +232,12 @@ void dump_tree(StringBuilder& builder, Layout::Node const& layout_node, bool sho auto& line_box = block.paint_box()->line_boxes()[line_box_index]; for (size_t i = 0; i < indent; ++i) builder.append(" "); - builder.appendff(" {}line {}{} width: {}, bottom: {}, baseline: {}\n", + builder.appendff(" {}line {}{} width: {}, height: {}, bottom: {}, baseline: {}\n", line_box_color_on, line_box_index, color_off, line_box.width(), + line_box.height(), line_box.bottom(), line_box.baseline()); for (size_t fragment_index = 0; fragment_index < line_box.fragments().size(); ++fragment_index) { diff --git a/Userland/Libraries/LibWeb/Layout/LineBox.h b/Userland/Libraries/LibWeb/Layout/LineBox.h index 2f92e8e514..50986488da 100644 --- a/Userland/Libraries/LibWeb/Layout/LineBox.h +++ b/Userland/Libraries/LibWeb/Layout/LineBox.h @@ -16,6 +16,7 @@ public: LineBox() = default; float width() const { return m_width; } + float height() const { return m_height; } float bottom() const { return m_bottom; } float baseline() const { return m_baseline; } @@ -36,6 +37,7 @@ private: Vector m_fragments; float m_width { 0 }; + float m_height { 0 }; float m_bottom { 0 }; float m_baseline { 0 }; }; diff --git a/Userland/Libraries/LibWeb/Layout/LineBoxFragment.cpp b/Userland/Libraries/LibWeb/Layout/LineBoxFragment.cpp index 795c4dad41..fba9839902 100644 --- a/Userland/Libraries/LibWeb/Layout/LineBoxFragment.cpp +++ b/Userland/Libraries/LibWeb/Layout/LineBoxFragment.cpp @@ -139,4 +139,49 @@ Gfx::FloatRect LineBoxFragment::selection_rect(const Gfx::Font& font) const return {}; } +float LineBoxFragment::height_of_inline_level_box(FormattingState const& state) const +{ + auto height = [&] { + // From "10.8 Line height calculations: the 'line-height' and 'vertical-align' properties" + // https://www.w3.org/TR/CSS22/visudet.html#line-height + + // For replaced elements, inline-block elements, and inline-table elements, this is the height of their margin box. + // FIXME: Support inline-table elements. + if (layout_node().is_replaced_box() || layout_node().is_inline_block()) { + auto const& fragment_box_state = state.get(static_cast(layout_node())); + return fragment_box_state.margin_box_height(); + } + // For inline boxes, this is their 'line-height'. + return layout_node().line_height(); + }(); + if (auto length_percentage = layout_node().computed_values().vertical_align().get_pointer(); length_percentage && length_percentage->is_length()) + height += length_percentage->length().to_px(layout_node()); + return height; +} + +float LineBoxFragment::top_of_inline_level_box(FormattingState const& state) const +{ + // FIXME: Support inline-table elements. + if (layout_node().is_replaced_box() || layout_node().is_inline_block()) { + auto const& fragment_box_state = state.get(static_cast(layout_node())); + return m_offset.y() - fragment_box_state.margin_box_top(); + } + return m_offset.y() - (layout_node().line_height() - layout_node().computed_values().font_size()) / 2; +} + +float LineBoxFragment::bottom_of_inline_level_box(FormattingState const& state) const +{ + auto bottom = [&] { + // FIXME: Support inline-table elements. + if (layout_node().is_replaced_box() || layout_node().is_inline_block()) { + auto const& fragment_box_state = state.get(static_cast(layout_node())); + return m_offset.y() + fragment_box_state.content_height + fragment_box_state.margin_box_bottom(); + } + return m_offset.y() + (layout_node().line_height() - layout_node().computed_values().font_size()) / 2; + }(); + if (auto length_percentage = layout_node().computed_values().vertical_align().get_pointer(); length_percentage && length_percentage->is_length()) + bottom += length_percentage->length().to_px(layout_node()); + return bottom; +} + } diff --git a/Userland/Libraries/LibWeb/Layout/LineBoxFragment.h b/Userland/Libraries/LibWeb/Layout/LineBoxFragment.h index ee7eb04622..0a5fe51610 100644 --- a/Userland/Libraries/LibWeb/Layout/LineBoxFragment.h +++ b/Userland/Libraries/LibWeb/Layout/LineBoxFragment.h @@ -63,6 +63,10 @@ public: Gfx::FloatRect selection_rect(const Gfx::Font&) const; + float height_of_inline_level_box(FormattingState const&) const; + float top_of_inline_level_box(FormattingState const&) const; + float bottom_of_inline_level_box(FormattingState const&) const; + private: Node const& m_layout_node; int m_start { 0 }; diff --git a/Userland/Libraries/LibWeb/Layout/LineBuilder.cpp b/Userland/Libraries/LibWeb/Layout/LineBuilder.cpp index fcb82cbf09..b55e0b10d7 100644 --- a/Userland/Libraries/LibWeb/Layout/LineBuilder.cpp +++ b/Userland/Libraries/LibWeb/Layout/LineBuilder.cpp @@ -119,7 +119,6 @@ void LineBuilder::update_last_line() auto text_align = m_context.containing_block().computed_values().text_align(); float x_offset = m_context.leftmost_x_offset_at(m_current_y); - float bottom = m_current_y + m_context.containing_block().line_height(); float excess_horizontal_space = m_containing_block_state.content_width - line_box.width(); switch (text_align) { @@ -137,25 +136,39 @@ void LineBuilder::update_last_line() } auto fragment_baseline = [&](auto const& fragment) -> float { - if (fragment.layout_node().is_text_node()) - return fragment.layout_node().font().baseline(); - auto const& box = verify_cast(fragment.layout_node()); - return box_baseline(m_formatting_state, box); + float fragment_baseline = 0; + if (fragment.layout_node().is_text_node()) { + fragment_baseline = fragment.layout_node().font().baseline(); + } else { + auto const& box = verify_cast(fragment.layout_node()); + fragment_baseline = box_baseline(m_formatting_state, box); + } + return fragment_baseline; }; auto line_box_baseline = [&] { float line_box_baseline = 0; for (auto const& fragment : line_box.fragments()) { auto baseline = fragment_baseline(fragment); + if (fragment.height() < m_context.containing_block().line_height()) + baseline += (m_context.containing_block().line_height() - fragment.height()) / 2; // NOTE: For fragments with a vertical-align, shift the fragment baseline down by the length. // This ensures that we make enough vertical space on the line for any manually-aligned fragments. - if (auto length_percentage = fragment.layout_node().computed_values().vertical_align().get_pointer(); length_percentage && length_percentage->is_length()) + if (auto length_percentage = fragment.layout_node().computed_values().vertical_align().template get_pointer(); length_percentage && length_percentage->is_length()) baseline += length_percentage->length().to_px(fragment.layout_node()); + line_box_baseline = max(line_box_baseline, baseline); } return line_box_baseline; }(); + // Start with the "strut", an imaginary zero-width box at the start of each line box. + auto strut_top = m_current_y; + auto strut_bottom = m_current_y + m_context.containing_block().line_height(); + + float uppermost_box_top = strut_top; + float lowermost_box_bottom = strut_bottom; + for (size_t i = 0; i < line_box.fragments().size(); ++i) { auto& fragment = line_box.fragments()[i]; @@ -190,10 +203,14 @@ void LineBuilder::update_last_line() fragment.set_offset({ new_fragment_x, new_fragment_y }); - bottom = max(bottom, new_fragment_y + fragment.height() + fragment.border_box_bottom()); + uppermost_box_top = min(uppermost_box_top, fragment.top_of_inline_level_box(m_formatting_state)); + lowermost_box_bottom = max(lowermost_box_bottom, fragment.bottom_of_inline_level_box(m_formatting_state)); } - line_box.m_bottom = bottom; + // 3. The line box height is the distance between the uppermost box top and the lowermost box bottom. + line_box.m_height = max(m_context.containing_block().line_height(), lowermost_box_bottom - uppermost_box_top); + + line_box.m_bottom = m_current_y + line_box.m_height; line_box.m_baseline = line_box_baseline; } diff --git a/Userland/Services/WebContent/main.cpp b/Userland/Services/WebContent/main.cpp index 351fd4b159..5d282482ca 100644 --- a/Userland/Services/WebContent/main.cpp +++ b/Userland/Services/WebContent/main.cpp @@ -16,6 +16,7 @@ ErrorOr serenity_main(Main::Arguments) Core::EventLoop event_loop; TRY(Core::System::pledge("stdio recvfd sendfd accept unix rpath")); TRY(Core::System::unveil("/res", "r")); + TRY(Core::System::unveil("/home", "r")); TRY(Core::System::unveil("/etc/timezone", "r")); TRY(Core::System::unveil("/tmp/portal/request", "rw")); TRY(Core::System::unveil("/tmp/portal/image", "rw"));