Commit 9feeebdf authored by Xianzhu Wang's avatar Xianzhu Wang Committed by Commit Bot

More accurate layout shift tracking for texts

Previously when a text shifted, we assumed the whole area below the
text in the containing block shifted, which might report too big
shifted area, e.g. if the text shift was just in one line.

Now pass the logical height of the shifted text and calculate the
shifted area for the text only. Shifts of other texts below are
checked separately.

Bug: 1121405
Change-Id: I029c5dfcf6d0537a9be4f8c33cac9d63bd4b4f1d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2386842
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#803755}
parent 93332342
......@@ -65,14 +65,19 @@ FloatPoint StartingPoint(const PhysicalOffset& paint_offset,
// Returns the part a rect logically below a starting point.
PhysicalRect RectBelowStartingPoint(const PhysicalRect& rect,
const PhysicalOffset& starting_point,
LayoutUnit logical_height,
WritingDirectionMode writing_direction) {
PhysicalRect result = rect;
if (writing_direction.IsHorizontal())
if (writing_direction.IsHorizontal()) {
result.ShiftTopEdgeTo(starting_point.top);
else if (writing_direction.IsFlippedBlocks())
result.ShiftRightEdgeTo(starting_point.left);
else
result.ShiftLeftEdgeTo(starting_point.left);
result.SetHeight(logical_height);
} else {
result.SetWidth(logical_height);
if (writing_direction.IsFlippedBlocks())
result.ShiftRightEdgeTo(starting_point.left);
else
result.ShiftLeftEdgeTo(starting_point.left);
}
return result;
}
......@@ -155,7 +160,7 @@ bool LayoutShiftTracker::NeedsToTrack(const LayoutObject& object) const {
return false;
if (object.IsText())
return ContainingBlockScope::top_;
return !object.IsBR() && ContainingBlockScope::top_;
if (!object.IsBox())
return false;
......@@ -350,19 +355,12 @@ void LayoutShiftTracker::NotifyTextPrePaint(
const LogicalOffset& old_starting_point,
const LogicalOffset& new_starting_point,
const PhysicalOffset& old_paint_offset,
const PhysicalOffset& new_paint_offset) {
const PhysicalOffset& new_paint_offset,
LayoutUnit logical_height) {
DCHECK(NeedsToTrack(text));
auto* block = ContainingBlockScope::top_;
DCHECK(block);
LayoutUnit distance = std::max(
(new_starting_point.inline_offset - old_starting_point.inline_offset)
.Abs(),
(new_starting_point.block_offset - old_starting_point.block_offset)
.Abs());
if (distance <= block->max_text_shift_distance_)
return;
block->max_text_shift_distance_ = distance;
auto writing_direction = text.StyleRef().GetWritingDirection();
PhysicalOffset old_physical_starting_point =
old_paint_offset + old_starting_point.ConvertToPhysical(writing_direction,
......@@ -373,12 +371,14 @@ void LayoutShiftTracker::NotifyTextPrePaint(
block->new_size_,
PhysicalSize());
PhysicalRect old_rect = RectBelowStartingPoint(
block->old_rect_, old_physical_starting_point, writing_direction);
PhysicalRect old_rect =
RectBelowStartingPoint(block->old_rect_, old_physical_starting_point,
logical_height, writing_direction);
if (old_rect.IsEmpty())
return;
PhysicalRect new_rect = RectBelowStartingPoint(
block->new_rect_, new_physical_starting_point, writing_direction);
PhysicalRect new_rect =
RectBelowStartingPoint(block->new_rect_, new_physical_starting_point,
logical_height, writing_direction);
if (new_rect.IsEmpty())
return;
......
......@@ -58,7 +58,8 @@ class CORE_EXPORT LayoutShiftTracker final
const LogicalOffset& old_starting_point,
const LogicalOffset& new_starting_point,
const PhysicalOffset& old_paint_offset,
const PhysicalOffset& new_paint_offset);
const PhysicalOffset& new_paint_offset,
const LayoutUnit logical_height);
void NotifyPrePaintFinished();
void NotifyInput(const WebInputEvent&);
......@@ -137,7 +138,6 @@ class CORE_EXPORT LayoutShiftTracker final
PhysicalSize new_size_;
PhysicalRect old_rect_;
PhysicalRect new_rect_;
LayoutUnit max_text_shift_distance_;
};
private:
......
......@@ -43,6 +43,7 @@
#include "third_party/blink/renderer/core/layout/api/line_layout_api_shim.h"
#include "third_party/blink/renderer/core/layout/api/line_layout_box.h"
#include "third_party/blink/renderer/core/layout/geometry/logical_rect.h"
#include "third_party/blink/renderer/core/layout/geometry/writing_mode_converter.h"
#include "third_party/blink/renderer/core/layout/layout_block.h"
#include "third_party/blink/renderer/core/layout/layout_object_factory.h"
#include "third_party/blink/renderer/core/layout/layout_table_cell.h"
......@@ -1691,23 +1692,40 @@ PhysicalOffset LayoutText::FirstLineBoxTopLeft() const {
return PhysicalOffset();
}
LogicalOffset LayoutText::LogicalStartingPoint() const {
void LayoutText::LogicalStartingPointAndHeight(
LogicalOffset& logical_starting_point,
LayoutUnit& logical_height) const {
if (IsInLayoutNGInlineFormattingContext()) {
NGInlineCursor cursor;
cursor.MoveTo(*this);
if (!cursor)
return LogicalOffset();
return;
PhysicalOffset physical_offset = cursor.Current().OffsetInContainerBlock();
if (StyleRef().GetWritingDirection().IsHorizontalLtr())
return {physical_offset.left, physical_offset.top};
return physical_offset.ConvertToLogical(
StyleRef().GetWritingDirection(),
PhysicalSizeToBeNoop(ContainingBlock()->Size()),
cursor.Current().Size());
}
if (const auto* text_box = FirstTextBox())
return {text_box->LogicalLeft(), text_box->LogicalTop()};
return LogicalOffset();
if (StyleRef().GetWritingDirection().IsHorizontalLtr()) {
cursor.MoveToLastForSameLayoutObject();
logical_height = cursor.Current().RectInContainerBlock().Bottom() -
physical_offset.top;
logical_starting_point = {physical_offset.left, physical_offset.top};
return;
}
PhysicalSize outer_size = PhysicalSizeToBeNoop(ContainingBlock()->Size());
logical_starting_point = physical_offset.ConvertToLogical(
StyleRef().GetWritingDirection(), outer_size, cursor.Current().Size());
cursor.MoveToLastForSameLayoutObject();
PhysicalRect last_physical_rect = cursor.Current().RectInContainerBlock();
LogicalOffset logical_ending_point =
WritingModeConverter(StyleRef().GetWritingDirection(), outer_size)
.ToLogical(last_physical_rect)
.EndOffset();
logical_height =
logical_ending_point.block_offset - logical_starting_point.block_offset;
return;
}
if (const auto* text_box = FirstTextBox()) {
logical_starting_point = {text_box->LogicalLeft(), text_box->LogicalTop()};
logical_height = LastTextBox()->LogicalBottom() - text_box->LogicalTop();
}
}
bool LayoutText::CanOptimizeSetText() const {
......
......@@ -332,8 +332,10 @@ class CORE_EXPORT LayoutText : public LayoutObject {
void DetachAbstractInlineTextBoxesIfNeeded();
// Returns the logical location of the first line box.
LogicalOffset LogicalStartingPoint() const;
// Returns the logical location of the first line box, and the logical height
// of the LayoutText.
void LogicalStartingPointAndHeight(LogicalOffset& logical_starting_point,
LayoutUnit& logical_height) const;
// For LayoutShiftTracker. Saves the value of LogicalStartingPoint() value
// during the previous paint invalidation.
......
......@@ -180,7 +180,9 @@ void PaintInvalidator::UpdateLayoutShiftTracking(
if (object.IsText()) {
const auto& text = ToLayoutText(object);
LogicalOffset new_starting_point = text.LogicalStartingPoint();
LogicalOffset new_starting_point;
LayoutUnit logical_height;
text.LogicalStartingPointAndHeight(new_starting_point, logical_height);
LogicalOffset old_starting_point = text.PreviousLogicalStartingPoint();
if (new_starting_point == old_starting_point)
return;
......@@ -198,7 +200,7 @@ void PaintInvalidator::UpdateLayoutShiftTracking(
context.old_paint_offset -
tree_builder_context.current
.additional_offset_to_layout_shift_root_delta,
tree_builder_context.current.paint_offset);
tree_builder_context.current.paint_offset, logical_height);
return;
}
......
<!DOCTYPE html>
<title>Layout Instability: inline/text movement is detected</title>
<link rel="help" href="https://wicg.github.io/layout-instability/" />
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="resources/util.js"></script>
<div style="width: 200px; font-size: 20px; line-height: 25px">
1AAAAAAA<br>
2AAAAAAA<br>
3AAAAAAA<br>
<div id="inline-block" style="display: inline-block">4AAAAAAA</div><br>
<div id="shift" style="display: inline-block"></div>5AAAAAAA<br>
6AAAAAAA<br>
7AAAAAAA<br>
</div>
<script>
promise_test(async () => {
const watcher = new ScoreWatcher;
// Wait for the initial render to complete.
await waitForAnimationFrames(2);
// Modify the width of |shift|.
document.querySelector("#shift").style.width = '50px';
// Only the line after |shift| is shifted right by 50px.
// The implementation may measure the real width of the shifted text
// or use the available width (i.e. width of the containing block).
// Also tolerate extra 10% error.
const text_width = document.querySelector("#inline-block").clientWidth;
const expectedScoreMin = computeExpectedScore((text_width + 50) * 20, 50) * 0.9;
const expectedScoreMax = computeExpectedScore(200 * 25, 50) * 1.1;
// Observer fires after the frame is painted.
assert_equals(watcher.score, 0);
await watcher.promise;
assert_between_exclusive(watcher.score, expectedScoreMin, expectedScoreMax);
}, 'Inline flow movement.');
</script>
......@@ -31,8 +31,8 @@ promise_test(async () => {
// or use the available width (i.e. width of the containing block).
// Also tolerate extra 10% error.
const text_width = inline_block.offsetWidth;
const expectedScoreMin = computeExpectedScore(text_width * (30 * 3 + 50), 50) * 0.9;
const expectedScoreMax = computeExpectedScore(200 * (30 * 3 + 50), 50) * 1.1;
const expectedScoreMin = computeExpectedScore(text_width * (20 * 3 + 50), 50) * 0.9;
const expectedScoreMax = computeExpectedScore(200 * (25 * 3 + 50), 50) * 1.1;
// Observer fires after the frame is painted.
assert_equals(watcher.score, 0);
......
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