Commit 717c1c29 authored by Xianzhu Wang's avatar Xianzhu Wang Committed by Commit Bot

[PE] Don't combine clips across non-invertible transform

GeometryMapper::SourceToDestinationProjection(parent, child) returns
identity if child transform is not invertible, so it's not reliable
to use it to check if parent and child are in the same transform space.

Now avoid the function, while still handle most common cases to combine
clips through identity transforms.

Bug: 849382
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I747e857d3828a5fb6eea0f69ed8746843912baf4
Reviewed-on: https://chromium-review.googlesource.com/1086492Reviewed-by: default avatarTien-Ren Chen <trchen@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#565078}
parent 2364c5d3
...@@ -278,10 +278,11 @@ static bool CombineClip(const ClipPaintPropertyNode* clip, ...@@ -278,10 +278,11 @@ static bool CombineClip(const ClipPaintPropertyNode* clip,
return false; return false;
// Don't combine clips in different transform spaces. // Don't combine clips in different transform spaces.
if (clip->LocalTransformSpace() != clip->Parent()->LocalTransformSpace() && const auto* transform_space = clip->LocalTransformSpace();
!GeometryMapper::SourceToDestinationProjection( const auto* parent_transform_space = clip->Parent()->LocalTransformSpace();
clip->Parent()->LocalTransformSpace(), clip->LocalTransformSpace()) if (transform_space != parent_transform_space &&
.IsIdentity()) (transform_space->Parent() != parent_transform_space ||
!transform_space->Matrix().IsIdentity()))
return false; return false;
// Don't combine two rounded clip rects. // Don't combine two rounded clip rects.
......
...@@ -119,7 +119,8 @@ class PaintRecordMatcher ...@@ -119,7 +119,8 @@ class PaintRecordMatcher
do { \ do { \
const auto* concat = (op_buffer).GetOpAtForTesting<cc::ConcatOp>(index); \ const auto* concat = (op_buffer).GetOpAtForTesting<cc::ConcatOp>(index); \
ASSERT_NE(nullptr, concat); \ ASSERT_NE(nullptr, concat); \
EXPECT_EQ(transform, TransformationMatrix(concat->matrix)); \ EXPECT_EQ(SkMatrix(TransformationMatrix::ToSkMatrix44(transform)), \
concat->matrix); \
} while (false) } while (false)
#define EXPECT_TRANSLATE(x, y, op_buffer, index) \ #define EXPECT_TRANSLATE(x, y, op_buffer, index) \
...@@ -740,6 +741,50 @@ TEST_F(PaintChunksToCcLayerTest, CombineClips) { ...@@ -740,6 +741,50 @@ TEST_F(PaintChunksToCcLayerTest, CombineClips) {
cc::PaintOpType::Restore})); // </c1+c2> cc::PaintOpType::Restore})); // </c1+c2>
} }
TEST_F(PaintChunksToCcLayerTest, CombineClipsAcrossTransform) {
FloatRoundedRect clip_rect(0, 0, 100, 100);
auto identity = CreateTransform(t0(), TransformationMatrix());
auto non_identity =
CreateTransform(*identity, TransformationMatrix().Scale(2));
auto non_invertible =
CreateTransform(*non_identity, TransformationMatrix().Scale(0));
EXPECT_FALSE(non_invertible->Matrix().IsInvertible());
auto c1 = CreateClip(c0(), &t0(), FloatRoundedRect(0, 0, 100, 100));
auto c2 = CreateClip(*c1, identity.get(), FloatRoundedRect(50, 50, 100, 100));
auto c3 = CreateClip(*c2, non_identity.get(), FloatRoundedRect(1, 2, 3, 4));
auto c4 = CreateClip(*c3, non_invertible.get(), FloatRoundedRect(5, 6, 7, 8));
TestChunks chunks;
chunks.AddChunk(*non_invertible, *c4, e0());
sk_sp<PaintRecord> output =
PaintChunksToCcLayer::Convert(
chunks.chunks, PropertyTreeState::Root(), gfx::Vector2dF(),
chunks.items, cc::DisplayItemList::kToBeReleasedAsPaintOpBuffer)
->ReleaseAsRecord();
// We combine c1/c2 across |identity|, but not c2/c3 across |non_identity|
// and c3/c4 across |non_invertible|.
EXPECT_THAT(
*output,
PaintRecordMatcher::Make(
{cc::PaintOpType::Save, cc::PaintOpType::ClipRect, // <c1+c2>
cc::PaintOpType::Save, cc::PaintOpType::Concat, // <non_identity
cc::PaintOpType::ClipRect, // c3>
cc::PaintOpType::Save, cc::PaintOpType::Concat, // <non_invertible
cc::PaintOpType::ClipRect, // c4>
cc::PaintOpType::DrawRecord, // <p0/>
cc::PaintOpType::Restore, // </c4 non_invertible>
cc::PaintOpType::Restore, // </c3 non_identity>
cc::PaintOpType::Restore})); // </c1+c2>
EXPECT_CLIP(FloatRect(50, 50, 50, 50), *output, 1);
EXPECT_TRANSFORM_MATRIX(non_identity->Matrix(), *output, 3);
EXPECT_CLIP(FloatRect(1, 2, 3, 4), *output, 4);
EXPECT_TRANSFORM_MATRIX(non_invertible->Matrix(), *output, 6);
EXPECT_CLIP(FloatRect(5, 6, 7, 8), *output, 7);
}
TEST_F(PaintChunksToCcLayerTest, CombineClipsWithRoundedRects) { TEST_F(PaintChunksToCcLayerTest, CombineClipsWithRoundedRects) {
FloatRoundedRect clip_rect(0, 0, 100, 100); FloatRoundedRect clip_rect(0, 0, 100, 100);
FloatSize corner(5, 5); FloatSize corner(5, 5);
......
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