Commit 41212cbd authored by Philip Rogers's avatar Philip Rogers Committed by Commit Bot

[BGPT] Ensure cc::TransformNode::in_subtree_of_page_scale_layer is set

cc::TransformNode::in_subtree_of_page_scale_layer is important for
avoiding raster during pinch-zoom (see ideal_page_scale calculation
in PictureLayerImpl::UpdateIdealScales) and was not being set at the
blink->cc boundary when BlinkGenPropertyTrees was enabled.

This patch adds in_subtree_of_page_scale to TransformPaintPropertyNode
and sets it when building the VisualViewport property nodes. This is
then used to update cc::TransformNode::in_subtree_of_page_scale_layer
in PaintPropertyTreeManager.

Bug: 951861
Change-Id: Ic3bd0f48fb3a383825503b04b4f42d7db51bd651
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1586996Reviewed-by: default avatarXianzhu Wang <wangxianzhu@chromium.org>
Auto-Submit: Philip Rogers <pdr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#654866}
parent 7578f765
...@@ -130,6 +130,7 @@ PaintPropertyChangeType VisualViewport::UpdatePaintPropertyNodesIfNeeded( ...@@ -130,6 +130,7 @@ PaintPropertyChangeType VisualViewport::UpdatePaintPropertyNodesIfNeeded(
GetChromeClient()->GetDeviceEmulationTransform(); GetChromeClient()->GetDeviceEmulationTransform();
if (!device_emulation_transform.IsIdentity()) { if (!device_emulation_transform.IsIdentity()) {
TransformPaintPropertyNode::State state{device_emulation_transform}; TransformPaintPropertyNode::State state{device_emulation_transform};
state.in_subtree_of_page_scale = false;
if (!device_emulation_transform_node_) { if (!device_emulation_transform_node_) {
device_emulation_transform_node_ = TransformPaintPropertyNode::Create( device_emulation_transform_node_ = TransformPaintPropertyNode::Create(
*transform_parent, std::move(state)); *transform_parent, std::move(state));
...@@ -152,7 +153,10 @@ PaintPropertyChangeType VisualViewport::UpdatePaintPropertyNodesIfNeeded( ...@@ -152,7 +153,10 @@ PaintPropertyChangeType VisualViewport::UpdatePaintPropertyNodesIfNeeded(
} }
{ {
DCHECK(!transform_parent->IsInSubtreeOfPageScale());
TransformPaintPropertyNode::State state; TransformPaintPropertyNode::State state;
state.in_subtree_of_page_scale = false;
state.compositor_element_id = GetCompositorOverscrollElasticityElementId(); state.compositor_element_id = GetCompositorOverscrollElasticityElementId();
// TODO(crbug.com/877794) Should create overscroll elasticity transform node // TODO(crbug.com/877794) Should create overscroll elasticity transform node
// based on settings. // based on settings.
...@@ -170,6 +174,7 @@ PaintPropertyChangeType VisualViewport::UpdatePaintPropertyNodesIfNeeded( ...@@ -170,6 +174,7 @@ PaintPropertyChangeType VisualViewport::UpdatePaintPropertyNodesIfNeeded(
{ {
TransformPaintPropertyNode::State state{ TransformPaintPropertyNode::State state{
TransformationMatrix().Scale(Scale())}; TransformationMatrix().Scale(Scale())};
state.in_subtree_of_page_scale = false;
state.compositor_element_id = GetCompositorElementId(); state.compositor_element_id = GetCompositorElementId();
if (!scale_transform_node_) { if (!scale_transform_node_) {
......
...@@ -2594,5 +2594,36 @@ TEST_P(VisualViewportTest, DirectPinchZoomPropertyUpdate) { ...@@ -2594,5 +2594,36 @@ TEST_P(VisualViewportTest, DirectPinchZoomPropertyUpdate) {
EXPECT_EQ(3.f, visual_viewport.Scale()); EXPECT_EQ(3.f, visual_viewport.Scale());
} }
// |TransformPaintPropertyNode::in_subtree_of_page_scale| should be false for
// the page scale transform node and all ancestors, and should be true for
// descendants of the page scale transform node.
TEST_P(VisualViewportTest, InSubtreeOfPageScale) {
InitializeWithAndroidSettings();
RegisterMockedHttpURLLoad("200-by-800-viewport.html");
NavigateTo(base_url_ + "200-by-800-viewport.html");
UpdateAllLifecyclePhases();
VisualViewport& visual_viewport = GetFrame()->GetPage()->GetVisualViewport();
const auto* page_scale = visual_viewport.GetPageScaleNode();
// The page scale is not in its own subtree.
EXPECT_FALSE(page_scale->IsInSubtreeOfPageScale());
// Ancestors of the page scale are not in the page scale's subtree.
for (const auto* ancestor = page_scale->Parent(); ancestor;
ancestor = ancestor->Parent()) {
EXPECT_FALSE(ancestor->IsInSubtreeOfPageScale());
}
const auto* view = GetFrame()->View()->GetLayoutView();
const auto& view_contents_transform =
view->FirstFragment().ContentsProperties().Transform();
// Descendants of the page scale node should have |IsInSubtreeOfPageScale|.
EXPECT_TRUE(view_contents_transform.IsInSubtreeOfPageScale());
for (const auto* ancestor = view_contents_transform.Parent();
ancestor != page_scale; ancestor = ancestor->Parent()) {
EXPECT_TRUE(ancestor->IsInSubtreeOfPageScale());
}
}
} // namespace } // namespace
} // namespace blink } // namespace blink
...@@ -3398,6 +3398,7 @@ TEST_P(PaintArtifactCompositorTest, CreatesViewportNodes) { ...@@ -3398,6 +3398,7 @@ TEST_P(PaintArtifactCompositorTest, CreatesViewportNodes) {
TransformationMatrix matrix; TransformationMatrix matrix;
matrix.Scale(2); matrix.Scale(2);
TransformPaintPropertyNode::State transform_state{matrix}; TransformPaintPropertyNode::State transform_state{matrix};
transform_state.in_subtree_of_page_scale = false;
transform_state.compositor_element_id = transform_state.compositor_element_id =
CompositorElementIdFromUniqueObjectId(1); CompositorElementIdFromUniqueObjectId(1);
...@@ -3420,6 +3421,58 @@ TEST_P(PaintArtifactCompositorTest, CreatesViewportNodes) { ...@@ -3420,6 +3421,58 @@ TEST_P(PaintArtifactCompositorTest, CreatesViewportNodes) {
EXPECT_TRUE(cc_transform_node->pre_local.IsIdentity()); EXPECT_TRUE(cc_transform_node->pre_local.IsIdentity());
} }
// Test that |cc::TransformNode::in_subtree_of_page_scale_layer| is not set on
// the page scale transform node or ancestors, and is set on descendants.
TEST_P(PaintArtifactCompositorTest, InSubtreeOfPageScale) {
TransformPaintPropertyNode::State ancestor_transform_state;
ancestor_transform_state.in_subtree_of_page_scale = false;
auto ancestor_transform = TransformPaintPropertyNode::Create(
TransformPaintPropertyNode::Root(), std::move(ancestor_transform_state));
TransformPaintPropertyNode::State page_scale_transform_state;
page_scale_transform_state.in_subtree_of_page_scale = false;
page_scale_transform_state.compositor_element_id =
CompositorElementIdFromUniqueObjectId(1);
auto page_scale_transform = TransformPaintPropertyNode::Create(
*ancestor_transform, std::move(page_scale_transform_state));
TransformPaintPropertyNode::State descendant_transform_state;
descendant_transform_state.compositor_element_id =
CompositorElementIdFromUniqueObjectId(2);
descendant_transform_state.in_subtree_of_page_scale = true;
descendant_transform_state.direct_compositing_reasons =
CompositingReason::kWillChangeTransform;
auto descendant_transform = TransformPaintPropertyNode::Create(
*page_scale_transform, std::move(descendant_transform_state));
TestPaintArtifact artifact;
artifact.Chunk(*descendant_transform, c0(), e0())
.RectDrawing(FloatRect(0, 0, 10, 10), Color::kBlack);
ViewportProperties viewport_properties;
viewport_properties.page_scale = page_scale_transform.get();
CompositorElementIdSet element_ids;
Update(artifact.Build(), element_ids, viewport_properties);
cc::TransformTree& transform_tree = GetPropertyTrees().transform_tree;
const auto* cc_page_scale_transform = transform_tree.FindNodeFromElementId(
page_scale_transform_state.compositor_element_id);
// The page scale node is not in a subtree of the page scale layer.
EXPECT_FALSE(cc_page_scale_transform->in_subtree_of_page_scale_layer);
// Ancestors of the page scale node are not in a page scale subtree.
auto cc_ancestor_id = cc_page_scale_transform->parent_id;
while (cc_ancestor_id != cc::TransformTree::kInvalidNodeId) {
const auto* ancestor = transform_tree.Node(cc_ancestor_id);
EXPECT_FALSE(ancestor->in_subtree_of_page_scale_layer);
cc_ancestor_id = ancestor->parent_id;
}
// Descendants of the page scale node should be in the page scale subtree.
const auto* cc_descendant_transform = transform_tree.FindNodeFromElementId(
descendant_transform_state.compositor_element_id);
EXPECT_TRUE(cc_descendant_transform->in_subtree_of_page_scale_layer);
}
enum { enum {
kNoRenderSurface = 0, kNoRenderSurface = 0,
kHasRenderSurface = 1 << 0, kHasRenderSurface = 1 << 0,
......
...@@ -365,6 +365,9 @@ int PropertyTreeManager::EnsureCompositorTransformNode( ...@@ -365,6 +365,9 @@ int PropertyTreeManager::EnsureCompositorTransformNode(
GetTransformTree().AddNodeAffectedByOuterViewportBoundsDelta(id); GetTransformTree().AddNodeAffectedByOuterViewportBoundsDelta(id);
} }
compositor_node.in_subtree_of_page_scale_layer =
transform_node.IsInSubtreeOfPageScale();
if (const auto* sticky_constraint = transform_node.GetStickyConstraint()) { if (const auto* sticky_constraint = transform_node.GetStickyConstraint()) {
DCHECK(sticky_constraint->is_sticky); DCHECK(sticky_constraint->is_sticky);
cc::StickyPositionNodeData* sticky_data = cc::StickyPositionNodeData* sticky_data =
...@@ -467,6 +470,7 @@ void PropertyTreeManager::SetCcTransformNodeScrollToTransformTranslation( ...@@ -467,6 +470,7 @@ void PropertyTreeManager::SetCcTransformNodeScrollToTransformTranslation(
int PropertyTreeManager::EnsureCompositorPageScaleTransformNode( int PropertyTreeManager::EnsureCompositorPageScaleTransformNode(
const TransformPaintPropertyNode& node) { const TransformPaintPropertyNode& node) {
DCHECK(!node.IsInSubtreeOfPageScale());
int id = EnsureCompositorTransformNode(node); int id = EnsureCompositorTransformNode(node);
DCHECK(GetTransformTree().Node(id)); DCHECK(GetTransformTree().Node(id));
cc::TransformNode& compositor_node = *GetTransformTree().Node(id); cc::TransformNode& compositor_node = *GetTransformTree().Node(id);
......
...@@ -12,7 +12,11 @@ const TransformPaintPropertyNode& TransformPaintPropertyNode::Root() { ...@@ -12,7 +12,11 @@ const TransformPaintPropertyNode& TransformPaintPropertyNode::Root() {
DEFINE_STATIC_REF( DEFINE_STATIC_REF(
TransformPaintPropertyNode, root, TransformPaintPropertyNode, root,
base::AdoptRef(new TransformPaintPropertyNode( base::AdoptRef(new TransformPaintPropertyNode(
nullptr, State{FloatSize(), &ScrollPaintPropertyNode::Root()}, nullptr,
State{FloatSize(), &ScrollPaintPropertyNode::Root(),
false /* flattens_inherited_transform */,
false /* affected_by_outer_viewport_bounds_delta */,
false /* in_subtree_of_page_scale */},
true /* is_parent_alias */))); true /* is_parent_alias */)));
return *root; return *root;
} }
...@@ -59,6 +63,8 @@ std::unique_ptr<JSONObject> TransformPaintPropertyNode::ToJSON() const { ...@@ -59,6 +63,8 @@ std::unique_ptr<JSONObject> TransformPaintPropertyNode::ToJSON() const {
} }
if (!state_.flattens_inherited_transform) if (!state_.flattens_inherited_transform)
json->SetBoolean("flattensInheritedTransform", false); json->SetBoolean("flattensInheritedTransform", false);
if (!state_.in_subtree_of_page_scale)
json->SetBoolean("in_subtree_of_page_scale", false);
if (state_.backface_visibility != BackfaceVisibility::kInherited) { if (state_.backface_visibility != BackfaceVisibility::kInherited) {
json->SetString("backface", json->SetString("backface",
state_.backface_visibility == BackfaceVisibility::kVisible state_.backface_visibility == BackfaceVisibility::kVisible
......
...@@ -116,6 +116,7 @@ class PLATFORM_EXPORT TransformPaintPropertyNode ...@@ -116,6 +116,7 @@ class PLATFORM_EXPORT TransformPaintPropertyNode
scoped_refptr<const ScrollPaintPropertyNode> scroll; scoped_refptr<const ScrollPaintPropertyNode> scroll;
bool flattens_inherited_transform = false; bool flattens_inherited_transform = false;
bool affected_by_outer_viewport_bounds_delta = false; bool affected_by_outer_viewport_bounds_delta = false;
bool in_subtree_of_page_scale = true;
BackfaceVisibility backface_visibility = BackfaceVisibility::kInherited; BackfaceVisibility backface_visibility = BackfaceVisibility::kInherited;
unsigned rendering_context_id = 0; unsigned rendering_context_id = 0;
CompositingReasons direct_compositing_reasons = CompositingReason::kNone; CompositingReasons direct_compositing_reasons = CompositingReason::kNone;
...@@ -128,6 +129,7 @@ class PLATFORM_EXPORT TransformPaintPropertyNode ...@@ -128,6 +129,7 @@ class PLATFORM_EXPORT TransformPaintPropertyNode
if (flattens_inherited_transform != other.flattens_inherited_transform || if (flattens_inherited_transform != other.flattens_inherited_transform ||
affected_by_outer_viewport_bounds_delta != affected_by_outer_viewport_bounds_delta !=
other.affected_by_outer_viewport_bounds_delta || other.affected_by_outer_viewport_bounds_delta ||
in_subtree_of_page_scale != other.in_subtree_of_page_scale ||
backface_visibility != other.backface_visibility || backface_visibility != other.backface_visibility ||
rendering_context_id != other.rendering_context_id || rendering_context_id != other.rendering_context_id ||
compositor_element_id != other.compositor_element_id || compositor_element_id != other.compositor_element_id ||
...@@ -275,6 +277,12 @@ class PLATFORM_EXPORT TransformPaintPropertyNode ...@@ -275,6 +277,12 @@ class PLATFORM_EXPORT TransformPaintPropertyNode
return state_.affected_by_outer_viewport_bounds_delta; return state_.affected_by_outer_viewport_bounds_delta;
} }
// If true, this node is a descendant of the page scale transform. This is
// important for avoiding raster during pinch-zoom (see: crbug.com/951861).
bool IsInSubtreeOfPageScale() const {
return state_.in_subtree_of_page_scale;
}
const cc::LayerStickyPositionConstraint* GetStickyConstraint() const { const cc::LayerStickyPositionConstraint* GetStickyConstraint() const {
return state_.sticky_constraint.get(); return state_.sticky_constraint.get();
} }
......
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