Skip to content

Commit dd9d6d2

Browse files
Calme1709awesomekling
authored andcommitted
LibWeb: Iterate over fewer properties in start_needed_transitions
We don't need to iterate every property in start_needed_transitions, only those that appear in transition-property or have an existing transition Reduces the time spent in start_needed_transitions from ~5% to ~0.03% when loading https://en.wikipedia.org/wiki/2023_in_American_television
1 parent bbb344d commit dd9d6d2

File tree

3 files changed

+58
-28
lines changed

3 files changed

+58
-28
lines changed

Libraries/LibWeb/Animations/Animatable.cpp

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -179,6 +179,16 @@ void Animatable::add_transitioned_properties(Optional<CSS::PseudoElement> pseudo
179179
}
180180
}
181181

182+
Vector<CSS::PropertyID> Animatable::property_ids_with_matching_transition_property_entry(Optional<CSS::PseudoElement> pseudo_element) const
183+
{
184+
auto const* maybe_transition = ensure_transition(pseudo_element);
185+
186+
if (!maybe_transition)
187+
return {};
188+
189+
return maybe_transition->transition_attribute_indices.keys();
190+
}
191+
182192
Optional<Animatable::TransitionAttributes const&> Animatable::property_transition_attributes(Optional<CSS::PseudoElement> pseudo_element, CSS::PropertyID property) const
183193
{
184194
auto* maybe_transition = ensure_transition(pseudo_element);
@@ -190,6 +200,16 @@ Optional<Animatable::TransitionAttributes const&> Animatable::property_transitio
190200
return {};
191201
}
192202

