Commit ddd9f0ee authored by Stephen McGruer's avatar Stephen McGruer Committed by Commit Bot

Composite ScrollTimeline scrollSource when appropriate.

This CL makes being the scrollSource of an active (i.e. the scrollSource is
scrollable) and attached (i.e. it is being used by at least one WorkletAnimation
as a timeline) ScrollTimeline a compositing reason. It does not change the
requirement that the scrollSource must be composited for the WorkletAnimation to
be sent to the compositor, nor does it change the case where a scrollSource stops
being composited whilst a WorkletAnimation is running. Those will be addressed in
follow-up CLs.

Bug: 776533
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Icae43e1c9cfe2720cc739401c4766b592571f231
Reviewed-on: https://chromium-review.googlesource.com/1036934
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Reviewed-by: default avatarMajid Valipour <majidvp@chromium.org>
Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#556541}
parent cbce434f
...@@ -16,7 +16,6 @@ ...@@ -16,7 +16,6 @@
overflow: auto; overflow: auto;
height: 100px; height: 100px;
width: 100px; width: 100px;
will-change: transform; /* force compositing */
} }
#contents { #contents {
......
...@@ -1633,6 +1633,7 @@ jumbo_source_set("unit_tests") { ...@@ -1633,6 +1633,7 @@ jumbo_source_set("unit_tests") {
"animation/keyframe_effect_model_test.cc", "animation/keyframe_effect_model_test.cc",
"animation/keyframe_effect_test.cc", "animation/keyframe_effect_test.cc",
"animation/property_handle_test.cc", "animation/property_handle_test.cc",
"animation/scroll_timeline_test.cc",
"animation/timing_calculations_test.cc", "animation/timing_calculations_test.cc",
"animation/timing_input_test.cc", "animation/timing_input_test.cc",
"clipboard/data_object_test.cc", "clipboard/data_object_test.cc",
......
...@@ -6,11 +6,18 @@ ...@@ -6,11 +6,18 @@
#include "third_party/blink/renderer/core/dom/exception_code.h" #include "third_party/blink/renderer/core/dom/exception_code.h"
#include "third_party/blink/renderer/core/layout/layout_box.h" #include "third_party/blink/renderer/core/layout/layout_box.h"
#include "third_party/blink/renderer/core/layout/layout_view.h"
#include "third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h" #include "third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h"
namespace blink { namespace blink {
namespace { namespace {
using ActiveScrollTimelineSet = PersistentHeapHashCountedSet<WeakMember<Node>>;
ActiveScrollTimelineSet& GetActiveScrollTimelineSet() {
DEFINE_STATIC_LOCAL(ActiveScrollTimelineSet, set, ());
return set;
}
bool StringToScrollDirection(String scroll_direction, bool StringToScrollDirection(String scroll_direction,
ScrollTimeline::ScrollDirection& result) { ScrollTimeline::ScrollDirection& result) {
// TODO(smcgruer): Support 'auto' value. // TODO(smcgruer): Support 'auto' value.
...@@ -46,17 +53,17 @@ ScrollTimeline* ScrollTimeline::Create(Document& document, ...@@ -46,17 +53,17 @@ ScrollTimeline* ScrollTimeline::Create(Document& document,
return nullptr; return nullptr;
} }
return new ScrollTimeline(document, scroll_source, orientation, return new ScrollTimeline(scroll_source, orientation,
options.timeRange().GetAsDouble()); options.timeRange().GetAsDouble());
} }
ScrollTimeline::ScrollTimeline(const Document& document, ScrollTimeline::ScrollTimeline(Element* scroll_source,
Element* scroll_source,
ScrollDirection orientation, ScrollDirection orientation,
double time_range) double time_range)
: scroll_source_(scroll_source), : scroll_source_(scroll_source),
orientation_(orientation), orientation_(orientation),
time_range_(time_range) { time_range_(time_range) {
DCHECK(scroll_source_);
} }
double ScrollTimeline::currentTime(bool& is_null) { double ScrollTimeline::currentTime(bool& is_null) {
...@@ -140,9 +147,36 @@ void ScrollTimeline::timeRange(DoubleOrScrollTimelineAutoKeyword& result) { ...@@ -140,9 +147,36 @@ void ScrollTimeline::timeRange(DoubleOrScrollTimelineAutoKeyword& result) {
result.SetDouble(time_range_); result.SetDouble(time_range_);
} }
void ScrollTimeline::AttachAnimation() {
if (!scroll_source_)
return;
GetActiveScrollTimelineSet().insert(scroll_source_);
scroll_source_->GetDocument()
.GetLayoutView()
->Compositor()
->SetNeedsCompositingUpdate(kCompositingUpdateRebuildTree);
}
void ScrollTimeline::DetachAnimation() {
if (!scroll_source_)
return;
DCHECK(GetActiveScrollTimelineSet().Contains(scroll_source_));
GetActiveScrollTimelineSet().erase(scroll_source_);
scroll_source_->GetDocument()
.GetLayoutView()
->Compositor()
->SetNeedsCompositingUpdate(kCompositingUpdateRebuildTree);
}
void ScrollTimeline::Trace(blink::Visitor* visitor) { void ScrollTimeline::Trace(blink::Visitor* visitor) {
visitor->Trace(scroll_source_); visitor->Trace(scroll_source_);
AnimationTimeline::Trace(visitor); AnimationTimeline::Trace(visitor);
} }
bool ScrollTimeline::HasActiveScrollTimeline(Node* node) {
ActiveScrollTimelineSet& set = GetActiveScrollTimelineSet();
auto it = set.find(node);
return it != set.end() && it->value > 0;
}
} // namespace blink } // namespace blink
...@@ -46,10 +46,22 @@ class CORE_EXPORT ScrollTimeline final : public AnimationTimeline { ...@@ -46,10 +46,22 @@ class CORE_EXPORT ScrollTimeline final : public AnimationTimeline {
ScrollDirection GetOrientation() const { return orientation_; } ScrollDirection GetOrientation() const { return orientation_; }
// Must be called when this ScrollTimeline is attached/unattached from an
// animation.
void AttachAnimation();
void DetachAnimation();
void Trace(blink::Visitor*) override; void Trace(blink::Visitor*) override;
// For the AnimationWorklet origin trial, we need to automatically composite
// elements that are targets of ScrollTimelines (http://crbug.com/776533). We
// expose a static lookup method to enable this.
//
// TODO(crbug.com/839341): Remove once WorkletAnimations can run on main.
static bool HasActiveScrollTimeline(Node* node);
private: private:
ScrollTimeline(const Document&, Element*, ScrollDirection, double); ScrollTimeline(Element*, ScrollDirection, double);
Member<Element> scroll_source_; Member<Element> scroll_source_;
ScrollDirection orientation_; ScrollDirection orientation_;
......
// Copyright 2018 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "third_party/blink/renderer/core/animation/scroll_timeline.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/renderer/bindings/core/v8/exception_state.h"
#include "third_party/blink/renderer/core/paint/paint_layer.h"
#include "third_party/blink/renderer/core/testing/core_unit_test_helper.h"
#include "third_party/blink/renderer/core/testing/dummy_page_holder.h"
namespace blink {
using ScrollTimelineTest = RenderingTest;
TEST_F(ScrollTimelineTest,
AttachingAndDetachingAnimationCausesCompositingUpdate) {
EnableCompositing();
SetBodyInnerHTML(R"HTML(
<style>#scroller { overflow: scroll; width: 100px; height: 100px; }</style>
<div id='scroller'></div>
)HTML");
LayoutBoxModelObject* scroller =
ToLayoutBoxModelObject(GetLayoutObjectByElementId("scroller"));
ASSERT_TRUE(scroller);
// Invariant: the scroller is not composited by default.
EXPECT_EQ(DocumentLifecycle::kPaintClean,
GetDocument().Lifecycle().GetState());
EXPECT_EQ(kNotComposited, scroller->Layer()->GetCompositingState());
// Create the ScrollTimeline. This shouldn't cause the scrollSource to need
// compositing, as it isn't attached to any animation yet.
ScrollTimelineOptions options;
DoubleOrScrollTimelineAutoKeyword time_range =
DoubleOrScrollTimelineAutoKeyword::FromDouble(100);
options.setTimeRange(time_range);
options.setScrollSource(GetElementById("scroller"));
ScrollTimeline* scroll_timeline =
ScrollTimeline::Create(GetDocument(), options, ASSERT_NO_EXCEPTION);
EXPECT_EQ(DocumentLifecycle::kPaintClean,
GetDocument().Lifecycle().GetState());
EXPECT_EQ(kNotComposited, scroller->Layer()->GetCompositingState());
// Now attach an animation. This should require a compositing update.
scroll_timeline->AttachAnimation();
UpdateAllLifecyclePhases();
EXPECT_NE(scroller->Layer()->GetCompositingState(), kNotComposited);
// Now detach an animation. This should again require a compositing update.
scroll_timeline->DetachAnimation();
UpdateAllLifecyclePhases();
EXPECT_EQ(scroller->Layer()->GetCompositingState(), kNotComposited);
}
} // namespace blink
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "third_party/blink/renderer/core/paint/compositing/compositing_reason_finder.h" #include "third_party/blink/renderer/core/paint/compositing/compositing_reason_finder.h"
#include "third_party/blink/renderer/core/animation/scroll_timeline.h"
#include "third_party/blink/renderer/core/css_property_names.h" #include "third_party/blink/renderer/core/css_property_names.h"
#include "third_party/blink/renderer/core/dom/document.h" #include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/frame/local_frame_view.h" #include "third_party/blink/renderer/core/frame/local_frame_view.h"
...@@ -177,6 +178,14 @@ CompositingReasons CompositingReasonFinder::NonStyleDeterminedDirectReasons( ...@@ -177,6 +178,14 @@ CompositingReasons CompositingReasonFinder::NonStyleDeterminedDirectReasons(
if (RequiresCompositingForScrollDependentPosition(layer, ignore_lcd_text)) if (RequiresCompositingForScrollDependentPosition(layer, ignore_lcd_text))
direct_reasons |= CompositingReason::kScrollDependentPosition; direct_reasons |= CompositingReason::kScrollDependentPosition;
// TODO(crbug.com/839341): Remove once we support main-thread AnimationWorklet
// and don't need to promote the scroll-source.
if (layer->GetScrollableArea() && layer->GetLayoutObject().GetNode() &&
ScrollTimeline::HasActiveScrollTimeline(
layer->GetLayoutObject().GetNode())) {
direct_reasons |= CompositingReason::kScrollTimelineTarget;
}
direct_reasons |= layout_object.AdditionalCompositingReasons(); direct_reasons |= layout_object.AdditionalCompositingReasons();
DCHECK( DCHECK(
......
...@@ -231,6 +231,9 @@ WorkletAnimation::WorkletAnimation( ...@@ -231,6 +231,9 @@ WorkletAnimation::WorkletAnimation(
AnimationEffect* target_effect = effects_.at(0); AnimationEffect* target_effect = effects_.at(0);
target_effect->Attach(this); target_effect->Attach(this);
if (timeline_.IsScrollTimeline())
timeline.GetAsScrollTimeline()->AttachAnimation();
} }
String WorkletAnimation::playState() { String WorkletAnimation::playState() {
...@@ -431,6 +434,8 @@ KeyframeEffect* WorkletAnimation::GetEffect() const { ...@@ -431,6 +434,8 @@ KeyframeEffect* WorkletAnimation::GetEffect() const {
void WorkletAnimation::Dispose() { void WorkletAnimation::Dispose() {
DCHECK(IsMainThread()); DCHECK(IsMainThread());
if (timeline_.IsScrollTimeline())
timeline_.GetAsScrollTimeline()->DetachAnimation();
DestroyCompositorAnimation(); DestroyCompositorAnimation();
} }
......
...@@ -35,6 +35,7 @@ using CompositingReasons = uint64_t; ...@@ -35,6 +35,7 @@ using CompositingReasons = uint64_t;
V(WillChangeCompositingHint) \ V(WillChangeCompositingHint) \
V(BackdropFilter) \ V(BackdropFilter) \
V(RootScroller) \ V(RootScroller) \
V(ScrollTimelineTarget) \
\ \
/* Overlap reasons that require knowing what's behind you in paint-order \ /* Overlap reasons that require knowing what's behind you in paint-order \
before knowing the answer. */ \ before knowing the answer. */ \
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment