From c044b11a0cdf5bfc7c13c1b2dfb41b6b415e18f5 Mon Sep 17 00:00:00 2001 From: sunxd Date: Wed, 16 Mar 2016 09:23:20 -0700 Subject: Clean scroll logic in LayerTreeImpl: * Move DidUpdateScrollOffset to LayerTreeImpl * Make SyncedScrollOffset transparent to LayerImpl BUG=568830 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Review URL: https://codereview.chromium.org/1803643002 Cr-Commit-Position: refs/heads/master@{#381467} --- cc/trees/layer_tree_host_common_unittest.cc | 12 +++---- cc/trees/layer_tree_host_impl_unittest.cc | 50 ++++++++++++++++++----------- cc/trees/layer_tree_host_unittest_scroll.cc | 9 ++++-- cc/trees/layer_tree_impl.cc | 36 +++++++++++++++++++++ cc/trees/layer_tree_impl.h | 1 + cc/trees/property_tree.cc | 47 +++++++++++++++++++++++---- cc/trees/property_tree.h | 14 ++++++-- cc/trees/property_tree_builder.cc | 4 +-- cc/trees/property_tree_unittest.cc | 2 +- cc/trees/tree_synchronizer_unittest.cc | 8 ++--- 10 files changed, 141 insertions(+), 42 deletions(-) (limited to 'cc/trees') diff --git a/cc/trees/layer_tree_host_common_unittest.cc b/cc/trees/layer_tree_host_common_unittest.cc index 1db2eea..67fa301 100644 --- a/cc/trees/layer_tree_host_common_unittest.cc +++ b/cc/trees/layer_tree_host_common_unittest.cc @@ -9807,6 +9807,8 @@ TEST_F(LayerTreeHostCommonTest, ScrollTreeBuilderTest) { // Property tree root ScrollTree scroll_tree = host()->property_trees()->scroll_tree; PropertyTrees property_trees; + property_trees.is_main_thread = true; + property_trees.is_active = false; ScrollTree expected_scroll_tree = property_trees.scroll_tree; ScrollNode* property_tree_root = expected_scroll_tree.Node(0); property_tree_root->id = kRootPropertyTreeNodeId; @@ -9890,12 +9892,10 @@ TEST_F(LayerTreeHostCommonTest, ScrollTreeBuilderTest) { scroll_parent5.data.transform_id = parent5->transform_tree_index(); expected_scroll_tree.Insert(scroll_parent5, 1); - expected_scroll_tree.synced_scroll_offset(parent2->id()) - ->PushFromMainThread(gfx::ScrollOffset(0, 0)); - expected_scroll_tree.synced_scroll_offset(child7->id()) - ->PushFromMainThread(gfx::ScrollOffset(0, 0)); - expected_scroll_tree.synced_scroll_offset(grand_child11->id()) - ->PushFromMainThread(gfx::ScrollOffset(0, 0)); + expected_scroll_tree.SetScrollOffset(parent2->id(), gfx::ScrollOffset(0, 0)); + expected_scroll_tree.SetScrollOffset(child7->id(), gfx::ScrollOffset(0, 0)); + expected_scroll_tree.SetScrollOffset(grand_child11->id(), + gfx::ScrollOffset(0, 0)); expected_scroll_tree.set_needs_update(false); EXPECT_EQ(expected_scroll_tree, scroll_tree); diff --git a/cc/trees/layer_tree_host_impl_unittest.cc b/cc/trees/layer_tree_host_impl_unittest.cc index 1bfa546..e838eae 100644 --- a/cc/trees/layer_tree_host_impl_unittest.cc +++ b/cc/trees/layer_tree_host_impl_unittest.cc @@ -225,13 +225,11 @@ class LayerTreeHostImplTest : public testing::Test, } static gfx::Vector2dF ScrollDelta(LayerImpl* layer_impl) { - if (layer_impl->IsActive()) - return gfx::Vector2dF(layer_impl->synced_scroll_offset()->Delta().x(), - layer_impl->synced_scroll_offset()->Delta().y()); - else - return gfx::Vector2dF( - layer_impl->synced_scroll_offset()->PendingDelta().get().x(), - layer_impl->synced_scroll_offset()->PendingDelta().get().y()); + gfx::ScrollOffset delta = + layer_impl->layer_tree_impl() + ->property_trees() + ->scroll_tree.GetScrollOffsetDeltaForTesting(layer_impl->id()); + return gfx::Vector2dF(delta.x(), delta.y()); } static void ExpectClearedScrollDeltasRecursive(LayerImpl* layer) { @@ -474,9 +472,12 @@ class LayerTreeHostImplTest : public testing::Test, static void SetScrollOffsetDelta(LayerImpl* layer_impl, const gfx::Vector2dF& delta) { - layer_impl->SetCurrentScrollOffset( - layer_impl->synced_scroll_offset()->ActiveBase() + - gfx::ScrollOffset(delta)); + if (layer_impl->layer_tree_impl() + ->property_trees() + ->scroll_tree.SetScrollOffsetDeltaForTesting(layer_impl->id(), + delta)) + layer_impl->layer_tree_impl()->DidUpdateScrollOffset( + layer_impl->id(), layer_impl->transform_tree_index()); } FakeImplTaskRunnerProvider task_runner_provider_; @@ -2037,7 +2038,9 @@ TEST_F(LayerTreeHostImplTest, PinchGesture) { host_impl_->active_tree()->PushPageScaleFromMainThread(1.f, min_page_scale, max_page_scale); SetScrollOffsetDelta(scroll_layer, gfx::Vector2d()); - scroll_layer->synced_scroll_offset()->PullDeltaForMainThread(); + scroll_layer->layer_tree_impl() + ->property_trees() + ->scroll_tree.CollectScrollDeltasForTesting(); scroll_layer->layer_tree_impl() ->property_trees() ->scroll_tree.UpdateScrollOffsetBaseForTesting( @@ -2063,7 +2066,9 @@ TEST_F(LayerTreeHostImplTest, PinchGesture) { host_impl_->active_tree()->PushPageScaleFromMainThread(1.f, min_page_scale, max_page_scale); SetScrollOffsetDelta(scroll_layer, gfx::Vector2d()); - scroll_layer->synced_scroll_offset()->PullDeltaForMainThread(); + scroll_layer->layer_tree_impl() + ->property_trees() + ->scroll_tree.CollectScrollDeltasForTesting(); scroll_layer->layer_tree_impl() ->property_trees() ->scroll_tree.UpdateScrollOffsetBaseForTesting( @@ -2089,7 +2094,9 @@ TEST_F(LayerTreeHostImplTest, PinchGesture) { host_impl_->active_tree()->PushPageScaleFromMainThread(1.f, min_page_scale, max_page_scale); SetScrollOffsetDelta(scroll_layer, gfx::Vector2d()); - scroll_layer->synced_scroll_offset()->PullDeltaForMainThread(); + scroll_layer->layer_tree_impl() + ->property_trees() + ->scroll_tree.CollectScrollDeltasForTesting(); scroll_layer->layer_tree_impl() ->property_trees() ->scroll_tree.UpdateScrollOffsetBaseForTesting( @@ -2117,7 +2124,9 @@ TEST_F(LayerTreeHostImplTest, PinchGesture) { { host_impl_->active_tree()->PushPageScaleFromMainThread(0.5f, 0.5f, 4.f); SetScrollOffsetDelta(scroll_layer, gfx::Vector2d()); - scroll_layer->synced_scroll_offset()->PullDeltaForMainThread(); + scroll_layer->layer_tree_impl() + ->property_trees() + ->scroll_tree.CollectScrollDeltasForTesting(); scroll_layer->layer_tree_impl() ->property_trees() ->scroll_tree.UpdateScrollOffsetBaseForTesting(scroll_layer->id(), @@ -2697,7 +2706,9 @@ class LayerTreeHostImplTestScrollbarAnimation : public LayerTreeHostImplTest { ->scroll_tree.UpdateScrollOffsetBaseForTesting( host_impl_->InnerViewportScrollLayer()->id(), gfx::ScrollOffset(5, 5))) - host_impl_->InnerViewportScrollLayer()->DidUpdateScrollOffset(); + host_impl_->active_tree()->DidUpdateScrollOffset( + host_impl_->InnerViewportScrollLayer()->id(), + host_impl_->InnerViewportScrollLayer()->transform_tree_index()); EXPECT_FALSE(did_request_next_frame_); EXPECT_FALSE(did_request_redraw_); EXPECT_EQ(base::TimeDelta::FromMilliseconds(20), @@ -10323,11 +10334,14 @@ TEST_F(LayerTreeHostImplTest, JitterTest) { // delta is not zero, the delta would be counted twice. // This hacking here is to restore the damaged scroll offset. gfx::ScrollOffset pending_base = - last_scrolled_layer->synced_scroll_offset()->PendingBase(); + pending_tree->property_trees() + ->scroll_tree.GetScrollOffsetBaseForTesting( + last_scrolled_layer->id()); pending_tree->property_trees()->needs_rebuild = true; pending_tree->BuildPropertyTreesForTesting(); - last_scrolled_layer->synced_scroll_offset()->PushFromMainThread( - pending_base); + pending_tree->property_trees() + ->scroll_tree.UpdateScrollOffsetBaseForTesting( + last_scrolled_layer->id(), pending_base); pending_tree->set_needs_update_draw_properties(); pending_tree->UpdateDrawProperties(false); diff --git a/cc/trees/layer_tree_host_unittest_scroll.cc b/cc/trees/layer_tree_host_unittest_scroll.cc index 6648f15..024e65b 100644 --- a/cc/trees/layer_tree_host_unittest_scroll.cc +++ b/cc/trees/layer_tree_host_unittest_scroll.cc @@ -1399,9 +1399,12 @@ class LayerTreeHostScrollTestLayerStructureChange static void SetScrollOffsetDelta(LayerImpl* layer_impl, const gfx::Vector2dF& delta) { - layer_impl->SetCurrentScrollOffset( - layer_impl->synced_scroll_offset()->ActiveBase() + - gfx::ScrollOffset(delta)); + if (layer_impl->layer_tree_impl() + ->property_trees() + ->scroll_tree.SetScrollOffsetDeltaForTesting(layer_impl->id(), + delta)) + layer_impl->layer_tree_impl()->DidUpdateScrollOffset( + layer_impl->id(), layer_impl->transform_tree_index()); } FakeLayerScrollClient root_scroll_layer_client_; diff --git a/cc/trees/layer_tree_impl.cc b/cc/trees/layer_tree_impl.cc index dae2e2c..074ab1d 100644 --- a/cc/trees/layer_tree_impl.cc +++ b/cc/trees/layer_tree_impl.cc @@ -142,6 +142,42 @@ bool LayerTreeImpl::IsViewportLayerId(int id) const { return false; } +// TODO(sunxd): when we have a layer_id to property_tree index map in property +// trees, use the transform_id parameter instead of looking for indices from +// LayerImpls. +void LayerTreeImpl::DidUpdateScrollOffset(int layer_id, int transform_id) { + DidUpdateScrollState(layer_id); + TransformTree& transform_tree = property_trees()->transform_tree; + ScrollTree& scroll_tree = property_trees()->scroll_tree; + + // If pending tree topology changed and we still want to notify the pending + // tree about scroll offset in the active tree, we may not find the + // corresponding pending layer. + if (LayerById(layer_id)) { + transform_id = LayerById(layer_id)->transform_tree_index(); + } else { + DCHECK(!IsActiveTree()); + return; + } + + if (transform_id != -1) { + TransformNode* node = transform_tree.Node(transform_id); + if (node->data.scroll_offset != + scroll_tree.current_scroll_offset(layer_id)) { + node->data.scroll_offset = scroll_tree.current_scroll_offset(layer_id); + node->data.needs_local_transform_update = true; + transform_tree.set_needs_update(true); + } + node->data.transform_changed = true; + property_trees()->changed = true; + set_needs_update_draw_properties(); + } + + if (IsActiveTree() && layer_tree_host_impl_->pending_tree()) + layer_tree_host_impl_->pending_tree()->DidUpdateScrollOffset(layer_id, + transform_id); +} + void LayerTreeImpl::DidUpdateScrollState(int layer_id) { if (!IsActiveTree()) return; diff --git a/cc/trees/layer_tree_impl.h b/cc/trees/layer_tree_impl.h index 2af669d..76ff68c 100644 --- a/cc/trees/layer_tree_impl.h +++ b/cc/trees/layer_tree_impl.h @@ -421,6 +421,7 @@ class CC_EXPORT LayerTreeImpl { void GatherFrameTimingRequestIds(std::vector* request_ids); + void DidUpdateScrollOffset(int layer_id, int transform_id); void DidUpdateScrollState(int layer_id); bool IsAnimatingFilterProperty(const LayerImpl* layer) const; diff --git a/cc/trees/property_tree.cc b/cc/trees/property_tree.cc index 34f8c49..c3514e1 100644 --- a/cc/trees/property_tree.cc +++ b/cc/trees/property_tree.cc @@ -1477,8 +1477,6 @@ gfx::Transform ScrollTree::ScreenSpaceTransform(int scroll_node_id) const { return screen_space_transform; } -// TODO(sunxd): Make this function private once scroll offset access is fully -// directed to scroll tree. SyncedScrollOffset* ScrollTree::synced_scroll_offset(int layer_id) { if (layer_id_to_scroll_offset_map_.find(layer_id) == layer_id_to_scroll_offset_map_.end()) { @@ -1495,6 +1493,13 @@ const SyncedScrollOffset* ScrollTree::synced_scroll_offset(int layer_id) const { return layer_id_to_scroll_offset_map_.at(layer_id).get(); } +const gfx::ScrollOffset ScrollTree::current_scroll_offset(int layer_id) const { + return synced_scroll_offset(layer_id) + ? synced_scroll_offset(layer_id)->Current( + property_trees()->is_active) + : gfx::ScrollOffset(); +} + gfx::ScrollOffset ScrollTree::PullDeltaForMainThread( SyncedScrollOffset* scroll_offset) { // TODO(miletus): Remove all this temporary flooring machinery when @@ -1528,6 +1533,12 @@ void ScrollTree::CollectScrollDeltas(ScrollAndScaleSet* scroll_info) { } } +void ScrollTree::CollectScrollDeltasForTesting() { + for (auto map_entry : layer_id_to_scroll_offset_map_) { + PullDeltaForMainThread(map_entry.second.get()); + } +} + void ScrollTree::UpdateScrollOffsetMapEntry( int key, ScrollTree::ScrollOffsetMap* new_scroll_offset_map, @@ -1543,13 +1554,15 @@ void ScrollTree::UpdateScrollOffsetMapEntry( if (new_scroll_offset_map->at(key)->clobber_active_value()) { synced_scroll_offset(key)->set_clobber_active_value(); } - if (changed) - layer_tree_impl->LayerById(key)->DidUpdateScrollOffset(); + if (changed) { + layer_tree_impl->DidUpdateScrollOffset(key, -1); + } } else { layer_id_to_scroll_offset_map_[key] = new_scroll_offset_map->at(key); changed |= synced_scroll_offset(key)->PushPendingToActive(); - if (changed) - layer_tree_impl->LayerById(key)->DidUpdateScrollOffset(); + if (changed) { + layer_tree_impl->DidUpdateScrollOffset(key, -1); + } } } @@ -1592,6 +1605,11 @@ void ScrollTree::ApplySentScrollDeltasFromAbortedCommit() { map_entry.second->AbortCommit(); } +bool ScrollTree::SetBaseScrollOffset(int layer_id, + const gfx::ScrollOffset& scroll_offset) { + return synced_scroll_offset(layer_id)->PushFromMainThread(scroll_offset); +} + bool ScrollTree::SetScrollOffset(int layer_id, const gfx::ScrollOffset& scroll_offset) { if (property_trees()->is_main_thread) @@ -1611,6 +1629,12 @@ bool ScrollTree::UpdateScrollOffsetBaseForTesting( return changed; } +bool ScrollTree::SetScrollOffsetDeltaForTesting(int layer_id, + const gfx::Vector2dF& delta) { + return synced_scroll_offset(layer_id)->SetCurrent( + synced_scroll_offset(layer_id)->ActiveBase() + gfx::ScrollOffset(delta)); +} + const gfx::ScrollOffset ScrollTree::GetScrollOffsetBaseForTesting( int layer_id) const { DCHECK(!property_trees()->is_main_thread); @@ -1622,6 +1646,17 @@ const gfx::ScrollOffset ScrollTree::GetScrollOffsetBaseForTesting( return gfx::ScrollOffset(); } +const gfx::ScrollOffset ScrollTree::GetScrollOffsetDeltaForTesting( + int layer_id) const { + DCHECK(!property_trees()->is_main_thread); + if (synced_scroll_offset(layer_id)) + return property_trees()->is_active + ? synced_scroll_offset(layer_id)->Delta() + : synced_scroll_offset(layer_id)->PendingDelta().get(); + else + return gfx::ScrollOffset(); +} + PropertyTrees::PropertyTrees() : needs_rebuild(true), non_root_surfaces_enabled(true), diff --git a/cc/trees/property_tree.h b/cc/trees/property_tree.h index ddd73c7..96d8d8e 100644 --- a/cc/trees/property_tree.h +++ b/cc/trees/property_tree.h @@ -575,23 +575,33 @@ class CC_EXPORT ScrollTree final : public PropertyTree { void set_currently_scrolling_node(int scroll_node_id); gfx::Transform ScreenSpaceTransform(int scroll_node_id) const; - SyncedScrollOffset* synced_scroll_offset(int layer_id); - const SyncedScrollOffset* synced_scroll_offset(int layer_id) const; + const gfx::ScrollOffset current_scroll_offset(int layer_id) const; void CollectScrollDeltas(ScrollAndScaleSet* scroll_info); void UpdateScrollOffsetMap(ScrollOffsetMap* new_scroll_offset_map, LayerTreeImpl* layer_tree_impl); ScrollOffsetMap& scroll_offset_map(); const ScrollOffsetMap& scroll_offset_map() const; void ApplySentScrollDeltasFromAbortedCommit(); + bool SetBaseScrollOffset(int layer_id, + const gfx::ScrollOffset& scroll_offset); bool SetScrollOffset(int layer_id, const gfx::ScrollOffset& scroll_offset); + void SetScrollOffsetClobberActiveValue(int layer_id) { + synced_scroll_offset(layer_id)->set_clobber_active_value(); + } bool UpdateScrollOffsetBaseForTesting(int layer_id, const gfx::ScrollOffset& offset); + bool SetScrollOffsetDeltaForTesting(int layer_id, + const gfx::Vector2dF& delta); const gfx::ScrollOffset GetScrollOffsetBaseForTesting(int layer_id) const; + const gfx::ScrollOffset GetScrollOffsetDeltaForTesting(int layer_id) const; + void CollectScrollDeltasForTesting(); private: int currently_scrolling_node_id_; ScrollOffsetMap layer_id_to_scroll_offset_map_; + SyncedScrollOffset* synced_scroll_offset(int layer_id); + const SyncedScrollOffset* synced_scroll_offset(int layer_id) const; gfx::ScrollOffset PullDeltaForMainThread(SyncedScrollOffset* scroll_offset); void UpdateScrollOffsetMapEntry(int key, ScrollOffsetMap* new_scroll_offset_map, diff --git a/cc/trees/property_tree_builder.cc b/cc/trees/property_tree_builder.cc index 0b887a4..6b59138 100644 --- a/cc/trees/property_tree_builder.cc +++ b/cc/trees/property_tree_builder.cc @@ -716,8 +716,8 @@ void AddScrollNodeIfNeeded( scroll_node_uninheritable_criteria; if (node.data.scrollable) { - data_for_children->scroll_tree->synced_scroll_offset(layer->id()) - ->PushFromMainThread(layer->CurrentScrollOffset()); + data_for_children->scroll_tree->SetBaseScrollOffset( + layer->id(), layer->CurrentScrollOffset()); } } diff --git a/cc/trees/property_tree_unittest.cc b/cc/trees/property_tree_unittest.cc index 0d70672..c13ca99 100644 --- a/cc/trees/property_tree_unittest.cc +++ b/cc/trees/property_tree_unittest.cc @@ -273,7 +273,7 @@ TEST(PropertyTreeSerializationTest, ScrollTreeSerialization) { original.Insert(third, 1); original.set_currently_scrolling_node(1); - original.synced_scroll_offset(1)->PushFromMainThread(gfx::ScrollOffset(1, 2)); + original.SetScrollOffset(1, gfx::ScrollOffset(1, 2)); proto::PropertyTree proto; original.ToProtobuf(&proto); diff --git a/cc/trees/tree_synchronizer_unittest.cc b/cc/trees/tree_synchronizer_unittest.cc index 5b2f95c..ee4041a 100644 --- a/cc/trees/tree_synchronizer_unittest.cc +++ b/cc/trees/tree_synchronizer_unittest.cc @@ -787,8 +787,8 @@ TEST_F(TreeSynchronizerTest, SynchronizeScrollTreeScrollOffsetMap) { host_impl->active_tree()->LayerById(scroll_layer->id()); ScrollTree& scroll_tree = host_impl->active_tree()->property_trees()->scroll_tree; - scroll_tree.synced_scroll_offset(scroll_layer_impl->id()) - ->SetCurrent(gfx::ScrollOffset(20, 30)); + scroll_tree.SetScrollOffset(scroll_layer_impl->id(), + gfx::ScrollOffset(20, 30)); // Pull ScrollOffset delta for main thread, and change offset on main thread scoped_ptr scroll_info(new ScrollAndScaleSet()); @@ -799,8 +799,8 @@ TEST_F(TreeSynchronizerTest, SynchronizeScrollTreeScrollOffsetMap) { scroll_layer->SetScrollOffset(gfx::ScrollOffset(100, 100)); // More update to ScrollOffset active delta: gfx::ScrollOffset(20, 20) - scroll_tree.synced_scroll_offset(scroll_layer_impl->id()) - ->SetCurrent(gfx::ScrollOffset(40, 50)); + scroll_tree.SetScrollOffset(scroll_layer_impl->id(), + gfx::ScrollOffset(40, 50)); host_impl->active_tree()->SetCurrentlyScrollingLayer(scroll_layer_impl); // Make one layer unscrollable so that scroll tree topology changes -- cgit v1.1