Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 14d712d

Browse files
flarflar
authored
Avoid unnecessary transform resets for pixel snapping (#51337)
During some new development work that might make transform resets more expensive we realized that the resets were mostly coming from the calls to snap the transform to a pixel translate value and many of those were NOPs since the transform was already on a pixel translate value. This PR will avoid those trivially unnecessary reset operations.
1 parent cc1e7d1 commit 14d712d

8 files changed

+412
-52
lines changed

‎flow/diff_context.cc‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,17 @@ void DiffContext::MakeCurrentTransformIntegral() {
6161
// TODO(knopp): This is duplicated from LayerStack. Maybe should be part of
6262
// clip tracker?
6363
if (clip_tracker_.using_4x4_matrix()) {
64-
clip_tracker_.setTransform(
65-
RasterCacheUtil::GetIntegralTransCTM(clip_tracker_.matrix_4x4()));
64+
SkM44 integral;
65+
if (RasterCacheUtil::ComputeIntegralTransCTM(clip_tracker_.matrix_4x4(),
66+
&integral)) {
67+
clip_tracker_.setTransform(integral);
68+
}
6669
} else {
67-
clip_tracker_.setTransform(
68-
RasterCacheUtil::GetIntegralTransCTM(clip_tracker_.matrix_3x3()));
70+
SkMatrix integral;
71+
if (RasterCacheUtil::ComputeIntegralTransCTM(clip_tracker_.matrix_3x3(),
72+
&integral)) {
73+
clip_tracker_.setTransform(integral);
74+
}
6975
}
7076
}
7177

‎flow/layers/display_list_layer_unittests.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ TEST_F(DisplayListLayerTest, CachedIncompatibleDisplayListOpacityInheritance) {
249249
&paint_context(), false);
250250

251251
int opacity_alpha = 0x7F;
252-
SkPoint opacity_offset = SkPoint::Make(10, 10);
252+
SkPoint opacity_offset = SkPoint::Make(10.2, 10.2);
253253
auto opacity_layer =
254254
std::make_shared<OpacityLayer>(opacity_alpha, opacity_offset);
255255
opacity_layer->Add(display_list_layer);

‎flow/layers/image_filter_layer_unittests.cc‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -454,9 +454,8 @@ TEST_F(ImageFilterLayerTest, CacheChildren) {
454454
expected_builder.Save();
455455
{
456456
expected_builder.Translate(offset.fX, offset.fY);
457-
// snap translation components to pixels due to using raster cache
458-
expected_builder.TransformReset();
459-
expected_builder.Transform(snapped_matrix);
457+
// translation components already snapped to pixels, intent to
458+
// use raster cache won't change them
460459
DlPaint dl_paint;
461460
dl_paint.setImageFilter(transformed_filter.get());
462461
raster_cache()->Draw(cacheable_image_filter_item->GetId().value(),

‎flow/layers/layer_state_stack.cc‎

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -114,8 +114,10 @@ class DlCanvasDelegate : public LayerStateStack::Delegate {
114114
canvas_->Transform(matrix);
115115
}
116116
void integralTransform() override {
117-
SkM44 matrix = RasterCacheUtil::GetIntegralTransCTM(matrix_4x4());
118-
canvas_->SetTransform(matrix);
117+
SkM44 integral;
118+
if (RasterCacheUtil::ComputeIntegralTransCTM(matrix_4x4(), &integral)) {
119+
canvas_->SetTransform(integral);
120+
}
119121
}
120122

121123
void clipRect(const SkRect& rect, ClipOp op, bool is_aa) override {
@@ -168,11 +170,17 @@ class PrerollDelegate : public LayerStateStack::Delegate {
168170
}
169171
void integralTransform() override {
170172
if (tracker_.using_4x4_matrix()) {
171-
tracker_.setTransform(
172-
RasterCacheUtil::GetIntegralTransCTM(tracker_.matrix_4x4()));
173+
SkM44 integral;
174+
if (RasterCacheUtil::ComputeIntegralTransCTM(tracker_.matrix_4x4(),
175+
&integral)) {
176+
tracker_.setTransform(integral);
177+
}
173178
} else {
174-
tracker_.setTransform(
175-
RasterCacheUtil::GetIntegralTransCTM(tracker_.matrix_3x3()));
179+
SkMatrix integral;
180+
if (RasterCacheUtil::ComputeIntegralTransCTM(tracker_.matrix_3x3(),
181+
&integral)) {
182+
tracker_.setTransform(integral);
183+
}
176184
}
177185
}
178186

‎flow/layers/shader_mask_layer_unittests.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -443,10 +443,10 @@ TEST_F(ShaderMaskLayerTest, SimpleFilterWithRasterCacheLayerNotCached) {
443443
/* (ShaderMask)layer::Paint */ {
444444
expected_builder.Save();
445445
{
446-
expected_builder.TransformReset();
447-
// The layer will perform this Identity transform operation by default,
448-
// but it should be ignored both here and in the layer paint
449-
expected_builder.Transform(SkMatrix());
446+
// The layer will notice that the CTM is already an integer matrix
447+
// and will not perform an Integral CTM operation.
448+
// expected_builder.TransformReset();
449+
// expected_builder.Transform(SkMatrix());
450450
expected_builder.SaveLayer(&child_bounds);
451451
{
452452
/* mock_layer::Paint */ {

‎flow/raster_cache_unittests.cc‎

Lines changed: 275 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -809,6 +809,281 @@ TEST_F(RasterCacheTest, RasterCacheKeyIDLayerChildrenIds) {
809809
ASSERT_EQ(ids, expected_ids);
810810
}
811811

812+
TEST(RasterCacheUtilsTest, SkMatrixIntegralTransCTM) {
813+
#define EXPECT_EQ_WITH_TRANSLATE(test, expected, expected_tx, expected_ty) \
814+
do { \
815+
EXPECT_EQ(test[SkMatrix::kMScaleX], expected[SkMatrix::kMScaleX]); \
816+
EXPECT_EQ(test[SkMatrix::kMSkewX], expected[SkMatrix::kMSkewX]); \
817+
EXPECT_EQ(test[SkMatrix::kMScaleY], expected[SkMatrix::kMScaleY]); \
818+
EXPECT_EQ(test[SkMatrix::kMSkewY], expected[SkMatrix::kMSkewY]); \
819+
EXPECT_EQ(test[SkMatrix::kMSkewX], expected[SkMatrix::kMSkewX]); \
820+
EXPECT_EQ(test[SkMatrix::kMPersp0], expected[SkMatrix::kMPersp0]); \
821+
EXPECT_EQ(test[SkMatrix::kMPersp1], expected[SkMatrix::kMPersp1]); \
822+
EXPECT_EQ(test[SkMatrix::kMPersp2], expected[SkMatrix::kMPersp2]); \
823+
EXPECT_EQ(test[SkMatrix::kMTransX], expected_tx); \
824+
EXPECT_EQ(test[SkMatrix::kMTransY], expected_ty); \
825+
} while (0)
826+
827+
#define EXPECT_NON_INTEGER_TRANSLATION(matrix) \
828+
EXPECT_TRUE(SkScalarFraction(matrix[SkMatrix::kMTransX]) != 0.0f || \
829+
SkScalarFraction(matrix[SkMatrix::kMTransY]) != 0.0f)
830+
831+
{
832+
// Identity
833+
SkMatrix matrix = SkMatrix::I();
834+
SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
835+
SkMatrix compute;
836+
EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
837+
EXPECT_EQ(get, matrix);
838+
}
839+
{
840+
// Integer translate
841+
SkMatrix matrix = SkMatrix::Translate(10.0f, 12.0f);
842+
SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
843+
SkMatrix compute;
844+
EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
845+
EXPECT_EQ(get, matrix);
846+
}
847+
{
848+
// Fractional x translate
849+
SkMatrix matrix = SkMatrix::Translate(10.2f, 12.0f);
850+
EXPECT_NON_INTEGER_TRANSLATION(matrix);
851+
SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
852+
SkMatrix compute;
853+
EXPECT_TRUE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
854+
EXPECT_EQ_WITH_TRANSLATE(get, matrix, 10.0f, 12.0f);
855+
EXPECT_EQ(get, compute);
856+
}
857+
{
858+
// Fractional y translate
859+
SkMatrix matrix = SkMatrix::Translate(10.0f, 12.3f);
860+
EXPECT_NON_INTEGER_TRANSLATION(matrix);
861+
SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
862+
SkMatrix compute;
863+
EXPECT_TRUE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
864+
EXPECT_EQ_WITH_TRANSLATE(get, matrix, 10.0f, 12.0f);
865+
EXPECT_EQ(get, compute);
866+
}
867+
{
868+
// Fractional x & y translate
869+
SkMatrix matrix = SkMatrix::Translate(10.7f, 12.3f);
870+
EXPECT_NON_INTEGER_TRANSLATION(matrix);
871+
SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
872+
SkMatrix compute;
873+
EXPECT_TRUE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
874+
EXPECT_EQ_WITH_TRANSLATE(get, matrix, 11.0f, 12.0f);
875+
EXPECT_EQ(get, compute);
876+
}
877+
{
878+
// Scale
879+
SkMatrix matrix = SkMatrix::Scale(2.0f, 3.0f);
880+
SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
881+
SkMatrix compute;
882+
EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
883+
EXPECT_EQ(get, matrix);
884+
}
885+
{
886+
// Scale, Integer translate
887+
SkMatrix matrix = SkMatrix::Scale(2.0f, 3.0f);
888+
matrix.preTranslate(10.0f, 12.0f);
889+
SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
890+
SkMatrix compute;
891+
EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
892+
EXPECT_EQ(get, matrix);
893+
}
894+
{
895+
// Scale, Fractional translate
896+
SkMatrix matrix = SkMatrix::Scale(2.0f, 3.0f);
897+
matrix.preTranslate(10.7f, 12.1f);
898+
EXPECT_NON_INTEGER_TRANSLATION(matrix);
899+
SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
900+
SkMatrix compute;
901+
EXPECT_TRUE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
902+
EXPECT_EQ_WITH_TRANSLATE(get, matrix, 21.0f, 36.0f);
903+
EXPECT_EQ(get, compute);
904+
}
905+
{
906+
// Skew
907+
SkMatrix matrix = SkMatrix::Skew(0.5f, 0.1f);
908+
SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
909+
SkMatrix compute;
910+
EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
911+
EXPECT_EQ(get, matrix);
912+
}
913+
{
914+
// Skew, Fractional translate - should be NOP
915+
SkMatrix matrix = SkMatrix::Skew(0.5f, 0.1f);
916+
matrix.preTranslate(10.7f, 12.1f);
917+
EXPECT_NON_INTEGER_TRANSLATION(matrix);
918+
SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
919+
SkMatrix compute;
920+
EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
921+
EXPECT_EQ(get, matrix);
922+
}
923+
{
924+
// Rotate
925+
SkMatrix matrix = SkMatrix::RotateDeg(45);
926+
SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
927+
SkMatrix compute;
928+
EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
929+
EXPECT_EQ(get, matrix);
930+
}
931+
{
932+
// Rotate, Fractional Translate - should be NOP
933+
SkMatrix matrix = SkMatrix::RotateDeg(45);
934+
matrix.preTranslate(10.7f, 12.1f);
935+
EXPECT_NON_INTEGER_TRANSLATION(matrix);
936+
SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
937+
SkMatrix compute;
938+
EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
939+
EXPECT_EQ(get, matrix);
940+
}
941+
{
942+
// Perspective x
943+
SkMatrix matrix = SkMatrix::I();
944+
matrix.setPerspX(0.1);
945+
SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
946+
SkMatrix compute;
947+
EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
948+
EXPECT_EQ(get, matrix);
949+
}
950+
{
951+
// Perspective x, Fractional Translate - should be NOP
952+
SkMatrix matrix = SkMatrix::I();
953+
matrix.setPerspX(0.1);
954+
matrix.preTranslate(10.7f, 12.1f);
955+
EXPECT_NON_INTEGER_TRANSLATION(matrix);
956+
SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
957+
SkMatrix compute;
958+
EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
959+
EXPECT_EQ(get, matrix);
960+
}
961+
{
962+
// Perspective y
963+
SkMatrix matrix = SkMatrix::I();
964+
matrix.setPerspY(0.1);
965+
SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
966+
SkMatrix compute;
967+
EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
968+
EXPECT_EQ(get, matrix);
969+
}
970+
{
971+
// Perspective y, Fractional Translate - should be NOP
972+
SkMatrix matrix = SkMatrix::I();
973+
matrix.setPerspY(0.1);
974+
matrix.preTranslate(10.7f, 12.1f);
975+
EXPECT_NON_INTEGER_TRANSLATION(matrix);
976+
SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
977+
SkMatrix compute;
978+
EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
979+
EXPECT_EQ(get, matrix);
980+
}
981+
{
982+
// Perspective weight
983+
// clang-format off
984+
SkMatrix matrix = SkMatrix::MakeAll(
985+
1.0f, 0.0f, 0.0f,
986+
0.0f, 1.0f, 0.0f,
987+
0.0f, 0.0f, 0.9f);
988+
// clang-format on
989+
SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
990+
SkMatrix compute;
991+
EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
992+
EXPECT_EQ(get, matrix);
993+
}
994+
{
995+
// Perspective weight, Fractional Translate - should be NOP
996+
// clang-format off
997+
SkMatrix matrix = SkMatrix::MakeAll(
998+
1.0f, 0.0f, 0.0f,
999+
0.0f, 1.0f, 0.0f,
1000+
0.0f, 0.0f, 0.9f);
1001+
// clang-format on
1002+
matrix.preTranslate(10.7f, 12.1f);
1003+
EXPECT_NON_INTEGER_TRANSLATION(matrix);
1004+
SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix);
1005+
SkMatrix compute;
1006+
EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute));
1007+
EXPECT_EQ(get, matrix);
1008+
}
1009+
#undef EXPECT_NON_INTEGER_TRANSLATION
1010+
#undef EXPECT_EQ_WITH_TRANSLATE
1011+
}
1012+
1013+
TEST(RasterCacheUtilsTest, SkM44IntegralTransCTM) {
1014+
#define EXPECT_EQ_WITH_TRANSLATE(test, expected, tx, ty, label) \
1015+
do { \
1016+
EXPECT_EQ(test.rc(0, 0), expected.rc(0, 0)) << label; \
1017+
EXPECT_EQ(test.rc(0, 1), expected.rc(0, 1)) << label; \
1018+
EXPECT_EQ(test.rc(0, 2), expected.rc(0, 2)) << label; \
1019+
EXPECT_EQ(test.rc(0, 3), tx) << label; \
1020+
EXPECT_EQ(test.rc(1, 0), expected.rc(1, 0)) << label; \
1021+
EXPECT_EQ(test.rc(1, 1), expected.rc(1, 1)) << label; \
1022+
EXPECT_EQ(test.rc(1, 2), expected.rc(1, 2)) << label; \
1023+
EXPECT_EQ(test.rc(1, 3), ty) << label; \
1024+
EXPECT_EQ(test.rc(2, 0), expected.rc(2, 0)) << label; \
1025+
EXPECT_EQ(test.rc(2, 1), expected.rc(2, 1)) << label; \
1026+
EXPECT_EQ(test.rc(2, 2), expected.rc(2, 2)) << label; \
1027+
EXPECT_EQ(test.rc(2, 3), expected.rc(2, 3)) << label; \
1028+
EXPECT_EQ(test.rc(3, 0), expected.rc(3, 0)) << label; \
1029+
EXPECT_EQ(test.rc(3, 1), expected.rc(3, 1)) << label; \
1030+
EXPECT_EQ(test.rc(3, 2), expected.rc(3, 2)) << label; \
1031+
EXPECT_EQ(test.rc(3, 3), expected.rc(3, 3)) << label; \
1032+
} while (0)
1033+
1034+
#define EXPECT_NON_INTEGER_TRANSLATION(matrix) \
1035+
EXPECT_TRUE(SkScalarFraction(matrix.rc(0, 3)) != 0.0f || \
1036+
SkScalarFraction(matrix.rc(1, 3)) != 0.0f)
1037+
1038+
for (int r = 0; r < 4; r++) {
1039+
for (int c = 0; c < 4; c++) {
1040+
bool snaps;
1041+
switch (r) {
1042+
case 0: // X equation
1043+
if (c == 3) {
1044+
continue; // TranslateX, the value we are testing, skip
1045+
}
1046+
snaps = (c == 0); // X Scale value yes, Skew by Y or Z no
1047+
break;
1048+
case 1: // Y equation
1049+
if (c == 3) {
1050+
continue; // TranslateY, the value we are testing, skip
1051+
}
1052+
snaps = (c == 1); // Y Scale value yes, Skew by X or Z no
1053+
break;
1054+
case 2: // Z equation, ignored, will snap
1055+
snaps = true;
1056+
break;
1057+
case 3: // W equation, modifications prevent snapping
1058+
snaps = false;
1059+
break;
1060+
default:
1061+
FML_UNREACHABLE();
1062+
}
1063+
auto label = std::to_string(r) + ", " + std::to_string(c);
1064+
SkM44 matrix = SkM44::Translate(10.7f, 12.1f);
1065+
EXPECT_NON_INTEGER_TRANSLATION(matrix) << label;
1066+
matrix.setRC(r, c, 0.5f);
1067+
if (snaps) {
1068+
SkM44 compute;
1069+
SkM44 get = RasterCacheUtil::GetIntegralTransCTM(matrix);
1070+
EXPECT_TRUE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute))
1071+
<< label;
1072+
EXPECT_EQ_WITH_TRANSLATE(get, matrix, 11.0f, 12.0f, label);
1073+
EXPECT_EQ(get, compute) << label;
1074+
} else {
1075+
SkM44 compute;
1076+
SkM44 get = RasterCacheUtil::GetIntegralTransCTM(matrix);
1077+
EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute))
1078+
<< label;
1079+
EXPECT_EQ(get, matrix) << label;
1080+
}
1081+
}
1082+
}
1083+
#undef EXPECT_NON_INTEGER_TRANSLATION
1084+
#undef EXPECT_EQ_WITH_TRANSLATE
1085+
}
1086+
8121087
} // namespace testing
8131088
} // namespace flutter
8141089

0 commit comments

Comments
 (0)