From e8a946c674c302e4551c17517230631aa16241f9 Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Sat, 27 May 2023 12:28:25 +0100 Subject: [PATCH] LibWeb: Remove StyleValue::has/to_length() Specifically, stop letting NumericStyleValues holding `0` from pretending to hold a Length. The parser is now smart enough that we don't have to do this. :^) --- .../Libraries/LibWeb/CSS/Parser/Parser.cpp | 10 +++----- .../Libraries/LibWeb/CSS/StyleComputer.cpp | 10 ++++---- .../Libraries/LibWeb/CSS/StyleProperties.cpp | 25 ++++++++++--------- Userland/Libraries/LibWeb/CSS/StyleValue.h | 2 -- .../LibWeb/CSS/StyleValues/LengthStyleValue.h | 2 -- .../CSS/StyleValues/NumericStyleValue.h | 3 --- Userland/Libraries/LibWeb/Layout/Node.cpp | 9 ++++--- 7 files changed, 27 insertions(+), 34 deletions(-) diff --git a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp index f05fc4b841..b8ca4484b0 100644 --- a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp +++ b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp @@ -4405,8 +4405,8 @@ static Optional style_value_to_length_percentage(auto value) { if (value->is_percentage()) return LengthPercentage { value->as_percentage().percentage() }; - if (value->has_length()) - return LengthPercentage { value->to_length() }; + if (value->is_length()) + return LengthPercentage { value->as_length().length() }; if (value->is_calculated()) return LengthPercentage { value->as_calculated() }; return {}; @@ -4674,8 +4674,8 @@ ErrorOr> Parser::parse_single_background_size_value(TokenStre return LengthPercentage { Length::make_auto() }; if (style_value.is_percentage()) return LengthPercentage { style_value.as_percentage().percentage() }; - if (style_value.has_length()) - return LengthPercentage { style_value.to_length() }; + if (style_value.is_length()) + return LengthPercentage { style_value.as_length().length() }; return {}; }; @@ -6239,8 +6239,6 @@ ErrorOr> Parser::parse_transform_origin_value(Vectoras_percentage() }; if (value->is_length()) return AxisOffset { Axis::None, value->as_length() }; - if (value->has_length()) - return AxisOffset { Axis::None, TRY(LengthStyleValue::create(value->to_length())) }; if (value->is_identifier()) { switch (value->to_identifier()) { case ValueID::Top: diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp index 4fd9c2b5b1..acad231baf 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp +++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp @@ -1121,7 +1121,7 @@ Length::FontMetrics StyleComputer::calculate_root_element_font_metrics(StyleProp auto font_pixel_metrics = style.computed_font().pixel_metrics(); Length::FontMetrics font_metrics { m_default_font_metrics.font_size, font_pixel_metrics, font_pixel_metrics.line_spacing() }; - font_metrics.font_size = root_value->to_length().to_px(viewport_rect(), font_metrics, font_metrics); + font_metrics.font_size = root_value->as_length().length().to_px(viewport_rect(), font_metrics, font_metrics); font_metrics.line_height = style.line_height(viewport_rect(), font_metrics, font_metrics); return font_metrics; @@ -1249,7 +1249,7 @@ void StyleComputer::compute_font(StyleProperties& style, DOM::Element const* ele return font_size_in_px; auto value = parent_element->computed_css_values()->property(CSS::PropertyID::FontSize); if (value->is_length()) { - auto length = static_cast(*value).to_length(); + auto length = value->as_length().length(); auto parent_line_height = parent_or_root_element_line_height(parent_element, {}); if (length.is_absolute() || length.is_relative()) { Length::FontMetrics font_metrics { font_size_in_px, font_pixel_metrics, parent_line_height }; @@ -1265,7 +1265,7 @@ void StyleComputer::compute_font(StyleProperties& style, DOM::Element const* ele maybe_length = Length::make_px(static_cast(font_size->as_percentage().percentage().as_fraction()) * parent_font_size()); } else if (font_size->is_length()) { - maybe_length = font_size->to_length(); + maybe_length = font_size->as_length().length(); } else if (font_size->is_calculated()) { // FIXME: Support font-size: calc(...) @@ -1412,7 +1412,7 @@ CSSPixels StyleComputer::parent_or_root_element_line_height(DOM::Element const* return m_root_element_font_metrics.line_height; auto const* computed_values = parent_element->computed_css_values(); auto parent_font_pixel_metrics = computed_values->computed_font().pixel_metrics(); - auto parent_font_size = computed_values->property(CSS::PropertyID::FontSize)->to_length(); + auto parent_font_size = computed_values->property(CSS::PropertyID::FontSize)->as_length().length(); // FIXME: Can the parent font size be non-absolute here? auto parent_font_size_value = parent_font_size.is_absolute() ? parent_font_size.absolute_length_to_px() : m_root_element_font_metrics.font_size; auto parent_parent_line_height = parent_or_root_element_line_height(parent_element, {}); @@ -1428,7 +1428,7 @@ ErrorOr StyleComputer::absolutize_values(StyleProperties& style, DOM::Elem Length::FontMetrics font_metrics { m_root_element_font_metrics.font_size, font_pixel_metrics, parent_or_root_line_height }; - auto font_size = style.property(CSS::PropertyID::FontSize)->to_length().to_px(viewport_rect(), font_metrics, m_root_element_font_metrics); + auto font_size = style.property(CSS::PropertyID::FontSize)->as_length().length().to_px(viewport_rect(), font_metrics, m_root_element_font_metrics); font_metrics.font_size = font_size; // NOTE: Percentage line-height values are relative to the font-size of the element. diff --git a/Userland/Libraries/LibWeb/CSS/StyleProperties.cpp b/Userland/Libraries/LibWeb/CSS/StyleProperties.cpp index 6e083fed7f..da20b86d46 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleProperties.cpp +++ b/Userland/Libraries/LibWeb/CSS/StyleProperties.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -88,11 +89,11 @@ CSS::Size StyleProperties::size_value(CSS::PropertyID id) const if (value->is_percentage()) return CSS::Size::make_percentage(value->as_percentage().percentage()); - if (value->has_length()) { - auto length = value->to_length(); + if (value->is_length()) { + auto length = value->as_length().length(); if (length.is_auto()) return CSS::Size::make_auto(); - return CSS::Size::make_length(value->to_length()); + return CSS::Size::make_length(length); } // FIXME: Support `fit-content()` @@ -115,8 +116,8 @@ Optional StyleProperties::length_percentage(CSS::PropertyID id if (value->is_percentage()) return value->as_percentage().percentage(); - if (value->has_length()) - return value->to_length(); + if (value->is_length()) + return value->as_length().length(); if (value->has_auto()) return LengthPercentage { Length::make_auto() }; @@ -165,7 +166,7 @@ CSSPixels StyleProperties::line_height(CSSPixelRect const& viewport_rect, Length return font_metrics.line_height; if (line_height->is_length()) { - auto line_height_length = line_height->to_length(); + auto line_height_length = line_height->as_length().length(); if (!line_height_length.is_auto()) return line_height_length.to_px(viewport_rect, font_metrics, root_font_metrics); } @@ -195,7 +196,7 @@ CSSPixels StyleProperties::line_height(Layout::Node const& layout_node) const return layout_node.font().pixel_metrics().line_spacing(); if (line_height->is_length()) { - auto line_height_length = line_height->to_length(); + auto line_height_length = line_height->as_length().length(); if (!line_height_length.is_auto()) return line_height_length.to_px(layout_node); } @@ -316,8 +317,8 @@ Optional StyleProperties::flex_basis() const if (value->is_percentage()) return { { CSS::FlexBasis::LengthPercentage, value->as_percentage().percentage() } }; - if (value->has_length()) - return { { CSS::FlexBasis::LengthPercentage, value->to_length() } }; + if (value->is_length()) + return { { CSS::FlexBasis::LengthPercentage, value->as_length().length() } }; if (value->is_calculated()) return { { CSS::FlexBasis::LengthPercentage, CSS::LengthPercentage { value->as_calculated() } } }; @@ -392,7 +393,7 @@ Vector StyleProperties::transformations() const Vector values; for (auto& transformation_value : transformation_style_value.values()) { if (transformation_value->is_length()) { - values.append({ transformation_value->to_length() }); + values.append({ transformation_value->as_length().length() }); } else if (transformation_value->is_percentage()) { values.append({ transformation_value->as_percentage().percentage() }); } else if (transformation_value->is_numeric()) { @@ -412,7 +413,7 @@ Vector StyleProperties::transformations() const static Optional length_percentage_for_style_value(StyleValue const& value) { if (value.is_length()) - return value.to_length(); + return value.as_length().length(); if (value.is_percentage()) return value.as_percentage().percentage(); return {}; @@ -747,7 +748,7 @@ Variant StyleProperties::vertical_ali return value_id_to_vertical_align(value->to_identifier()).release_value(); if (value->is_length()) - return CSS::LengthPercentage(value->to_length()); + return CSS::LengthPercentage(value->as_length().length()); if (value->is_percentage()) return CSS::LengthPercentage(value->as_percentage().percentage()); diff --git a/Userland/Libraries/LibWeb/CSS/StyleValue.h b/Userland/Libraries/LibWeb/CSS/StyleValue.h index 5e10039398..87472265bf 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleValue.h +++ b/Userland/Libraries/LibWeb/CSS/StyleValue.h @@ -293,13 +293,11 @@ public: bool has_auto() const; virtual bool has_color() const { return false; } - virtual bool has_length() const { return false; } virtual ErrorOr> absolutized(CSSPixelRect const& viewport_rect, Length::FontMetrics const& font_metrics, Length::FontMetrics const& root_font_metrics) const; virtual Color to_color(Optional) const { return {}; } ValueID to_identifier() const; - virtual Length to_length() const { VERIFY_NOT_REACHED(); } virtual ErrorOr to_string() const = 0; [[nodiscard]] int to_font_weight() const; diff --git a/Userland/Libraries/LibWeb/CSS/StyleValues/LengthStyleValue.h b/Userland/Libraries/LibWeb/CSS/StyleValues/LengthStyleValue.h index e305af7c12..0c6b8b4f95 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleValues/LengthStyleValue.h +++ b/Userland/Libraries/LibWeb/CSS/StyleValues/LengthStyleValue.h @@ -20,9 +20,7 @@ public: Length const& length() const { return m_length; } - virtual bool has_length() const override { return true; } virtual ErrorOr to_string() const override { return m_length.to_string(); } - virtual Length to_length() const override { return m_length; } virtual ErrorOr> absolutized(CSSPixelRect const& viewport_rect, Length::FontMetrics const& font_metrics, Length::FontMetrics const& root_font_metrics) const override; bool properties_equal(LengthStyleValue const& other) const { return m_length == other.m_length; } diff --git a/Userland/Libraries/LibWeb/CSS/StyleValues/NumericStyleValue.h b/Userland/Libraries/LibWeb/CSS/StyleValues/NumericStyleValue.h index 7895ed422e..74d222a565 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleValues/NumericStyleValue.h +++ b/Userland/Libraries/LibWeb/CSS/StyleValues/NumericStyleValue.h @@ -25,9 +25,6 @@ public: return adopt_nonnull_ref_or_enomem(new (nothrow) NumericStyleValue(value)); } - virtual bool has_length() const override { return number() == 0; } - virtual Length to_length() const override { return Length::make_px(0); } - float number() const { return m_value.visit( diff --git a/Userland/Libraries/LibWeb/Layout/Node.cpp b/Userland/Libraries/LibWeb/Layout/Node.cpp index 6804d63ed7..86665dbadb 100644 --- a/Userland/Libraries/LibWeb/Layout/Node.cpp +++ b/Userland/Libraries/LibWeb/Layout/Node.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -294,7 +295,7 @@ void NodeWithStyle::apply_style(const CSS::StyleProperties& computed_style) // m_font is used by Length::to_px() when resolving sizes against this layout node. // That's why it has to be set before everything else. m_font = computed_style.computed_font(); - computed_values.set_font_size(computed_style.property(CSS::PropertyID::FontSize)->to_length().to_px(*this).value()); + computed_values.set_font_size(computed_style.property(CSS::PropertyID::FontSize)->as_length().length().to_px(*this).value()); computed_values.set_font_weight(computed_style.property(CSS::PropertyID::FontWeight)->as_numeric().integer()); m_line_height = computed_style.line_height(*this); @@ -625,8 +626,8 @@ void NodeWithStyle::apply_style(const CSS::StyleProperties& computed_style) auto value = computed_style.property(width_property); if (value->is_calculated()) return value->as_calculated().resolve_length(*this)->to_px(*this).value(); - if (value->has_length()) - return value->to_length().to_px(*this).value(); + if (value->is_length()) + return value->as_length().length().to_px(*this).value(); if (value->is_identifier()) { // https://www.w3.org/TR/css-backgrounds-3/#valdef-line-width-thin switch (value->to_identifier()) { @@ -679,7 +680,7 @@ void NodeWithStyle::apply_style(const CSS::StyleProperties& computed_style) if (stroke_width->is_numeric()) computed_values.set_stroke_width(CSS::Length::make_px(stroke_width->as_numeric().number())); else if (stroke_width->is_length()) - computed_values.set_stroke_width(stroke_width->to_length()); + computed_values.set_stroke_width(stroke_width->as_length().length()); else if (stroke_width->is_percentage()) computed_values.set_stroke_width(CSS::LengthPercentage { stroke_width->as_percentage().percentage() });