diff --git a/Libraries/LibWeb/CSS/Parser/ValueParsing.cpp b/Libraries/LibWeb/CSS/Parser/ValueParsing.cpp index 4127436b97d..6bf231d44f4 100644 --- a/Libraries/LibWeb/CSS/Parser/ValueParsing.cpp +++ b/Libraries/LibWeb/CSS/Parser/ValueParsing.cpp @@ -4339,8 +4339,10 @@ NonnullRefPtr Parser::resolve_unresolved_style_value(Parsin NonnullRefPtr Parser::resolve_unresolved_style_value(DOM::AbstractElement& element, GuardedSubstitutionContexts& guarded_contexts, PropertyIDOrCustomPropertyName property, UnresolvedStyleValue const& unresolved) { // AD-HOC: Report that we might rely on custom properties. - // FIXME: This over-invalidates. Find a way of invalidating only when we need to - specifically, when var() is used. - element.element().set_style_uses_css_custom_properties(true); + if (unresolved.includes_attr_function()) + element.element().set_style_uses_attr_css_function(); + if (unresolved.includes_var_function()) + element.element().set_style_uses_var_css_function(); // To replace substitution functions in a property prop: auto const& property_name = property.visit( diff --git a/Libraries/LibWeb/CSS/StyleComputer.cpp b/Libraries/LibWeb/CSS/StyleComputer.cpp index ad3cb3bf727..953d8628d2e 100644 --- a/Libraries/LibWeb/CSS/StyleComputer.cpp +++ b/Libraries/LibWeb/CSS/StyleComputer.cpp @@ -2454,17 +2454,17 @@ GC::Ref StyleComputer::create_document_style() const return style; } -GC::Ref StyleComputer::compute_style(DOM::Element& element, Optional pseudo_element) const +GC::Ref StyleComputer::compute_style(DOM::Element& element, Optional pseudo_element, Optional did_change_custom_properties) const { - return *compute_style_impl(element, move(pseudo_element), ComputeStyleMode::Normal); + return *compute_style_impl(element, move(pseudo_element), ComputeStyleMode::Normal, did_change_custom_properties); } -GC::Ptr StyleComputer::compute_pseudo_element_style_if_needed(DOM::Element& element, Optional pseudo_element) const +GC::Ptr StyleComputer::compute_pseudo_element_style_if_needed(DOM::Element& element, Optional pseudo_element, Optional did_change_custom_properties) const { - return compute_style_impl(element, move(pseudo_element), ComputeStyleMode::CreatePseudoElementStyleIfNeeded); + return compute_style_impl(element, move(pseudo_element), ComputeStyleMode::CreatePseudoElementStyleIfNeeded, did_change_custom_properties); } -GC::Ptr StyleComputer::compute_style_impl(DOM::Element& element, Optional pseudo_element, ComputeStyleMode mode) const +GC::Ptr StyleComputer::compute_style_impl(DOM::Element& element, Optional pseudo_element, ComputeStyleMode mode, Optional did_change_custom_properties) const { build_rule_cache_if_needed(); @@ -2488,6 +2488,9 @@ GC::Ptr StyleComputer::compute_style_impl(DOM::Element& elem PseudoClassBitmap attempted_pseudo_class_matches; auto matching_rule_set = build_matching_rule_set(element, pseudo_element, attempted_pseudo_class_matches, did_match_any_pseudo_element_rules, mode); + DOM::AbstractElement abstract_element { element, pseudo_element }; + auto old_custom_properties = abstract_element.custom_properties(); + // Resolve all the CSS custom properties ("variables") for this element: // FIXME: Also resolve !important custom properties, in a second cascade. if (!pseudo_element.has_value() || pseudo_element_supports_property(*pseudo_element, PropertyID::Custom)) { @@ -2533,6 +2536,11 @@ GC::Ptr StyleComputer::compute_style_impl(DOM::Element& elem auto computed_properties = compute_properties(element, pseudo_element, cascaded_properties); computed_properties->set_attempted_pseudo_class_matches(attempted_pseudo_class_matches); + + if (did_change_custom_properties.has_value() && abstract_element.custom_properties() != old_custom_properties) { + *did_change_custom_properties = true; + } + return computed_properties; } diff --git a/Libraries/LibWeb/CSS/StyleComputer.h b/Libraries/LibWeb/CSS/StyleComputer.h index ed098b197e5..f0110a85806 100644 --- a/Libraries/LibWeb/CSS/StyleComputer.h +++ b/Libraries/LibWeb/CSS/StyleComputer.h @@ -145,8 +145,8 @@ public: [[nodiscard]] GC::Ref create_document_style() const; - [[nodiscard]] GC::Ref compute_style(DOM::Element&, Optional = {}) const; - [[nodiscard]] GC::Ptr compute_pseudo_element_style_if_needed(DOM::Element&, Optional) const; + [[nodiscard]] GC::Ref compute_style(DOM::Element&, Optional = {}, Optional did_change_custom_properties = {}) const; + [[nodiscard]] GC::Ptr compute_pseudo_element_style_if_needed(DOM::Element&, Optional, Optional did_change_custom_properties) const; [[nodiscard]] RuleCache const& get_pseudo_class_rule_cache(PseudoClass) const; @@ -217,7 +217,7 @@ private: [[nodiscard]] MatchingRuleSet build_matching_rule_set(DOM::Element const&, Optional, PseudoClassBitmap& attempted_pseudo_class_matches, bool& did_match_any_pseudo_element_rules, ComputeStyleMode) const; LogicalAliasMappingContext compute_logical_alias_mapping_context(DOM::Element&, Optional, ComputeStyleMode, MatchingRuleSet const&) const; - [[nodiscard]] GC::Ptr compute_style_impl(DOM::Element&, Optional, ComputeStyleMode) const; + [[nodiscard]] GC::Ptr compute_style_impl(DOM::Element&, Optional, ComputeStyleMode, Optional did_change_custom_properties) const; [[nodiscard]] GC::Ref compute_cascaded_values(DOM::Element&, Optional, bool did_match_any_pseudo_element_rules, ComputeStyleMode, MatchingRuleSet const&, Optional, ReadonlySpan properties_to_cascade) const; static RefPtr find_matching_font_weight_ascending(Vector const& candidates, int target_weight, float font_size_in_pt, bool inclusive); static RefPtr find_matching_font_weight_descending(Vector const& candidates, int target_weight, float font_size_in_pt, bool inclusive); diff --git a/Libraries/LibWeb/CSS/StyleProperty.cpp b/Libraries/LibWeb/CSS/StyleProperty.cpp index 334376c9c0b..03573b33919 100644 --- a/Libraries/LibWeb/CSS/StyleProperty.cpp +++ b/Libraries/LibWeb/CSS/StyleProperty.cpp @@ -11,4 +11,11 @@ namespace Web::CSS { StyleProperty::~StyleProperty() = default; +bool StyleProperty::operator==(StyleProperty const& other) const +{ + if (important != other.important || property_id != other.property_id || custom_name != other.custom_name) + return false; + return value->equals(*other.value); +} + } diff --git a/Libraries/LibWeb/CSS/StyleProperty.h b/Libraries/LibWeb/CSS/StyleProperty.h index 12964bb75ff..ba880781017 100644 --- a/Libraries/LibWeb/CSS/StyleProperty.h +++ b/Libraries/LibWeb/CSS/StyleProperty.h @@ -23,6 +23,8 @@ struct StyleProperty { CSS::PropertyID property_id; NonnullRefPtr value; FlyString custom_name {}; + + bool operator==(StyleProperty const& other) const; }; } diff --git a/Libraries/LibWeb/CSS/StyleValues/UnresolvedStyleValue.h b/Libraries/LibWeb/CSS/StyleValues/UnresolvedStyleValue.h index f69bf3730e9..9d88b5fbdce 100644 --- a/Libraries/LibWeb/CSS/StyleValues/UnresolvedStyleValue.h +++ b/Libraries/LibWeb/CSS/StyleValues/UnresolvedStyleValue.h @@ -25,6 +25,8 @@ public: Vector const& values() const { return m_values; } bool contains_arbitrary_substitution_function() const { return m_substitution_functions_presence.has_any(); } + bool includes_attr_function() const { return m_substitution_functions_presence.attr; } + bool includes_var_function() const { return m_substitution_functions_presence.var; } virtual bool equals(CSSStyleValue const& other) const override; diff --git a/Libraries/LibWeb/DOM/Document.cpp b/Libraries/LibWeb/DOM/Document.cpp index 0f3ccb4a411..8a62b3dbaff 100644 --- a/Libraries/LibWeb/DOM/Document.cpp +++ b/Libraries/LibWeb/DOM/Document.cpp @@ -1427,7 +1427,7 @@ void Document::update_layout(UpdateLayoutReason reason) } } -[[nodiscard]] static CSS::RequiredInvalidationAfterStyleChange update_style_recursively(Node& node, CSS::StyleComputer& style_computer, bool needs_inherited_style_update) +[[nodiscard]] static CSS::RequiredInvalidationAfterStyleChange update_style_recursively(Node& node, CSS::StyleComputer& style_computer, bool needs_inherited_style_update, bool recompute_elements_depending_on_custom_properties) { bool const needs_full_style_update = node.document().needs_full_style_update(); CSS::RequiredInvalidationAfterStyleChange invalidation; @@ -1440,12 +1440,14 @@ void Document::update_layout(UpdateLayoutReason reason) // We will still recompute style for the children, though. bool is_display_none = false; + bool did_change_custom_properties = false; CSS::RequiredInvalidationAfterStyleChange node_invalidation; if (is(node)) { - if (needs_full_style_update || node.needs_style_update()) { - node_invalidation = static_cast(node).recompute_style(); + auto& element = static_cast(node); + if (needs_full_style_update || node.needs_style_update() || (recompute_elements_depending_on_custom_properties && element.style_uses_var_css_function())) { + node_invalidation = element.recompute_style(did_change_custom_properties); } else if (needs_inherited_style_update) { - node_invalidation = static_cast(node).recompute_inherited_style(); + node_invalidation = element.recompute_inherited_style(); } is_display_none = static_cast(node).computed_properties()->display().is_none(); } @@ -1464,12 +1466,16 @@ void Document::update_layout(UpdateLayoutReason reason) node.set_needs_style_update(false); invalidation |= node_invalidation; + if (did_change_custom_properties) { + recompute_elements_depending_on_custom_properties = true; + } + bool children_need_inherited_style_update = !invalidation.is_none(); - if (needs_full_style_update || node.child_needs_style_update() || children_need_inherited_style_update) { + if (needs_full_style_update || node.child_needs_style_update() || children_need_inherited_style_update || recompute_elements_depending_on_custom_properties) { if (node.is_element()) { if (auto shadow_root = static_cast(node).shadow_root()) { if (needs_full_style_update || shadow_root->needs_style_update() || shadow_root->child_needs_style_update()) { - auto subtree_invalidation = update_style_recursively(*shadow_root, style_computer, children_need_inherited_style_update); + auto subtree_invalidation = update_style_recursively(*shadow_root, style_computer, children_need_inherited_style_update, recompute_elements_depending_on_custom_properties); if (!is_display_none) invalidation |= subtree_invalidation; } @@ -1477,8 +1483,8 @@ void Document::update_layout(UpdateLayoutReason reason) } node.for_each_child([&](auto& child) { - if (needs_full_style_update || child.needs_style_update() || children_need_inherited_style_update || child.child_needs_style_update()) { - auto subtree_invalidation = update_style_recursively(child, style_computer, children_need_inherited_style_update); + if (needs_full_style_update || child.needs_style_update() || children_need_inherited_style_update || child.child_needs_style_update() || recompute_elements_depending_on_custom_properties) { + auto subtree_invalidation = update_style_recursively(child, style_computer, children_need_inherited_style_update, recompute_elements_depending_on_custom_properties); if (!is_display_none) invalidation |= subtree_invalidation; } @@ -1523,7 +1529,7 @@ void Document::update_style() style_computer().reset_ancestor_filter(); - auto invalidation = update_style_recursively(*this, style_computer(), false); + auto invalidation = update_style_recursively(*this, style_computer(), false, false); if (!invalidation.is_none()) invalidate_display_list(); if (invalidation.rebuild_stacking_context_tree) diff --git a/Libraries/LibWeb/DOM/Element.cpp b/Libraries/LibWeb/DOM/Element.cpp index 06672b16da3..50df89c7aa6 100644 --- a/Libraries/LibWeb/DOM/Element.cpp +++ b/Libraries/LibWeb/DOM/Element.cpp @@ -683,10 +683,12 @@ static CSS::RequiredInvalidationAfterStyleChange compute_required_invalidation(C return invalidation; } -CSS::RequiredInvalidationAfterStyleChange Element::recompute_style() +CSS::RequiredInvalidationAfterStyleChange Element::recompute_style(bool& did_change_custom_properties) { VERIFY(parent()); + m_style_uses_attr_css_function = false; + m_style_uses_var_css_function = false; m_affected_by_has_pseudo_class_in_subject_position = false; m_affected_by_has_pseudo_class_in_non_subject_position = false; m_affected_by_has_pseudo_class_with_relative_selector_that_has_sibling_combinator = false; @@ -697,7 +699,7 @@ CSS::RequiredInvalidationAfterStyleChange Element::recompute_style() m_sibling_invalidation_distance = 0; auto& style_computer = document().style_computer(); - auto new_computed_properties = style_computer.compute_style(*this); + auto new_computed_properties = style_computer.compute_style(*this, {}, did_change_custom_properties); // Tables must not inherit -libweb-* values for text-align. // FIXME: Find the spec for this. @@ -737,7 +739,7 @@ CSS::RequiredInvalidationAfterStyleChange Element::recompute_style() style_computer.push_ancestor(*this); auto pseudo_element_style = computed_properties(pseudo_element); - auto new_pseudo_element_style = style_computer.compute_pseudo_element_style_if_needed(*this, pseudo_element); + auto new_pseudo_element_style = style_computer.compute_pseudo_element_style_if_needed(*this, pseudo_element, did_change_custom_properties); // TODO: Can we be smarter about invalidation? if (pseudo_element_style && new_pseudo_element_style) { @@ -2472,24 +2474,13 @@ void Element::invalidate_style_after_attribute_change(FlyString const& attribute { Vector changed_properties; StyleInvalidationOptions style_invalidation_options; - if (is_presentational_hint(attribute_name)) { + if (is_presentational_hint(attribute_name) || style_uses_attr_css_function()) { style_invalidation_options.invalidate_self = true; } - if (style_uses_css_custom_properties()) { - // A css custom property can be hooked on to this element by any attribute - // so invalidate elements and rerender them in that scenario - style_invalidation_options.invalidate_elements_that_use_css_custom_properties = true; - } - if (attribute_name == HTML::AttributeNames::style) { style_invalidation_options.invalidate_self = true; - // even if we don't have custom properties, the new "style" attribute could add one - style_invalidation_options.invalidate_elements_that_use_css_custom_properties = true; } else if (attribute_name == HTML::AttributeNames::class_) { - // adding or removing classes can add new custom properties to this element - style_invalidation_options.invalidate_elements_that_use_css_custom_properties = true; - Vector old_classes; Vector new_classes; if (old_value.has_value()) diff --git a/Libraries/LibWeb/DOM/Element.h b/Libraries/LibWeb/DOM/Element.h index da24c609bc6..abf3a63da6f 100644 --- a/Libraries/LibWeb/DOM/Element.h +++ b/Libraries/LibWeb/DOM/Element.h @@ -201,7 +201,7 @@ public: void run_attribute_change_steps(FlyString const& local_name, Optional const& old_value, Optional const& value, Optional const& namespace_); - CSS::RequiredInvalidationAfterStyleChange recompute_style(); + CSS::RequiredInvalidationAfterStyleChange recompute_style(bool& did_change_custom_properties); CSS::RequiredInvalidationAfterStyleChange recompute_inherited_style(); Optional use_pseudo_element() const { return m_use_pseudo_element; } @@ -256,8 +256,10 @@ public: void set_custom_properties(Optional, HashMap custom_properties); [[nodiscard]] HashMap const& custom_properties(Optional) const; - bool style_uses_css_custom_properties() const { return m_style_uses_css_custom_properties; } - void set_style_uses_css_custom_properties(bool value) { m_style_uses_css_custom_properties = value; } + bool style_uses_attr_css_function() const { return m_style_uses_attr_css_function; } + void set_style_uses_attr_css_function() { m_style_uses_attr_css_function = true; } + bool style_uses_var_css_function() const { return m_style_uses_var_css_function; } + void set_style_uses_var_css_function() { m_style_uses_var_css_function = true; } // NOTE: The function is wrapped in a GC::HeapFunction immediately. HTML::TaskID queue_an_element_task(HTML::Task::Source, Function); @@ -605,7 +607,8 @@ private: bool m_in_top_layer : 1 { false }; bool m_rendered_in_top_layer : 1 { false }; - bool m_style_uses_css_custom_properties : 1 { false }; + bool m_style_uses_attr_css_function : 1 { false }; + bool m_style_uses_var_css_function : 1 { false }; bool m_affected_by_has_pseudo_class_in_subject_position : 1 { false }; bool m_affected_by_has_pseudo_class_in_non_subject_position : 1 { false }; bool m_affected_by_direct_sibling_combinator : 1 { false }; diff --git a/Libraries/LibWeb/DOM/Node.cpp b/Libraries/LibWeb/DOM/Node.cpp index 241dc014018..77cbdcc98a0 100644 --- a/Libraries/LibWeb/DOM/Node.cpp +++ b/Libraries/LibWeb/DOM/Node.cpp @@ -515,11 +515,11 @@ void Node::invalidate_style(StyleInvalidationReason reason, Vector const&, StyleInvalidationOptions); diff --git a/Libraries/LibWeb/DOM/StyleInvalidator.cpp b/Libraries/LibWeb/DOM/StyleInvalidator.cpp index 180cf17cba5..fde3a354940 100644 --- a/Libraries/LibWeb/DOM/StyleInvalidator.cpp +++ b/Libraries/LibWeb/DOM/StyleInvalidator.cpp @@ -26,15 +26,12 @@ void StyleInvalidator::invalidate(Node& node) m_pending_invalidations.clear(); } -void StyleInvalidator::add_pending_invalidation(GC::Ref node, CSS::InvalidationSet&& invalidation_set, bool invalidate_elements_that_use_css_custom_properties) +void StyleInvalidator::add_pending_invalidation(GC::Ref node, CSS::InvalidationSet&& invalidation_set) { - auto& pending_invalidation = m_pending_invalidations.ensure(node, [&] { - return PendingInvalidation {}; + auto& pending_invalidation_set = m_pending_invalidations.ensure(node, [&] { + return CSS::InvalidationSet {}; }); - pending_invalidation.invalidation_set.include_all_from(invalidation_set); - if (invalidate_elements_that_use_css_custom_properties) { - pending_invalidation.invalidate_elements_that_use_css_custom_properties = true; - } + pending_invalidation_set.include_all_from(invalidation_set); } // This function makes a full pass over the entire DOM and: @@ -51,24 +48,17 @@ void StyleInvalidator::perform_pending_style_invalidations(Node& node, bool inva } auto previous_subtree_invalidations_sets_size = m_subtree_invalidation_sets.size(); - auto previous_invalidate_elements_that_use_css_custom_properties = m_invalidate_elements_that_use_css_custom_properties; - ScopeGuard restore_state = [this, previous_subtree_invalidations_sets_size, previous_invalidate_elements_that_use_css_custom_properties] { + ScopeGuard restore_state = [this, previous_subtree_invalidations_sets_size] { m_subtree_invalidation_sets.shrink(previous_subtree_invalidations_sets_size); - m_invalidate_elements_that_use_css_custom_properties = previous_invalidate_elements_that_use_css_custom_properties; }; if (!invalidate_entire_subtree) { - auto pending_invalidation = m_pending_invalidations.get(node); - if (pending_invalidation.has_value()) { - m_subtree_invalidation_sets.append(pending_invalidation->invalidation_set); - m_invalidate_elements_that_use_css_custom_properties = m_invalidate_elements_that_use_css_custom_properties || pending_invalidation->invalidate_elements_that_use_css_custom_properties; + auto invalidation_set = m_pending_invalidations.get(node); + if (invalidation_set.has_value()) { + m_subtree_invalidation_sets.append(*invalidation_set); } auto affected_by_invalidation_sets_or_invalidation_flags = [this](Element const& element) { - if (m_invalidate_elements_that_use_css_custom_properties && element.style_uses_css_custom_properties()) { - return true; - } - for (auto& invalidation_set : m_subtree_invalidation_sets) { if (element.includes_properties_from_invalidation_set(invalidation_set)) return true; diff --git a/Libraries/LibWeb/DOM/StyleInvalidator.h b/Libraries/LibWeb/DOM/StyleInvalidator.h index 0f40494b39f..f85bcc4fccd 100644 --- a/Libraries/LibWeb/DOM/StyleInvalidator.h +++ b/Libraries/LibWeb/DOM/StyleInvalidator.h @@ -18,7 +18,7 @@ class StyleInvalidator : public GC::Cell { public: void invalidate(Node& node); - void add_pending_invalidation(GC::Ref, CSS::InvalidationSet&&, bool invalidate_elements_that_use_css_custom_properties); + void add_pending_invalidation(GC::Ref, CSS::InvalidationSet&&); bool has_pending_invalidations() const { return !m_pending_invalidations.is_empty(); } virtual void visit_edges(Cell::Visitor& visitor) override; @@ -26,14 +26,8 @@ public: private: void perform_pending_style_invalidations(Node& node, bool invalidate_entire_subtree); - struct PendingInvalidation { - bool invalidate_elements_that_use_css_custom_properties { false }; - CSS::InvalidationSet invalidation_set; - }; - HashMap, PendingInvalidation> m_pending_invalidations; - + HashMap, CSS::InvalidationSet> m_pending_invalidations; Vector m_subtree_invalidation_sets; - bool m_invalidate_elements_that_use_css_custom_properties { false }; }; }