Skip to content

Commit

Permalink
Reland "Remove some blink optimizations about 2d translations"
Browse files Browse the repository at this point in the history
This reverts commit 67bedec.

Reason for revert: Fix the build issue.

Original change's description:
> Revert "Remove some blink optimizations about 2d translations"
>
> This reverts commit 41bc3ae.
>
> Reason for revert: broken build https://ci.chromium.org/ui/p/chromium/builders/ci/Mac%20Builder/179363/overview
>
> Original change's description:
> > Remove some blink optimizations about 2d translations
> >
> > We have combined blink::TransformationMatrix and gfx::Transform.
> > gfx::Transform has built-in optimization for simple 2d
> > scale/translation transforms, so the blink optimizations about 2d
> > translations are redundant.
> >
> > Bug: 1359528
> > Change-Id: I914634e458183defc9d539e3cf4d034124e97270
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4009620
> > Reviewed-by: Philip Rogers <pdr@chromium.org>
> > Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
> > Owners-Override: Xianzhu Wang <wangxianzhu@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#1070507}
>
> Bug: 1359528
> Change-Id: I4b3d231d933c79d92ad05e6bf01bebb28b5883f8
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4024029
> Commit-Queue: Eugene Zemtsov <ezemtsov@google.com>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Owners-Override: Eugene Zemtsov <ezemtsov@google.com>
> Cr-Commit-Position: refs/heads/main@{#1070519}

Bug: 1359528
Change-Id: Ib6e219f018f0fb32454316ccbdb80cfce7799bb2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4024030
Auto-Submit: Xianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Owners-Override: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1070701}
  • Loading branch information
wangxianzhu authored and Chromium LUCI CQ committed Nov 12, 2022
1 parent 70faa04 commit 364ea96
Show file tree
Hide file tree
Showing 48 changed files with 508 additions and 717 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2043,9 +2043,7 @@ namespace {

void UpdateDummyTransformNode(ObjectPaintProperties& properties,
CompositingReasons reasons) {
// Initialize with gfx::Transform() to avoid 2d translation optimization
// in case of transform animation.
TransformPaintPropertyNode::State state{gfx::Transform()};
TransformPaintPropertyNode::State state;
state.direct_compositing_reasons = reasons;
properties.UpdateTransform(TransformPaintPropertyNode::Root(),
std::move(state));
Expand Down
7 changes: 4 additions & 3 deletions third_party/blink/renderer/core/frame/visual_viewport.cc
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ PaintPropertyChangeType VisualViewport::UpdatePaintPropertyNodesIfNeeded(
const auto& device_emulation_transform =
GetChromeClient()->GetDeviceEmulationTransform();
if (!device_emulation_transform.IsIdentity()) {
TransformPaintPropertyNode::State state{device_emulation_transform};
TransformPaintPropertyNode::State state{{device_emulation_transform}};
state.flags.in_subtree_of_page_scale = false;
if (!device_emulation_transform_node_) {
device_emulation_transform_node_ = TransformPaintPropertyNode::Create(
Expand Down Expand Up @@ -228,7 +228,7 @@ PaintPropertyChangeType VisualViewport::UpdatePaintPropertyNodesIfNeeded(

TransformPaintPropertyNode::State state;
if (scale_ != 1.f)
state.transform_and_origin = {gfx::Transform::MakeScale(scale_)};
state.transform_and_origin.matrix = gfx::Transform::MakeScale(scale_);
state.flags.in_subtree_of_page_scale = false;
state.direct_compositing_reasons = CompositingReason::kViewport;
state.compositor_element_id = page_scale_element_id_;
Expand Down Expand Up @@ -309,7 +309,8 @@ PaintPropertyChangeType VisualViewport::UpdatePaintPropertyNodesIfNeeded(
}

{
TransformPaintPropertyNode::State state{-GetScrollOffset()};
TransformPaintPropertyNode::State state{
{gfx::Transform::MakeTranslation(-offset_)}};
state.scroll = scroll_node_;
state.direct_compositing_reasons = CompositingReason::kViewport;
if (!scroll_translation_node_) {
Expand Down
20 changes: 8 additions & 12 deletions third_party/blink/renderer/core/frame/visual_viewport_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -371,14 +371,13 @@ TEST_P(VisualViewportTest, TestResizeAfterVerticalScroll) {
EXPECT_EQ(gfx::Transform::MakeScale(2),
visual_viewport.GetPageScaleNode()->Matrix());
EXPECT_EQ(gfx::Vector2dF(0, -300),
visual_viewport.GetScrollTranslationNode()->Translation2D());
visual_viewport.GetScrollTranslationNode()->Get2dTranslation());
auto expected_projection = gfx::Transform::MakeScale(2);
expected_projection.Translate(0, -300);
EXPECT_EQ(expected_projection,
GeometryMapper::SourceToDestinationProjection(
*visual_viewport.GetScrollTranslationNode(),
TransformPaintPropertyNode::Root())
.Matrix());
TransformPaintPropertyNode::Root()));
}

// Perform the resizing
Expand All @@ -397,14 +396,13 @@ TEST_P(VisualViewportTest, TestResizeAfterVerticalScroll) {
EXPECT_EQ(gfx::Transform::MakeScale(4),
visual_viewport.GetPageScaleNode()->Matrix());
EXPECT_EQ(gfx::Vector2dF(0, -75),
visual_viewport.GetScrollTranslationNode()->Translation2D());
visual_viewport.GetScrollTranslationNode()->Get2dTranslation());
auto expected_projection = gfx::Transform::MakeScale(4);
expected_projection.Translate(0, -75);
EXPECT_EQ(expected_projection,
GeometryMapper::SourceToDestinationProjection(
*visual_viewport.GetScrollTranslationNode(),
TransformPaintPropertyNode::Root())
.Matrix());
TransformPaintPropertyNode::Root()));
}
}

Expand Down Expand Up @@ -462,14 +460,13 @@ TEST_P(VisualViewportTest, TestResizeAfterHorizontalScroll) {
EXPECT_EQ(gfx::Transform::MakeScale(2),
visual_viewport.GetPageScaleNode()->Matrix());
EXPECT_EQ(gfx::Vector2dF(-150, 0),
visual_viewport.GetScrollTranslationNode()->Translation2D());
visual_viewport.GetScrollTranslationNode()->Get2dTranslation());
auto expected_projection = gfx::Transform::MakeScale(2);
expected_projection.Translate(-150, 0);
EXPECT_EQ(expected_projection,
GeometryMapper::SourceToDestinationProjection(
*visual_viewport.GetScrollTranslationNode(),
TransformPaintPropertyNode::Root())
.Matrix());
TransformPaintPropertyNode::Root()));
}

