From de3acd0940a4029ced154d9c166e293a3a3b439b Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Mon, 11 Mar 2024 15:02:45 -0700 Subject: [PATCH 1/2] Avoid unnecessary transform resets for pixel snapping --- flow/diff_context.cc | 14 ++-- flow/layers/display_list_layer_unittests.cc | 2 +- flow/layers/image_filter_layer_unittests.cc | 5 +- flow/layers/layer_state_stack.cc | 20 ++++-- flow/layers/shader_mask_layer_unittests.cc | 8 +-- flow/raster_cache_util.cc | 65 +++++++++++++++++- flow/raster_cache_util.h | 75 ++++++++++++--------- 7 files changed, 137 insertions(+), 52 deletions(-) diff --git a/flow/diff_context.cc b/flow/diff_context.cc index 36bce667cb377..8bfec18230aa0 100644 --- a/flow/diff_context.cc +++ b/flow/diff_context.cc @@ -61,11 +61,17 @@ void DiffContext::MakeCurrentTransformIntegral() { // TODO(knopp): This is duplicated from LayerStack. Maybe should be part of // clip tracker? if (clip_tracker_.using_4x4_matrix()) { - clip_tracker_.setTransform( - RasterCacheUtil::GetIntegralTransCTM(clip_tracker_.matrix_4x4())); + SkM44 integral; + if (RasterCacheUtil::ComputeIntegralTransCTM(clip_tracker_.matrix_4x4(), + &integral)) { + clip_tracker_.setTransform(integral); + } } else { - clip_tracker_.setTransform( - RasterCacheUtil::GetIntegralTransCTM(clip_tracker_.matrix_3x3())); + SkMatrix integral; + if (RasterCacheUtil::ComputeIntegralTransCTM(clip_tracker_.matrix_3x3(), + &integral)) { + clip_tracker_.setTransform(integral); + } } } diff --git a/flow/layers/display_list_layer_unittests.cc b/flow/layers/display_list_layer_unittests.cc index 9a85aa3b2f5b7..2552cf79c41f5 100644 --- a/flow/layers/display_list_layer_unittests.cc +++ b/flow/layers/display_list_layer_unittests.cc @@ -249,7 +249,7 @@ TEST_F(DisplayListLayerTest, CachedIncompatibleDisplayListOpacityInheritance) { &paint_context(), false); int opacity_alpha = 0x7F; - SkPoint opacity_offset = SkPoint::Make(10, 10); + SkPoint opacity_offset = SkPoint::Make(10.2, 10.2); auto opacity_layer = std::make_shared(opacity_alpha, opacity_offset); opacity_layer->Add(display_list_layer); diff --git a/flow/layers/image_filter_layer_unittests.cc b/flow/layers/image_filter_layer_unittests.cc index 67cf817c0899f..17ed929dd3b08 100644 --- a/flow/layers/image_filter_layer_unittests.cc +++ b/flow/layers/image_filter_layer_unittests.cc @@ -454,9 +454,8 @@ TEST_F(ImageFilterLayerTest, CacheChildren) { expected_builder.Save(); { expected_builder.Translate(offset.fX, offset.fY); - // snap translation components to pixels due to using raster cache - expected_builder.TransformReset(); - expected_builder.Transform(snapped_matrix); + // translation components already snapped to pixels, intent to + // use raster cache won't change them DlPaint dl_paint; dl_paint.setImageFilter(transformed_filter.get()); raster_cache()->Draw(cacheable_image_filter_item->GetId().value(), diff --git a/flow/layers/layer_state_stack.cc b/flow/layers/layer_state_stack.cc index 0d3f9fd6303c3..492931278bd88 100644 --- a/flow/layers/layer_state_stack.cc +++ b/flow/layers/layer_state_stack.cc @@ -114,8 +114,10 @@ class DlCanvasDelegate : public LayerStateStack::Delegate { canvas_->Transform(matrix); } void integralTransform() override { - SkM44 matrix = RasterCacheUtil::GetIntegralTransCTM(matrix_4x4()); - canvas_->SetTransform(matrix); + SkM44 integral; + if (RasterCacheUtil::ComputeIntegralTransCTM(matrix_4x4(), &integral)) { + canvas_->SetTransform(integral); + } } void clipRect(const SkRect& rect, ClipOp op, bool is_aa) override { @@ -168,11 +170,17 @@ class PrerollDelegate : public LayerStateStack::Delegate { } void integralTransform() override { if (tracker_.using_4x4_matrix()) { - tracker_.setTransform( - RasterCacheUtil::GetIntegralTransCTM(tracker_.matrix_4x4())); + SkM44 integral; + if (RasterCacheUtil::ComputeIntegralTransCTM(tracker_.matrix_4x4(), + &integral)) { + tracker_.setTransform(integral); + } } else { - tracker_.setTransform( - RasterCacheUtil::GetIntegralTransCTM(tracker_.matrix_3x3())); + SkMatrix integral; + if (RasterCacheUtil::ComputeIntegralTransCTM(tracker_.matrix_3x3(), + &integral)) { + tracker_.setTransform(integral); + } } } diff --git a/flow/layers/shader_mask_layer_unittests.cc b/flow/layers/shader_mask_layer_unittests.cc index 5588caa369478..933ba70664835 100644 --- a/flow/layers/shader_mask_layer_unittests.cc +++ b/flow/layers/shader_mask_layer_unittests.cc @@ -443,10 +443,10 @@ TEST_F(ShaderMaskLayerTest, SimpleFilterWithRasterCacheLayerNotCached) { /* (ShaderMask)layer::Paint */ { expected_builder.Save(); { - expected_builder.TransformReset(); - // The layer will perform this Identity transform operation by default, - // but it should be ignored both here and in the layer paint - expected_builder.Transform(SkMatrix()); + // The layer will notice that the CTM is already an integer matrix + // and will not perform an Integral CTM operation. + // expected_builder.TransformReset(); + // expected_builder.Transform(SkMatrix()); expected_builder.SaveLayer(&child_bounds); { /* mock_layer::Paint */ { diff --git a/flow/raster_cache_util.cc b/flow/raster_cache_util.cc index 25061f29f07b8..fa636b5b78b58 100644 --- a/flow/raster_cache_util.cc +++ b/flow/raster_cache_util.cc @@ -4,4 +4,67 @@ #include "flutter/flow/raster_cache_util.h" -namespace flutter {} +namespace flutter { + +bool RasterCacheUtil::ComputeIntegralTransCTM(const SkMatrix& in, + SkMatrix* out) { + // Avoid integral snapping if the matrix has complex transformation to avoid + // the artifact observed in https://github.com/flutter/flutter/issues/41654. + if (!in.isScaleTranslate()) { + return false; + } + + SkScalar in_tx = in.getTranslateX(); + SkScalar in_ty = in.getTranslateY(); + SkScalar out_tx = SkScalarRoundToScalar(in_tx); + SkScalar out_ty = SkScalarRoundToScalar(in_ty); + if (out_tx != in_tx || out_ty != in_ty) { + // As a side effect of those tests we also know that neither translation + // component was a NaN + *out = in; + (*out)[SkMatrix::kMTransX] = out_tx; + (*out)[SkMatrix::kMTransY] = out_ty; + return true; + } + + return false; +} + +bool RasterCacheUtil::ComputeIntegralTransCTM(const SkM44& in, SkM44* out) { + // Avoid integral snapping if the matrix has complex transformation to avoid + // the artifact observed in https://github.com/flutter/flutter/issues/41654. + if (in.rc(0, 1) != 0 || in.rc(0, 2) != 0) { + // X multiplied by either Y or Z + return false; + } + if (in.rc(1, 0) != 0 || in.rc(1, 2) != 0) { + // Y multiplied by either X or Z + return false; + } + // We do not need to worry about the Z row unless the W row + // has perspective entries... + if (in.rc(3, 0) != 0 || in.rc(3, 1) != 0 || in.rc(3, 2) != 0 || + in.rc(3, 3) != 1) { + // W not identity row, therefore perspective is applied + return false; + } + + SkScalar in_tx = in.rc(0, 3); + SkScalar in_ty = in.rc(1, 3); + SkScalar out_tx = SkScalarRoundToScalar(in_tx); + SkScalar out_ty = SkScalarRoundToScalar(in_ty); + if (out_tx != in_tx || out_ty != in_ty) { + // As a side effect of those tests we also know that neither translation + // component was a NaN + *out = in; + out->setRC(0, 3, out_tx); + out->setRC(1, 3, out_ty); + // No need to worry about Z translation because it has no effect + // without perspective entries... + return true; + } + + return false; +} + +} // namespace flutter diff --git a/flow/raster_cache_util.h b/flow/raster_cache_util.h index 795174da92c17..4df70494cb4f5 100644 --- a/flow/raster_cache_util.h +++ b/flow/raster_cache_util.h @@ -78,17 +78,29 @@ struct RasterCacheUtil { * @return SkMatrix the snapped transformation matrix. */ static SkMatrix GetIntegralTransCTM(const SkMatrix& ctm) { - // Avoid integral snapping if the matrix has complex transformation to avoid - // the artifact observed in https://github.com/flutter/flutter/issues/41654. - if (!ctm.isScaleTranslate()) { - return ctm; - } - SkMatrix result = ctm; - result[SkMatrix::kMTransX] = SkScalarRoundToScalar(ctm.getTranslateX()); - result[SkMatrix::kMTransY] = SkScalarRoundToScalar(ctm.getTranslateY()); - return result; + SkMatrix integral; + return ComputeIntegralTransCTM(ctm, &integral) ? integral : ctm; } + /** + * @brief Snap the translation components of the |in| matrix to integers + * and store the snapped matrix in |out|. + * + * The snapping will only happen if the matrix only has scale and translation + * transformations. This is used, along with GetRoundedOutDeviceBounds, to + * ensure that the textures drawn by the raster cache are exactly aligned to + * physical pixels. Any layers that participate in raster caching must align + * themselves to physical pixels even when not cached to prevent a change in + * apparent location if caching is later applied. + * + * The |out| matrix will not be modified if this method returns false. + * + * @param in the current transformation matrix. + * @param out the storage for the snapped matrix. + * @return true if the integral modification was needed, false otherwise. + */ + static bool ComputeIntegralTransCTM(const SkMatrix& in, SkMatrix* out); + /** * @brief Snap the translation components of the matrix to integers. * @@ -103,31 +115,28 @@ struct RasterCacheUtil { * @return SkM44 the snapped transformation matrix. */ static SkM44 GetIntegralTransCTM(const SkM44& ctm) { - // Avoid integral snapping if the matrix has complex transformation to avoid - // the artifact observed in https://github.com/flutter/flutter/issues/41654. - if (ctm.rc(0, 1) != 0 || ctm.rc(0, 2) != 0) { - // X multiplied by either Y or Z - return ctm; - } - if (ctm.rc(1, 0) != 0 || ctm.rc(1, 2) != 0) { - // Y multiplied by either X or Z - return ctm; - } - // We do not need to worry about the Z row unless the W row - // has perspective entries... - if (ctm.rc(3, 0) != 0 || ctm.rc(3, 1) != 0 || ctm.rc(3, 2) != 0 || - ctm.rc(3, 3) != 1) { - // W not identity row, therefore perspective is applied - return ctm; - } - - SkM44 result = ctm; - result.setRC(0, 3, SkScalarRoundToScalar(ctm.rc(0, 3))); - result.setRC(1, 3, SkScalarRoundToScalar(ctm.rc(1, 3))); - // No need to worry about Z translation because it has no effect - // without perspective entries... - return result; + SkM44 integral; + return ComputeIntegralTransCTM(ctm, &integral) ? integral : ctm; } + + /** + * @brief Snap the translation components of the |in| matrix to integers + * and store the snapped matrix in |out|. + * + * The snapping will only happen if the matrix only has scale and translation + * transformations. This is used, along with GetRoundedOutDeviceBounds, to + * ensure that the textures drawn by the raster cache are exactly aligned to + * physical pixels. Any layers that participate in raster caching must align + * themselves to physical pixels even when not cached to prevent a change in + * apparent location if caching is later applied. + * + * The |out| matrix will not be modified if this method returns false. + * + * @param in the current transformation matrix. + * @param out the storage for the snapped matrix. + * @return true if the integral modification was needed, false otherwise. + */ + static bool ComputeIntegralTransCTM(const SkM44& in, SkM44* out); }; } // namespace flutter From d5d1dab5a2c246766f8df423898c3a791bc5cbbc Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Thu, 14 Mar 2024 01:37:01 -0700 Subject: [PATCH 2/2] add tests for IntegralTransCTM functions --- flow/raster_cache_unittests.cc | 275 +++++++++++++++++++++++++++++++++ flow/raster_cache_util.cc | 4 +- 2 files changed, 277 insertions(+), 2 deletions(-) diff --git a/flow/raster_cache_unittests.cc b/flow/raster_cache_unittests.cc index e0dfda6593c8c..94339690fca92 100644 --- a/flow/raster_cache_unittests.cc +++ b/flow/raster_cache_unittests.cc @@ -809,6 +809,281 @@ TEST_F(RasterCacheTest, RasterCacheKeyIDLayerChildrenIds) { ASSERT_EQ(ids, expected_ids); } +TEST(RasterCacheUtilsTest, SkMatrixIntegralTransCTM) { +#define EXPECT_EQ_WITH_TRANSLATE(test, expected, expected_tx, expected_ty) \ + do { \ + EXPECT_EQ(test[SkMatrix::kMScaleX], expected[SkMatrix::kMScaleX]); \ + EXPECT_EQ(test[SkMatrix::kMSkewX], expected[SkMatrix::kMSkewX]); \ + EXPECT_EQ(test[SkMatrix::kMScaleY], expected[SkMatrix::kMScaleY]); \ + EXPECT_EQ(test[SkMatrix::kMSkewY], expected[SkMatrix::kMSkewY]); \ + EXPECT_EQ(test[SkMatrix::kMSkewX], expected[SkMatrix::kMSkewX]); \ + EXPECT_EQ(test[SkMatrix::kMPersp0], expected[SkMatrix::kMPersp0]); \ + EXPECT_EQ(test[SkMatrix::kMPersp1], expected[SkMatrix::kMPersp1]); \ + EXPECT_EQ(test[SkMatrix::kMPersp2], expected[SkMatrix::kMPersp2]); \ + EXPECT_EQ(test[SkMatrix::kMTransX], expected_tx); \ + EXPECT_EQ(test[SkMatrix::kMTransY], expected_ty); \ + } while (0) + +#define EXPECT_NON_INTEGER_TRANSLATION(matrix) \ + EXPECT_TRUE(SkScalarFraction(matrix[SkMatrix::kMTransX]) != 0.0f || \ + SkScalarFraction(matrix[SkMatrix::kMTransY]) != 0.0f) + + { + // Identity + SkMatrix matrix = SkMatrix::I(); + SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix); + SkMatrix compute; + EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute)); + EXPECT_EQ(get, matrix); + } + { + // Integer translate + SkMatrix matrix = SkMatrix::Translate(10.0f, 12.0f); + SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix); + SkMatrix compute; + EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute)); + EXPECT_EQ(get, matrix); + } + { + // Fractional x translate + SkMatrix matrix = SkMatrix::Translate(10.2f, 12.0f); + EXPECT_NON_INTEGER_TRANSLATION(matrix); + SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix); + SkMatrix compute; + EXPECT_TRUE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute)); + EXPECT_EQ_WITH_TRANSLATE(get, matrix, 10.0f, 12.0f); + EXPECT_EQ(get, compute); + } + { + // Fractional y translate + SkMatrix matrix = SkMatrix::Translate(10.0f, 12.3f); + EXPECT_NON_INTEGER_TRANSLATION(matrix); + SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix); + SkMatrix compute; + EXPECT_TRUE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute)); + EXPECT_EQ_WITH_TRANSLATE(get, matrix, 10.0f, 12.0f); + EXPECT_EQ(get, compute); + } + { + // Fractional x & y translate + SkMatrix matrix = SkMatrix::Translate(10.7f, 12.3f); + EXPECT_NON_INTEGER_TRANSLATION(matrix); + SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix); + SkMatrix compute; + EXPECT_TRUE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute)); + EXPECT_EQ_WITH_TRANSLATE(get, matrix, 11.0f, 12.0f); + EXPECT_EQ(get, compute); + } + { + // Scale + SkMatrix matrix = SkMatrix::Scale(2.0f, 3.0f); + SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix); + SkMatrix compute; + EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute)); + EXPECT_EQ(get, matrix); + } + { + // Scale, Integer translate + SkMatrix matrix = SkMatrix::Scale(2.0f, 3.0f); + matrix.preTranslate(10.0f, 12.0f); + SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix); + SkMatrix compute; + EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute)); + EXPECT_EQ(get, matrix); + } + { + // Scale, Fractional translate + SkMatrix matrix = SkMatrix::Scale(2.0f, 3.0f); + matrix.preTranslate(10.7f, 12.1f); + EXPECT_NON_INTEGER_TRANSLATION(matrix); + SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix); + SkMatrix compute; + EXPECT_TRUE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute)); + EXPECT_EQ_WITH_TRANSLATE(get, matrix, 21.0f, 36.0f); + EXPECT_EQ(get, compute); + } + { + // Skew + SkMatrix matrix = SkMatrix::Skew(0.5f, 0.1f); + SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix); + SkMatrix compute; + EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute)); + EXPECT_EQ(get, matrix); + } + { + // Skew, Fractional translate - should be NOP + SkMatrix matrix = SkMatrix::Skew(0.5f, 0.1f); + matrix.preTranslate(10.7f, 12.1f); + EXPECT_NON_INTEGER_TRANSLATION(matrix); + SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix); + SkMatrix compute; + EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute)); + EXPECT_EQ(get, matrix); + } + { + // Rotate + SkMatrix matrix = SkMatrix::RotateDeg(45); + SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix); + SkMatrix compute; + EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute)); + EXPECT_EQ(get, matrix); + } + { + // Rotate, Fractional Translate - should be NOP + SkMatrix matrix = SkMatrix::RotateDeg(45); + matrix.preTranslate(10.7f, 12.1f); + EXPECT_NON_INTEGER_TRANSLATION(matrix); + SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix); + SkMatrix compute; + EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute)); + EXPECT_EQ(get, matrix); + } + { + // Perspective x + SkMatrix matrix = SkMatrix::I(); + matrix.setPerspX(0.1); + SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix); + SkMatrix compute; + EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute)); + EXPECT_EQ(get, matrix); + } + { + // Perspective x, Fractional Translate - should be NOP + SkMatrix matrix = SkMatrix::I(); + matrix.setPerspX(0.1); + matrix.preTranslate(10.7f, 12.1f); + EXPECT_NON_INTEGER_TRANSLATION(matrix); + SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix); + SkMatrix compute; + EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute)); + EXPECT_EQ(get, matrix); + } + { + // Perspective y + SkMatrix matrix = SkMatrix::I(); + matrix.setPerspY(0.1); + SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix); + SkMatrix compute; + EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute)); + EXPECT_EQ(get, matrix); + } + { + // Perspective y, Fractional Translate - should be NOP + SkMatrix matrix = SkMatrix::I(); + matrix.setPerspY(0.1); + matrix.preTranslate(10.7f, 12.1f); + EXPECT_NON_INTEGER_TRANSLATION(matrix); + SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix); + SkMatrix compute; + EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute)); + EXPECT_EQ(get, matrix); + } + { + // Perspective weight + // clang-format off + SkMatrix matrix = SkMatrix::MakeAll( + 1.0f, 0.0f, 0.0f, + 0.0f, 1.0f, 0.0f, + 0.0f, 0.0f, 0.9f); + // clang-format on + SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix); + SkMatrix compute; + EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute)); + EXPECT_EQ(get, matrix); + } + { + // Perspective weight, Fractional Translate - should be NOP + // clang-format off + SkMatrix matrix = SkMatrix::MakeAll( + 1.0f, 0.0f, 0.0f, + 0.0f, 1.0f, 0.0f, + 0.0f, 0.0f, 0.9f); + // clang-format on + matrix.preTranslate(10.7f, 12.1f); + EXPECT_NON_INTEGER_TRANSLATION(matrix); + SkMatrix get = RasterCacheUtil::GetIntegralTransCTM(matrix); + SkMatrix compute; + EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute)); + EXPECT_EQ(get, matrix); + } +#undef EXPECT_NON_INTEGER_TRANSLATION +#undef EXPECT_EQ_WITH_TRANSLATE +} + +TEST(RasterCacheUtilsTest, SkM44IntegralTransCTM) { +#define EXPECT_EQ_WITH_TRANSLATE(test, expected, tx, ty, label) \ + do { \ + EXPECT_EQ(test.rc(0, 0), expected.rc(0, 0)) << label; \ + EXPECT_EQ(test.rc(0, 1), expected.rc(0, 1)) << label; \ + EXPECT_EQ(test.rc(0, 2), expected.rc(0, 2)) << label; \ + EXPECT_EQ(test.rc(0, 3), tx) << label; \ + EXPECT_EQ(test.rc(1, 0), expected.rc(1, 0)) << label; \ + EXPECT_EQ(test.rc(1, 1), expected.rc(1, 1)) << label; \ + EXPECT_EQ(test.rc(1, 2), expected.rc(1, 2)) << label; \ + EXPECT_EQ(test.rc(1, 3), ty) << label; \ + EXPECT_EQ(test.rc(2, 0), expected.rc(2, 0)) << label; \ + EXPECT_EQ(test.rc(2, 1), expected.rc(2, 1)) << label; \ + EXPECT_EQ(test.rc(2, 2), expected.rc(2, 2)) << label; \ + EXPECT_EQ(test.rc(2, 3), expected.rc(2, 3)) << label; \ + EXPECT_EQ(test.rc(3, 0), expected.rc(3, 0)) << label; \ + EXPECT_EQ(test.rc(3, 1), expected.rc(3, 1)) << label; \ + EXPECT_EQ(test.rc(3, 2), expected.rc(3, 2)) << label; \ + EXPECT_EQ(test.rc(3, 3), expected.rc(3, 3)) << label; \ + } while (0) + +#define EXPECT_NON_INTEGER_TRANSLATION(matrix) \ + EXPECT_TRUE(SkScalarFraction(matrix.rc(0, 3)) != 0.0f || \ + SkScalarFraction(matrix.rc(1, 3)) != 0.0f) + + for (int r = 0; r < 4; r++) { + for (int c = 0; c < 4; c++) { + bool snaps; + switch (r) { + case 0: // X equation + if (c == 3) { + continue; // TranslateX, the value we are testing, skip + } + snaps = (c == 0); // X Scale value yes, Skew by Y or Z no + break; + case 1: // Y equation + if (c == 3) { + continue; // TranslateY, the value we are testing, skip + } + snaps = (c == 1); // Y Scale value yes, Skew by X or Z no + break; + case 2: // Z equation, ignored, will snap + snaps = true; + break; + case 3: // W equation, modifications prevent snapping + snaps = false; + break; + default: + FML_UNREACHABLE(); + } + auto label = std::to_string(r) + ", " + std::to_string(c); + SkM44 matrix = SkM44::Translate(10.7f, 12.1f); + EXPECT_NON_INTEGER_TRANSLATION(matrix) << label; + matrix.setRC(r, c, 0.5f); + if (snaps) { + SkM44 compute; + SkM44 get = RasterCacheUtil::GetIntegralTransCTM(matrix); + EXPECT_TRUE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute)) + << label; + EXPECT_EQ_WITH_TRANSLATE(get, matrix, 11.0f, 12.0f, label); + EXPECT_EQ(get, compute) << label; + } else { + SkM44 compute; + SkM44 get = RasterCacheUtil::GetIntegralTransCTM(matrix); + EXPECT_FALSE(RasterCacheUtil::ComputeIntegralTransCTM(matrix, &compute)) + << label; + EXPECT_EQ(get, matrix) << label; + } + } + } +#undef EXPECT_NON_INTEGER_TRANSLATION +#undef EXPECT_EQ_WITH_TRANSLATE +} + } // namespace testing } // namespace flutter diff --git a/flow/raster_cache_util.cc b/flow/raster_cache_util.cc index fa636b5b78b58..8fbd19cb1a2a2 100644 --- a/flow/raster_cache_util.cc +++ b/flow/raster_cache_util.cc @@ -41,13 +41,13 @@ bool RasterCacheUtil::ComputeIntegralTransCTM(const SkM44& in, SkM44* out) { // Y multiplied by either X or Z return false; } - // We do not need to worry about the Z row unless the W row - // has perspective entries... if (in.rc(3, 0) != 0 || in.rc(3, 1) != 0 || in.rc(3, 2) != 0 || in.rc(3, 3) != 1) { // W not identity row, therefore perspective is applied return false; } + // We do not need to worry about the Z row unless the W row + // has perspective entries, which we've just eliminated... SkScalar in_tx = in.rc(0, 3); SkScalar in_ty = in.rc(1, 3);