Skip to content

Commit

Permalink
[HitTestOpaqueness] Use view scroll as the scroll state of fixed-posi…
Browse files Browse the repository at this point in the history
…tion layer

Scroll gesture in a fixed-position layer is special in that though
the layer itself doesn't scroll, the gesture should scroll the
viewport. In pre-HitTestOpaqueness, such scrolls are treated as
unreliable and the main thread will scroll the viewport.
In HitTestOpaqueness, a scroll gesture may be reliable in an
hit-test-opaque fixed-layer, and the scroll state of the layer
is now adjusted to be the viewport scroll so that the compositor
can scroll the viewport, instead of crbug.com/1481779.

Bug: 1413877, 1481779
Change-Id: I5f2edb77f7f6562e0e5c07be7b5d6ecf794ed9cb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4874317
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1198117}
  • Loading branch information
wangxianzhu authored and Chromium LUCI CQ committed Sep 19, 2023
1 parent 01838f8 commit e34768e
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -150,14 +150,19 @@ std::unique_ptr<JSONObject> PaintArtifactCompositor::GetLayersAsJSON(
}

const TransformPaintPropertyNode&
PaintArtifactCompositor::NearestScrollTranslationForLayer(
PaintArtifactCompositor::ScrollTranslationStateForLayer(
const PendingLayer& pending_layer) {
if (pending_layer.GetCompositingType() == PendingLayer::kScrollHitTestLayer)
if (pending_layer.GetCompositingType() == PendingLayer::kScrollHitTestLayer) {
return pending_layer.ScrollTranslationForScrollHitTestLayer();
}

return pending_layer.GetPropertyTreeState()
.Transform()
.NearestScrollTranslationNode();
// When HitTestOpaqueness is enabled, use the correct scroll state for fixed
// position content, so scrolls on fixed content is correctly handled on the
// compositor if the fixed content is opaque to hit test.
const auto& transform = pending_layer.GetPropertyTreeState().Transform();
return RuntimeEnabledFeatures::HitTestOpaquenessEnabled()
? transform.ScrollTranslationState()
: transform.NearestScrollTranslationNode();
}

bool PaintArtifactCompositor::NeedsCompositedScrolling(
Expand Down Expand Up @@ -883,13 +888,9 @@ void PaintArtifactCompositor::Update(
effect.GetCompositorElementId();
}

// The compositor scroll node is not directly stored in the property tree
// state but can be created via the scroll offset translation node.
const auto& scroll_translation =
NearestScrollTranslationForLayer(pending_layer);
int scroll_id =
property_tree_manager.EnsureCompositorScrollAndTransformNode(
scroll_translation);
ScrollTranslationStateForLayer(pending_layer));

layer_list_builder.Add(&layer);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ class PLATFORM_EXPORT PaintArtifactCompositor final
const EffectPaintPropertyNode& effect,
wtf_size_t layer_index);

const TransformPaintPropertyNode& NearestScrollTranslationForLayer(
const TransformPaintPropertyNode& ScrollTranslationStateForLayer(
const PendingLayer&);

// if |needs_layer| is false, no cc::Layer is created, |mask_effect_id| is
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1606,6 +1606,61 @@ TEST_P(PaintArtifactCompositorTest,
scroll_tree.current_scroll_offset(scroll_node_c.element_id));
}