WebView()->MainFrameViewWidget()->Resize(gfx::Size(200, 100));
Expand All @@ -487,14 +484,13 @@ TEST_P(VisualViewportTest, TestResizeAfterHorizontalScroll) {
EXPECT_EQ(gfx::Transform::MakeScale(4),
visual_viewport.GetPageScaleNode()->Matrix());
EXPECT_EQ(gfx::Vector2dF(-150, 0),
visual_viewport.GetScrollTranslationNode()->Translation2D());
visual_viewport.GetScrollTranslationNode()->Get2dTranslation());
auto expected_projection = gfx::Transform::MakeScale(4);
expected_projection.Translate(-150, 0);
EXPECT_EQ(expected_projection,
GeometryMapper::SourceToDestinationProjection(
*visual_viewport.GetScrollTranslationNode(),
TransformPaintPropertyNode::Root())
.Matrix());
TransformPaintPropertyNode::Root()));
}
}

Expand Down
9 changes: 3 additions & 6 deletions third_party/blink/renderer/core/layout/layout_object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1999,12 +1999,9 @@ bool LayoutObject::HasDistortingVisualEffects() const {
const auto& root_properties = root_fragment.LocalBorderBoxProperties();

// The only allowed transforms are 2D translation and proportional up-scaling.
const auto& translation_2d_or_matrix =
GeometryMapper::SourceToDestinationProjection(
paint_properties.Transform(), root_properties.Transform());
if (!translation_2d_or_matrix.IsIdentityOr2DTranslation() &&
!translation_2d_or_matrix.Matrix()
.Is2dProportionalUpscaleAndOr2dTranslation())
gfx::Transform projection = GeometryMapper::SourceToDestinationProjection(
paint_properties.Transform(), root_properties.Transform());
if (!projection.Is2dProportionalUpscaleAndOr2dTranslation())
return true;

return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,9 +309,9 @@ void LayoutShiftTracker::ObjectShifted(
// TODO(crbug.com/1187979): Shift by |scroll_delta| to keep backward
// compatibility in https://crrev.com/c/2754969. See the bug for details.
old_rect_in_root.Offset(scroll_delta);
transform.MapRect(old_rect_in_root);
old_rect_in_root = transform.MapRect(old_rect_in_root);
gfx::RectF new_rect_in_root(new_rect);
transform.MapRect(new_rect_in_root);
new_rect_in_root = transform.MapRect(new_rect_in_root);

gfx::Rect visible_old_rect = gfx::ToRoundedRect(
gfx::IntersectRects(old_rect_in_root, clip_rect.Rect()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ void MobileFriendlinessChecker::UpdateBeyondViewportAreaSizes(
auto projection = GeometryMapper::SourceToDestinationProjection(
current_transform, *viewport_transform_);
if (projection.IsIdentityOr2DTranslation()) {
current_x_offset_ = projection.Translation2D().x();
current_x_offset_ = projection.To2dTranslation().x();
previous_transform_ = &current_transform;
} else {
// For now we ignore offsets caused by non-2d-translation transforms.
Expand Down
5 changes: 2 additions & 3 deletions third_party/blink/renderer/core/paint/cull_rect_updater.cc
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,7 @@ bool ShouldUseInfiniteCullRect(
// "transform: perspective(100px) rotateY(45deg)". In these cases, we
// also want to skip cull rect mapping. See http://crbug.com/887558 for
// details.
if (!transform->IsIdentityOr2DTranslation() &&
transform->Matrix().HasPerspective()) {
if (transform->Matrix().HasPerspective()) {
subtree_should_use_infinite_cull_rect = true;
return true;
}
Expand Down Expand Up @@ -165,7 +164,7 @@ bool HasScrolledEnough(const LayoutObject& object) {
if (const auto* scroll_translation = properties->ScrollTranslation()) {
const auto* scrollable_area = To<LayoutBox>(object).GetScrollableArea();
DCHECK(scrollable_area);
gfx::Vector2dF delta = -scroll_translation->Translation2D() -
gfx::Vector2dF delta = -scroll_translation->Get2dTranslation() -
scrollable_area->LastCullRectUpdateScrollOffset();
return object.FirstFragment().GetContentsCullRect().HasScrolledEnough(
delta, *scroll_translation);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class FindPaintOffsetNeedingUpdateScope {
if (const auto* properties = fragment_data.PaintProperties()) {
if (const auto* translation = properties->PaintOffsetTranslation()) {
old_parent_ = translation->Parent();
old_translation_ = translation->Translation2D();
old_translation_ = translation->Get2dTranslation();
}
}
}
Expand All @@ -54,7 +54,7 @@ class FindPaintOffsetNeedingUpdateScope {
if (const auto* properties = fragment_data_.PaintProperties()) {
if (const auto* translation = properties->PaintOffsetTranslation()) {
new_parent = translation->Parent();
new_translation = translation->Translation2D();
new_translation = translation->Get2dTranslation();
}
}
DCHECK_EQ(!!old_translation_, !!new_translation) << object_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,11 @@ void HitTestingTransformState::Translate(const gfx::Vector2dF& offset) {

void HitTestingTransformState::ApplyTransform(
const TransformPaintPropertyNode& transform) {
if (transform.IsIdentityOr2DTranslation()) {
Translate(transform.Translation2D());
} else {
accumulated_transform_.PreConcat(transform.MatrixWithOriginApplied());
}
accumulated_transform_.PreConcat(transform.MatrixWithOriginApplied());
}

void HitTestingTransformState::ApplyTransform(
const GeometryMapper::Translation2DOrMatrix& transform) {
if (transform.IsIdentityOr2DTranslation()) {
Translate(transform.Translation2D());
} else {
accumulated_transform_.PreConcat(transform.Matrix());
}
void HitTestingTransformState::ApplyTransform(const gfx::Transform& transform) {
accumulated_transform_.PreConcat(transform);
}

void HitTestingTransformState::Flatten() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class HitTestingTransformState {

void Translate(const gfx::Vector2dF&);
void ApplyTransform(const TransformPaintPropertyNode&);
void ApplyTransform(const GeometryMapper::Translation2DOrMatrix&);
void ApplyTransform(const gfx::Transform&);

gfx::PointF MappedPoint() const;
gfx::QuadF MappedQuad() const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,7 @@ bool ShouldUseInfiniteCullRect(const PaintLayer& layer,
// "transform: perspective(100px) rotateY(45deg)". In these cases, we
// also want to skip cull rect mapping. See http://crbug.com/887558 for
// details.
if (!transform->IsIdentityOr2DTranslation() &&
transform->Matrix().HasPerspective()) {
if (transform->Matrix().HasPerspective()) {
subtree_should_use_infinite_cull_rect = true;
return true;
}
Expand Down Expand Up @@ -468,7 +467,7 @@ void OldCullRectUpdater::PaintPropertiesChanged(
// - if the current contents cull rect is infinite and no descendants
// need cull rect update.
needs_cull_rect_update =
scroll_translation->Translation2D() != old_scroll_offset;
scroll_translation->Get2dTranslation() != old_scroll_offset;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -925,7 +925,7 @@ TEST_P(PaintAndRasterInvalidationTest, ScrollingInvalidatesStickyOffset) {
EXPECT_EQ(gfx::Vector2dF(0, 50), sticky->FirstFragment()
.PaintProperties()
->StickyTranslation()
->Translation2D());
->Get2dTranslation());
const auto* inner = GetLayoutObjectByElementId("inner");
EXPECT_EQ(PhysicalOffset(), inner->FirstFragment().PaintOffset());

Expand All @@ -936,7 +936,7 @@ TEST_P(PaintAndRasterInvalidationTest, ScrollingInvalidatesStickyOffset) {
EXPECT_EQ(gfx::Vector2dF(0, 150), sticky->FirstFragment()
.PaintProperties()
->StickyTranslation()
->Translation2D());
->Get2dTranslation());
EXPECT_EQ(PhysicalOffset(), inner->FirstFragment().PaintOffset());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1123,7 +1123,7 @@ TEST_P(MAYBE_PaintLayerScrollableAreaTest, CompositedStickyDescendant) {
EXPECT_EQ(gfx::Vector2dF(0, 50), sticky->FirstFragment()
.PaintProperties()
->StickyTranslation()
->Translation2D());
->Get2dTranslation());
}

TEST_P(MAYBE_PaintLayerScrollableAreaTest, StickyPositionUseCounter) {
Expand Down

0 comments on commit 364ea96

Please sign in to comment.