Commit de75c7d0 authored by Daniel Libby's avatar Daniel Libby Committed by Commit Bot

Change handling of saturated size of uniting PhysicalRects

If a layer has a child that has a large negative inset for the top/left
properties, along with other content, the size of the layer can exceed
the max layout unit in either dimension. When this happens, currently the
min of left/top is used when creating the clip rect for the layer, but
because of the size limitations, the content that is actually in view
is not painted.

Instead of using the min for left/top, favor the positive (right/bottom)
portion of the rect when the size is saturated up against the
LayoutUnit max value.

Bug: 1122807
Change-Id: I8276f1556eebb3c9f197e0fba36571f7e7dd4546
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2391389Reviewed-by: default avatarXianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Daniel Libby <dlibby@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#804560}
parent b7f7d543
...@@ -34,8 +34,10 @@ void LogicalRect::Unite(const LogicalRect& other) { ...@@ -34,8 +34,10 @@ void LogicalRect::Unite(const LogicalRect& other) {
} }
LogicalOffset new_end_offset(Max(EndOffset(), other.EndOffset())); LogicalOffset new_end_offset(Max(EndOffset(), other.EndOffset()));
offset = Min(offset, other.offset); LogicalOffset new_start_offset(Min(offset, other.offset));
size = new_end_offset - offset; size = new_end_offset - new_start_offset;
offset = {new_end_offset.inline_offset - size.inline_size,
new_end_offset.block_offset - size.block_size};
} }
String LogicalRect::ToString() const { String LogicalRect::ToString() const {
......
...@@ -22,6 +22,14 @@ struct LogicalRectUniteTestData { ...@@ -22,6 +22,14 @@ struct LogicalRectUniteTestData {
{"b empty", {1, 2, 3, 4}, {}, {1, 2, 3, 4}}, {"b empty", {1, 2, 3, 4}, {}, {1, 2, 3, 4}},
{"a larger", {100, 50, 300, 200}, {200, 50, 200, 200}, {100, 50, 300, 200}}, {"a larger", {100, 50, 300, 200}, {200, 50, 200, 200}, {100, 50, 300, 200}},
{"b larger", {200, 50, 200, 200}, {100, 50, 300, 200}, {100, 50, 300, 200}}, {"b larger", {200, 50, 200, 200}, {100, 50, 300, 200}, {100, 50, 300, 200}},
{"saturated width",
{-1000, 0, 200, 200},
{33554402, 500, 30, 100},
{0, 0, 99999999.0f, 600}},
{"saturated height",
{0, -1000, 200, 200},
{0, 33554402, 100, 30},
{0, 0, 200, 99999999.0f}},
}; };
std::ostream& operator<<(std::ostream& os, std::ostream& operator<<(std::ostream& os,
...@@ -41,7 +49,21 @@ TEST_P(LogicalRectUniteTest, Data) { ...@@ -41,7 +49,21 @@ TEST_P(LogicalRectUniteTest, Data) {
const auto& data = GetParam(); const auto& data = GetParam();
LogicalRect actual = data.a; LogicalRect actual = data.a;
actual.Unite(data.b); actual.Unite(data.b);
EXPECT_EQ(data.expected, actual);
LogicalRect expected = data.expected;
constexpr int kExtraForSaturation = 2000;
// On arm, you cannot actually get the true saturated value just by
// setting via LayoutUnit constructor. Instead, add to the expected
// value to actually get a saturated expectation (which is what happens in
// the Unite operation).
if (data.expected.size.inline_size == GetMaxSaturatedSetResultForTesting()) {
expected.size.inline_size += kExtraForSaturation;
}
if (data.expected.size.block_size == GetMaxSaturatedSetResultForTesting()) {
expected.size.block_size += kExtraForSaturation;
}
EXPECT_EQ(expected, actual);
} }
} // namespace } // namespace
......
...@@ -58,8 +58,15 @@ void PhysicalRect::UniteEvenIfEmpty(const PhysicalRect& other) { ...@@ -58,8 +58,15 @@ void PhysicalRect::UniteEvenIfEmpty(const PhysicalRect& other) {
LayoutUnit top = std::min(offset.top, other.offset.top); LayoutUnit top = std::min(offset.top, other.offset.top);
LayoutUnit right = std::max(Right(), other.Right()); LayoutUnit right = std::max(Right(), other.Right());
LayoutUnit bottom = std::max(Bottom(), other.Bottom()); LayoutUnit bottom = std::max(Bottom(), other.Bottom());
offset = {left, top};
size = {right - left, bottom - top}; size = {right - left, bottom - top};
// If either width or height are not saturated, right - width == left and
// bottom - height == top. If they are saturated, instead of using left/top
// directly for the offset, the subtraction results in the united rect to
// favor content in the positive directions.
// Note that this is just a heuristic as the true rect would normally be
// larger than the max LayoutUnit value.
offset = {right - size.width, bottom - size.height};
} }
void PhysicalRect::Expand(const NGPhysicalBoxStrut& strut) { void PhysicalRect::Expand(const NGPhysicalBoxStrut& strut) {
......
...@@ -22,6 +22,14 @@ struct PhysicalOffsetRectUniteTestData { ...@@ -22,6 +22,14 @@ struct PhysicalOffsetRectUniteTestData {
{"b empty", {1, 2, 3, 4}, {}, {1, 2, 3, 4}}, {"b empty", {1, 2, 3, 4}, {}, {1, 2, 3, 4}},
{"a larger", {100, 50, 300, 200}, {200, 50, 200, 200}, {100, 50, 300, 200}}, {"a larger", {100, 50, 300, 200}, {200, 50, 200, 200}, {100, 50, 300, 200}},
{"b larger", {200, 50, 200, 200}, {100, 50, 300, 200}, {100, 50, 300, 200}}, {"b larger", {200, 50, 200, 200}, {100, 50, 300, 200}, {100, 50, 300, 200}},
{"saturated width",
{-1000, 0, 200, 200},
{33554402, 500, 30, 100},
{0, 0, 99999999.0f, 600}},
{"saturated height",
{0, -1000, 200, 200},
{0, 33554402, 100, 30},
{0, 0, 200, 99999999.0f}},
}; };
std::ostream& operator<<(std::ostream& os, std::ostream& operator<<(std::ostream& os,
...@@ -42,7 +50,20 @@ TEST_P(PhysicalRectUniteTest, Data) { ...@@ -42,7 +50,20 @@ TEST_P(PhysicalRectUniteTest, Data) {
const auto& data = GetParam(); const auto& data = GetParam();
PhysicalRect actual = data.a; PhysicalRect actual = data.a;
actual.Unite(data.b); actual.Unite(data.b);
EXPECT_EQ(data.expected, actual); auto expected = data.expected;
constexpr int kExtraForSaturation = 2000;
// On arm, you cannot actually get the true saturated value just by
// setting via LayoutUnit constructor. Instead, add to the expected
// value to actually get a saturated expectation (which is what happens in
// the Unite operation).
if (data.expected.size.width == GetMaxSaturatedSetResultForTesting()) {
expected.size.width += kExtraForSaturation;
}
if (data.expected.size.height == GetMaxSaturatedSetResultForTesting()) {
expected.size.height += kExtraForSaturation;
}
EXPECT_EQ(expected, actual);
} }
TEST(PhysicalRectTest, InclusiveIntersect) { TEST(PhysicalRectTest, InclusiveIntersect) {
......
...@@ -121,8 +121,8 @@ void LayoutRect::UniteEvenIfEmpty(const LayoutRect& other) { ...@@ -121,8 +121,8 @@ void LayoutRect::UniteEvenIfEmpty(const LayoutRect& other) {
LayoutPoint new_max_point(std::max(MaxX(), other.MaxX()), LayoutPoint new_max_point(std::max(MaxX(), other.MaxX()),
std::max(MaxY(), other.MaxY())); std::max(MaxY(), other.MaxY()));
location_ = new_location;
size_ = new_max_point - new_location; size_ = new_max_point - new_location;
location_ = new_max_point - size_;
} }
void LayoutRect::Scale(float s) { void LayoutRect::Scale(float s) {
......
...@@ -283,4 +283,59 @@ TEST(LayoutRectTest, ExpandEdgesToPixelBoundaries) { ...@@ -283,4 +283,59 @@ TEST(LayoutRectTest, ExpandEdgesToPixelBoundaries) {
EXPECT_EQ(LayoutRect(IntRect(-101, -151, 201, 352)), fractional_negpos_rect3); EXPECT_EQ(LayoutRect(IntRect(-101, -151, 201, 352)), fractional_negpos_rect3);
} }
struct LayoutRectUniteTestData {
const char* test_case;
LayoutRect a;
LayoutRect b;
LayoutRect expected;
} layout_rect_unite_test_data[] = {
{"empty", {}, {}, {}},
{"a empty", {}, {1, 2, 3, 4}, {1, 2, 3, 4}},
{"b empty", {1, 2, 3, 4}, {}, {1, 2, 3, 4}},
{"a larger", {100, 50, 300, 200}, {200, 50, 200, 200}, {100, 50, 300, 200}},
{"b larger", {200, 50, 200, 200}, {100, 50, 300, 200}, {100, 50, 300, 200}},
{"saturated width",
{-1000, 0, 200, 200},
{33554402, 500, 30, 100},
{0, 0, 99999999.0f, 600}},
{"saturated height",
{0, -1000, 200, 200},
{0, 33554402, 100, 30},
{0, 0, 200, 99999999.0f}},
};
std::ostream& operator<<(std::ostream& os,
const LayoutRectUniteTestData& data) {
return os << "Unite " << data.test_case;
}
class LayoutRectUniteTest
: public testing::Test,
public testing::WithParamInterface<LayoutRectUniteTestData> {};
INSTANTIATE_TEST_SUITE_P(LayoutRectTest,
LayoutRectUniteTest,
testing::ValuesIn(layout_rect_unite_test_data));
TEST_P(LayoutRectUniteTest, Data) {
const auto& data = GetParam();
LayoutRect actual = data.a;
actual.Unite(data.b);
LayoutRect expected = data.expected;
constexpr int kExtraForSaturation = 2000;
// On arm, you cannot actually get the true saturated value just by
// setting via LayoutUnit constructor. Instead, add to the expected
// value to actually get a saturated expectation (which is what happens in
// the Unite operation).
if (data.expected.Width() == GetMaxSaturatedSetResultForTesting()) {
expected.Expand(LayoutSize(kExtraForSaturation, 0));
}
if (data.expected.Height() == GetMaxSaturatedSetResultForTesting()) {
expected.Expand(LayoutSize(0, kExtraForSaturation));
}
EXPECT_EQ(expected, actual);
}
} // namespace blink } // namespace blink
<!DOCTYPE html>
<style>
div {
background:green;
width:50px;
height:50px;
}
</style>
<div></div>
<!DOCTYPE html>
<html>
<title>CSS Position: Large negative box inset properties</title>
<link rel="help" href="https://drafts.csswg.org/css-position-3/#insets">
<link rel="match" href="position-absolute-large-negative-inset-ref.html">
<meta name="assert" content="This test verifies a box with very large negative insets within an element with will-change: transform does not affect the rendering of the element."/>
<style>
html {
will-change: transform;
}
span {
position: absolute;
left: -99999999999px;
}
div {
background:green;
width:50px;
height:50px;
}
</style>
<div></div>
<span>negative inset text</span>
</html>
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