From 36a255eedde53391941d459cf39bca224e4ed848 Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Wed, 28 May 2025 15:37:53 +0200 Subject: [PATCH] LibWeb: Fix glitchy CSS transitions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `start_needed_transitions()` decides which animations need to be started based on previous and current style property values. Before this change, we were using the style value without animations applied as the "current" value. This caused issues such as starting a new transition from the animation’s end value when an ongoing animation was interrupted. --- Libraries/LibWeb/CSS/CSSTransition.cpp | 10 ---------- Libraries/LibWeb/CSS/CSSTransition.h | 1 - Libraries/LibWeb/CSS/StyleComputer.cpp | 8 ++++---- 3 files changed, 4 insertions(+), 15 deletions(-) diff --git a/Libraries/LibWeb/CSS/CSSTransition.cpp b/Libraries/LibWeb/CSS/CSSTransition.cpp index 47f73a41f77..3f70ef4a2f9 100644 --- a/Libraries/LibWeb/CSS/CSSTransition.cpp +++ b/Libraries/LibWeb/CSS/CSSTransition.cpp @@ -150,14 +150,4 @@ double CSSTransition::timing_function_output_at_time(double t) const return m_keyframe_effect->timing_function().evaluate_at(progress, before_flag); } -NonnullRefPtr CSSTransition::value_at_time(double t, AllowDiscrete allow_discrete) const -{ - // https://drafts.csswg.org/css-transitions/#application - auto progress = timing_function_output_at_time(t); - auto result = interpolate_property(*m_keyframe_effect->target(), m_transition_property, m_start_value, m_end_value, progress, allow_discrete); - if (result) - return result.release_nonnull(); - return m_start_value; -} - } diff --git a/Libraries/LibWeb/CSS/CSSTransition.h b/Libraries/LibWeb/CSS/CSSTransition.h index 77a8bb48452..ea4bd8ed13a 100644 --- a/Libraries/LibWeb/CSS/CSSTransition.h +++ b/Libraries/LibWeb/CSS/CSSTransition.h @@ -38,7 +38,6 @@ public: double reversing_shortening_factor() const { return m_reversing_shortening_factor; } double timing_function_output_at_time(double t) const; - NonnullRefPtr value_at_time(double t, AllowDiscrete allow_discrete) const; // This is designed to be created from AnimationEffect::Phase. enum class Phase : u8 { diff --git a/Libraries/LibWeb/CSS/StyleComputer.cpp b/Libraries/LibWeb/CSS/StyleComputer.cpp index 4baf031cd73..6a55580ec00 100644 --- a/Libraries/LibWeb/CSS/StyleComputer.cpp +++ b/Libraries/LibWeb/CSS/StyleComputer.cpp @@ -1475,7 +1475,7 @@ void StyleComputer::start_needed_transitions(ComputedProperties const& previous_ for (auto i = to_underlying(CSS::first_longhand_property_id); i <= to_underlying(CSS::last_longhand_property_id); ++i) { auto property_id = static_cast(i); auto matching_transition_properties = element.property_transition_attributes(property_id); - auto const& before_change_value = previous_style.property(property_id, ComputedProperties::WithAnimationsApplied::No); + auto const& before_change_value = previous_style.property(property_id, ComputedProperties::WithAnimationsApplied::Yes); auto const& after_change_value = new_style.property(property_id, ComputedProperties::WithAnimationsApplied::No); auto existing_transition = element.property_transition(property_id); @@ -1488,7 +1488,7 @@ void StyleComputer::start_needed_transitions(ComputedProperties const& previous_ auto transition = CSSTransition::start_a_transition(element, property_id, document().transition_generation(), start_time, end_time, start_value, end_value, reversing_adjusted_start_value, reversing_shortening_factor); // Immediately set the property's value to the transition's current value, to prevent single-frame jumps. - new_style.set_animated_property(property_id, transition->value_at_time(style_change_event_time, matching_transition_properties->transition_behavior == TransitionBehavior::AllowDiscrete ? AllowDiscrete::Yes : AllowDiscrete::No)); + collect_animation_into(element, {}, as(*transition->effect()), new_style, AnimationRefresh::No); }; // 1. If all of the following are true: @@ -1560,8 +1560,8 @@ void StyleComputer::start_needed_transitions(ComputedProperties const& previous_ // 1. If the current value of the property in the running transition is equal to the value of the property in the after-change style, // or if these two values are not transitionable, // then implementations must cancel the running transition. - auto current_value = existing_transition->value_at_time(style_change_event_time, matching_transition_properties->transition_behavior == TransitionBehavior::AllowDiscrete ? AllowDiscrete::Yes : AllowDiscrete::No); - if (current_value->equals(after_change_value) || !property_values_are_transitionable(property_id, current_value, after_change_value, element, matching_transition_properties->transition_behavior)) { + auto& current_value = new_style.property(property_id, ComputedProperties::WithAnimationsApplied::Yes); + if (current_value.equals(after_change_value) || !property_values_are_transitionable(property_id, current_value, after_change_value, element, matching_transition_properties->transition_behavior)) { dbgln_if(CSS_TRANSITIONS_DEBUG, "Transition step 4.1"); existing_transition->cancel(); }