From 0f9f6aef81d14d12763501da4a65330170271d52 Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Wed, 19 Apr 2023 12:00:38 +0100 Subject: [PATCH] LibWeb: Simplify StyleValue API now that `auto` isn't a length Now that LengthStyleValue never contains `auto`, IdentifierStyleValue is the only type that can hold an identifier. This lets us remove a couple of virtual methods from StyleValue. I've kept `has_auto()` and `to_identifier()` for convenience, but they are now simple non-virtual methods. --- Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp | 4 ++-- Userland/Libraries/LibWeb/CSS/StyleValue.cpp | 12 ++++++++++++ Userland/Libraries/LibWeb/CSS/StyleValue.h | 5 ++--- .../LibWeb/CSS/StyleValues/IdentifierStyleValue.h | 3 --- .../LibWeb/CSS/StyleValues/LengthStyleValue.h | 3 --- Userland/Libraries/LibWeb/Layout/Node.cpp | 8 ++++---- 6 files changed, 20 insertions(+), 15 deletions(-) diff --git a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp index d47553b0ab..268c0d886c 100644 --- a/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp +++ b/Userland/Libraries/LibWeb/CSS/Parser/Parser.cpp @@ -4388,7 +4388,7 @@ RefPtr Parser::parse_single_background_position_value(TokenStreamhas_identifier()) { + if (value->is_identifier()) { auto identifier = value->to_identifier(); if (is_horizontal(identifier)) { bool offset_provided = false; @@ -4474,7 +4474,7 @@ RefPtr Parser::parse_single_background_position_x_or_y_value(TokenSt }; auto value = parse_value(tokens.next_token()); - if (value->has_identifier()) { + if (value->is_identifier()) { auto identifier = value->to_identifier(); if (identifier == ValueID::Center) { transaction.commit(); diff --git a/Userland/Libraries/LibWeb/CSS/StyleValue.cpp b/Userland/Libraries/LibWeb/CSS/StyleValue.cpp index fa9c965fa1..bfdbb076f5 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleValue.cpp +++ b/Userland/Libraries/LibWeb/CSS/StyleValue.cpp @@ -336,4 +336,16 @@ ValueComparingNonnullRefPtr StyleValue::absolutized(CSSPixelRe return *this; } +bool StyleValue::has_auto() const +{ + return is_identifier() && as_identifier().id() == ValueID::Auto; +} + +ValueID StyleValue::to_identifier() const +{ + if (is_identifier()) + return as_identifier().id(); + return ValueID::Invalid; +} + } diff --git a/Userland/Libraries/LibWeb/CSS/StyleValue.h b/Userland/Libraries/LibWeb/CSS/StyleValue.h index 829f3ff4c2..76661d942b 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleValue.h +++ b/Userland/Libraries/LibWeb/CSS/StyleValue.h @@ -276,9 +276,8 @@ public: UnsetStyleValue& as_unset() { return const_cast(const_cast(*this).as_unset()); } StyleValueList& as_value_list() { return const_cast(const_cast(*this).as_value_list()); } - virtual bool has_auto() const { return false; } + bool has_auto() const; virtual bool has_color() const { return false; } - virtual bool has_identifier() const { return false; } virtual bool has_length() const { return false; } virtual bool has_rect() const { return false; } virtual bool has_number() const { return false; } @@ -287,7 +286,7 @@ public: virtual ValueComparingNonnullRefPtr absolutized(CSSPixelRect const& viewport_rect, Gfx::FontPixelMetrics const& font_metrics, CSSPixels font_size, CSSPixels root_font_size, CSSPixels line_height, CSSPixels root_line_height) const; virtual Color to_color(Layout::NodeWithStyle const&) const { return {}; } - virtual CSS::ValueID to_identifier() const { return ValueID::Invalid; } + ValueID to_identifier() const; virtual Length to_length() const { VERIFY_NOT_REACHED(); } virtual float to_number() const { return 0; } virtual float to_integer() const { return 0; } diff --git a/Userland/Libraries/LibWeb/CSS/StyleValues/IdentifierStyleValue.h b/Userland/Libraries/LibWeb/CSS/StyleValues/IdentifierStyleValue.h index 52f451bbc4..225506f047 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleValues/IdentifierStyleValue.h +++ b/Userland/Libraries/LibWeb/CSS/StyleValues/IdentifierStyleValue.h @@ -24,9 +24,6 @@ public: ValueID id() const { return m_id; } - virtual bool has_auto() const override { return m_id == ValueID::Auto; } - virtual bool has_identifier() const override { return true; } - virtual ValueID to_identifier() const override { return m_id; } virtual bool has_color() const override; virtual Color to_color(Layout::NodeWithStyle const& node) const override; virtual ErrorOr to_string() const override; diff --git a/Userland/Libraries/LibWeb/CSS/StyleValues/LengthStyleValue.h b/Userland/Libraries/LibWeb/CSS/StyleValues/LengthStyleValue.h index 5f23c207b8..37d43cca64 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleValues/LengthStyleValue.h +++ b/Userland/Libraries/LibWeb/CSS/StyleValues/LengthStyleValue.h @@ -20,12 +20,9 @@ public: Length const& length() const { return m_length; } - virtual bool has_auto() const override { return m_length.is_auto(); } virtual bool has_length() const override { return true; } - virtual bool has_identifier() const override { return has_auto(); } virtual ErrorOr to_string() const override { return m_length.to_string(); } virtual Length to_length() const override { return m_length; } - virtual ValueID to_identifier() const override { return has_auto() ? ValueID::Auto : ValueID::Invalid; } virtual ValueComparingNonnullRefPtr absolutized(CSSPixelRect const& viewport_rect, Gfx::FontPixelMetrics const& font_metrics, CSSPixels font_size, CSSPixels root_font_size, CSSPixels line_height, CSSPixels root_line_height) const override; bool properties_equal(LengthStyleValue const& other) const { return m_length == other.m_length; } diff --git a/Userland/Libraries/LibWeb/Layout/Node.cpp b/Userland/Libraries/LibWeb/Layout/Node.cpp index d40e6656df..b7688f1e84 100644 --- a/Userland/Libraries/LibWeb/Layout/Node.cpp +++ b/Userland/Libraries/LibWeb/Layout/Node.cpp @@ -334,7 +334,7 @@ void NodeWithStyle::apply_style(const CSS::StyleProperties& computed_style) } } - if (auto attachment_value = value_for_layer(attachments, layer_index); attachment_value && attachment_value->has_identifier()) { + if (auto attachment_value = value_for_layer(attachments, layer_index); attachment_value && attachment_value->is_identifier()) { switch (attachment_value->to_identifier()) { case CSS::ValueID::Fixed: layer.attachment = CSS::BackgroundAttachment::Fixed; @@ -363,11 +363,11 @@ void NodeWithStyle::apply_style(const CSS::StyleProperties& computed_style) } }; - if (auto origin_value = value_for_layer(origins, layer_index); origin_value && origin_value->has_identifier()) { + if (auto origin_value = value_for_layer(origins, layer_index); origin_value && origin_value->is_identifier()) { layer.origin = as_box(origin_value->to_identifier()); } - if (auto clip_value = value_for_layer(clips, layer_index); clip_value && clip_value->has_identifier()) { + if (auto clip_value = value_for_layer(clips, layer_index); clip_value && clip_value->is_identifier()) { layer.clip = as_box(clip_value->to_identifier()); } @@ -389,7 +389,7 @@ void NodeWithStyle::apply_style(const CSS::StyleProperties& computed_style) layer.size_type = CSS::BackgroundSize::LengthPercentage; layer.size_x = size.size_x(); layer.size_y = size.size_y(); - } else if (size_value->has_identifier()) { + } else if (size_value->is_identifier()) { switch (size_value->to_identifier()) { case CSS::ValueID::Contain: layer.size_type = CSS::BackgroundSize::Contain;