Skip to content

Commit

Permalink
Consider transform change countering layout shift
Browse files Browse the repository at this point in the history
This is to ignore layout shift when transform and location change at the
same time, and visual representation is kept unchanged. This happens in
many websites containing carousel UI.

Summary of layout_shift.cluster_telemetry
https://ct.skia.org/results/cluster-telemetry/tasks/chromium_perf_runs/wangxianzhu-ChromiumPerf-5629/html/index.html:
mainFrameCumulativeLayoutShift	-4.5%
overallCumulativeLayoutShift	-2.5%

Bug: 1169326
Change-Id: Icb8f5748e927753ef244429699cc1396d8cb7f71
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2673965
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#854104}
  • Loading branch information
wangxianzhu authored and Chromium LUCI CQ committed Feb 16, 2021
1 parent b69b75d commit af4f44f
Show file tree
Hide file tree
Showing 9 changed files with 147 additions and 59 deletions.
5 changes: 5 additions & 0 deletions docs/speed/metrics_changelog/2021_02_cls.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@
[Source code for change 3](https://chromium-review.googlesource.com/c/chromium/src/+/2665761)
[Source code for change 4](https://chromium-review.googlesource.com/c/chromium/src/+/2690998)

### Consider transform change countering layout shift

Corresponds to [the spec change](https://github.com/WICG/layout-instability/pull/94)
[Source code](https://chromium-review.googlesource.com/c/chromium/src/+/2673965)

## When were users affected?

Chrome 90 is currently scheduled to be released the week of April 30, 2021.
1 change: 1 addition & 0 deletions docs/speed/metrics_changelog/cls.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ This is a list of changes to [Cumulative Layout Shift](https://web.dev/cls).

* Chrome 90
* Metric definition improvement: [Bug fixes involving changes to transform, effect, clip or position](2021_02_cls.md)
* Metric definition improvement: [Consider transform change countering layout shift](2021_02_cls.md)
* Chrome 89
* Metric definition improvement: [Ignore layout shift under opacity:0](2020_12_cls.md)
* Metric definition improvement: [Clip layout shift rect by visual viewport](2020_12_cls.md)
Expand Down
28 changes: 28 additions & 0 deletions third_party/blink/renderer/core/layout/layout_shift_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ void LayoutShiftTracker::ObjectShifted(
const PhysicalRect& old_rect,
const PhysicalRect& new_rect,
const FloatPoint& old_starting_point,
const FloatPoint& old_transform_indifferent_starting_point,
const FloatPoint& new_starting_point) {
// The caller should ensure these conditions.
DCHECK(!old_rect.IsEmpty());
Expand All @@ -217,6 +218,11 @@ void LayoutShiftTracker::ObjectShifted(
threshold_physical_px))
return;

if (old_transform_indifferent_starting_point != old_starting_point &&
EqualWithinMovementThreshold(old_transform_indifferent_starting_point,
new_starting_point, threshold_physical_px))
return;

if (SmallerThanRegionGranularity(old_rect) &&
SmallerThanRegionGranularity(new_rect))
return;
Expand Down Expand Up @@ -257,6 +263,20 @@ void LayoutShiftTracker::ObjectShifted(
return;
}

if (old_transform_indifferent_starting_point != old_starting_point) {
FloatPoint old_transform_indifferent_starting_point_in_root =
transform.MapPoint(old_transform_indifferent_starting_point);
if (EqualWithinMovementThreshold(
old_transform_indifferent_starting_point_in_root,
new_starting_point_in_root, threshold_physical_px))
return;
if (EqualWithinMovementThreshold(
old_transform_indifferent_starting_point_in_root +
frame_scroll_delta_,
new_starting_point_in_root, threshold_physical_px))
return;
}

FloatRect old_rect_in_root(old_rect);
transform.MapRect(old_rect_in_root);
FloatRect new_rect_in_root(new_rect);
Expand Down Expand Up @@ -353,10 +373,13 @@ void LayoutShiftTracker::NotifyBoxPrePaint(
const PhysicalRect& old_rect,
const PhysicalRect& new_rect,
const PhysicalOffset& old_paint_offset,
const PhysicalOffset& old_transform_indifferent_paint_offset,
const PhysicalOffset& new_paint_offset) {
DCHECK(NeedsToTrack(box));
ObjectShifted(box, property_tree_state, old_rect, new_rect,
StartingPoint(old_paint_offset, box, box.PreviousSize()),
StartingPoint(old_transform_indifferent_paint_offset, box,
box.PreviousSize()),
StartingPoint(new_paint_offset, box, box.Size()));
}

Expand All @@ -366,6 +389,7 @@ void LayoutShiftTracker::NotifyTextPrePaint(
const LogicalOffset& old_starting_point,
const LogicalOffset& new_starting_point,
const PhysicalOffset& old_paint_offset,
const PhysicalOffset& old_transform_indifferent_paint_offset,
const PhysicalOffset& new_paint_offset,
LayoutUnit logical_height) {
DCHECK(NeedsToTrack(text));
Expand All @@ -377,6 +401,9 @@ void LayoutShiftTracker::NotifyTextPrePaint(
old_paint_offset + old_starting_point.ConvertToPhysical(writing_direction,
block->old_size_,
PhysicalSize());
PhysicalOffset old_transform_indifferent_physical_starting_point =
old_physical_starting_point + old_transform_indifferent_paint_offset -
old_paint_offset;
PhysicalOffset new_physical_starting_point =
new_paint_offset + new_starting_point.ConvertToPhysical(writing_direction,
block->new_size_,
Expand All @@ -395,6 +422,7 @@ void LayoutShiftTracker::NotifyTextPrePaint(

ObjectShifted(text, property_tree_state, old_rect, new_rect,
FloatPoint(old_physical_starting_point),
FloatPoint(old_transform_indifferent_physical_starting_point),
FloatPoint(new_physical_starting_point));
}

Expand Down
35 changes: 21 additions & 14 deletions third_party/blink/renderer/core/layout/layout_shift_tracker.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,26 @@ class CORE_EXPORT LayoutShiftTracker final
// |old_rect| and |old_paint_offset| so that we can calculate the correct old
// visual representation and old starting point in the initial containing
// block and the viewport with the new property tree state in most cases.
void NotifyBoxPrePaint(const LayoutBox& box,
const PropertyTreeStateOrAlias& property_tree_state,
const PhysicalRect& old_rect,
const PhysicalRect& new_rect,
const PhysicalOffset& old_paint_offset,
const PhysicalOffset& new_paint_offset);

void NotifyTextPrePaint(const LayoutText& text,
const PropertyTreeStateOrAlias& property_tree_state,
const LogicalOffset& old_starting_point,
const LogicalOffset& new_starting_point,
const PhysicalOffset& old_paint_offset,
const PhysicalOffset& new_paint_offset,
const LayoutUnit logical_height);
// |old_transform_indifferent_paint_offset| is the adjusted old paint offset
// with transform changes excluded.
void NotifyBoxPrePaint(
const LayoutBox& box,
const PropertyTreeStateOrAlias& property_tree_state,
const PhysicalRect& old_rect,
const PhysicalRect& new_rect,
const PhysicalOffset& old_paint_offset,
const PhysicalOffset& old_transform_indifferent_paint_offset,
const PhysicalOffset& new_paint_offset);

void NotifyTextPrePaint(
const LayoutText& text,
const PropertyTreeStateOrAlias& property_tree_state,
const LogicalOffset& old_starting_point,
const LogicalOffset& new_starting_point,
const PhysicalOffset& old_paint_offset,
const PhysicalOffset& old_transform_indifferent_paint_offset,
const PhysicalOffset& new_paint_offset,
const LayoutUnit logical_height);

void NotifyPrePaintFinished();
void NotifyInput(const WebInputEvent&);
Expand Down Expand Up @@ -146,6 +152,7 @@ class CORE_EXPORT LayoutShiftTracker final
const PhysicalRect& old_rect,
const PhysicalRect& new_rect,
const FloatPoint& old_starting_point,
const FloatPoint& old_transform_indifferent_starting_point,
const FloatPoint& new_starting_point);

void ReportShift(double score_delta, double weighted_score_delta);
Expand Down
73 changes: 35 additions & 38 deletions third_party/blink/renderer/core/paint/paint_invalidator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,17 @@ void PaintInvalidator::UpdateLayoutShiftTracking(
*tree_builder_context.current.transform,
*tree_builder_context.current.clip, *tree_builder_context.current_effect);

// Adjust old_paint_offset so that LayoutShiftTracker will see the change of
// offset caused by change of paint offset translations below the layout shift
// root.
PhysicalOffset adjusted_old_transform_indifferent_paint_offset =
context.old_paint_offset -
tree_builder_context.current.additional_offset_to_layout_shift_root_delta;
PhysicalOffset adjusted_old_paint_offset =
adjusted_old_transform_indifferent_paint_offset -
tree_builder_context.translation_2d_to_layout_shift_root_delta;
PhysicalOffset new_paint_offset = tree_builder_context.current.paint_offset;

if (object.IsText()) {
const auto& text = To<LayoutText>(object);
LogicalOffset new_starting_point;
Expand All @@ -217,19 +228,33 @@ void PaintInvalidator::UpdateLayoutShiftTracking(

layout_shift_tracker.NotifyTextPrePaint(
text, property_tree_state, old_starting_point, new_starting_point,
// Similar to the adjustment of old_paint_offset for LayoutBox.
context.old_paint_offset -
tree_builder_context.current
.additional_offset_to_layout_shift_root_delta,
tree_builder_context.current.paint_offset, logical_height);
adjusted_old_paint_offset,
adjusted_old_transform_indifferent_paint_offset, new_paint_offset,
logical_height);
return;
}

DCHECK(object.IsBox());
const auto& box = To<LayoutBox>(object);

PhysicalRect new_rect = box.PhysicalVisualOverflowRect();
new_rect.Move(new_paint_offset);
PhysicalRect old_rect = box.PreviousPhysicalVisualOverflowRect();
old_rect.Move(adjusted_old_paint_offset);

bool should_create_containing_block_scope =
// TODO(crbug.com/1178618): Support multiple-fragments when switching to
// LayoutNGFragmentTraversal.
context.fragment_data == &box.FirstFragment() &&
box.IsLayoutBlockFlow() && box.ChildrenInline() && box.SlowFirstChild();
if (should_create_containing_block_scope) {
// For layout shift tracking of contained LayoutTexts.
context.containing_block_scope_ =
std::make_unique<LayoutShiftTracker::ContainingBlockScope>(
PhysicalSizeToBeNoop(box.PreviousSize()),
PhysicalSizeToBeNoop(box.Size()), old_rect, new_rect);
}

bool should_report_layout_shift = [&]() -> bool {
if (box.ShouldSkipNextLayoutShiftTracking()) {
box.GetMutableForPainting().SetShouldSkipNextLayoutShiftTracking(false);
Expand Down Expand Up @@ -258,41 +283,13 @@ void PaintInvalidator::UpdateLayoutShiftTracking(
// different from that of the parent).
return parent_context->fragment_data->PaintOffset() -
parent_context->old_paint_offset !=
tree_builder_context.current.paint_offset - context.old_paint_offset;
new_paint_offset - context.old_paint_offset;
}();

bool should_create_containing_block_scope =
// TODO(crbug.com/1104064): Support multiple-fragments when switching to
// LayoutNGFragmentTraversal.
context.fragment_data == &box.FirstFragment() &&
box.IsLayoutBlockFlow() && box.ChildrenInline() && box.SlowFirstChild();
if (!should_report_layout_shift && !should_create_containing_block_scope)
return;

new_rect.Move(tree_builder_context.current.paint_offset);
old_rect.Move(context.old_paint_offset);
// Adjust old_visual_rect so that LayoutShiftTracker can see the change of
// offset caused by change of transforms below the layout shift root.
old_rect.Move(-tree_builder_context.current
.additional_offset_to_layout_shift_root_delta);

if (should_create_containing_block_scope) {
// For layout shift tracking of contained LayoutTexts.
context.containing_block_scope_ =
std::make_unique<LayoutShiftTracker::ContainingBlockScope>(
PhysicalSizeToBeNoop(box.PreviousSize()),
PhysicalSizeToBeNoop(box.Size()), old_rect, new_rect);
if (!should_report_layout_shift)
return;
if (should_report_layout_shift) {
layout_shift_tracker.NotifyBoxPrePaint(
box, property_tree_state, old_rect, new_rect, adjusted_old_paint_offset,
adjusted_old_transform_indifferent_paint_offset, new_paint_offset);
}

// Adjust old_paint_offset similarly.
PhysicalOffset old_paint_offset =
context.old_paint_offset -
tree_builder_context.current.additional_offset_to_layout_shift_root_delta;
layout_shift_tracker.NotifyBoxPrePaint(
box, property_tree_state, old_rect, new_rect, old_paint_offset,
tree_builder_context.current.paint_offset);
}

bool PaintInvalidator::InvalidatePaint(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -953,16 +953,19 @@ void FragmentPaintPropertyTreeBuilder::UpdateTransform() {
// properties_->Transform() is present if a CSS transform is present,
// and is also present if transform-style: preserve-3d is set.
// See NeedsTransform.
if (properties_->Transform()) {
context_.current.transform = properties_->Transform();
if (const auto* transform = properties_->Transform()) {
context_.current.transform = transform;
if (object_.StyleRef().Preserves3D()) {
context_.current.rendering_context_id =
properties_->Transform()->RenderingContextId();
context_.current.rendering_context_id = transform->RenderingContextId();
context_.current.should_flatten_inherited_transform = false;
} else {
context_.current.rendering_context_id = 0;
context_.current.should_flatten_inherited_transform = true;
}
if (transform->IsIdentityOr2DTranslation()) {
context_.translation_2d_to_layout_shift_root_delta +=
PhysicalOffset::FromFloatSizeRound(transform->Translation2D());
}
} else if (RuntimeEnabledFeatures::TransformInteropEnabled() &&
!object_.IsAnonymous()) {
// With kTransformInterop enabled, 3D rendering contexts follow the
Expand Down Expand Up @@ -2621,7 +2624,7 @@ void FragmentPaintPropertyTreeBuilder::UpdateForSelf() {
// the object itself.
if (IsA<LayoutView>(object_)) {
context_.current.additional_offset_to_layout_shift_root_delta =
PhysicalOffset();
context_.translation_2d_to_layout_shift_root_delta = PhysicalOffset();
}
}

Expand Down Expand Up @@ -2656,6 +2659,7 @@ void FragmentPaintPropertyTreeBuilder::UpdateForChildren() {
// additional_offset_to_layout_shift_root_delta.
context_.current.additional_offset_to_layout_shift_root_delta =
context_.old_paint_offset - fragment_data_.PaintOffset();
context_.translation_2d_to_layout_shift_root_delta = PhysicalOffset();
}

#if DCHECK_IS_ON()
Expand Down Expand Up @@ -2691,6 +2695,12 @@ void PaintPropertyTreeBuilder::InitFragmentPaintProperties(
context.pending_additional_offset_to_layout_shift_root_delta =
-PhysicalOffset::FromFloatSizeRound(translation->Translation2D());
}
if (const auto* transform = properties->Transform()) {
if (transform->IsIdentityOr2DTranslation()) {
context.translation_2d_to_layout_shift_root_delta -=
PhysicalOffset::FromFloatSizeRound(transform->Translation2D());
}
}
}

if (needs_paint_properties) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,10 @@ struct PaintPropertyTreeBuilderFragmentContext {
// ContainingBlockContext is set, this value should be added to
// ContainingBlockContext::additional_offset_to_layout_shift_root_delta.
PhysicalOffset pending_additional_offset_to_layout_shift_root_delta;

// The delta between the old and new accumulated offsets of 2d translation
// transforms to the layout shift root.
PhysicalOffset translation_2d_to_layout_shift_root_delta;
};

struct PaintPropertyTreeBuilderContext {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<!DOCTYPE html>
<title>Layout Instability: transform change</title>
<title>Layout Instability: no layout shift for transform change</title>
<link rel="help" href="https://wicg.github.io/layout-instability/" />
<style>
body { margin: 0; }
Expand Down Expand Up @@ -28,6 +28,6 @@
await waitForAnimationFrames(2);
// No shift should be reported.
assert_equals(watcher.score, 0);
}, 'transform change');
}, 'no layout shift for transform change');

</script>
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<!DOCTYPE html>
<title>Layout Instability: no layout shift if transform change counters location change</title>
<link rel="help" href="https://wicg.github.io/layout-instability/" />
<style>
body { margin: 0; }
#transformed { position: relative; transform: translateX(20px); width: 100px; height: 100px; }
#child { width: 400px; height: 400px; }
</style>
<div id="transformed">
<div id="child"></div>
</div>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="resources/util.js"></script>
<script>

promise_test(async () => {
const watcher = new ScoreWatcher;

// Wait for the initial render to complete.
await waitForAnimationFrames(2);

// Modify the transform and the location at the same time, and the values
// cancel each other visually, for which no shift should be reported.
transformed.style.transform = 'translateY(100px)';
transformed.style.top = '-100px';
transformed.style.left = '20px';
// Change size of child, for which no shift should be reported, either.
child.style.width = '300px';

await waitForAnimationFrames(2);
// No shift should be reported.
assert_equals(watcher.score, 0);
}, 'no layout shift if transform change counters location change');

</script>

0 comments on commit af4f44f

Please sign in to comment.