203+
Vector<CSS::PropertyID> Animatable::property_ids_with_existing_transitions(Optional<CSS::PseudoElement> pseudo_element) const
204+
{
205+
auto const* maybe_transition = ensure_transition(pseudo_element);
206+
207+
if (!maybe_transition)
208+
return {};
209+
210+
return maybe_transition->associated_transitions.keys();
211+
}
212+
193213
GC::Ptr<CSS::CSSTransition> Animatable::property_transition(Optional<CSS::PseudoElement> pseudo_element, CSS::PropertyID property) const
194214
{
195215
auto* maybe_transition = ensure_transition(pseudo_element);

Libraries/LibWeb/Animations/Animatable.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,11 @@ class WEB_API Animatable {
6060
void set_cached_transition_property_source(Optional<CSS::PseudoElement>, GC::Ptr<CSS::CSSStyleDeclaration const> value);
6161

6262
void add_transitioned_properties(Optional<CSS::PseudoElement>, Vector<Vector<CSS::PropertyID>> properties, CSS::StyleValueVector delays, CSS::StyleValueVector durations, CSS::StyleValueVector timing_functions, CSS::StyleValueVector transition_behaviors);
63+
Vector<CSS::PropertyID> property_ids_with_matching_transition_property_entry(Optional<CSS::PseudoElement>) const;
6364
Optional<TransitionAttributes const&> property_transition_attributes(Optional<CSS::PseudoElement>, CSS::PropertyID) const;
6465
void set_transition(Optional<CSS::PseudoElement>, CSS::PropertyID, GC::Ref<CSS::CSSTransition>);
6566
void remove_transition(Optional<CSS::PseudoElement>, CSS::PropertyID);
67+
Vector<CSS::PropertyID> property_ids_with_existing_transitions(Optional<CSS::PseudoElement>) const;
6668
GC::Ptr<CSS::CSSTransition> property_transition(Optional<CSS::PseudoElement>, CSS::PropertyID) const;
6769
void clear_transitions(Optional<CSS::PseudoElement>);
6870

Libraries/LibWeb/CSS/StyleComputer.cpp

Lines changed: 36 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1374,9 +1374,10 @@ void StyleComputer::start_needed_transitions(ComputedProperties const& previous_
13741374
auto& element = abstract_element.element();
13751375
auto pseudo_element = abstract_element.pseudo_element();
13761376

1377-
for (auto i = to_underlying(CSS::first_longhand_property_id); i <= to_underlying(CSS::last_longhand_property_id); ++i) {
1378-
auto property_id = static_cast<CSS::PropertyID>(i);
1379-
auto matching_transition_properties = element.property_transition_attributes(pseudo_element, property_id);
1377+
// OPTIMIZATION: Instead of iterating over all properties we split the logic into two loops, one for the properties
1378+
// which appear in transition-property and one for those which have existing transitions
1379+
for (auto property_id : element.property_ids_with_matching_transition_property_entry(pseudo_element)) {
1380+
auto matching_transition_properties = element.property_transition_attributes(pseudo_element, property_id).value();
13801381
auto const& before_change_value = previous_style.property(property_id, ComputedProperties::WithAnimationsApplied::Yes);
13811382
auto const& after_change_value = new_style.property(property_id, ComputedProperties::WithAnimationsApplied::No);
13821383

@@ -1398,14 +1399,14 @@ void StyleComputer::start_needed_transitions(ComputedProperties const& previous_
13981399
// - the element does not have a running transition for the property,
13991400
(!has_running_transition) &&
14001401
// - there is a matching transition-property value, and
1401-
(matching_transition_properties.has_value()) &&
1402+
// NOTE: We only iterate over properties for which this is true
14021403
// - the before-change style is different from the after-change style for that property, and the values for the property are transitionable,
1403-
(!before_change_value.equals(after_change_value) && property_values_are_transitionable(property_id, before_change_value, after_change_value, element, matching_transition_properties->transition_behavior)) &&
1404+
(!before_change_value.equals(after_change_value) && property_values_are_transitionable(property_id, before_change_value, after_change_value, element, matching_transition_properties.transition_behavior)) &&
14041405
// - the element does not have a completed transition for the property
14051406
// or the end value of the completed transition is different from the after-change style for the property,
14061407
(!has_completed_transition || !existing_transition->transition_end_value()->equals(after_change_value)) &&
14071408
// - the combined duration is greater than 0s,
1408-
(combined_duration(matching_transition_properties.value()) > 0)) {
1409+
(combined_duration(matching_transition_properties) > 0)) {
14091410

14101411
dbgln_if(CSS_TRANSITIONS_DEBUG, "Transition step 1.");
14111412

@@ -1415,13 +1416,13 @@ void StyleComputer::start_needed_transitions(ComputedProperties const& previous_
14151416
// and start a transition whose:
14161417

14171418
// AD-HOC: We pass delay to the constructor separately so we can use it to construct the contained KeyframeEffect
1418-
auto delay = matching_transition_properties->delay;
1419+
auto delay = matching_transition_properties.delay;
14191420

14201421
// - start time is the time of the style change event plus the matching transition delay,
14211422
auto start_time = style_change_event_time;
14221423

14231424
// - end time is the start time plus the matching transition duration,
1424-
auto end_time = start_time + matching_transition_properties->duration;
1425+
auto end_time = start_time + matching_transition_properties.duration;
14251426

14261427
// - start value is the value of the transitioning property in the before-change style,
14271428
auto const& start_value = before_change_value;
@@ -1446,36 +1447,27 @@ void StyleComputer::start_needed_transitions(ComputedProperties const& previous_
14461447
element.remove_transition(pseudo_element, property_id);
14471448
}
14481449

1449-
// 3. If the element has a running transition or completed transition for the property,
1450-
// and there is not a matching transition-property value,
1451-
if (existing_transition && !matching_transition_properties.has_value()) {
1452-
// then implementations must cancel the running transition or remove the completed transition from the set of completed transitions.
1453-
dbgln_if(CSS_TRANSITIONS_DEBUG, "Transition step 3.");
1454-
if (has_running_transition)
1455-
existing_transition->cancel();
1456-
else
1457-
element.remove_transition(pseudo_element, property_id);
1458-
}
1450+
// NOTE: Step 3 is handled in a separate loop below for performance reasons
14591451

14601452
// 4. If the element has a running transition for the property,
14611453
// there is a matching transition-property value,
14621454
// and the end value of the running transition is not equal to the value of the property in the after-change style, then:
1463-
if (has_running_transition && matching_transition_properties.has_value() && !existing_transition->transition_end_value()->equals(after_change_value)) {
1455+
if (has_running_transition && !existing_transition->transition_end_value()->equals(after_change_value)) {
14641456
dbgln_if(CSS_TRANSITIONS_DEBUG, "Transition step 4. existing end value = {}, after change value = {}", existing_transition->transition_end_value()->to_string(SerializationMode::Normal), after_change_value.to_string(SerializationMode::Normal));
14651457
// 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,
14661458
// or if these two values are not transitionable,
14671459
// then implementations must cancel the running transition.
14681460
auto& current_value = new_style.property(property_id, ComputedProperties::WithAnimationsApplied::Yes);
1469-
if (current_value.equals(after_change_value) || !property_values_are_transitionable(property_id, current_value, after_change_value, element, matching_transition_properties->transition_behavior)) {
1461+
if (current_value.equals(after_change_value) || !property_values_are_transitionable(property_id, current_value, after_change_value, element, matching_transition_properties.transition_behavior)) {
14701462
dbgln_if(CSS_TRANSITIONS_DEBUG, "Transition step 4.1");
14711463
existing_transition->cancel();
14721464
}
14731465

14741466
// 2. Otherwise, if the combined duration is less than or equal to 0s,
14751467
// or if the current value of the property in the running transition is not transitionable with the value of the property in the after-change style,
14761468
// then implementations must cancel the running transition.
1477-
else if ((combined_duration(matching_transition_properties.value()) <= 0)
1478-
|| !property_values_are_transitionable(property_id, current_value, after_change_value, element, matching_transition_properties->transition_behavior)) {
1469+
else if ((combined_duration(matching_transition_properties) <= 0)
1470+
|| !property_values_are_transitionable(property_id, current_value, after_change_value, element, matching_transition_properties.transition_behavior)) {
14791471
dbgln_if(CSS_TRANSITIONS_DEBUG, "Transition step 4.2");
14801472
existing_transition->cancel();
14811473
}
@@ -1502,17 +1494,17 @@ void StyleComputer::start_needed_transitions(ComputedProperties const& previous_
15021494
double reversing_shortening_factor = clamp(abs(term_1 + term_2), 0.0, 1.0);
15031495

15041496
// AD-HOC: We pass delay to the constructor separately so we can use it to construct the contained KeyframeEffect
1505-
auto delay = (matching_transition_properties->delay >= 0
1506-
? (matching_transition_properties->delay)
1507-
: (reversing_shortening_factor * matching_transition_properties->delay));
1497+
auto delay = (matching_transition_properties.delay >= 0
1498+
? (matching_transition_properties.delay)
1499+
: (reversing_shortening_factor * matching_transition_properties.delay));
15081500

15091501
// - start time is the time of the style change event plus:
15101502
// 1. if the matching transition delay is nonnegative, the matching transition delay, or
15111503
// 2. if the matching transition delay is negative, the product of the new transition’s reversing shortening factor and the matching transition delay,
15121504
auto start_time = style_change_event_time;
15131505

15141506
// - end time is the start time plus the product of the matching transition duration and the new transition’s reversing shortening factor,
1515-
auto end_time = start_time + (matching_transition_properties->duration * reversing_shortening_factor);
1507+
auto end_time = start_time + (matching_transition_properties.duration * reversing_shortening_factor);
15161508

15171509
// - start value is the current value of the property in the running transition,
15181510
auto const& start_value = current_value;
@@ -1533,13 +1525,13 @@ void StyleComputer::start_needed_transitions(ComputedProperties const& previous_
15331525
element.remove_transition(pseudo_element, property_id);
15341526

15351527
// AD-HOC: We pass delay to the constructor separately so we can use it to construct the contained KeyframeEffect
1536-
auto delay = matching_transition_properties->delay;
1528+
auto delay = matching_transition_properties.delay;
15371529

15381530
// - start time is the time of the style change event plus the matching transition delay,
15391531
auto start_time = style_change_event_time;
15401532

15411533
// - end time is the start time plus the matching transition duration,
1542-
auto end_time = start_time + matching_transition_properties->duration;
1534+
auto end_time = start_time + matching_transition_properties.duration;
15431535

15441536
// - start value is the current value of the property in the running transition,
15451537
auto const& start_value = current_value;
@@ -1557,6 +1549,22 @@ void StyleComputer::start_needed_transitions(ComputedProperties const& previous_
15571549
}
15581550
}
15591551
}
1552+
1553+
for (auto property_id : element.property_ids_with_existing_transitions(pseudo_element)) {
1554+
// 3. If the element has a running transition or completed transition for the property, and there is not a
1555+
// matching transition-property value, then implementations must cancel the running transition or remove the
1556+
// completed transition from the set of completed transitions.
1557+
if (element.property_transition_attributes(pseudo_element, property_id).has_value())
1558+
continue;
1559+
1560+
auto const& existing_transition = element.property_transition(pseudo_element, property_id);
1561+
1562+
dbgln_if(CSS_TRANSITIONS_DEBUG, "Transition step 3.");
1563+
if (!existing_transition->is_finished())
1564+
existing_transition->cancel();
1565+
else
1566+
element.remove_transition(pseudo_element, property_id);
1567+
}
15601568
}
15611569

15621570
StyleComputer::MatchingRuleSet StyleComputer::build_matching_rule_set(DOM::AbstractElement abstract_element, PseudoClassBitmap& attempted_pseudo_class_matches, bool& did_match_any_pseudo_element_rules, ComputeStyleMode mode) const

0 commit comments

Comments
 (0)