Skip to content

Commit 693dd7b

Browse files
Psychpsyogmta
authored andcommitted
LibWeb: Avoid unnecessary sorting work when getting animations
This way, the list is not re-sorted on every recursive call.
1 parent 1abc91c commit 693dd7b

File tree

6 files changed

+26
-11
lines changed

6 files changed

+26
-11
lines changed

Libraries/LibWeb/Animations/Animatable.cpp

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,10 @@ WebIDL::ExceptionOr<GC::Ref<Animation>> Animatable::animate(Optional<GC::Root<JS
7272
WebIDL::ExceptionOr<Vector<GC::Ref<Animation>>> Animatable::get_animations(Optional<GetAnimationsOptions> options)
7373
{
7474
as<DOM::Element>(*this).document().update_style();
75-
return get_animations_internal(options);
75+
return get_animations_internal(GetAnimationsSorted::Yes, options);
7676
}
7777

78-
WebIDL::ExceptionOr<Vector<GC::Ref<Animation>>> Animatable::get_animations_internal(Optional<GetAnimationsOptions> options)
78+
WebIDL::ExceptionOr<Vector<GC::Ref<Animation>>> Animatable::get_animations_internal(GetAnimationsSorted sorted, Optional<GetAnimationsOptions> options)
7979
{
8080
// 1. Let object be the object on which this method was called.
8181

@@ -107,18 +107,20 @@ WebIDL::ExceptionOr<Vector<GC::Ref<Animation>>> Animatable::get_animations_inter
107107
if (options.has_value() && options->subtree) {
108108
Optional<WebIDL::Exception> exception;
109109
TRY(target->for_each_child_of_type_fallible<DOM::Element>([&](auto& child) -> WebIDL::ExceptionOr<IterationDecision> {
110-
relevant_animations.extend(TRY(child.get_animations(options)));
110+
relevant_animations.extend(TRY(child.get_animations_internal(GetAnimationsSorted::No, options)));
111111
return IterationDecision::Continue;
112112
}));
113113
}
114114

115115
// The returned list is sorted using the composite order described for the associated animations of effects in
116116
// §5.4.2 The effect stack.
117-
quick_sort(relevant_animations, [](GC::Ref<Animation>& a, GC::Ref<Animation>& b) {
118-
auto& a_effect = as<KeyframeEffect>(*a->effect());
119-
auto& b_effect = as<KeyframeEffect>(*b->effect());
120-
return KeyframeEffect::composite_order(a_effect, b_effect) < 0;
121-
});
117+
if (sorted == GetAnimationsSorted::Yes) {
118+
quick_sort(relevant_animations, [](GC::Ref<Animation>& a, GC::Ref<Animation>& b) {
119+
auto& a_effect = as<KeyframeEffect>(*a->effect());
120+
auto& b_effect = as<KeyframeEffect>(*b->effect());
121+
return KeyframeEffect::composite_order(a_effect, b_effect) < 0;
122+
});
123+
}
122124

123125
return relevant_animations;
124126
}

Libraries/LibWeb/Animations/Animatable.h

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,14 @@ class WEB_API Animatable {
4545

4646
virtual ~Animatable() = default;
4747

48+
enum class GetAnimationsSorted {
49+
No,
50+
Yes
51+
};
52+
4853
WebIDL::ExceptionOr<GC::Ref<Animation>> animate(Optional<GC::Root<JS::Object>> keyframes, Variant<Empty, double, KeyframeAnimationOptions> options = {});
4954
WebIDL::ExceptionOr<Vector<GC::Ref<Animation>>> get_animations(Optional<GetAnimationsOptions> options = {});
50-
WebIDL::ExceptionOr<Vector<GC::Ref<Animation>>> get_animations_internal(Optional<GetAnimationsOptions> options = {});
55+
WebIDL::ExceptionOr<Vector<GC::Ref<Animation>>> get_animations_internal(GetAnimationsSorted sorted, Optional<GetAnimationsOptions> options = {});
5156

5257
void associate_with_animation(GC::Ref<Animation>);
5358
void disassociate_with_animation(GC::Ref<Animation>);

Libraries/LibWeb/CSS/StyleComputer.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
#include <LibGfx/Font/Typeface.h>
2424
#include <LibGfx/Font/WOFF/Loader.h>
2525
#include <LibGfx/Font/WOFF2/Loader.h>
26+
#include <LibWeb/Animations/Animatable.h>
2627
#include <LibWeb/Animations/AnimationEffect.h>
2728
#include <LibWeb/Animations/DocumentTimeline.h>
2829
#include <LibWeb/Bindings/PrincipalHostDefined.h>
@@ -2573,7 +2574,9 @@ GC::Ref<ComputedProperties> StyleComputer::compute_properties(DOM::AbstractEleme
25732574
// 5. Add or modify CSS-defined animations
25742575
process_animation_definitions(computed_style, abstract_element);
25752576

2576-
auto animations = abstract_element.element().get_animations_internal(Animations::GetAnimationsOptions { .subtree = false });
2577+
auto animations = abstract_element.element().get_animations_internal(
2578+
Animations::Animatable::GetAnimationsSorted::Yes,
2579+
Animations::GetAnimationsOptions { .subtree = false });
25772580
if (animations.is_exception()) {
25782581
dbgln("Error getting animations for element {}", abstract_element.debug_description());
25792582
} else {

Libraries/LibWeb/DOM/Document.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5457,6 +5457,7 @@ void Document::remove_replaced_animations()
54575457

54585458
WebIDL::ExceptionOr<Vector<GC::Ref<Animations::Animation>>> Document::get_animations()
54595459
{
5460+
update_style();
54605461
return calculate_get_animations(*this);
54615462
}
54625463

Libraries/LibWeb/DOM/DocumentOrShadowRoot.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#pragma once
99

10+
#include <LibWeb/Animations/Animatable.h>
1011
#include <LibWeb/Animations/Animation.h>
1112
#include <LibWeb/DOM/Document.h>
1213
#include <LibWeb/DOM/Utils.h>
@@ -56,7 +57,9 @@ WebIDL::ExceptionOr<Vector<GC::Ref<Animations::Animation>>> calculate_get_animat
5657
// method is called.
5758
Vector<GC::Ref<Animations::Animation>> relevant_animations;
5859
TRY(self.template for_each_child_of_type_fallible<Element>([&](auto& child) -> WebIDL::ExceptionOr<IterationDecision> {
59-
relevant_animations.extend(TRY(child.get_animations(Animations::GetAnimationsOptions { .subtree = true })));
60+
relevant_animations.extend(TRY(child.get_animations_internal(
61+
Animations::Animatable::GetAnimationsSorted::No,
62+
Animations::GetAnimationsOptions { .subtree = true })));
6063
return IterationDecision::Continue;
6164
}));
6265

Libraries/LibWeb/DOM/ShadowRoot.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ void ShadowRoot::for_each_active_css_style_sheet(Function<void(CSS::CSSStyleShee
215215

216216
WebIDL::ExceptionOr<Vector<GC::Ref<Animations::Animation>>> ShadowRoot::get_animations()
217217
{
218+
document().update_style();
218219
return calculate_get_animations(*this);
219220
}
220221

0 commit comments

Comments
 (0)