From c61747fb2a4e841233a9209db8ce84f073548e34 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Mon, 21 Feb 2022 16:24:12 +0100 Subject: [PATCH] LibWeb: Respect font-size specified by CSS in "em" length calculations "5em" means 5*font-size, but by forcing "em" to mean the presentation size of the bitmap font actually used, we broke a bunch of layouts that depended on a correct interpretation of "em". This means that "em" units will no longer be relative to the exact size of the bitmap font in use, but I think that's a compromise we'll have to make, since accurate layouts are more important. This yields a visual progression on both ACID2 and ACID3. :^) --- Userland/Libraries/LibWeb/CSS/ComputedValues.h | 9 +++++++++ Userland/Libraries/LibWeb/CSS/Length.cpp | 6 +++--- Userland/Libraries/LibWeb/CSS/Length.h | 6 +++--- Userland/Libraries/LibWeb/CSS/MediaQuery.cpp | 9 +++++---- Userland/Libraries/LibWeb/CSS/StyleComputer.cpp | 12 +++++++++--- Userland/Libraries/LibWeb/Layout/Node.cpp | 3 +++ 6 files changed, 32 insertions(+), 13 deletions(-) diff --git a/Userland/Libraries/LibWeb/CSS/ComputedValues.h b/Userland/Libraries/LibWeb/CSS/ComputedValues.h index b7e73e80bb..242ec7983b 100644 --- a/Userland/Libraries/LibWeb/CSS/ComputedValues.h +++ b/Userland/Libraries/LibWeb/CSS/ComputedValues.h @@ -14,6 +14,8 @@ namespace Web::CSS { class InitialValues { public: + static float font_size() { return 10; } + static int font_weight() { return 400; } static CSS::Float float_() { return CSS::Float::None; } static CSS::Clear clear() { return CSS::Clear::None; } static CSS::Cursor cursor() { return CSS::Cursor::Auto; } @@ -146,6 +148,9 @@ public: Vector transformations() const { return m_noninherited.transformations; } + float font_size() const { return m_inherited.font_size; } + int font_weight() const { return m_inherited.font_weight; } + ComputedValues clone_inherited_values() const { ComputedValues clone; @@ -155,6 +160,8 @@ public: protected: struct { + float font_size { InitialValues::font_size() }; + int font_weight { InitialValues::font_weight() }; Color color { InitialValues::color() }; CSS::Cursor cursor { InitialValues::cursor() }; CSS::ImageRendering image_rendering { InitialValues::image_rendering() }; @@ -217,6 +224,8 @@ class ImmutableComputedValues final : public ComputedValues { class MutableComputedValues final : public ComputedValues { public: + void set_font_size(float font_size) { m_inherited.font_size = font_size; } + void set_font_weight(int font_weight) { m_inherited.font_weight = font_weight; } void set_color(const Color& color) { m_inherited.color = color; } void set_cursor(CSS::Cursor cursor) { m_inherited.cursor = cursor; } void set_image_rendering(CSS::ImageRendering value) { m_inherited.image_rendering = value; } diff --git a/Userland/Libraries/LibWeb/CSS/Length.cpp b/Userland/Libraries/LibWeb/CSS/Length.cpp index c19a6505a2..18b07935ea 100644 --- a/Userland/Libraries/LibWeb/CSS/Length.cpp +++ b/Userland/Libraries/LibWeb/CSS/Length.cpp @@ -65,13 +65,13 @@ Length Length::resolved(Layout::Node const& layout_node) const return *this; } -float Length::relative_length_to_px(Gfx::IntRect const& viewport_rect, Gfx::FontMetrics const& font_metrics, float root_font_size) const +float Length::relative_length_to_px(Gfx::IntRect const& viewport_rect, Gfx::FontMetrics const& font_metrics, float font_size, float root_font_size) const { switch (m_type) { case Type::Ex: return m_value * font_metrics.x_height; case Type::Em: - return m_value * font_metrics.size; + return m_value * font_size; case Type::Ch: // FIXME: Use layout_node.font().glyph_height() when writing-mode is not horizontal-tb (it has to be implemented first) return m_value * (font_metrics.glyph_width + font_metrics.glyph_spacing); @@ -101,7 +101,7 @@ float Length::to_px(Layout::Node const& layout_node) const auto* root_element = layout_node.document().document_element(); if (!root_element || !root_element->layout_node()) return 0; - return to_px(viewport_rect, layout_node.font().metrics('M'), root_element->layout_node()->font().presentation_size()); + return to_px(viewport_rect, layout_node.font().metrics('M'), layout_node.computed_values().font_size(), root_element->layout_node()->computed_values().font_size()); } const char* Length::unit_name() const diff --git a/Userland/Libraries/LibWeb/CSS/Length.h b/Userland/Libraries/LibWeb/CSS/Length.h index be96bc2ea7..d0e48bcca8 100644 --- a/Userland/Libraries/LibWeb/CSS/Length.h +++ b/Userland/Libraries/LibWeb/CSS/Length.h @@ -77,12 +77,12 @@ public: float to_px(Layout::Node const&) const; - ALWAYS_INLINE float to_px(Gfx::IntRect const& viewport_rect, Gfx::FontMetrics const& font_metrics, float root_font_size) const + ALWAYS_INLINE float to_px(Gfx::IntRect const& viewport_rect, Gfx::FontMetrics const& font_metrics, float font_size, float root_font_size) const { if (is_auto()) return 0; if (is_relative()) - return relative_length_to_px(viewport_rect, font_metrics, root_font_size); + return relative_length_to_px(viewport_rect, font_metrics, font_size, root_font_size); if (is_calculated()) VERIFY_NOT_REACHED(); // We can't resolve a calculated length from here. :^( return absolute_length_to_px(); @@ -129,7 +129,7 @@ public: return !(*this == other); } - float relative_length_to_px(Gfx::IntRect const& viewport_rect, Gfx::FontMetrics const& font_metrics, float root_font_size) const; + float relative_length_to_px(Gfx::IntRect const& viewport_rect, Gfx::FontMetrics const& font_metrics, float font_size, float root_font_size) const; private: const char* unit_name() const; diff --git a/Userland/Libraries/LibWeb/CSS/MediaQuery.cpp b/Userland/Libraries/LibWeb/CSS/MediaQuery.cpp index 83a00ef6ab..ae5a37c9dd 100644 --- a/Userland/Libraries/LibWeb/CSS/MediaQuery.cpp +++ b/Userland/Libraries/LibWeb/CSS/MediaQuery.cpp @@ -153,12 +153,13 @@ bool MediaFeature::compare(DOM::Window const& window, MediaFeatureValue left, Co // FIXME: This isn't right - we want to query the initial-value font, which is the one used // if no author styles are defined. - auto const& font = window.associated_document().root().layout_node()->font(); + auto& root_layout_node = *window.associated_document().root().layout_node(); + auto const& font = root_layout_node.font(); Gfx::FontMetrics const& font_metrics = font.metrics('M'); - float root_font_size = font.presentation_size(); + float root_font_size = root_layout_node.computed_values().font_size(); - left_px = left.length().to_px(viewport_rect, font_metrics, root_font_size); - right_px = right.length().to_px(viewport_rect, font_metrics, root_font_size); + left_px = left.length().to_px(viewport_rect, font_metrics, root_font_size, root_font_size); + right_px = right.length().to_px(viewport_rect, font_metrics, root_font_size, root_font_size); } switch (comparison) { diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp index 4129758f8e..9780c02e67 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp +++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp @@ -770,7 +770,7 @@ void StyleComputer::compute_font(StyleProperties& style, DOM::Element const* ele if (value->is_length()) { auto length = static_cast(*value).to_length(); if (length.is_absolute() || length.is_relative()) - parent_font_size = length.to_px(viewport_rect, font_metrics, root_font_size); + parent_font_size = length.to_px(viewport_rect, font_metrics, size, root_font_size); } } maybe_length = Length::make_px(percentage.as_fraction() * parent_font_size); @@ -785,7 +785,7 @@ void StyleComputer::compute_font(StyleProperties& style, DOM::Element const* ele // FIXME: Support font-size: calc(...) // Theoretically we can do this now, but to resolve it we need a layout_node which we might not have. :^( if (!maybe_length->is_calculated()) { - auto px = maybe_length.value().to_px(viewport_rect, font_metrics, root_font_size); + auto px = maybe_length.value().to_px(viewport_rect, font_metrics, size, root_font_size); if (px != 0) size = px; } @@ -874,6 +874,9 @@ void StyleComputer::compute_font(StyleProperties& style, DOM::Element const* ele FontCache::the().set(font_selector, *found_font); + style.set_property(CSS::PropertyID::FontSize, LengthStyleValue::create(CSS::Length::make_px(size))); + style.set_property(CSS::PropertyID::FontWeight, NumericStyleValue::create_integer(weight)); + style.set_computed_font(found_font.release_nonnull()); } @@ -883,13 +886,16 @@ void StyleComputer::absolutize_values(StyleProperties& style, DOM::Element const auto font_metrics = style.computed_font().metrics('M'); // FIXME: Get the root element font. float root_font_size = 10; + float font_size = style.property(CSS::PropertyID::FontSize).value()->to_length().to_px(viewport_rect, font_metrics, root_font_size, root_font_size); for (auto& value_slot : style.m_property_values) { if (!value_slot) continue; value_slot->visit_lengths([&](Length& length) { + if (length.is_px()) + return; if (length.is_absolute() || length.is_relative()) { - auto px = length.to_px(viewport_rect, font_metrics, root_font_size); + auto px = length.to_px(viewport_rect, font_metrics, font_size, root_font_size); length = Length::make_px(px); } }); diff --git a/Userland/Libraries/LibWeb/Layout/Node.cpp b/Userland/Libraries/LibWeb/Layout/Node.cpp index 910d63f73c..8e1bb855af 100644 --- a/Userland/Libraries/LibWeb/Layout/Node.cpp +++ b/Userland/Libraries/LibWeb/Layout/Node.cpp @@ -322,6 +322,9 @@ void NodeWithStyle::apply_style(const CSS::StyleProperties& specified_style) computed_values.set_box_sizing(specified_style.box_sizing()); + computed_values.set_font_size(specified_style.property(CSS::PropertyID::FontSize).value()->to_length().to_px(*this)); + computed_values.set_font_weight(specified_style.property(CSS::PropertyID::FontWeight).value()->to_integer()); + // FIXME: BorderXRadius properties are now BorderRadiusStyleValues, so make use of that. auto border_bottom_left_radius = specified_style.property(CSS::PropertyID::BorderBottomLeftRadius); if (border_bottom_left_radius.has_value() && border_bottom_left_radius.value()->is_border_radius())