From 4049cce40cce31c45e528bbdf301b4e63b8912db Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Thu, 1 Aug 2024 15:43:44 +0300 Subject: [PATCH] LibWeb: Add slots for pseudo-elements animation cache in Animatable Fixes the bug when animation does not run at all if an element has a pseudo-element, because both of them use the same cache. --- .../LibWeb/Animations/Animatable.cpp | 38 ++++++++++++++++++- .../Libraries/LibWeb/Animations/Animatable.h | 13 ++++--- .../Libraries/LibWeb/CSS/StyleComputer.cpp | 16 ++++---- 3 files changed, 51 insertions(+), 16 deletions(-) diff --git a/Userland/Libraries/LibWeb/Animations/Animatable.cpp b/Userland/Libraries/LibWeb/Animations/Animatable.cpp index e0ea78139b7..5630637ed85 100644 --- a/Userland/Libraries/LibWeb/Animations/Animatable.cpp +++ b/Userland/Libraries/LibWeb/Animations/Animatable.cpp @@ -102,8 +102,42 @@ void Animatable::disassociate_with_animation(JS::NonnullGCPtr animati void Animatable::visit_edges(JS::Cell::Visitor& visitor) { visitor.visit(m_associated_animations); - visitor.visit(m_cached_animation_name_source); - visitor.visit(m_cached_animation_name_animation); + for (auto const& cached_animation_source : m_cached_animation_name_source) + visitor.visit(cached_animation_source); + for (auto const& cached_animation_name : m_cached_animation_name_animation) + visitor.visit(cached_animation_name); +} + +JS::GCPtr Animatable::cached_animation_name_source(Optional pseudo_element) const +{ + if (pseudo_element.has_value()) + return m_cached_animation_name_source[to_underlying(pseudo_element.value()) + 1]; + return m_cached_animation_name_source[0]; +} + +void Animatable::set_cached_animation_name_source(JS::GCPtr value, Optional pseudo_element) +{ + if (pseudo_element.has_value()) { + m_cached_animation_name_source[to_underlying(pseudo_element.value()) + 1] = value; + } else { + m_cached_animation_name_source[0] = value; + } +} + +JS::GCPtr Animatable::cached_animation_name_animation(Optional pseudo_element) const +{ + if (pseudo_element.has_value()) + return m_cached_animation_name_animation[to_underlying(pseudo_element.value()) + 1]; + return m_cached_animation_name_animation[0]; +} + +void Animatable::set_cached_animation_name_animation(JS::GCPtr value, Optional pseudo_element) +{ + if (pseudo_element.has_value()) { + m_cached_animation_name_animation[to_underlying(pseudo_element.value()) + 1] = value; + } else { + m_cached_animation_name_animation[0] = value; + } } } diff --git a/Userland/Libraries/LibWeb/Animations/Animatable.h b/Userland/Libraries/LibWeb/Animations/Animatable.h index 61b700894d5..9f4b374d76b 100644 --- a/Userland/Libraries/LibWeb/Animations/Animatable.h +++ b/Userland/Libraries/LibWeb/Animations/Animatable.h @@ -33,11 +33,11 @@ public: void associate_with_animation(JS::NonnullGCPtr); void disassociate_with_animation(JS::NonnullGCPtr); - JS::GCPtr cached_animation_name_source() const { return m_cached_animation_name_source; } - void set_cached_animation_name_source(JS::GCPtr value) { m_cached_animation_name_source = value; } + JS::GCPtr cached_animation_name_source(Optional) const; + void set_cached_animation_name_source(JS::GCPtr value, Optional); - JS::GCPtr cached_animation_name_animation() const { return m_cached_animation_name_animation; } - void set_cached_animation_name_animation(JS::GCPtr value) { m_cached_animation_name_animation = value; } + JS::GCPtr cached_animation_name_animation(Optional) const; + void set_cached_animation_name_animation(JS::GCPtr value, Optional); protected: void visit_edges(JS::Cell::Visitor&); @@ -45,8 +45,9 @@ protected: private: Vector> m_associated_animations; bool m_is_sorted_by_composite_order { true }; - JS::GCPtr m_cached_animation_name_source; - JS::GCPtr m_cached_animation_name_animation; + + Array, to_underlying(CSS::Selector::PseudoElement::Type::KnownPseudoElementCount) + 1> m_cached_animation_name_source; + Array, to_underlying(CSS::Selector::PseudoElement::Type::KnownPseudoElementCount) + 1> m_cached_animation_name_animation; }; } diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp index d6ddfb53bc0..b1e6a456e81 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp +++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp @@ -1774,11 +1774,11 @@ void StyleComputer::compute_cascaded_values(StyleProperties& style, DOM::Element if (auto source_declaration = style.property_source_declaration(PropertyID::AnimationName); source_declaration) { auto& realm = element.realm(); - if (source_declaration != element.cached_animation_name_source()) { + if (source_declaration != element.cached_animation_name_source(pseudo_element)) { // This animation name is new, so we need to create a new animation for it. - if (auto existing_animation = element.cached_animation_name_animation()) + if (auto existing_animation = element.cached_animation_name_animation(pseudo_element)) existing_animation->cancel(Animations::Animation::ShouldInvalidate::No); - element.set_cached_animation_name_source(source_declaration); + element.set_cached_animation_name_source(source_declaration, pseudo_element); auto effect = Animations::KeyframeEffect::create(realm); auto animation = CSSAnimation::create(realm); @@ -1795,21 +1795,21 @@ void StyleComputer::compute_cascaded_values(StyleProperties& style, DOM::Element effect->set_key_frame_set(keyframe_set.value()); effect->set_target(&element); - element.set_cached_animation_name_animation(animation); + element.set_cached_animation_name_animation(animation, pseudo_element); HTML::TemporaryExecutionContext context(m_document->relevant_settings_object()); animation->play().release_value_but_fixme_should_propagate_errors(); } else { // The animation hasn't changed, but some properties of the animation may have - apply_animation_properties(m_document, style, *element.cached_animation_name_animation()); + apply_animation_properties(m_document, style, *element.cached_animation_name_animation(pseudo_element)); } } } else { // If the element had an existing animation, cancel it - if (auto existing_animation = element.cached_animation_name_animation()) { + if (auto existing_animation = element.cached_animation_name_animation(pseudo_element)) { existing_animation->cancel(Animations::Animation::ShouldInvalidate::No); - element.set_cached_animation_name_animation({}); - element.set_cached_animation_name_source({}); + element.set_cached_animation_name_animation({}, pseudo_element); + element.set_cached_animation_name_source({}, pseudo_element); } }