Commit 5a5647c5 authored by Xianzhu Wang's avatar Xianzhu Wang Committed by Commit Bot

[PE] Fix fragment clip and paint offset under multicol vertical-rl scrolled

The previous code in VisualOffsetFromPaintOffsetRoot() converting the result
into scrolling contents space used ScrolledContentOffset which mismatched
the new ScrollTranslation which also includes scroll origin. Now use
ScrollTranslation instead.

Also removed the wrong remedy about scroll origin in one of the callers.

Bug: 887423
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I9a72ed6ddc066c5b4c9970037ed4c4287c5a2f0f
Reviewed-on: https://chromium-review.googlesource.com/1239170
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593572}
parent 3f1011a3
......@@ -767,6 +767,7 @@ crbug.com/591099 virtual/layout_ng_experimental/external/wpt/css/css-multicol/mu
crbug.com/591099 virtual/layout_ng_experimental/external/wpt/css/css-multicol/multicol-table-cell-height-001.xht [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/external/wpt/css/css-multicol/multicol-table-cell-height-002.xht [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/external/wpt/css/css-multicol/multicol-table-cell-vertical-align-001.xht [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/external/wpt/css/css-multicol/multicol-under-vertical-rl-scroll.html [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/external/wpt/css/css-multicol/multicol-width-001.xht [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/external/wpt/css/css-multicol/multicol-width-002.xht [ Failure ]
crbug.com/591099 virtual/layout_ng_experimental/external/wpt/css/css-multicol/multicol-width-003.xht [ Failure ]
......
<!DOCTYPE html>
<title>Multicol under vertical-rl scrolling container</title>
<p>Passes if there are two green squares</p>
<div id="scroller" style="width: 200px; height: 200px; overflow: scroll; border: 1px solid black">
<div style="width: 580px; height: 500px; position: relative">
<div style="position: absolute; right: 0">
<div style="width: 80px; height: 80px; background: green"></div>
<div style="height: 20px"></div>
<div style="width: 80px; height: 80px; background: green"></div>
</div>
</div>
</div>
<script>
// Scroll all the way to the right.
scroller.scrollLeft = 800;
</script>
<!DOCTYPE html>
<title>Multicol under vertical-rl scrolling container</title>
<link rel="match" href="multicol-under-vertical-rl-scroll-ref.html">
<link rel="help" href="https://drafts.csswg.org/css-multicol">
<p>Passes if there are two green squares</p>
<div style="width: 200px; height: 200px; overflow: scroll; writing-mode: vertical-rl; border: 1px solid black">
<div style="columns: 2; column-gap: 20px; width: 80px; height: 180px">
<div style="width: 160px; background: green"></div>
</div>
<div style="width: 500px; height: 500px"></div>
</div>
......@@ -1199,10 +1199,14 @@ static LayoutPoint VisualOffsetFromPaintOffsetRoot(
&painting_layer->GetLayoutObject()));
}
// Don't include scroll offset of paint_offset_root. Any scroll is
// already included in a separate transform node.
if (paint_offset_root->HasOverflowClip())
result += ToLayoutBox(paint_offset_root)->ScrolledContentOffset();
// Convert the result into the space of the scrolling contents space.
if (const auto* properties =
paint_offset_root->FirstFragment().PaintProperties()) {
if (const auto* scroll_translation = properties->ScrollTranslation()) {
DCHECK(scroll_translation->Matrix().IsIdentityOr2DTranslation());
result += -LayoutSize(scroll_translation->Matrix().To2DTranslation());
}
}
return result;
}
......@@ -1848,16 +1852,8 @@ void FragmentPaintPropertyTreeBuilder::UpdatePaintOffset() {
const auto& paint_offset_root_fragment =
context_.current.paint_offset_root->FirstFragment();
paint_offset.MoveBy(paint_offset_root_fragment.PaintOffset());
if (paint_offset_root_fragment.PaintProperties() &&
paint_offset_root_fragment.PaintProperties()->ScrollTranslation()) {
// This duplicates the logic of the additional paint offset for scrolling
// contents in UpdateScrollTranslation().
paint_offset.MoveBy(
ToLayoutBox(context_.current.paint_offset_root)->ScrollOrigin());
}
context_.current.paint_offset = paint_offset;
return;
}
......
......@@ -439,6 +439,49 @@ TEST_P(PaintPropertyTreeBuilderTest, OverflowScrollRTL) {
EXPECT_EQ(FloatRoundedRect(25, 10, 85, 85), overflow_clip->ClipRect());
}
TEST_P(PaintPropertyTreeBuilderTest, OverflowScrollVerticalRLMulticol) {
SetBodyInnerHTML(R"HTML(
<style>::-webkit-scrollbar {width: 15px; height: 15px}</style>
<div id='scroller'
style='width: 100px; height: 100px; overflow: scroll;
writing-mode: vertical-rl; border: 10px solid blue'>
<div id="multicol"
style="width: 50px; height: 400px; columns: 2; column-gap: 0">
<div style="width: 100px"></div>
</div>
<div style='width: 400px; height: 400px'></div>
</div>
)HTML");
const auto* flow_thread =
GetLayoutObjectByElementId("multicol")->SlowFirstChild();
auto check_fragments = [flow_thread]() {
ASSERT_EQ(2u, NumFragments(flow_thread));
EXPECT_EQ(410, FragmentAt(flow_thread, 0)
.PaintProperties()
->FragmentClip()
->ClipRect()
.Rect()
.X());
EXPECT_EQ(LayoutPoint(360, 10), FragmentAt(flow_thread, 0).PaintOffset());
EXPECT_EQ(460, FragmentAt(flow_thread, 1)
.PaintProperties()
->FragmentClip()
->ClipRect()
.Rect()
.MaxX());
EXPECT_EQ(LayoutPoint(410, 210), FragmentAt(flow_thread, 1).PaintOffset());
};
check_fragments();
// Fragment geometries are not affected by parent scrolling.
ToLayoutBox(GetLayoutObjectByElementId("scroller"))
->GetScrollableArea()
->ScrollBy(ScrollOffset(-100, 200), kUserScroll);
GetDocument().View()->UpdateAllLifecyclePhases();
check_fragments();
}
TEST_P(PaintPropertyTreeBuilderTest, DocScrollingTraditional) {
SetBodyInnerHTML("<style> body { height: 10000px; } </style>");
......
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