TEST_P(PaintArtifactCompositorTest, FixedPositionScrollState) {
auto scroll_state_a = ScrollState1();
auto& scroll_a = *scroll_state_a.Transform().ScrollNode();
auto fixed_transform = CreateFixedPositionTranslation(
t0(), 100, 200, scroll_state_a.Transform());
PropertyTreeState fixed_state(*fixed_transform, scroll_state_a.Clip(),
scroll_state_a.Effect());
// scroll_state_b's has fixed transform space, while the scroll parent is
// scroll_a.
auto scroll_state_b = CreateCompositedScrollTranslationState(
fixed_state, scroll_a, 11, 22, gfx::Rect(10, 20), gfx::Size(50, 60));
auto& scroll_b = *scroll_state_b.Transform().ScrollNode();

TestPaintArtifact artifact;
CreateScrollableChunk(artifact, scroll_state_a);
artifact.Chunk(fixed_state).RectDrawing(gfx::Rect(50, 100), Color::kBlack);
CreateScrollableChunk(artifact, scroll_state_b);
Update(artifact.Build());

auto& scroll_tree = GetPropertyTrees().scroll_tree();
auto& transform_tree = GetPropertyTrees().transform_tree();
// Node #0 reserved for null. #1 for root render surface. #2 is for scroll_a.
// scroll #3 and transform #4 are for scroll_b. Transform #3 is for
// fixed_transform.
ASSERT_EQ(4u, scroll_tree.size());
ASSERT_EQ(5u, transform_tree.size());
ASSERT_EQ(3u, LayerCount());

auto* scroll_node_a = scroll_tree.Node(2);
auto* layer_a = LayerAt(0);
EXPECT_EQ(scroll_a.GetCompositorElementId(), scroll_node_a->element_id);
EXPECT_EQ(1, scroll_node_a->parent_id);
EXPECT_EQ(2, scroll_node_a->transform_id);
EXPECT_EQ(2, layer_a->scroll_tree_index());
EXPECT_EQ(1, layer_a->transform_tree_index());

auto* fixed_layer = LayerAt(1);
if (RuntimeEnabledFeatures::HitTestOpaquenessEnabled()) {
EXPECT_EQ(2, fixed_layer->scroll_tree_index());
} else {
EXPECT_EQ(1, fixed_layer->scroll_tree_index());
}
EXPECT_EQ(3, fixed_layer->transform_tree_index());
auto* fixed_transform_node = transform_tree.Node(3);
EXPECT_EQ(1, fixed_transform_node->parent_id);

auto* scroll_node_b = scroll_tree.Node(3);
auto* layer_b = LayerAt(2);
EXPECT_EQ(2, scroll_node_b->parent_id);
EXPECT_EQ(scroll_b.GetCompositorElementId(), scroll_node_b->element_id);
EXPECT_EQ(4, scroll_node_b->transform_id);
EXPECT_EQ(3, layer_b->scroll_tree_index());
EXPECT_EQ(3, layer_b->transform_tree_index());
}

TEST_P(PaintArtifactCompositorTest, MergeSimpleChunks) {
TestPaintArtifact test_artifact;
test_artifact.Chunk().RectDrawing(gfx::Rect(0, 0, 100, 100), Color::kWhite);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ void GeometryMapperTransformCache::Update(
screen_transform_updated_ = true;

DCHECK(node.ScrollNode());
nearest_scroll_translation_ = &node;
nearest_scroll_translation_ = scroll_translation_state_ = &node;
return;
}

Expand All @@ -54,6 +54,9 @@ void GeometryMapperTransformCache::Update(

nearest_scroll_translation_ =
node.ScrollNode() ? &node : parent.nearest_scroll_translation_;
scroll_translation_state_ = node.ScrollTranslationForFixed()
? node.ScrollTranslationForFixed()
: nearest_scroll_translation_;

nearest_directly_composited_ancestor_ =
node.HasDirectCompositingReasons()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ class PLATFORM_EXPORT GeometryMapperTransformCache {
DCHECK(nearest_scroll_translation_);
return *nearest_scroll_translation_;
}
const TransformPaintPropertyNode& scroll_translation_state() const {
DCHECK(scroll_translation_state_);
return *scroll_translation_state_;
}

const TransformPaintPropertyNode* nearest_directly_composited_ancestor()
const {
Expand Down Expand Up @@ -221,6 +225,7 @@ class PLATFORM_EXPORT GeometryMapperTransformCache {
absl::optional<ScreenTransform> screen_transform_;

const TransformPaintPropertyNode* nearest_scroll_translation_ = nullptr;
const TransformPaintPropertyNode* scroll_translation_state_ = nullptr;
const TransformPaintPropertyNode* nearest_directly_composited_ancestor_ =
nullptr;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,15 @@ class PLATFORM_EXPORT TransformPaintPropertyNode
return GetTransformCache().nearest_scroll_translation();
}

// This is different from NearestScrollTranslationNode in that for a
// fixed-position paint offset translation, this returns
// ScrollTranslationForFixed() instead of the ancestor scroll translation
// because a scroll gesture on a fixed-position element should scroll the
// containing view.
const TransformPaintPropertyNode& ScrollTranslationState() const {
return GetTransformCache().scroll_translation_state();
}

// Returns the nearest ancestor node (including |this|) that has direct
// compositing reasons.
const TransformPaintPropertyNode* NearestDirectlyCompositedAncestor() const {
Expand Down

0 comments on commit e34768e

Please sign in to comment.