From 52020d743523ee3b2211ed8c646cdee0bb7cc397 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Thu, 12 May 2022 17:41:42 -0700 Subject: [PATCH 01/11] add currentTransform/Clip methods to DL and ui.Canvas --- display_list/display_list_builder.cc | 52 ++++-- display_list/display_list_builder.h | 26 ++- display_list/display_list_unittests.cc | 203 ++++++++++++++++++++++ lib/ui/painting.dart | 14 ++ lib/ui/painting/canvas.cc | 25 +++ lib/ui/painting/canvas.h | 2 + testing/dart/canvas_test.dart | 177 ++++++++++++++++++- testing/dart/pubspec.yaml | 1 + third_party/tonic/typed_data/typed_list.h | 1 + 9 files changed, 485 insertions(+), 16 deletions(-) diff --git a/display_list/display_list_builder.cc b/display_list/display_list_builder.cc index 221b6db186eaf..8027b23b2839c 100644 --- a/display_list/display_list_builder.cc +++ b/display_list/display_list_builder.cc @@ -65,7 +65,7 @@ sk_sp DisplayListBuilder::Build() { DisplayListBuilder::DisplayListBuilder(const SkRect& cull_rect) : cull_rect_(cull_rect) { - layer_stack_.emplace_back(); + layer_stack_.emplace_back(SkM44(), cull_rect); current_layer_ = &layer_stack_.back(); } @@ -415,7 +415,7 @@ void DisplayListBuilder::setAttributesFromPaint( void DisplayListBuilder::save() { Push(0, 1); - layer_stack_.emplace_back(); + layer_stack_.emplace_back(current_layer_); current_layer_ = &layer_stack_.back(); } void DisplayListBuilder::restore() { @@ -476,7 +476,7 @@ void DisplayListBuilder::saveLayer(const SkRect* bounds, : Push(0, 1, options); } CheckLayerOpacityCompatibility(options.renders_with_attributes()); - layer_stack_.emplace_back(save_layer_offset, true); + layer_stack_.emplace_back(current_layer_, save_layer_offset, true); current_layer_ = &layer_stack_.back(); if (options.renders_with_attributes()) { // |current_opacity_compatibility_| does not take an ImageFilter into @@ -505,23 +505,27 @@ void DisplayListBuilder::translate(SkScalar tx, SkScalar ty) { if (SkScalarIsFinite(tx) && SkScalarIsFinite(ty) && (tx != 0.0 || ty != 0.0)) { Push(0, 1, tx, ty); + current_layer_->matrix.preTranslate(tx, ty); } } void DisplayListBuilder::scale(SkScalar sx, SkScalar sy) { if (SkScalarIsFinite(sx) && SkScalarIsFinite(sy) && (sx != 1.0 || sy != 1.0)) { Push(0, 1, sx, sy); + current_layer_->matrix.preScale(sx, sy); } } void DisplayListBuilder::rotate(SkScalar degrees) { if (SkScalarMod(degrees, 360.0) != 0.0) { Push(0, 1, degrees); + current_layer_->matrix.preConcat(SkMatrix::RotateDeg(degrees)); } } void DisplayListBuilder::skew(SkScalar sx, SkScalar sy) { if (SkScalarIsFinite(sx) && SkScalarIsFinite(sy) && (sx != 0.0 || sy != 0.0)) { Push(0, 1, sx, sy); + current_layer_->matrix.preConcat(SkMatrix::Skew(sx, sy)); } } @@ -539,6 +543,10 @@ void DisplayListBuilder::transform2DAffine( Push(0, 1, mxx, mxy, mxt, myx, myy, myt); + current_layer_->matrix.preConcat(SkM44(mxx, mxy, 0, mxt, + myx, myy, 0, myt, + 0, 0, 1, 0, + 0, 0, 0, 1)); } } // full 4x4 transform in row major order @@ -562,11 +570,16 @@ void DisplayListBuilder::transformFullPerspective( myx, myy, myz, myt, mzx, mzy, mzz, mzt, mwx, mwy, mwz, mwt); + current_layer_->matrix.preConcat(SkM44(mxx, mxy, mxz, mxt, + myx, myy, myz, myt, + mzx, mzy, mzz, mzt, + mwx, mwy, mwz, mwt)); } } // clang-format on void DisplayListBuilder::transformReset() { Push(0, 0); + current_layer_->matrix.setIdentity(); } void DisplayListBuilder::transform(const SkMatrix* matrix) { if (matrix != nullptr) { @@ -586,9 +599,14 @@ void DisplayListBuilder::transform(const SkM44* m44) { void DisplayListBuilder::clipRect(const SkRect& rect, SkClipOp clip_op, bool is_aa) { - clip_op == SkClipOp::kIntersect // - ? Push(0, 1, rect, is_aa) - : Push(0, 1, rect, is_aa); + if (clip_op == SkClipOp::kIntersect) { + Push(0, 1, rect, is_aa); + if (!current_layer_->clip_bounds.intersect(rect)) { + current_layer_->clip_bounds.setEmpty(); + } + } else { + Push(0, 1, rect, is_aa); + } } void DisplayListBuilder::clipRRect(const SkRRect& rrect, SkClipOp clip_op, @@ -596,9 +614,14 @@ void DisplayListBuilder::clipRRect(const SkRRect& rrect, if (rrect.isRect()) { clipRect(rrect.rect(), clip_op, is_aa); } else { - clip_op == SkClipOp::kIntersect // - ? Push(0, 1, rrect, is_aa) - : Push(0, 1, rrect, is_aa); + if (clip_op == SkClipOp::kIntersect) { + Push(0, 1, rrect, is_aa); + if (!current_layer_->clip_bounds.intersect(rrect.getBounds())) { + current_layer_->clip_bounds.setEmpty(); + } + } else { + Push(0, 1, rrect, is_aa); + } } } void DisplayListBuilder::clipPath(const SkPath& path, @@ -621,9 +644,14 @@ void DisplayListBuilder::clipPath(const SkPath& path, return; } } - clip_op == SkClipOp::kIntersect // - ? Push(0, 1, path, is_aa) - : Push(0, 1, path, is_aa); + if (clip_op == SkClipOp::kIntersect) { + Push(0, 1, path, is_aa); + if (!current_layer_->clip_bounds.intersect(path.getBounds())) { + current_layer_->clip_bounds.setEmpty(); + } + } else { + Push(0, 1, path, is_aa); + } } void DisplayListBuilder::drawPaint() { diff --git a/display_list/display_list_builder.h b/display_list/display_list_builder.h index 257b979fe9982..32d97934f998d 100644 --- a/display_list/display_list_builder.h +++ b/display_list/display_list_builder.h @@ -198,10 +198,16 @@ class DisplayListBuilder final : public virtual Dispatcher, void transform(const SkMatrix& matrix) { transform(&matrix); } void transform(const SkM44& matrix44) { transform(&matrix44); } + SkM44 getCurrentTransformFullPerspective() { return current_layer_->matrix; } + + SkMatrix getCurrentTransform() { return current_layer_->matrix.asM33(); } + void clipRect(const SkRect& rect, SkClipOp clip_op, bool is_aa) override; void clipRRect(const SkRRect& rrect, SkClipOp clip_op, bool is_aa) override; void clipPath(const SkPath& path, SkClipOp clip_op, bool is_aa) override; + SkRect getClipBounds() { return current_layer_->clip_bounds; } + void drawPaint() override; void drawPaint(const DlPaint& paint); void drawColor(DlColor color, DlBlendMode mode) override; @@ -350,11 +356,24 @@ class DisplayListBuilder final : public virtual Dispatcher, } struct LayerInfo { - LayerInfo(size_t save_layer_offset = 0, bool has_layer = false) + LayerInfo(const SkM44& matrix, + const SkRect& clip_bounds, + size_t save_layer_offset = 0, + bool has_layer = false) : save_layer_offset(save_layer_offset), has_layer(has_layer), cannot_inherit_opacity(false), - has_compatible_op(false) {} + has_compatible_op(false), + matrix(matrix), + clip_bounds(clip_bounds) {} + + LayerInfo(const LayerInfo* current_layer, + size_t save_layer_offset = 0, + bool has_layer = false) + : LayerInfo(current_layer->matrix, + current_layer->clip_bounds, + save_layer_offset, + has_layer) {} // The offset into the memory buffer where the saveLayer DLOp record // for this saveLayer() call is placed. This may be needed if the @@ -368,6 +387,9 @@ class DisplayListBuilder final : public virtual Dispatcher, bool cannot_inherit_opacity; bool has_compatible_op; + SkM44 matrix; + SkRect clip_bounds; + bool is_group_opacity_compatible() const { return !cannot_inherit_opacity; } void mark_incompatible() { cannot_inherit_opacity = true; } diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index e9ef2e5bc893c..773ef26abf126 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -1957,5 +1957,208 @@ TEST(DisplayList, FlutterSvgIssue661BoundsWereEmpty) { EXPECT_EQ(display_list->bytes(), sizeof(DisplayList) + 304u); } +TEST(DisplayList, TranslateAffectsCurrentTransform) { + DisplayListBuilder builder; + builder.translate(12.3, 14.5); + SkMatrix matrix = SkMatrix::Translate(12.3, 14.5); + SkM44 m44 = SkM44(matrix); + SkM44 curM44 = builder.getCurrentTransformFullPerspective(); + SkMatrix curMatrix = builder.getCurrentTransform(); + ASSERT_EQ(curM44, m44); + ASSERT_EQ(curMatrix, matrix); + builder.translate(10, 10); + // CurrentTransform has changed + ASSERT_NE(builder.getCurrentTransformFullPerspective(), m44); + ASSERT_NE(builder.getCurrentTransform(), curMatrix); + // Previous return values have not + ASSERT_EQ(curM44, m44); + ASSERT_EQ(curMatrix, matrix); +} + +TEST(DisplayList, ScaleAffectsCurrentTransform) { + DisplayListBuilder builder; + builder.scale(12.3, 14.5); + SkMatrix matrix = SkMatrix::Scale(12.3, 14.5); + SkM44 m44 = SkM44(matrix); + SkM44 curM44 = builder.getCurrentTransformFullPerspective(); + SkMatrix curMatrix = builder.getCurrentTransform(); + ASSERT_EQ(curM44, m44); + ASSERT_EQ(curMatrix, matrix); + builder.translate(10, 10); + // CurrentTransform has changed + ASSERT_NE(builder.getCurrentTransformFullPerspective(), m44); + ASSERT_NE(builder.getCurrentTransform(), curMatrix); + // Previous return values have not + ASSERT_EQ(curM44, m44); + ASSERT_EQ(curMatrix, matrix); +} + +TEST(DisplayList, RotateAffectsCurrentTransform) { + DisplayListBuilder builder; + builder.rotate(12.3); + SkMatrix matrix = SkMatrix::RotateDeg(12.3); + SkM44 m44 = SkM44(matrix); + SkM44 curM44 = builder.getCurrentTransformFullPerspective(); + SkMatrix curMatrix = builder.getCurrentTransform(); + ASSERT_EQ(curM44, m44); + ASSERT_EQ(curMatrix, matrix); + builder.translate(10, 10); + // CurrentTransform has changed + ASSERT_NE(builder.getCurrentTransformFullPerspective(), m44); + ASSERT_NE(builder.getCurrentTransform(), curMatrix); + // Previous return values have not + ASSERT_EQ(curM44, m44); + ASSERT_EQ(curMatrix, matrix); +} + +TEST(DisplayList, SkewAffectsCurrentTransform) { + DisplayListBuilder builder; + builder.skew(12.3, 14.5); + SkMatrix matrix = SkMatrix::Skew(12.3, 14.5); + SkM44 m44 = SkM44(matrix); + SkM44 curM44 = builder.getCurrentTransformFullPerspective(); + SkMatrix curMatrix = builder.getCurrentTransform(); + ASSERT_EQ(curM44, m44); + ASSERT_EQ(curMatrix, matrix); + builder.translate(10, 10); + // CurrentTransform has changed + ASSERT_NE(builder.getCurrentTransformFullPerspective(), m44); + ASSERT_NE(builder.getCurrentTransform(), curMatrix); + // Previous return values have not + ASSERT_EQ(curM44, m44); + ASSERT_EQ(curMatrix, matrix); +} + +TEST(DisplayList, TransformAffectsCurrentTransform) { + DisplayListBuilder builder; + builder.transform2DAffine(3, 0, 12.3, // + 1, 5, 14.5); + SkMatrix matrix = SkMatrix::MakeAll(3, 0, 12.3, // + 1, 5, 14.5, // + 0, 0, 1); + SkM44 m44 = SkM44(matrix); + SkM44 curM44 = builder.getCurrentTransformFullPerspective(); + SkMatrix curMatrix = builder.getCurrentTransform(); + ASSERT_EQ(curM44, m44); + ASSERT_EQ(curMatrix, matrix); + builder.translate(10, 10); + // CurrentTransform has changed + ASSERT_NE(builder.getCurrentTransformFullPerspective(), m44); + ASSERT_NE(builder.getCurrentTransform(), curMatrix); + // Previous return values have not + ASSERT_EQ(curM44, m44); + ASSERT_EQ(curMatrix, matrix); +} + +TEST(DisplayList, FullTransformAffectsCurrentTransform) { + DisplayListBuilder builder; + builder.transformFullPerspective(3, 0, 4, 12.3, // + 1, 5, 3, 14.5, // + 0, 0, 7, 16.2, // + 0, 0, 0, 1); + SkMatrix matrix = SkMatrix::MakeAll(3, 0, 12.3, // + 1, 5, 14.5, // + 0, 0, 1); + SkM44 m44 = SkM44(3, 0, 4, 12.3, // + 1, 5, 3, 14.5, // + 0, 0, 7, 16.2, // + 0, 0, 0, 1); + SkM44 curM44 = builder.getCurrentTransformFullPerspective(); + SkMatrix curMatrix = builder.getCurrentTransform(); + ASSERT_EQ(curM44, m44); + ASSERT_EQ(curMatrix, matrix); + builder.translate(10, 10); + // CurrentTransform has changed + ASSERT_NE(builder.getCurrentTransformFullPerspective(), m44); + ASSERT_NE(builder.getCurrentTransform(), curMatrix); + // Previous return values have not + ASSERT_EQ(curM44, m44); + ASSERT_EQ(curMatrix, matrix); +} + +TEST(DisplayList, ClipRectAffectsCurrentClipBounds) { + DisplayListBuilder builder; + SkRect clip = SkRect::MakeLTRB(10.2, 11.3, 20.4, 25.7); + builder.clipRect(clip, SkClipOp::kIntersect, false); + SkRect curBounds = builder.getClipBounds(); + ASSERT_EQ(curBounds, clip); + builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, false); + // CurrentTransform has changed + ASSERT_NE(builder.getClipBounds(), clip); + // Previous return values have not + ASSERT_EQ(curBounds, clip); +} + +TEST(DisplayList, ClipRRectAffectsCurrentClipBounds) { + DisplayListBuilder builder; + SkRRect clip = SkRRect::MakeRectXY({10.2, 11.3, 20.4, 25.7}, 3, 2); + builder.clipRRect(clip, SkClipOp::kIntersect, false); + SkRect curBounds = builder.getClipBounds(); + ASSERT_EQ(curBounds, clip.getBounds()); + builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, false); + // CurrentTransform has changed + ASSERT_NE(builder.getClipBounds(), clip.getBounds()); + // Previous return values have not + ASSERT_EQ(curBounds, clip.getBounds()); +} + +TEST(DisplayList, ClipPathAffectsCurrentClipBounds) { + DisplayListBuilder builder; + SkPath clip = SkPath().addCircle(10.2, 11.3, 2).addCircle(20.4, 25.7, 2); + builder.clipPath(clip, SkClipOp::kIntersect, false); + SkRect curBounds = builder.getClipBounds(); + ASSERT_EQ(curBounds, clip.getBounds()); + builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, false); + // CurrentTransform has changed + ASSERT_NE(builder.getClipBounds(), clip.getBounds()); + // Previous return values have not + ASSERT_EQ(curBounds, clip.getBounds()); +} + +TEST(DisplayList, DiffClipRectDoesNotAffectCurrentClipBounds) { + DisplayListBuilder builder; + SkRect diff_clip = SkRect::MakeLTRB(0, 0, 15, 15); + SkRect clip = SkRect::MakeLTRB(10.2, 11.3, 20.4, 25.7); + builder.clipRect(clip, SkClipOp::kIntersect, false); + builder.clipRect(diff_clip, SkClipOp::kDifference, false); + SkRect curBounds = builder.getClipBounds(); + ASSERT_EQ(curBounds, clip); + builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, false); + // CurrentTransform has changed + ASSERT_NE(builder.getClipBounds(), clip); + // Previous return values have not + ASSERT_EQ(curBounds, clip); +} + +TEST(DisplayList, DiffClipRRectAffectsCurrentClipBounds) { + DisplayListBuilder builder; + SkRRect diff_clip = SkRRect::MakeRectXY({0, 0, 15, 15}, 1, 1); + SkRRect clip = SkRRect::MakeRectXY({10.2, 11.3, 20.4, 25.7}, 3, 2); + builder.clipRRect(clip, SkClipOp::kIntersect, false); + builder.clipRRect(diff_clip, SkClipOp::kDifference, false); + SkRect curBounds = builder.getClipBounds(); + ASSERT_EQ(curBounds, clip.getBounds()); + builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, false); + // CurrentTransform has changed + ASSERT_NE(builder.getClipBounds(), clip.getBounds()); + // Previous return values have not + ASSERT_EQ(curBounds, clip.getBounds()); +} + +TEST(DisplayList, DiffClipPathAffectsCurrentClipBounds) { + DisplayListBuilder builder; + SkPath diff_clip = SkPath().addRect({0, 0, 15, 15}); + SkPath clip = SkPath().addCircle(10.2, 11.3, 2).addCircle(20.4, 25.7, 2); + builder.clipPath(clip, SkClipOp::kIntersect, false); + builder.clipPath(diff_clip, SkClipOp::kDifference, false); + SkRect curBounds = builder.getClipBounds(); + ASSERT_EQ(curBounds, clip.getBounds()); + builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, false); + // CurrentTransform has changed + ASSERT_NE(builder.getClipBounds(), clip.getBounds()); + // Previous return values have not + ASSERT_EQ(curBounds, clip.getBounds()); +} + } // namespace testing } // namespace flutter diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index ea1233e441aa8..4e3d9b84e0edc 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -4449,6 +4449,13 @@ class Canvas extends NativeFieldWrapperClass1 { } void _transform(Float64List matrix4) native 'Canvas_transform'; + Float64List getCurrentTransform() { + final Float64List matrix4 = Float64List(16); + _getCurrentTransform(matrix4); + return matrix4; + } + void _getCurrentTransform(Float64List matrix4) native 'Canvas_getCurrentTransform'; + /// Reduces the clip region to the intersection of the current clip and the /// given rectangle. /// @@ -4503,6 +4510,13 @@ class Canvas extends NativeFieldWrapperClass1 { } void _clipPath(Path path, bool doAntiAlias) native 'Canvas_clipPath'; + Rect getCurrentClipBounds() { + final Float64List bounds = Float64List(4); + _getCurrentClipBounds(bounds); + return Rect.fromLTRB(bounds[0], bounds[1], bounds[2], bounds[3]); + } + void _getCurrentClipBounds(Float64List bounds) native 'Canvas_getCurrentClipBounds'; + /// Paints the given [Color] onto the canvas, applying the given /// [BlendMode], with the given color being the source and the background /// being the destination. diff --git a/lib/ui/painting/canvas.cc b/lib/ui/painting/canvas.cc index ea5f583aaac8a..eea08dc6c1c91 100644 --- a/lib/ui/painting/canvas.cc +++ b/lib/ui/painting/canvas.cc @@ -46,9 +46,11 @@ IMPLEMENT_WRAPPERTYPEINFO(ui, Canvas); V(Canvas, rotate) \ V(Canvas, skew) \ V(Canvas, transform) \ + V(Canvas, getCurrentTransform) \ V(Canvas, clipRect) \ V(Canvas, clipRRect) \ V(Canvas, clipPath) \ + V(Canvas, getCurrentClipBounds) \ V(Canvas, drawColor) \ V(Canvas, drawLine) \ V(Canvas, drawPaint) \ @@ -219,6 +221,19 @@ void Canvas::transform(const tonic::Float64List& matrix4) { } } +void Canvas::getCurrentTransform(tonic::Float64List& matrix4) { + SkM44 sk_m44 = display_list_recorder_ + ? display_list_recorder_->builder() + ->getCurrentTransformFullPerspective() + : canvas_->getLocalToDevice(); + SkScalar m44_values[16]; + // The Float array stored by Dart Matrix4 is in column-major order + sk_m44.getColMajor(m44_values); + for (int i = 0; i < 16; i++) { + matrix4[i] = m44_values[i]; + } +} + void Canvas::clipRect(double left, double top, double right, @@ -255,6 +270,16 @@ void Canvas::clipPath(const CanvasPath* path, bool doAntiAlias) { } } +void Canvas::getCurrentClipBounds(tonic::Float64List& rect) { + SkRect bounds = display_list_recorder_ + ? display_list_recorder_->builder()->getClipBounds() + : canvas_->getLocalClipBounds(); + rect[0] = bounds.fLeft; + rect[1] = bounds.fTop; + rect[2] = bounds.fRight; + rect[3] = bounds.fBottom; +} + void Canvas::drawColor(SkColor color, DlBlendMode blend_mode) { if (display_list_recorder_) { builder()->drawColor(color, blend_mode); diff --git a/lib/ui/painting/canvas.h b/lib/ui/painting/canvas.h index 1d3c4e3bce19b..adfc1a35410c8 100644 --- a/lib/ui/painting/canvas.h +++ b/lib/ui/painting/canvas.h @@ -53,6 +53,7 @@ class Canvas : public RefCountedDartWrappable, DisplayListOpFlags { void rotate(double radians); void skew(double sx, double sy); void transform(const tonic::Float64List& matrix4); + void getCurrentTransform(tonic::Float64List& matrix4); void clipRect(double left, double top, @@ -62,6 +63,7 @@ class Canvas : public RefCountedDartWrappable, DisplayListOpFlags { bool doAntiAlias = true); void clipRRect(const RRect& rrect, bool doAntiAlias = true); void clipPath(const CanvasPath* path, bool doAntiAlias = true); + void getCurrentClipBounds(tonic::Float64List& rect); void drawColor(SkColor color, DlBlendMode blend_mode); void drawLine(double x1, diff --git a/testing/dart/canvas_test.dart b/testing/dart/canvas_test.dart index 5ec82d22e50a9..e90bfe6e61bbd 100644 --- a/testing/dart/canvas_test.dart +++ b/testing/dart/canvas_test.dart @@ -270,8 +270,8 @@ void main() { }); test('Canvas preserves perspective data in Matrix4', () async { - final double rotateAroundX = pi / 6; // 30 degrees - final double rotateAroundY = pi / 9; // 20 degrees + const double rotateAroundX = pi / 6; // 30 degrees + const double rotateAroundY = pi / 9; // 20 degrees const int width = 150; const int height = 150; const Color black = Color.fromARGB(255, 0, 0, 0); @@ -434,5 +434,178 @@ void main() { } else { expect(error, isNull); } + + Matcher closeToTransform(Float64List expected) => (dynamic v) { + Expect.type(v); + final Float64List value = v; + expect(expected.length, equals(16)); + expect(value.length, equals(16)); + for (int r = 0; r < 4; r++) { + for (int c = 0; c < 4; c++) { + final double vActual = value[r*4 + c]; + final double vExpected = expected[r*4 + c]; + if ((vActual - vExpected).abs() > 1e-10) { + Expect.fail('matrix mismatch at $r, $c, $vActual not close to $vExpected'); + } + } + } + }; + + Matcher notCloseToTransform(Float64List expected) => (dynamic v) { + Expect.type(v); + final Float64List value = v; + expect(expected.length, equals(16)); + expect(value.length, equals(16)); + for (int r = 0; r < 4; r++) { + for (int c = 0; c < 4; c++) { + final double vActual = value[r*4 + c]; + final double vExpected = expected[r*4 + c]; + if ((vActual - vExpected).abs() > 1e-10) { + return; + } + } + } + Expect.fail('$value is too close to $expected'); + }; + + test('Canvas.translate affects canvas.getCurrentTransform', () async { + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + canvas.translate(12, 14.5); + final Float64List matrix = Matrix4.translationValues(12, 14.5, 0).storage; + final Float64List curMatrix = canvas.getCurrentTransform(); + expect(curMatrix, closeToTransform(matrix)); + canvas.translate(10, 10); + final Float64List newCurMatrix = canvas.getCurrentTransform(); + expect(newCurMatrix, notCloseToTransform(matrix)); + expect(curMatrix, closeToTransform(matrix)); + }); + + test('Canvas.scale affects canvas.getCurrentTransform', () async { + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + canvas.scale(12, 14.5); + final Float64List matrix = Matrix4.diagonal3Values(12, 14.5, 1).storage; + final Float64List curMatrix = canvas.getCurrentTransform(); + expect(curMatrix, closeToTransform(matrix)); + canvas.translate(10, 10); + final Float64List newCurMatrix = canvas.getCurrentTransform(); + expect(newCurMatrix, notCloseToTransform(matrix)); + expect(curMatrix, closeToTransform(matrix)); + }); + + test('Canvas.rotate affects canvas.getCurrentTransform', () async { + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + canvas.rotate(pi); + final Float64List matrix = Matrix4.rotationZ(pi).storage; + final Float64List curMatrix = canvas.getCurrentTransform(); + expect(curMatrix, closeToTransform(matrix)); + canvas.translate(10, 10); + final Float64List newCurMatrix = canvas.getCurrentTransform(); + expect(newCurMatrix, notCloseToTransform(matrix)); + expect(curMatrix, closeToTransform(matrix)); + }); + + test('Canvas.skew affects canvas.getCurrentTransform', () async { + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + canvas.skew(12, 14.5); + final Float64List matrix = (Matrix4.identity()..setEntry(0, 1, 12)..setEntry(1, 0, 14.5)).storage; + final Float64List curMatrix = canvas.getCurrentTransform(); + expect(curMatrix, closeToTransform(matrix)); + canvas.translate(10, 10); + final Float64List newCurMatrix = canvas.getCurrentTransform(); + expect(newCurMatrix, notCloseToTransform(matrix)); + expect(curMatrix, closeToTransform(matrix)); + }); + + test('Canvas.transform affects canvas.getCurrentTransform', () async { + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + final Float64List matrix = (Matrix4.identity()..translate(12.0, 14.5)..scale(12.0, 14.5)).storage; + canvas.transform(matrix); + final Float64List curMatrix = canvas.getCurrentTransform(); + expect(curMatrix, closeToTransform(matrix)); + canvas.translate(10, 10); + final Float64List newCurMatrix = canvas.getCurrentTransform(); + expect(newCurMatrix, notCloseToTransform(matrix)); + expect(curMatrix, closeToTransform(matrix)); + }); + + Matcher closeToRect(Rect expected) => (dynamic v) { + Expect.type(v); + final Rect value = v; + expect(value.left, closeTo(expected.left, 1e-6)); + expect(value.top, closeTo(expected.top, 1e-6)); + expect(value.right, closeTo(expected.right, 1e-6)); + expect(value.bottom, closeTo(expected.bottom, 1e-6)); + }; + + Matcher notCloseToRect(Rect expected) => (dynamic v) { + Expect.type(v); + final Rect value = v; + if ((value.left - expected.left).abs() > 1e-6 || + (value.top - expected.top).abs() > 1e-6 || + (value.right - expected.right).abs() > 1e-6 || + (value.bottom - expected.bottom).abs() > 1e-6) { + return; + } + Expect.fail('$value is too close to $expected'); + }; + + test('Canvas.clipRect affects canvas.getCurrentClipBounds', () async { + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + const Rect clip = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); + canvas.clipRect(clip); + final Rect curBounds = canvas.getCurrentClipBounds(); + expect(curBounds, closeToRect(clip)); + canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15)); + final Rect newCurBounds = canvas.getCurrentClipBounds(); + expect(newCurBounds, notCloseToRect(clip)); + expect(curBounds, closeToRect(clip)); + }); + + test('Canvas.clipRRect affects canvas.getCurrentClipBounds', () async { + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + const Rect clipBounds = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); + final RRect clip = RRect.fromRectAndRadius(clipBounds, const Radius.circular(3)); + canvas.clipRRect(clip); + final Rect curBounds = canvas.getCurrentClipBounds(); + expect(curBounds, closeToRect(clipBounds)); + canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15)); + final Rect newCurBounds = canvas.getCurrentClipBounds(); + expect(newCurBounds, notCloseToRect(clipBounds)); + expect(curBounds, closeToRect(clipBounds)); + }); + + test('Canvas.clipPath affects canvas.getCurrentClipBounds', () async { + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + const Rect clipBounds = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); + final Path clip = Path()..addRect(clipBounds)..addOval(clipBounds); + canvas.clipPath(clip); + final Rect curBounds = canvas.getCurrentClipBounds(); + expect(curBounds, closeToRect(clipBounds)); + canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15)); + final Rect newCurBounds = canvas.getCurrentClipBounds(); + expect(newCurBounds, notCloseToRect(clipBounds)); + expect(curBounds, closeToRect(clipBounds)); + }); + + test('Canvas.clipRect(diff) does not affect canvas.getCurrentClipBounds', () async { + final PictureRecorder recorder = PictureRecorder(); + final Canvas canvas = Canvas(recorder); + const Rect clip = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); + canvas.clipRect(clip); + canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15), clipOp: ClipOp.difference); + final Rect curBounds = canvas.getCurrentClipBounds(); + expect(curBounds, closeToRect(clip)); + canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15)); + final Rect newCurBounds = canvas.getCurrentClipBounds(); + expect(newCurBounds, notCloseToRect(clip)); + expect(curBounds, closeToRect(clip)); }); } diff --git a/testing/dart/pubspec.yaml b/testing/dart/pubspec.yaml index b0a16c4e618e1..9cc8ac771c1c0 100644 --- a/testing/dart/pubspec.yaml +++ b/testing/dart/pubspec.yaml @@ -22,6 +22,7 @@ dependencies: sky_engine: any vector_math: any vm_service: any + matcher: any dependency_overrides: async_helper: diff --git a/third_party/tonic/typed_data/typed_list.h b/third_party/tonic/typed_data/typed_list.h index 008b8d4f883bf..369bca0072eb9 100644 --- a/third_party/tonic/typed_data/typed_list.h +++ b/third_party/tonic/typed_data/typed_list.h @@ -38,6 +38,7 @@ class TypedList { const ElemType& operator[](intptr_t i) const { return at(i); } const ElemType* data() const { return data_; } + ElemType* data() { return data_; } intptr_t num_elements() const { return num_elements_; } Dart_Handle dart_handle() const { return dart_handle_; } From 3decf0ff2d6664bc656fe9a4d8a9b7c853f598db Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Fri, 13 May 2022 00:11:47 -0700 Subject: [PATCH 02/11] remove lines of code not used for the fix --- testing/dart/pubspec.yaml | 1 - third_party/tonic/typed_data/typed_list.h | 1 - 2 files changed, 2 deletions(-) diff --git a/testing/dart/pubspec.yaml b/testing/dart/pubspec.yaml index 9cc8ac771c1c0..b0a16c4e618e1 100644 --- a/testing/dart/pubspec.yaml +++ b/testing/dart/pubspec.yaml @@ -22,7 +22,6 @@ dependencies: sky_engine: any vector_math: any vm_service: any - matcher: any dependency_overrides: async_helper: diff --git a/third_party/tonic/typed_data/typed_list.h b/third_party/tonic/typed_data/typed_list.h index 369bca0072eb9..008b8d4f883bf 100644 --- a/third_party/tonic/typed_data/typed_list.h +++ b/third_party/tonic/typed_data/typed_list.h @@ -38,7 +38,6 @@ class TypedList { const ElemType& operator[](intptr_t i) const { return at(i); } const ElemType* data() const { return data_; } - ElemType* data() { return data_; } intptr_t num_elements() const { return num_elements_; } Dart_Handle dart_handle() const { return dart_handle_; } From a156a6a55c01799d414e025649533877d739f6a8 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Tue, 17 May 2022 20:44:08 -0700 Subject: [PATCH 03/11] simpler name and some doc comments --- display_list/display_list_builder.h | 5 +-- display_list/display_list_unittests.cc | 48 +++++++++++----------- lib/ui/painting.dart | 56 +++++++++++++++++++++++--- lib/ui/painting/canvas.cc | 10 ++--- lib/ui/painting/canvas.h | 4 +- testing/dart/canvas_test.dart | 54 ++++++++++++------------- 6 files changed, 110 insertions(+), 67 deletions(-) diff --git a/display_list/display_list_builder.h b/display_list/display_list_builder.h index 32d97934f998d..da283e6e48feb 100644 --- a/display_list/display_list_builder.h +++ b/display_list/display_list_builder.h @@ -198,9 +198,8 @@ class DisplayListBuilder final : public virtual Dispatcher, void transform(const SkMatrix& matrix) { transform(&matrix); } void transform(const SkM44& matrix44) { transform(&matrix44); } - SkM44 getCurrentTransformFullPerspective() { return current_layer_->matrix; } - - SkMatrix getCurrentTransform() { return current_layer_->matrix.asM33(); } + SkM44 getTransformFullPerspective() { return current_layer_->matrix; } + SkMatrix getTransform() { return current_layer_->matrix.asM33(); } void clipRect(const SkRect& rect, SkClipOp clip_op, bool is_aa) override; void clipRRect(const SkRRect& rrect, SkClipOp clip_op, bool is_aa) override; diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index 773ef26abf126..e88de2e228004 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -1962,14 +1962,14 @@ TEST(DisplayList, TranslateAffectsCurrentTransform) { builder.translate(12.3, 14.5); SkMatrix matrix = SkMatrix::Translate(12.3, 14.5); SkM44 m44 = SkM44(matrix); - SkM44 curM44 = builder.getCurrentTransformFullPerspective(); - SkMatrix curMatrix = builder.getCurrentTransform(); + SkM44 curM44 = builder.getTransformFullPerspective(); + SkMatrix curMatrix = builder.getTransform(); ASSERT_EQ(curM44, m44); ASSERT_EQ(curMatrix, matrix); builder.translate(10, 10); // CurrentTransform has changed - ASSERT_NE(builder.getCurrentTransformFullPerspective(), m44); - ASSERT_NE(builder.getCurrentTransform(), curMatrix); + ASSERT_NE(builder.getTransformFullPerspective(), m44); + ASSERT_NE(builder.getTransform(), curMatrix); // Previous return values have not ASSERT_EQ(curM44, m44); ASSERT_EQ(curMatrix, matrix); @@ -1980,14 +1980,14 @@ TEST(DisplayList, ScaleAffectsCurrentTransform) { builder.scale(12.3, 14.5); SkMatrix matrix = SkMatrix::Scale(12.3, 14.5); SkM44 m44 = SkM44(matrix); - SkM44 curM44 = builder.getCurrentTransformFullPerspective(); - SkMatrix curMatrix = builder.getCurrentTransform(); + SkM44 curM44 = builder.getTransformFullPerspective(); + SkMatrix curMatrix = builder.getTransform(); ASSERT_EQ(curM44, m44); ASSERT_EQ(curMatrix, matrix); builder.translate(10, 10); // CurrentTransform has changed - ASSERT_NE(builder.getCurrentTransformFullPerspective(), m44); - ASSERT_NE(builder.getCurrentTransform(), curMatrix); + ASSERT_NE(builder.getTransformFullPerspective(), m44); + ASSERT_NE(builder.getTransform(), curMatrix); // Previous return values have not ASSERT_EQ(curM44, m44); ASSERT_EQ(curMatrix, matrix); @@ -1998,14 +1998,14 @@ TEST(DisplayList, RotateAffectsCurrentTransform) { builder.rotate(12.3); SkMatrix matrix = SkMatrix::RotateDeg(12.3); SkM44 m44 = SkM44(matrix); - SkM44 curM44 = builder.getCurrentTransformFullPerspective(); - SkMatrix curMatrix = builder.getCurrentTransform(); + SkM44 curM44 = builder.getTransformFullPerspective(); + SkMatrix curMatrix = builder.getTransform(); ASSERT_EQ(curM44, m44); ASSERT_EQ(curMatrix, matrix); builder.translate(10, 10); // CurrentTransform has changed - ASSERT_NE(builder.getCurrentTransformFullPerspective(), m44); - ASSERT_NE(builder.getCurrentTransform(), curMatrix); + ASSERT_NE(builder.getTransformFullPerspective(), m44); + ASSERT_NE(builder.getTransform(), curMatrix); // Previous return values have not ASSERT_EQ(curM44, m44); ASSERT_EQ(curMatrix, matrix); @@ -2016,14 +2016,14 @@ TEST(DisplayList, SkewAffectsCurrentTransform) { builder.skew(12.3, 14.5); SkMatrix matrix = SkMatrix::Skew(12.3, 14.5); SkM44 m44 = SkM44(matrix); - SkM44 curM44 = builder.getCurrentTransformFullPerspective(); - SkMatrix curMatrix = builder.getCurrentTransform(); + SkM44 curM44 = builder.getTransformFullPerspective(); + SkMatrix curMatrix = builder.getTransform(); ASSERT_EQ(curM44, m44); ASSERT_EQ(curMatrix, matrix); builder.translate(10, 10); // CurrentTransform has changed - ASSERT_NE(builder.getCurrentTransformFullPerspective(), m44); - ASSERT_NE(builder.getCurrentTransform(), curMatrix); + ASSERT_NE(builder.getTransformFullPerspective(), m44); + ASSERT_NE(builder.getTransform(), curMatrix); // Previous return values have not ASSERT_EQ(curM44, m44); ASSERT_EQ(curMatrix, matrix); @@ -2037,14 +2037,14 @@ TEST(DisplayList, TransformAffectsCurrentTransform) { 1, 5, 14.5, // 0, 0, 1); SkM44 m44 = SkM44(matrix); - SkM44 curM44 = builder.getCurrentTransformFullPerspective(); - SkMatrix curMatrix = builder.getCurrentTransform(); + SkM44 curM44 = builder.getTransformFullPerspective(); + SkMatrix curMatrix = builder.getTransform(); ASSERT_EQ(curM44, m44); ASSERT_EQ(curMatrix, matrix); builder.translate(10, 10); // CurrentTransform has changed - ASSERT_NE(builder.getCurrentTransformFullPerspective(), m44); - ASSERT_NE(builder.getCurrentTransform(), curMatrix); + ASSERT_NE(builder.getTransformFullPerspective(), m44); + ASSERT_NE(builder.getTransform(), curMatrix); // Previous return values have not ASSERT_EQ(curM44, m44); ASSERT_EQ(curMatrix, matrix); @@ -2063,14 +2063,14 @@ TEST(DisplayList, FullTransformAffectsCurrentTransform) { 1, 5, 3, 14.5, // 0, 0, 7, 16.2, // 0, 0, 0, 1); - SkM44 curM44 = builder.getCurrentTransformFullPerspective(); - SkMatrix curMatrix = builder.getCurrentTransform(); + SkM44 curM44 = builder.getTransformFullPerspective(); + SkMatrix curMatrix = builder.getTransform(); ASSERT_EQ(curM44, m44); ASSERT_EQ(curMatrix, matrix); builder.translate(10, 10); // CurrentTransform has changed - ASSERT_NE(builder.getCurrentTransformFullPerspective(), m44); - ASSERT_NE(builder.getCurrentTransform(), curMatrix); + ASSERT_NE(builder.getTransformFullPerspective(), m44); + ASSERT_NE(builder.getTransform(), curMatrix); // Previous return values have not ASSERT_EQ(curM44, m44); ASSERT_EQ(curMatrix, matrix); diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 4e3d9b84e0edc..c00d391883b63 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -4449,12 +4449,20 @@ class Canvas extends NativeFieldWrapperClass1 { } void _transform(Float64List matrix4) native 'Canvas_transform'; - Float64List getCurrentTransform() { + /// Returns the current transform including the combined result of all transform + /// methods executed since the creation of this [Canvas] object, and respecting the + /// save/restore history. + /// + /// Methods that can change the current transform include [translate], [scale], + /// [rotate], [skew], and [transform]. The [restore] method can also modify + /// the current transform by restoring it to the same value it had before its + /// associated [save] or [saveLayer] call. + Float64List getTransform() { final Float64List matrix4 = Float64List(16); - _getCurrentTransform(matrix4); + _getTransform(matrix4); return matrix4; } - void _getCurrentTransform(Float64List matrix4) native 'Canvas_getCurrentTransform'; + void _getTransform(Float64List matrix4) native 'Canvas_getTransform'; /// Reduces the clip region to the intersection of the current clip and the /// given rectangle. @@ -4510,12 +4518,48 @@ class Canvas extends NativeFieldWrapperClass1 { } void _clipPath(Path path, bool doAntiAlias) native 'Canvas_clipPath'; - Rect getCurrentClipBounds() { + /// Returns the bounds of the current clip including a conservative combined + /// result of all clip methods executed since the creation of this [Canvas] + /// object, and respecting the save/restore history. + /// + /// The returned bounds are a conservative estimate of the bounds of the actual + /// clip based on intersecting the bounds of each clip method that was executed + /// with [ClipOp.intersect] and potentially ignoring any clip method that was + /// executed with [ClipOp.difference]. These [ClipOp] arguments are only present + /// on the [clipRect] method. + /// + /// To understand how the bounds estimate can be conservative, consider the + /// following two clip method calls: + /// + /// ```dart + /// clipPath(Path() + /// ..addRect(const Rect.fromLTRB(10, 10, 20, 20)) + /// ..addRect(const Rect.fromLTRB(80, 80, 100, 100))); + /// clipPath(Path() + /// ..addRect(const Rect.fromLTRB(80, 10, 100, 20)) + /// ..addRect(const Rect.fromLTRB(10, 80, 20, 100))); + /// ``` + /// + /// After executing both of those calls there is no area left in which to draw + /// because the two paths have no overlapping regions. But, in this case, + /// [getClipBounds] would return a rectangle from `10, 10` to `100, 100` because it + /// only intersects the bounds of the two path objects to obtain its conservative + /// estimate. + /// + /// The clip bounds are not affected by the bounds of any enclosing + /// [saveLayer] call as the engine does not currently guarantee the strict + /// enforcement of those bounds during rendering. + /// + /// Methods that can change the current clip include [clipRect], [clipRRect], + /// and [clipPath]. The [restore] method can also modify the current clip by + /// restoring it to the same value it had before its associated [save] or + /// [saveLayer] call. + Rect getClipBounds() { final Float64List bounds = Float64List(4); - _getCurrentClipBounds(bounds); + _getClipBounds(bounds); return Rect.fromLTRB(bounds[0], bounds[1], bounds[2], bounds[3]); } - void _getCurrentClipBounds(Float64List bounds) native 'Canvas_getCurrentClipBounds'; + void _getClipBounds(Float64List bounds) native 'Canvas_getClipBounds'; /// Paints the given [Color] onto the canvas, applying the given /// [BlendMode], with the given color being the source and the background diff --git a/lib/ui/painting/canvas.cc b/lib/ui/painting/canvas.cc index eea08dc6c1c91..c5b2a0f58a69c 100644 --- a/lib/ui/painting/canvas.cc +++ b/lib/ui/painting/canvas.cc @@ -46,11 +46,11 @@ IMPLEMENT_WRAPPERTYPEINFO(ui, Canvas); V(Canvas, rotate) \ V(Canvas, skew) \ V(Canvas, transform) \ - V(Canvas, getCurrentTransform) \ + V(Canvas, getTransform) \ V(Canvas, clipRect) \ V(Canvas, clipRRect) \ V(Canvas, clipPath) \ - V(Canvas, getCurrentClipBounds) \ + V(Canvas, getClipBounds) \ V(Canvas, drawColor) \ V(Canvas, drawLine) \ V(Canvas, drawPaint) \ @@ -221,10 +221,10 @@ void Canvas::transform(const tonic::Float64List& matrix4) { } } -void Canvas::getCurrentTransform(tonic::Float64List& matrix4) { +void Canvas::getTransform(tonic::Float64List& matrix4) { SkM44 sk_m44 = display_list_recorder_ ? display_list_recorder_->builder() - ->getCurrentTransformFullPerspective() + ->getTransformFullPerspective() : canvas_->getLocalToDevice(); SkScalar m44_values[16]; // The Float array stored by Dart Matrix4 is in column-major order @@ -270,7 +270,7 @@ void Canvas::clipPath(const CanvasPath* path, bool doAntiAlias) { } } -void Canvas::getCurrentClipBounds(tonic::Float64List& rect) { +void Canvas::getClipBounds(tonic::Float64List& rect) { SkRect bounds = display_list_recorder_ ? display_list_recorder_->builder()->getClipBounds() : canvas_->getLocalClipBounds(); diff --git a/lib/ui/painting/canvas.h b/lib/ui/painting/canvas.h index adfc1a35410c8..b4af4054207a9 100644 --- a/lib/ui/painting/canvas.h +++ b/lib/ui/painting/canvas.h @@ -53,7 +53,7 @@ class Canvas : public RefCountedDartWrappable, DisplayListOpFlags { void rotate(double radians); void skew(double sx, double sy); void transform(const tonic::Float64List& matrix4); - void getCurrentTransform(tonic::Float64List& matrix4); + void getTransform(tonic::Float64List& matrix4); void clipRect(double left, double top, @@ -63,7 +63,7 @@ class Canvas : public RefCountedDartWrappable, DisplayListOpFlags { bool doAntiAlias = true); void clipRRect(const RRect& rrect, bool doAntiAlias = true); void clipPath(const CanvasPath* path, bool doAntiAlias = true); - void getCurrentClipBounds(tonic::Float64List& rect); + void getClipBounds(tonic::Float64List& rect); void drawColor(SkColor color, DlBlendMode blend_mode); void drawLine(double x1, diff --git a/testing/dart/canvas_test.dart b/testing/dart/canvas_test.dart index e90bfe6e61bbd..cb6c324117ea2 100644 --- a/testing/dart/canvas_test.dart +++ b/testing/dart/canvas_test.dart @@ -468,67 +468,67 @@ void main() { Expect.fail('$value is too close to $expected'); }; - test('Canvas.translate affects canvas.getCurrentTransform', () async { + test('Canvas.translate affects canvas.getTransform', () async { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder); canvas.translate(12, 14.5); final Float64List matrix = Matrix4.translationValues(12, 14.5, 0).storage; - final Float64List curMatrix = canvas.getCurrentTransform(); + final Float64List curMatrix = canvas.getTransform(); expect(curMatrix, closeToTransform(matrix)); canvas.translate(10, 10); - final Float64List newCurMatrix = canvas.getCurrentTransform(); + final Float64List newCurMatrix = canvas.getTransform(); expect(newCurMatrix, notCloseToTransform(matrix)); expect(curMatrix, closeToTransform(matrix)); }); - test('Canvas.scale affects canvas.getCurrentTransform', () async { + test('Canvas.scale affects canvas.getTransform', () async { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder); canvas.scale(12, 14.5); final Float64List matrix = Matrix4.diagonal3Values(12, 14.5, 1).storage; - final Float64List curMatrix = canvas.getCurrentTransform(); + final Float64List curMatrix = canvas.getTransform(); expect(curMatrix, closeToTransform(matrix)); canvas.translate(10, 10); - final Float64List newCurMatrix = canvas.getCurrentTransform(); + final Float64List newCurMatrix = canvas.getTransform(); expect(newCurMatrix, notCloseToTransform(matrix)); expect(curMatrix, closeToTransform(matrix)); }); - test('Canvas.rotate affects canvas.getCurrentTransform', () async { + test('Canvas.rotate affects canvas.getTransform', () async { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder); canvas.rotate(pi); final Float64List matrix = Matrix4.rotationZ(pi).storage; - final Float64List curMatrix = canvas.getCurrentTransform(); + final Float64List curMatrix = canvas.getTransform(); expect(curMatrix, closeToTransform(matrix)); canvas.translate(10, 10); - final Float64List newCurMatrix = canvas.getCurrentTransform(); + final Float64List newCurMatrix = canvas.getTransform(); expect(newCurMatrix, notCloseToTransform(matrix)); expect(curMatrix, closeToTransform(matrix)); }); - test('Canvas.skew affects canvas.getCurrentTransform', () async { + test('Canvas.skew affects canvas.getTransform', () async { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder); canvas.skew(12, 14.5); final Float64List matrix = (Matrix4.identity()..setEntry(0, 1, 12)..setEntry(1, 0, 14.5)).storage; - final Float64List curMatrix = canvas.getCurrentTransform(); + final Float64List curMatrix = canvas.getTransform(); expect(curMatrix, closeToTransform(matrix)); canvas.translate(10, 10); - final Float64List newCurMatrix = canvas.getCurrentTransform(); + final Float64List newCurMatrix = canvas.getTransform(); expect(newCurMatrix, notCloseToTransform(matrix)); expect(curMatrix, closeToTransform(matrix)); }); - test('Canvas.transform affects canvas.getCurrentTransform', () async { + test('Canvas.transform affects canvas.getTransform', () async { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder); final Float64List matrix = (Matrix4.identity()..translate(12.0, 14.5)..scale(12.0, 14.5)).storage; canvas.transform(matrix); - final Float64List curMatrix = canvas.getCurrentTransform(); + final Float64List curMatrix = canvas.getTransform(); expect(curMatrix, closeToTransform(matrix)); canvas.translate(10, 10); - final Float64List newCurMatrix = canvas.getCurrentTransform(); + final Float64List newCurMatrix = canvas.getTransform(); expect(newCurMatrix, notCloseToTransform(matrix)); expect(curMatrix, closeToTransform(matrix)); }); @@ -554,57 +554,57 @@ void main() { Expect.fail('$value is too close to $expected'); }; - test('Canvas.clipRect affects canvas.getCurrentClipBounds', () async { + test('Canvas.clipRect affects canvas.getClipBounds', () async { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder); const Rect clip = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); canvas.clipRect(clip); - final Rect curBounds = canvas.getCurrentClipBounds(); + final Rect curBounds = canvas.getClipBounds(); expect(curBounds, closeToRect(clip)); canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15)); - final Rect newCurBounds = canvas.getCurrentClipBounds(); + final Rect newCurBounds = canvas.getClipBounds(); expect(newCurBounds, notCloseToRect(clip)); expect(curBounds, closeToRect(clip)); }); - test('Canvas.clipRRect affects canvas.getCurrentClipBounds', () async { + test('Canvas.clipRRect affects canvas.getClipBounds', () async { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder); const Rect clipBounds = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); final RRect clip = RRect.fromRectAndRadius(clipBounds, const Radius.circular(3)); canvas.clipRRect(clip); - final Rect curBounds = canvas.getCurrentClipBounds(); + final Rect curBounds = canvas.getClipBounds(); expect(curBounds, closeToRect(clipBounds)); canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15)); - final Rect newCurBounds = canvas.getCurrentClipBounds(); + final Rect newCurBounds = canvas.getClipBounds(); expect(newCurBounds, notCloseToRect(clipBounds)); expect(curBounds, closeToRect(clipBounds)); }); - test('Canvas.clipPath affects canvas.getCurrentClipBounds', () async { + test('Canvas.clipPath affects canvas.getClipBounds', () async { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder); const Rect clipBounds = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); final Path clip = Path()..addRect(clipBounds)..addOval(clipBounds); canvas.clipPath(clip); - final Rect curBounds = canvas.getCurrentClipBounds(); + final Rect curBounds = canvas.getClipBounds(); expect(curBounds, closeToRect(clipBounds)); canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15)); - final Rect newCurBounds = canvas.getCurrentClipBounds(); + final Rect newCurBounds = canvas.getClipBounds(); expect(newCurBounds, notCloseToRect(clipBounds)); expect(curBounds, closeToRect(clipBounds)); }); - test('Canvas.clipRect(diff) does not affect canvas.getCurrentClipBounds', () async { + test('Canvas.clipRect(diff) does not affect canvas.getClipBounds', () async { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder); const Rect clip = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); canvas.clipRect(clip); canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15), clipOp: ClipOp.difference); - final Rect curBounds = canvas.getCurrentClipBounds(); + final Rect curBounds = canvas.getClipBounds(); expect(curBounds, closeToRect(clip)); canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15)); - final Rect newCurBounds = canvas.getCurrentClipBounds(); + final Rect newCurBounds = canvas.getClipBounds(); expect(newCurBounds, notCloseToRect(clip)); expect(curBounds, closeToRect(clip)); }); From a508b6a784d416d1811c989fb6e3de1b99fa1f43 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Tue, 17 May 2022 20:48:48 -0700 Subject: [PATCH 04/11] formatting --- lib/ui/painting/canvas.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/ui/painting/canvas.cc b/lib/ui/painting/canvas.cc index c5b2a0f58a69c..dc7a459496226 100644 --- a/lib/ui/painting/canvas.cc +++ b/lib/ui/painting/canvas.cc @@ -222,10 +222,10 @@ void Canvas::transform(const tonic::Float64List& matrix4) { } void Canvas::getTransform(tonic::Float64List& matrix4) { - SkM44 sk_m44 = display_list_recorder_ - ? display_list_recorder_->builder() - ->getTransformFullPerspective() - : canvas_->getLocalToDevice(); + SkM44 sk_m44 = + display_list_recorder_ + ? display_list_recorder_->builder()->getTransformFullPerspective() + : canvas_->getLocalToDevice(); SkScalar m44_values[16]; // The Float array stored by Dart Matrix4 is in column-major order sk_m44.getColMajor(m44_values); From 52c5c1c20f2dcd9eb37bee166cd1bf86b50112c2 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Fri, 20 May 2022 14:47:18 -0700 Subject: [PATCH 05/11] distinguish local and dest/device clip bounds and enhance tests accordingly --- display_list/display_list_builder.cc | 9 ++ display_list/display_list_builder.h | 15 +- display_list/display_list_unittests.cc | 191 ++++++++++++++++++------- lib/ui/painting.dart | 58 ++++++-- lib/ui/painting/canvas.cc | 87 ++++++----- lib/ui/painting/canvas.h | 3 +- lib/web_ui/lib/canvas.dart | 3 + testing/dart/canvas_test.dart | 135 +++++++++++++---- 8 files changed, 377 insertions(+), 124 deletions(-) diff --git a/display_list/display_list_builder.cc b/display_list/display_list_builder.cc index 8027b23b2839c..c0f83e4d37927 100644 --- a/display_list/display_list_builder.cc +++ b/display_list/display_list_builder.cc @@ -653,6 +653,15 @@ void DisplayListBuilder::clipPath(const SkPath& path, Push(0, 1, path, is_aa); } } +SkRect DisplayListBuilder::getLocalClipBounds() { + SkM44 inverse; + if (current_layer_->matrix.invert(&inverse)) { + SkRect devBounds; + current_layer_->clip_bounds.roundOut(&devBounds); + return inverse.asM33().mapRect(devBounds); + } + return kMaxCullRect_; +} void DisplayListBuilder::drawPaint() { Push(0, 1); diff --git a/display_list/display_list_builder.h b/display_list/display_list_builder.h index da283e6e48feb..ddee7ce9a5073 100644 --- a/display_list/display_list_builder.h +++ b/display_list/display_list_builder.h @@ -198,14 +198,27 @@ class DisplayListBuilder final : public virtual Dispatcher, void transform(const SkMatrix& matrix) { transform(&matrix); } void transform(const SkM44& matrix44) { transform(&matrix44); } + /// Returns the 4x4 full perspective transform representing all transform + /// operations executed so far in this DisplayList within the enclosing + /// save stack. SkM44 getTransformFullPerspective() { return current_layer_->matrix; } + /// Returns the 3x3 partial perspective transform representing all transform + /// operations executed so far in this DisplayList within the enclosing + /// save stack. SkMatrix getTransform() { return current_layer_->matrix.asM33(); } void clipRect(const SkRect& rect, SkClipOp clip_op, bool is_aa) override; void clipRRect(const SkRRect& rrect, SkClipOp clip_op, bool is_aa) override; void clipPath(const SkPath& path, SkClipOp clip_op, bool is_aa) override; - SkRect getClipBounds() { return current_layer_->clip_bounds; } + /// Conservative estimate of the bounds of all outstanding clip operations + /// measured in the coordinate space within which this DisplayList will + /// be rendered. + SkRect getDestinationClipBounds() { return current_layer_->clip_bounds; } + /// Conservative estimate of the bounds of all outstanding clip operations + /// transformed into the local coordinate space in which currently + /// recorded rendering operations are interpreted. + SkRect getLocalClipBounds(); void drawPaint() override; void drawPaint(const DlPaint& paint); diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index e88de2e228004..5feef8bbc722d 100644 --- a/display_list/display_list_unittests.cc +++ b/display_list/display_list_unittests.cc @@ -2076,88 +2076,179 @@ TEST(DisplayList, FullTransformAffectsCurrentTransform) { ASSERT_EQ(curMatrix, matrix); } -TEST(DisplayList, ClipRectAffectsCurrentClipBounds) { +TEST(DisplayList, ClipRectAffectsClipBounds) { DisplayListBuilder builder; - SkRect clip = SkRect::MakeLTRB(10.2, 11.3, 20.4, 25.7); - builder.clipRect(clip, SkClipOp::kIntersect, false); - SkRect curBounds = builder.getClipBounds(); - ASSERT_EQ(curBounds, clip); + SkRect clipBounds = SkRect::MakeLTRB(10.2, 11.3, 20.4, 25.7); + SkRect clipExpandedBounds = SkRect::MakeLTRB(10, 11, 21, 26); + builder.clipRect(clipBounds, SkClipOp::kIntersect, false); + + // Save initial return values for testing restored values + SkRect initialLocalBounds = builder.getLocalClipBounds(); + SkRect initialDestinationBounds = builder.getDestinationClipBounds(); + ASSERT_EQ(initialLocalBounds, clipExpandedBounds); + ASSERT_EQ(initialDestinationBounds, clipBounds); + + builder.save(); builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, false); - // CurrentTransform has changed - ASSERT_NE(builder.getClipBounds(), clip); - // Previous return values have not - ASSERT_EQ(curBounds, clip); + // Both clip bounds have changed + ASSERT_NE(builder.getLocalClipBounds(), clipExpandedBounds); + ASSERT_NE(builder.getDestinationClipBounds(), clipBounds); + // Previous return values have not changed + ASSERT_EQ(initialLocalBounds, clipExpandedBounds); + ASSERT_EQ(initialDestinationBounds, clipBounds); + builder.restore(); + + // save/restore returned the values to their original values + ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds); + ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds); + + builder.save(); + builder.scale(2, 2); + SkRect scaledExpandedBounds = SkRect::MakeLTRB(5, 5.5, 10.5, 13); + ASSERT_EQ(builder.getLocalClipBounds(), scaledExpandedBounds); + // Destination bounds are unaffected by transform + ASSERT_EQ(builder.getDestinationClipBounds(), clipBounds); + builder.restore(); + + // save/restore returned the values to their original values + ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds); + ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds); } -TEST(DisplayList, ClipRRectAffectsCurrentClipBounds) { +TEST(DisplayList, ClipRRectAffectsClipBounds) { DisplayListBuilder builder; - SkRRect clip = SkRRect::MakeRectXY({10.2, 11.3, 20.4, 25.7}, 3, 2); + SkRect clipBounds = SkRect::MakeLTRB(10.2, 11.3, 20.4, 25.7); + SkRect clipExpandedBounds = SkRect::MakeLTRB(10, 11, 21, 26); + SkRRect clip = SkRRect::MakeRectXY(clipBounds, 3, 2); builder.clipRRect(clip, SkClipOp::kIntersect, false); - SkRect curBounds = builder.getClipBounds(); - ASSERT_EQ(curBounds, clip.getBounds()); + + // Save initial return values for testing restored values + SkRect initialLocalBounds = builder.getLocalClipBounds(); + SkRect initialDestinationBounds = builder.getDestinationClipBounds(); + ASSERT_EQ(initialLocalBounds, clipExpandedBounds); + ASSERT_EQ(initialDestinationBounds, clipBounds); + + builder.save(); builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, false); - // CurrentTransform has changed - ASSERT_NE(builder.getClipBounds(), clip.getBounds()); - // Previous return values have not - ASSERT_EQ(curBounds, clip.getBounds()); + // Both clip bounds have changed + ASSERT_NE(builder.getLocalClipBounds(), clipExpandedBounds); + ASSERT_NE(builder.getDestinationClipBounds(), clipBounds); + // Previous return values have not changed + ASSERT_EQ(initialLocalBounds, clipExpandedBounds); + ASSERT_EQ(initialDestinationBounds, clipBounds); + builder.restore(); + + // save/restore returned the values to their original values + ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds); + ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds); + + builder.save(); + builder.scale(2, 2); + SkRect scaledExpandedBounds = SkRect::MakeLTRB(5, 5.5, 10.5, 13); + ASSERT_EQ(builder.getLocalClipBounds(), scaledExpandedBounds); + // Destination bounds are unaffected by transform + ASSERT_EQ(builder.getDestinationClipBounds(), clipBounds); + builder.restore(); + + // save/restore returned the values to their original values + ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds); + ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds); } -TEST(DisplayList, ClipPathAffectsCurrentClipBounds) { +TEST(DisplayList, ClipPathAffectsClipBounds) { DisplayListBuilder builder; SkPath clip = SkPath().addCircle(10.2, 11.3, 2).addCircle(20.4, 25.7, 2); + SkRect clipBounds = SkRect::MakeLTRB(8.2, 9.3, 22.4, 27.7); + SkRect clipExpandedBounds = SkRect::MakeLTRB(8, 9, 23, 28); builder.clipPath(clip, SkClipOp::kIntersect, false); - SkRect curBounds = builder.getClipBounds(); - ASSERT_EQ(curBounds, clip.getBounds()); + + // Save initial return values for testing restored values + SkRect initialLocalBounds = builder.getLocalClipBounds(); + SkRect initialDestinationBounds = builder.getDestinationClipBounds(); + ASSERT_EQ(initialLocalBounds, clipExpandedBounds); + ASSERT_EQ(initialDestinationBounds, clipBounds); + + builder.save(); builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, false); - // CurrentTransform has changed - ASSERT_NE(builder.getClipBounds(), clip.getBounds()); - // Previous return values have not - ASSERT_EQ(curBounds, clip.getBounds()); + // Both clip bounds have changed + ASSERT_NE(builder.getLocalClipBounds(), clipExpandedBounds); + ASSERT_NE(builder.getDestinationClipBounds(), clipBounds); + // Previous return values have not changed + ASSERT_EQ(initialLocalBounds, clipExpandedBounds); + ASSERT_EQ(initialDestinationBounds, clipBounds); + builder.restore(); + + // save/restore returned the values to their original values + ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds); + ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds); + + builder.save(); + builder.scale(2, 2); + SkRect scaledExpandedBounds = SkRect::MakeLTRB(4, 4.5, 11.5, 14); + ASSERT_EQ(builder.getLocalClipBounds(), scaledExpandedBounds); + // Destination bounds are unaffected by transform + ASSERT_EQ(builder.getDestinationClipBounds(), clipBounds); + builder.restore(); + + // save/restore returned the values to their original values + ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds); + ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds); } -TEST(DisplayList, DiffClipRectDoesNotAffectCurrentClipBounds) { +TEST(DisplayList, DiffClipRectDoesNotAffectClipBounds) { DisplayListBuilder builder; SkRect diff_clip = SkRect::MakeLTRB(0, 0, 15, 15); - SkRect clip = SkRect::MakeLTRB(10.2, 11.3, 20.4, 25.7); - builder.clipRect(clip, SkClipOp::kIntersect, false); + SkRect clipBounds = SkRect::MakeLTRB(10.2, 11.3, 20.4, 25.7); + SkRect clipExpandedBounds = SkRect::MakeLTRB(10, 11, 21, 26); + builder.clipRect(clipBounds, SkClipOp::kIntersect, false); + + // Save initial return values for testing after kDifference clip + SkRect initialLocalBounds = builder.getLocalClipBounds(); + SkRect initialDestinationBounds = builder.getDestinationClipBounds(); + ASSERT_EQ(initialLocalBounds, clipExpandedBounds); + ASSERT_EQ(initialDestinationBounds, clipBounds); + builder.clipRect(diff_clip, SkClipOp::kDifference, false); - SkRect curBounds = builder.getClipBounds(); - ASSERT_EQ(curBounds, clip); - builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, false); - // CurrentTransform has changed - ASSERT_NE(builder.getClipBounds(), clip); - // Previous return values have not - ASSERT_EQ(curBounds, clip); + ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds); + ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds); } -TEST(DisplayList, DiffClipRRectAffectsCurrentClipBounds) { +TEST(DisplayList, DiffClipRRectDoesNotAffectClipBounds) { DisplayListBuilder builder; SkRRect diff_clip = SkRRect::MakeRectXY({0, 0, 15, 15}, 1, 1); + SkRect clipBounds = SkRect::MakeLTRB(10.2, 11.3, 20.4, 25.7); + SkRect clipExpandedBounds = SkRect::MakeLTRB(10, 11, 21, 26); SkRRect clip = SkRRect::MakeRectXY({10.2, 11.3, 20.4, 25.7}, 3, 2); builder.clipRRect(clip, SkClipOp::kIntersect, false); + + // Save initial return values for testing after kDifference clip + SkRect initialLocalBounds = builder.getLocalClipBounds(); + SkRect initialDestinationBounds = builder.getDestinationClipBounds(); + ASSERT_EQ(initialLocalBounds, clipExpandedBounds); + ASSERT_EQ(initialDestinationBounds, clipBounds); + builder.clipRRect(diff_clip, SkClipOp::kDifference, false); - SkRect curBounds = builder.getClipBounds(); - ASSERT_EQ(curBounds, clip.getBounds()); - builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, false); - // CurrentTransform has changed - ASSERT_NE(builder.getClipBounds(), clip.getBounds()); - // Previous return values have not - ASSERT_EQ(curBounds, clip.getBounds()); + ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds); + ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds); } -TEST(DisplayList, DiffClipPathAffectsCurrentClipBounds) { +TEST(DisplayList, DiffClipPathDoesNotAffectClipBounds) { DisplayListBuilder builder; SkPath diff_clip = SkPath().addRect({0, 0, 15, 15}); SkPath clip = SkPath().addCircle(10.2, 11.3, 2).addCircle(20.4, 25.7, 2); + SkRect clipBounds = SkRect::MakeLTRB(8.2, 9.3, 22.4, 27.7); + SkRect clipExpandedBounds = SkRect::MakeLTRB(8, 9, 23, 28); builder.clipPath(clip, SkClipOp::kIntersect, false); + + // Save initial return values for testing after kDifference clip + SkRect initialLocalBounds = builder.getLocalClipBounds(); + SkRect initialDestinationBounds = builder.getDestinationClipBounds(); + ASSERT_EQ(initialLocalBounds, clipExpandedBounds); + ASSERT_EQ(initialDestinationBounds, clipBounds); + builder.clipPath(diff_clip, SkClipOp::kDifference, false); - SkRect curBounds = builder.getClipBounds(); - ASSERT_EQ(curBounds, clip.getBounds()); - builder.clipRect({0, 0, 15, 15}, SkClipOp::kIntersect, false); - // CurrentTransform has changed - ASSERT_NE(builder.getClipBounds(), clip.getBounds()); - // Previous return values have not - ASSERT_EQ(curBounds, clip.getBounds()); + ASSERT_EQ(builder.getLocalClipBounds(), initialLocalBounds); + ASSERT_EQ(builder.getDestinationClipBounds(), initialDestinationBounds); } } // namespace testing diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index c00d391883b63..f5ed75057e359 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -4518,15 +4518,27 @@ class Canvas extends NativeFieldWrapperClass1 { } void _clipPath(Path path, bool doAntiAlias) native 'Canvas_clipPath'; - /// Returns the bounds of the current clip including a conservative combined - /// result of all clip methods executed since the creation of this [Canvas] - /// object, and respecting the save/restore history. - /// - /// The returned bounds are a conservative estimate of the bounds of the actual - /// clip based on intersecting the bounds of each clip method that was executed - /// with [ClipOp.intersect] and potentially ignoring any clip method that was - /// executed with [ClipOp.difference]. These [ClipOp] arguments are only present - /// on the [clipRect] method. + /// Returns the conservative bounds of the combined result of all clip methods + /// executed within the current save stack of this [Canvas] object, as measured + /// in the local coordinate space under which rendering operations are curretnly + /// performed. + /// + /// The combined clip results are rounded out to an integer pixel boundary before + /// they are transformed back into the local coordinate space which accounts for + /// the pixel roundoff in rendering operations, particularly when antialiasing. + /// Because the [Picture] may eventually be rendered into a scene within the + /// context of transforming widgets or layers, the result may thus be overly + /// conservative due to premature rounding. Using the [getDestinationClipBounds] + /// method combined with the external transforms and rounding in the true device + /// coordinate system will produce more accurate results, but this value may + /// provide a more convenient approximation to compare rendering operations to + /// the established clip. + /// + /// {@template dart.ui.canvas.conservativeClipBounds} + /// The conservative estimate of the bounds is based on intersecting the bounds + /// of each clip method that was executed with [ClipOp.intersect] and potentially + /// ignoring any clip method that was executed with [ClipOp.difference]. The + /// [ClipOp] argument is only present on the [clipRect] method. /// /// To understand how the bounds estimate can be conservative, consider the /// following two clip method calls: @@ -4554,12 +4566,34 @@ class Canvas extends NativeFieldWrapperClass1 { /// and [clipPath]. The [restore] method can also modify the current clip by /// restoring it to the same value it had before its associated [save] or /// [saveLayer] call. - Rect getClipBounds() { + /// {@endtemplate} + Rect getLocalClipBounds() { + final Float64List bounds = Float64List(4); + _getLocalClipBounds(bounds); + return Rect.fromLTRB(bounds[0], bounds[1], bounds[2], bounds[3]); + } + void _getLocalClipBounds(Float64List bounds) native 'Canvas_getLocalClipBounds'; + + /// Returns the conservative bounds of the combined result of all clip methods + /// executed within the current save stack of this [Canvas] object, as measured + /// in the destination coordinate space in which the [Picture] will be rendered. + /// + /// Unlike [getLocalClipBounds], the bounds are not rounded out to an integer + /// pixel boundary as the Destination coordinate space may not represent pixels + /// if the [Picture] being constructed will be further transformed when it is + /// rendered or added to a scene. In order to determine the true pixels being + /// affected, those external transforms should be applied first before rounding + /// out the result to integer pixel boundaries. Most typically, [Picture] objects + /// are rendered in a scene with a scale transform representing the Device Pixel + /// Ratio. + /// + /// {@macro dart.ui.canvas.conservativeClipBounds} + Rect getDestinationClipBounds() { final Float64List bounds = Float64List(4); - _getClipBounds(bounds); + _getDestinationClipBounds(bounds); return Rect.fromLTRB(bounds[0], bounds[1], bounds[2], bounds[3]); } - void _getClipBounds(Float64List bounds) native 'Canvas_getClipBounds'; + void _getDestinationClipBounds(Float64List bounds) native 'Canvas_getDestinationClipBounds'; /// Paints the given [Color] onto the canvas, applying the given /// [BlendMode], with the given color being the source and the background diff --git a/lib/ui/painting/canvas.cc b/lib/ui/painting/canvas.cc index dc7a459496226..edd7fbc05198b 100644 --- a/lib/ui/painting/canvas.cc +++ b/lib/ui/painting/canvas.cc @@ -35,39 +35,40 @@ static void Canvas_constructor(Dart_NativeArguments args) { IMPLEMENT_WRAPPERTYPEINFO(ui, Canvas); -#define FOR_EACH_BINDING(V) \ - V(Canvas, save) \ - V(Canvas, saveLayerWithoutBounds) \ - V(Canvas, saveLayer) \ - V(Canvas, restore) \ - V(Canvas, getSaveCount) \ - V(Canvas, translate) \ - V(Canvas, scale) \ - V(Canvas, rotate) \ - V(Canvas, skew) \ - V(Canvas, transform) \ - V(Canvas, getTransform) \ - V(Canvas, clipRect) \ - V(Canvas, clipRRect) \ - V(Canvas, clipPath) \ - V(Canvas, getClipBounds) \ - V(Canvas, drawColor) \ - V(Canvas, drawLine) \ - V(Canvas, drawPaint) \ - V(Canvas, drawRect) \ - V(Canvas, drawRRect) \ - V(Canvas, drawDRRect) \ - V(Canvas, drawOval) \ - V(Canvas, drawCircle) \ - V(Canvas, drawArc) \ - V(Canvas, drawPath) \ - V(Canvas, drawImage) \ - V(Canvas, drawImageRect) \ - V(Canvas, drawImageNine) \ - V(Canvas, drawPicture) \ - V(Canvas, drawPoints) \ - V(Canvas, drawVertices) \ - V(Canvas, drawAtlas) \ +#define FOR_EACH_BINDING(V) \ + V(Canvas, save) \ + V(Canvas, saveLayerWithoutBounds) \ + V(Canvas, saveLayer) \ + V(Canvas, restore) \ + V(Canvas, getSaveCount) \ + V(Canvas, translate) \ + V(Canvas, scale) \ + V(Canvas, rotate) \ + V(Canvas, skew) \ + V(Canvas, transform) \ + V(Canvas, getTransform) \ + V(Canvas, clipRect) \ + V(Canvas, clipRRect) \ + V(Canvas, clipPath) \ + V(Canvas, getLocalClipBounds) \ + V(Canvas, getDestinationClipBounds) \ + V(Canvas, drawColor) \ + V(Canvas, drawLine) \ + V(Canvas, drawPaint) \ + V(Canvas, drawRect) \ + V(Canvas, drawRRect) \ + V(Canvas, drawDRRect) \ + V(Canvas, drawOval) \ + V(Canvas, drawCircle) \ + V(Canvas, drawArc) \ + V(Canvas, drawPath) \ + V(Canvas, drawImage) \ + V(Canvas, drawImageRect) \ + V(Canvas, drawImageNine) \ + V(Canvas, drawPicture) \ + V(Canvas, drawPoints) \ + V(Canvas, drawVertices) \ + V(Canvas, drawAtlas) \ V(Canvas, drawShadow) FOR_EACH_BINDING(DART_NATIVE_CALLBACK) @@ -270,9 +271,25 @@ void Canvas::clipPath(const CanvasPath* path, bool doAntiAlias) { } } -void Canvas::getClipBounds(tonic::Float64List& rect) { +void Canvas::getDestinationClipBounds(tonic::Float64List& rect) { + if (display_list_recorder_) { + SkRect bounds = builder()->getDestinationClipBounds(); + rect[0] = bounds.fLeft; + rect[1] = bounds.fTop; + rect[2] = bounds.fRight; + rect[3] = bounds.fBottom; + } else { + SkIRect bounds = canvas_->getDeviceClipBounds(); + rect[0] = bounds.fLeft; + rect[1] = bounds.fTop; + rect[2] = bounds.fRight; + rect[3] = bounds.fBottom; + } +} + +void Canvas::getLocalClipBounds(tonic::Float64List& rect) { SkRect bounds = display_list_recorder_ - ? display_list_recorder_->builder()->getClipBounds() + ? display_list_recorder_->builder()->getLocalClipBounds() : canvas_->getLocalClipBounds(); rect[0] = bounds.fLeft; rect[1] = bounds.fTop; diff --git a/lib/ui/painting/canvas.h b/lib/ui/painting/canvas.h index b4af4054207a9..ff5b39c3c0c29 100644 --- a/lib/ui/painting/canvas.h +++ b/lib/ui/painting/canvas.h @@ -63,7 +63,8 @@ class Canvas : public RefCountedDartWrappable, DisplayListOpFlags { bool doAntiAlias = true); void clipRRect(const RRect& rrect, bool doAntiAlias = true); void clipPath(const CanvasPath* path, bool doAntiAlias = true); - void getClipBounds(tonic::Float64List& rect); + void getDestinationClipBounds(tonic::Float64List& rect); + void getLocalClipBounds(tonic::Float64List& rect); void drawColor(SkColor color, DlBlendMode blend_mode); void drawLine(double x1, diff --git a/lib/web_ui/lib/canvas.dart b/lib/web_ui/lib/canvas.dart index a99877a2b1787..2b0de7976e76a 100644 --- a/lib/web_ui/lib/canvas.dart +++ b/lib/web_ui/lib/canvas.dart @@ -100,10 +100,13 @@ abstract class Canvas { void rotate(double radians); void skew(double sx, double sy); void transform(Float64List matrix4); + Float64List getTransform(); void clipRect(Rect rect, {ClipOp clipOp = ClipOp.intersect, bool doAntiAlias = true}); void clipRRect(RRect rrect, {bool doAntiAlias = true}); void clipPath(Path path, {bool doAntiAlias = true}); + Rect getLocalClipBounds(); + Rect getDestinationClipBounds(); void drawColor(Color color, BlendMode blendMode); void drawLine(Offset p1, Offset p2, Paint paint); void drawPaint(Paint paint); diff --git a/testing/dart/canvas_test.dart b/testing/dart/canvas_test.dart index cb6c324117ea2..90bb56121403a 100644 --- a/testing/dart/canvas_test.dart +++ b/testing/dart/canvas_test.dart @@ -557,55 +557,140 @@ void main() { test('Canvas.clipRect affects canvas.getClipBounds', () async { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder); - const Rect clip = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); - canvas.clipRect(clip); - final Rect curBounds = canvas.getClipBounds(); - expect(curBounds, closeToRect(clip)); + const Rect clipBounds = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); + const Rect clipExpandedBounds = Rect.fromLTRB(10, 11, 21, 26); + canvas.clipRect(clipBounds); + + // Save initial return values for testing restored values + final Rect initialLocalBounds = canvas.getLocalClipBounds(); + final Rect initialDestinationBounds = canvas.getDestinationClipBounds(); + expect(initialLocalBounds, closeToRect(clipExpandedBounds)); + expect(initialDestinationBounds, closeToRect(clipBounds)); + + canvas.save(); canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15)); - final Rect newCurBounds = canvas.getClipBounds(); - expect(newCurBounds, notCloseToRect(clip)); - expect(curBounds, closeToRect(clip)); + // Both clip bounds have changed + expect(canvas.getLocalClipBounds(), notCloseToRect(clipExpandedBounds)); + expect(canvas.getDestinationClipBounds(), notCloseToRect(clipBounds)); + // Previous return values have not changed + expect(initialLocalBounds, closeToRect(clipExpandedBounds)); + expect(initialDestinationBounds, closeToRect(clipBounds)); + canvas.restore(); + + // save/restore returned the values to their original values + expect(canvas.getLocalClipBounds(), initialLocalBounds); + expect(canvas.getDestinationClipBounds(), initialDestinationBounds); + + canvas.save(); + canvas.scale(2, 2); + const Rect scaledExpandedBounds = Rect.fromLTRB(5, 5.5, 10.5, 13); + expect(canvas.getLocalClipBounds(), closeToRect(scaledExpandedBounds)); + // Destination bounds are unaffected by transform + expect(canvas.getDestinationClipBounds(), closeToRect(clipBounds)); + canvas.restore(); + + // save/restore returned the values to their original values + expect(canvas.getLocalClipBounds(), initialLocalBounds); + expect(canvas.getDestinationClipBounds(), initialDestinationBounds); }); test('Canvas.clipRRect affects canvas.getClipBounds', () async { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder); const Rect clipBounds = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); + const Rect clipExpandedBounds = Rect.fromLTRB(10, 11, 21, 26); final RRect clip = RRect.fromRectAndRadius(clipBounds, const Radius.circular(3)); canvas.clipRRect(clip); - final Rect curBounds = canvas.getClipBounds(); - expect(curBounds, closeToRect(clipBounds)); + + // Save initial return values for testing restored values + final Rect initialLocalBounds = canvas.getLocalClipBounds(); + final Rect initialDestinationBounds = canvas.getDestinationClipBounds(); + expect(initialLocalBounds, closeToRect(clipExpandedBounds)); + expect(initialDestinationBounds, closeToRect(clipBounds)); + + canvas.save(); canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15)); - final Rect newCurBounds = canvas.getClipBounds(); - expect(newCurBounds, notCloseToRect(clipBounds)); - expect(curBounds, closeToRect(clipBounds)); + // Both clip bounds have changed + expect(canvas.getLocalClipBounds(), notCloseToRect(clipExpandedBounds)); + expect(canvas.getDestinationClipBounds(), notCloseToRect(clipBounds)); + // Previous return values have not changed + expect(initialLocalBounds, closeToRect(clipExpandedBounds)); + expect(initialDestinationBounds, closeToRect(clipBounds)); + canvas.restore(); + + // save/restore returned the values to their original values + expect(canvas.getLocalClipBounds(), initialLocalBounds); + expect(canvas.getDestinationClipBounds(), initialDestinationBounds); + + canvas.save(); + canvas.scale(2, 2); + const Rect scaledExpandedBounds = Rect.fromLTRB(5, 5.5, 10.5, 13); + expect(canvas.getLocalClipBounds(), closeToRect(scaledExpandedBounds)); + // Destination bounds are unaffected by transform + expect(canvas.getDestinationClipBounds(), closeToRect(clipBounds)); + canvas.restore(); + + // save/restore returned the values to their original values + expect(canvas.getLocalClipBounds(), initialLocalBounds); + expect(canvas.getDestinationClipBounds(), initialDestinationBounds); }); test('Canvas.clipPath affects canvas.getClipBounds', () async { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder); const Rect clipBounds = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); + const Rect clipExpandedBounds = Rect.fromLTRB(10, 11, 21, 26); final Path clip = Path()..addRect(clipBounds)..addOval(clipBounds); canvas.clipPath(clip); - final Rect curBounds = canvas.getClipBounds(); - expect(curBounds, closeToRect(clipBounds)); + + // Save initial return values for testing restored values + final Rect initialLocalBounds = canvas.getLocalClipBounds(); + final Rect initialDestinationBounds = canvas.getDestinationClipBounds(); + expect(initialLocalBounds, closeToRect(clipExpandedBounds)); + expect(initialDestinationBounds, closeToRect(clipBounds)); + + canvas.save(); canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15)); - final Rect newCurBounds = canvas.getClipBounds(); - expect(newCurBounds, notCloseToRect(clipBounds)); - expect(curBounds, closeToRect(clipBounds)); + // Both clip bounds have changed + expect(canvas.getLocalClipBounds(), notCloseToRect(clipExpandedBounds)); + expect(canvas.getDestinationClipBounds(), notCloseToRect(clipBounds)); + // Previous return values have not changed + expect(initialLocalBounds, closeToRect(clipExpandedBounds)); + expect(initialDestinationBounds, closeToRect(clipBounds)); + canvas.restore(); + + // save/restore returned the values to their original values + expect(canvas.getLocalClipBounds(), initialLocalBounds); + expect(canvas.getDestinationClipBounds(), initialDestinationBounds); + + canvas.save(); + canvas.scale(2, 2); + const Rect scaledExpandedBounds = Rect.fromLTRB(5, 5.5, 10.5, 13); + expect(canvas.getLocalClipBounds(), closeToRect(scaledExpandedBounds)); + // Destination bounds are unaffected by transform + expect(canvas.getDestinationClipBounds(), closeToRect(clipBounds)); + canvas.restore(); + + // save/restore returned the values to their original values + expect(canvas.getLocalClipBounds(), initialLocalBounds); + expect(canvas.getDestinationClipBounds(), initialDestinationBounds); }); test('Canvas.clipRect(diff) does not affect canvas.getClipBounds', () async { final PictureRecorder recorder = PictureRecorder(); final Canvas canvas = Canvas(recorder); - const Rect clip = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); - canvas.clipRect(clip); + const Rect clipBounds = Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); + const Rect clipExpandedBounds = Rect.fromLTRB(10, 11, 21, 26); + canvas.clipRect(clipBounds); + + // Save initial return values for testing restored values + final Rect initialLocalBounds = canvas.getLocalClipBounds(); + final Rect initialDestinationBounds = canvas.getDestinationClipBounds(); + expect(initialLocalBounds, closeToRect(clipExpandedBounds)); + expect(initialDestinationBounds, closeToRect(clipBounds)); + canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15), clipOp: ClipOp.difference); - final Rect curBounds = canvas.getClipBounds(); - expect(curBounds, closeToRect(clip)); - canvas.clipRect(const Rect.fromLTRB(0, 0, 15, 15)); - final Rect newCurBounds = canvas.getClipBounds(); - expect(newCurBounds, notCloseToRect(clip)); - expect(curBounds, closeToRect(clip)); + expect(canvas.getLocalClipBounds(), initialLocalBounds); + expect(canvas.getDestinationClipBounds(), initialDestinationBounds); }); } From 6b60eeddc443d6fc296dd1ff082f9b5e90be2b8d Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Fri, 20 May 2022 15:10:53 -0700 Subject: [PATCH 06/11] add unimpl stubs in web source files --- .../src/engine/canvaskit/canvaskit_canvas.dart | 16 ++++++++++++++++ lib/web_ui/lib/src/engine/html/canvas.dart | 15 +++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_canvas.dart b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_canvas.dart index 033fe9acddbc2..b0d190dc8357b 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_canvas.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_canvas.dart @@ -104,6 +104,11 @@ class CanvasKitCanvas implements ui.Canvas { _canvas.transform(matrix4); } + @override + Float64List getTransform() { + throw UnimplementedError('getTransform not implemented on CanvasKit back end'); + } + @override void clipRect(ui.Rect rect, {ui.ClipOp clipOp = ui.ClipOp.intersect, bool doAntiAlias = true}) { @@ -136,6 +141,17 @@ class CanvasKitCanvas implements ui.Canvas { _canvas.clipPath(path as CkPath, doAntiAlias); } + + @override + ui.Rect getLocalClipBounds() { + throw UnimplementedError('getLocalClipBounds not implemented on CanvasKit back end'); + } + + @override + ui.Rect getDestinationClipBounds() { + throw UnimplementedError('getDestinationClipBounds not implemented on CanvasKit back end'); + } + @override void drawColor(ui.Color color, ui.BlendMode blendMode) { assert(color != null); // ignore: unnecessary_null_comparison diff --git a/lib/web_ui/lib/src/engine/html/canvas.dart b/lib/web_ui/lib/src/engine/html/canvas.dart index 3a1ba9b3d6545..af6a1e0fb3db2 100644 --- a/lib/web_ui/lib/src/engine/html/canvas.dart +++ b/lib/web_ui/lib/src/engine/html/canvas.dart @@ -96,6 +96,11 @@ class SurfaceCanvas implements ui.Canvas { _canvas.transform(matrix4); } + @override + Float64List getTransform() { + throw UnimplementedError('getTransform not implemented on HTML back end'); + } + @override void clipRect(ui.Rect rect, {ui.ClipOp clipOp = ui.ClipOp.intersect, bool doAntiAlias = true}) { @@ -132,6 +137,16 @@ class SurfaceCanvas implements ui.Canvas { _canvas.clipPath(path, doAntiAlias: doAntiAlias); } + @override + ui.Rect getLocalClipBounds() { + throw UnimplementedError('getLocalClipBounds not implemented on HTML back end'); + } + + @override + ui.Rect getDestinationClipBounds() { + throw UnimplementedError('getDestinationClipBounds not implemented on HTML back end'); + } + @override void drawColor(ui.Color color, ui.BlendMode blendMode) { assert(color != null); // ignore: unnecessary_null_comparison From a8d6aee7c853058e563d3ff3d59c89a1f30be311 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Wed, 8 Jun 2022 16:21:58 -0700 Subject: [PATCH 07/11] prototype CanvasKit support --- .../lib/src/engine/canvaskit/canvas.dart | 8 ++++ .../src/engine/canvaskit/canvaskit_api.dart | 2 + .../engine/canvaskit/canvaskit_canvas.dart | 41 +++++++++++++++++-- lib/web_ui/lib/src/engine/vector_math.dart | 33 +++++++++++++++ .../test/canvaskit/canvaskit_api_test.dart | 1 + testing/dart/canvas_test.dart | 1 + 6 files changed, 82 insertions(+), 4 deletions(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/canvas.dart b/lib/web_ui/lib/src/engine/canvaskit/canvas.dart index e529ebd5f8244..2d08fff54f3f5 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/canvas.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/canvas.dart @@ -66,6 +66,10 @@ class CkCanvas { ); } + ui.Rect getDeviceClipBounds() { + return fromSkRect(skCanvas.getDeviceClipBounds()); + } + void drawArc( ui.Rect oval, double startAngle, @@ -313,6 +317,10 @@ class CkCanvas { skCanvas.translate(dx, dy); } + Float32List getLocalToDevice() { + return skCanvas.getLocalToDevice(); + } + CkPictureSnapshot? get pictureSnapshot => null; } diff --git a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart index 0a2e0e9ce1d4a..fc33e73bc5a8f 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart @@ -1683,6 +1683,7 @@ extension SkCanvasExtension on SkCanvas { SkClipOp clipOp, bool doAntiAlias, ); + external Float32List getDeviceClipBounds(); external void drawArc( Float32List oval, double startAngleDegrees, @@ -1816,6 +1817,7 @@ extension SkCanvasExtension on SkCanvas { external void skew(double x, double y); external void concat(Float32List matrix); external void translate(double x, double y); + external Float32List getLocalToDevice(); external void drawPicture(SkPicture picture); external void drawParagraph( SkParagraph paragraph, diff --git a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_canvas.dart b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_canvas.dart index b0d190dc8357b..3297e48dc5f0c 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_canvas.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_canvas.dart @@ -106,7 +106,7 @@ class CanvasKitCanvas implements ui.Canvas { @override Float64List getTransform() { - throw UnimplementedError('getTransform not implemented on CanvasKit back end'); + return toMatrix64(_canvas.getLocalToDevice()); } @override @@ -141,15 +141,48 @@ class CanvasKitCanvas implements ui.Canvas { _canvas.clipPath(path as CkPath, doAntiAlias); } - @override ui.Rect getLocalClipBounds() { - throw UnimplementedError('getLocalClipBounds not implemented on CanvasKit back end'); + Matrix4 transform = Matrix4.fromFloat32List(_canvas.getLocalToDevice()); + if (transform.invert() == 0) { + return ui.Rect.largest; + } + final ui.Rect destBounds = getDestinationClipBounds(); + final Float32List corners = Float32List(2); + double left = 0; + double top = 0; + double right = 0; + double bottom = 0; + for (int i = 0; i < 4; i++) { + corners[0] = (i & 1) == 0 ? destBounds.left : destBounds.right; + corners[1] = i < 2 ? destBounds.top : destBounds.bottom; + transform.transform2(corners); + if (i == 0) { + left = corners[0]; + right = corners[0]; + top = corners[1]; + bottom = corners[1]; + } else { + if (left > corners[0]) { + left = corners[0]; + } + if (right < corners[0]) { + right = corners[0]; + } + if (top > corners[1]) { + top = corners[1]; + } + if (bottom < corners[1]) { + bottom = corners[1]; + } + } + } + return ui.Rect.fromLTRB(left, top, right, bottom); } @override ui.Rect getDestinationClipBounds() { - throw UnimplementedError('getDestinationClipBounds not implemented on CanvasKit back end'); + return _canvas.getDeviceClipBounds(); } @override diff --git a/lib/web_ui/lib/src/engine/vector_math.dart b/lib/web_ui/lib/src/engine/vector_math.dart index a59ed54958e75..08b708a27fb41 100644 --- a/lib/web_ui/lib/src/engine/vector_math.dart +++ b/lib/web_ui/lib/src/engine/vector_math.dart @@ -1429,6 +1429,39 @@ Float32List toMatrix32(Float64List matrix64) { return matrix32; } +/// Converts a matrix represented using [Float32List] to one represented using +/// [Float64List]. +/// +/// 32-bit precision is sufficient because Flutter Engine itself (as well as +/// Skia) use 32-bit precision under the hood anyway. +/// +/// 32-bit matrices require 2x less memory and in V8 they are allocated on the +/// JavaScript heap, thus avoiding a malloc. +/// +/// See also: +/// * https://bugs.chromium.org/p/v8/issues/detail?id=9199 +/// * https://bugs.chromium.org/p/v8/issues/detail?id=2022 +Float64List toMatrix64(Float32List matrix32) { + final Float64List matrix64 = Float64List(16); + matrix64[15] = matrix32[15]; + matrix64[14] = matrix32[14]; + matrix64[13] = matrix32[13]; + matrix64[12] = matrix32[12]; + matrix64[11] = matrix32[11]; + matrix64[10] = matrix32[10]; + matrix64[9] = matrix32[9]; + matrix64[8] = matrix32[8]; + matrix64[7] = matrix32[7]; + matrix64[6] = matrix32[6]; + matrix64[5] = matrix32[5]; + matrix64[4] = matrix32[4]; + matrix64[3] = matrix32[3]; + matrix64[2] = matrix32[2]; + matrix64[1] = matrix32[1]; + matrix64[0] = matrix32[0]; + return matrix64; +} + // Stores matrix in a form that allows zero allocation transforms. // TODO(yjbanov): re-evaluate the need for this class. It may be an // over-optimization. It is only used by `GradientLinear` in the diff --git a/lib/web_ui/test/canvaskit/canvaskit_api_test.dart b/lib/web_ui/test/canvaskit/canvaskit_api_test.dart index f56d3865de008..95f6ef36e5e8e 100644 --- a/lib/web_ui/test/canvaskit/canvaskit_api_test.dart +++ b/lib/web_ui/test/canvaskit/canvaskit_api_test.dart @@ -1053,6 +1053,7 @@ void _canvasTests() { canvasKit.ClipOp.Intersect, true, ); + expect(canvas.getDeviceClipBounds(), ui.Rect.fromLTRB(0, 0, 100, 100)); }); test('drawArc', () { diff --git a/testing/dart/canvas_test.dart b/testing/dart/canvas_test.dart index 90bb56121403a..809e16145803d 100644 --- a/testing/dart/canvas_test.dart +++ b/testing/dart/canvas_test.dart @@ -434,6 +434,7 @@ void main() { } else { expect(error, isNull); } + }); Matcher closeToTransform(Float64List expected) => (dynamic v) { Expect.type(v); From 02718f95066ca86cd9a53819149aca0e4d62b72d Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Thu, 9 Jun 2022 16:04:07 -0700 Subject: [PATCH 08/11] roll CanvasKit to 0.34.1 --- DEPS | 2 +- lib/web_ui/README.md | 5 +++-- lib/web_ui/dev/canvaskit_lock.yaml | 2 +- lib/web_ui/lib/src/engine/configuration.dart | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/DEPS b/DEPS index 050f08a657e01..e54f6f3405003 100644 --- a/DEPS +++ b/DEPS @@ -31,7 +31,7 @@ vars = { # WARNING: DO NOT EDIT canvaskit_cipd_instance MANUALLY # See `lib/web_ui/README.md` for how to roll CanvasKit to a new version. - 'canvaskit_cipd_instance': '8MSYGWVWzrTJIoVL00ZquruZs-weuwLBy1kt1AawJiIC', + 'canvaskit_cipd_instance': '4PosNKiHa3EaBK4alMD4idrKYxAk0I0QiRVDDQplhOYC', # Do not download the Emscripten SDK by default. # This prevents us from downloading the Emscripten toolchain for builds diff --git a/lib/web_ui/README.md b/lib/web_ui/README.md index 72603c3994436..25609d7d454b4 100644 --- a/lib/web_ui/README.md +++ b/lib/web_ui/README.md @@ -209,8 +209,9 @@ directly), follow these steps to roll to the new version: - Make sure you have `depot_tools` installed (if you are regularly hacking on the engine code, you probably do). - If not already authenticated with CIPD, run `cipd auth-login` and follow - instructions (this step requires sufficient privileges; contact - #hackers-infra-🌡 on Flutter's Discord server). + instructions (this step requires sufficient privileges; file a github + infra ticket queue issue: https://github.com/flutter/flutter/wiki/Infra-Ticket-Queue + to get access) - Edit `dev/canvaskit_lock.yaml` and update the value of `canvaskit_version` to the new version. - Run `dart dev/canvaskit_roller.dart` and make sure it completes successfully. diff --git a/lib/web_ui/dev/canvaskit_lock.yaml b/lib/web_ui/dev/canvaskit_lock.yaml index d83e801d16940..5bfca239fc92d 100644 --- a/lib/web_ui/dev/canvaskit_lock.yaml +++ b/lib/web_ui/dev/canvaskit_lock.yaml @@ -1,4 +1,4 @@ # Specifies the version of CanvasKit to use for Flutter Web apps. # # See `lib/web_ui/README.md` for how to update this file. -canvaskit_version: "0.33.0" +canvaskit_version: "0.34.1" diff --git a/lib/web_ui/lib/src/engine/configuration.dart b/lib/web_ui/lib/src/engine/configuration.dart index 03dfd5960bdba..0817a80c023c0 100644 --- a/lib/web_ui/lib/src/engine/configuration.dart +++ b/lib/web_ui/lib/src/engine/configuration.dart @@ -32,7 +32,7 @@ import 'package:js/js.dart'; /// The version of CanvasKit used by the web engine by default. // DO NOT EDIT THE NEXT LINE OF CODE MANUALLY // See `lib/web_ui/README.md` for how to roll CanvasKit to a new version. -const String _canvaskitVersion = '0.33.0'; +const String _canvaskitVersion = '0.34.1'; /// The Web Engine configuration for the current application. FlutterConfiguration get configuration => _configuration ??= FlutterConfiguration(_jsConfiguration); From 381ebe922c86d00c8ada7d52b532c0f4d294123e Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Thu, 9 Jun 2022 20:33:29 -0700 Subject: [PATCH 09/11] finish web (dom and ck) implementations --- lib/web_ui/README.md | 2 + .../lib/src/engine/canvaskit/canvas.dart | 9 +++- .../src/engine/canvaskit/canvaskit_api.dart | 13 ++++- .../engine/canvaskit/canvaskit_canvas.dart | 38 ++------------ lib/web_ui/lib/src/engine/html/canvas.dart | 19 +++++-- .../lib/src/engine/html/recording_canvas.dart | 17 +++++++ .../test/canvaskit/canvaskit_api_test.dart | 50 +++++++++++++++++-- 7 files changed, 103 insertions(+), 45 deletions(-) diff --git a/lib/web_ui/README.md b/lib/web_ui/README.md index 25609d7d454b4..4284463585a3c 100644 --- a/lib/web_ui/README.md +++ b/lib/web_ui/README.md @@ -218,6 +218,8 @@ directly), follow these steps to roll to the new version: The script uploads the new version of CanvasKit to the `flutter/web/canvaskit_bundle` CIPD package, and writes the CIPD package instance ID to the DEPS file. +- Rerun `gclient sync` and do a clean build to test that the new version is + picked up. - Send a pull request containing the above file changes. If the new version contains breaking changes, the PR must also contain corresponding fixes. diff --git a/lib/web_ui/lib/src/engine/canvaskit/canvas.dart b/lib/web_ui/lib/src/engine/canvaskit/canvas.dart index 2d08fff54f3f5..194bbf68dc0dc 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/canvas.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/canvas.dart @@ -67,7 +67,7 @@ class CkCanvas { } ui.Rect getDeviceClipBounds() { - return fromSkRect(skCanvas.getDeviceClipBounds()); + return rectFromSkIRect(skCanvas.getDeviceClipBounds()); } void drawArc( @@ -318,7 +318,12 @@ class CkCanvas { } Float32List getLocalToDevice() { - return skCanvas.getLocalToDevice(); + final List list = skCanvas.getLocalToDevice(); + final Float32List matrix = Float32List(16); + for (int i = 0; i < 16; i++) { + matrix[i] = list[i]; + } + return matrix; } CkPictureSnapshot? get pictureSnapshot => null; diff --git a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart index fc33e73bc5a8f..9ee1a22bc6465 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_api.dart @@ -1569,6 +1569,15 @@ ui.Rect fromSkRect(Float32List skRect) { return ui.Rect.fromLTRB(skRect[0], skRect[1], skRect[2], skRect[3]); } +ui.Rect rectFromSkIRect(Int32List skIRect) { + return ui.Rect.fromLTRB( + skIRect[0].toDouble(), + skIRect[1].toDouble(), + skIRect[2].toDouble(), + skIRect[3].toDouble(), + ); +} + // TODO(hterkelsen): Use a shared malloc'ed array for performance. Float32List toSkRRect(ui.RRect rrect) { final Float32List skRRect = Float32List(12); @@ -1683,7 +1692,7 @@ extension SkCanvasExtension on SkCanvas { SkClipOp clipOp, bool doAntiAlias, ); - external Float32List getDeviceClipBounds(); + external Int32List getDeviceClipBounds(); external void drawArc( Float32List oval, double startAngleDegrees, @@ -1817,7 +1826,7 @@ extension SkCanvasExtension on SkCanvas { external void skew(double x, double y); external void concat(Float32List matrix); external void translate(double x, double y); - external Float32List getLocalToDevice(); + external List getLocalToDevice(); external void drawPicture(SkPicture picture); external void drawParagraph( SkParagraph paragraph, diff --git a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_canvas.dart b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_canvas.dart index 3297e48dc5f0c..e21cd2f655557 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_canvas.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_canvas.dart @@ -6,6 +6,7 @@ import 'dart:typed_data'; import 'package:ui/ui.dart' as ui; +import '../util.dart'; import '../validators.dart'; import '../vector_math.dart'; import 'canvas.dart'; @@ -143,41 +144,12 @@ class CanvasKitCanvas implements ui.Canvas { @override ui.Rect getLocalClipBounds() { - Matrix4 transform = Matrix4.fromFloat32List(_canvas.getLocalToDevice()); + final Matrix4 transform = Matrix4.fromFloat32List(_canvas.getLocalToDevice()); if (transform.invert() == 0) { - return ui.Rect.largest; + // non-invertible transforms collapse space to a line or point + return ui.Rect.zero; } - final ui.Rect destBounds = getDestinationClipBounds(); - final Float32List corners = Float32List(2); - double left = 0; - double top = 0; - double right = 0; - double bottom = 0; - for (int i = 0; i < 4; i++) { - corners[0] = (i & 1) == 0 ? destBounds.left : destBounds.right; - corners[1] = i < 2 ? destBounds.top : destBounds.bottom; - transform.transform2(corners); - if (i == 0) { - left = corners[0]; - right = corners[0]; - top = corners[1]; - bottom = corners[1]; - } else { - if (left > corners[0]) { - left = corners[0]; - } - if (right < corners[0]) { - right = corners[0]; - } - if (top > corners[1]) { - top = corners[1]; - } - if (bottom < corners[1]) { - bottom = corners[1]; - } - } - } - return ui.Rect.fromLTRB(left, top, right, bottom); + return transformRect(transform, getDestinationClipBounds()); } @override diff --git a/lib/web_ui/lib/src/engine/html/canvas.dart b/lib/web_ui/lib/src/engine/html/canvas.dart index af6a1e0fb3db2..0b3c1a65409bf 100644 --- a/lib/web_ui/lib/src/engine/html/canvas.dart +++ b/lib/web_ui/lib/src/engine/html/canvas.dart @@ -98,7 +98,7 @@ class SurfaceCanvas implements ui.Canvas { @override Float64List getTransform() { - throw UnimplementedError('getTransform not implemented on HTML back end'); + return Float64List.fromList(_canvas.getCurrentMatrixUnsafe()); } @override @@ -138,13 +138,22 @@ class SurfaceCanvas implements ui.Canvas { } @override - ui.Rect getLocalClipBounds() { - throw UnimplementedError('getLocalClipBounds not implemented on HTML back end'); + ui.Rect getDestinationClipBounds() { + return _canvas.getDestinationClipBounds() ?? ui.Rect.largest; } @override - ui.Rect getDestinationClipBounds() { - throw UnimplementedError('getDestinationClipBounds not implemented on HTML back end'); + ui.Rect getLocalClipBounds() { + final ui.Rect? destBounds = _canvas.getDestinationClipBounds(); + if (destBounds == null) { + return ui.Rect.largest; + } + final Matrix4 transform = Matrix4.fromFloat32List(_canvas.getCurrentMatrixUnsafe()); + if (transform.invert() == 0) { + // non-invertible transforms collapse space to a line or point + return ui.Rect.zero; + } + return transformRect(transform, destBounds); } @override diff --git a/lib/web_ui/lib/src/engine/html/recording_canvas.dart b/lib/web_ui/lib/src/engine/html/recording_canvas.dart index 92c32052a303c..ed62343165ac4 100644 --- a/lib/web_ui/lib/src/engine/html/recording_canvas.dart +++ b/lib/web_ui/lib/src/engine/html/recording_canvas.dart @@ -293,6 +293,8 @@ class RecordingCanvas { _commands.add(PaintTransform(matrix4)); } + Float32List getCurrentMatrixUnsafe() => _paintBounds._currentMatrix.storage; + void skew(double sx, double sy) { assert(!_recordingEnded); renderStrategy.hasArbitraryPaint = true; @@ -331,6 +333,8 @@ class RecordingCanvas { _commands.add(command); } + ui.Rect? getDestinationClipBounds() => _paintBounds.getDestinationClipBounds(); + void drawColor(ui.Color color, ui.BlendMode blendMode) { assert(!_recordingEnded); final PaintDrawColor command = PaintDrawColor(color, blendMode); @@ -1800,6 +1804,19 @@ class _PaintBounds { } } + ui.Rect? getDestinationClipBounds() { + if (!_clipRectInitialized) { + return null; + } else { + return ui.Rect.fromLTRB( + _currentClipLeft, + _currentClipTop, + _currentClipRight, + _currentClipBottom, + ); + } + } + /// Grow painted area to include given rectangle. void grow(ui.Rect r, DrawCommand command) { growLTRB(r.left, r.top, r.right, r.bottom, command); diff --git a/lib/web_ui/test/canvaskit/canvaskit_api_test.dart b/lib/web_ui/test/canvaskit/canvaskit_api_test.dart index 95f6ef36e5e8e..21b6313fb859c 100644 --- a/lib/web_ui/test/canvaskit/canvaskit_api_test.dart +++ b/lib/web_ui/test/canvaskit/canvaskit_api_test.dart @@ -1037,6 +1037,7 @@ void _canvasTests() { canvasKit.ClipOp.Intersect, true, ); + expect(canvas.getDeviceClipBounds(), [10, 10, 20, 20]); }); test('clipRRect', () { @@ -1045,6 +1046,7 @@ void _canvasTests() { canvasKit.ClipOp.Intersect, true, ); + expect(canvas.getDeviceClipBounds(), [0, 0, 100, 100]); }); test('clipRect', () { @@ -1053,7 +1055,7 @@ void _canvasTests() { canvasKit.ClipOp.Intersect, true, ); - expect(canvas.getDeviceClipBounds(), ui.Rect.fromLTRB(0, 0, 100, 100)); + expect(canvas.getDeviceClipBounds(), [0, 0, 100, 100]); }); test('drawArc', () { @@ -1250,23 +1252,65 @@ void _canvasTests() { }); test('rotate', () { - canvas.rotate(5, 10, 20); + canvas.rotate(90, 10, 20); + expect(canvas.getLocalToDevice(), [ + 0, -1, 0, 30, // tx = 10 - (-20) == 30 + 1, 0, 0, 10, // ty = 20 - 10 == 10 + 0, 0, 1, 0, + 0, 0, 0, 1, + ]); }); test('scale', () { canvas.scale(2, 3); + expect(canvas.getLocalToDevice(), [ + 2, 0, 0, 0, + 0, 3, 0, 0, + 0, 0, 1, 0, + 0, 0, 0, 1, + ]); }); test('skew', () { canvas.skew(4, 5); + expect(canvas.getLocalToDevice(), [ + 1, 4, 0, 0, + 5, 1, 0, 0, + 0, 0, 1, 0, + 0, 0, 0, 1, + ]); }); test('concat', () { - canvas.concat(toSkMatrixFromFloat32(Matrix4.identity().storage)); + canvas.concat(toSkM44FromFloat32(Matrix4.identity().storage)); + expect(canvas.getLocalToDevice(), [ + 1, 0, 0, 0, + 0, 1, 0, 0, + 0, 0, 1, 0, + 0, 0, 0, 1, + ]); + canvas.concat(Float32List.fromList([ + 11, 12, 13, 14, + 21, 22, 23, 24, + 31, 32, 33, 34, + 41, 42, 43, 44, + ])); + expect(canvas.getLocalToDevice(), [ + 11, 12, 13, 14, + 21, 22, 23, 24, + 31, 32, 33, 34, + 41, 42, 43, 44, + ]); }); test('translate', () { canvas.translate(4, 5); + expect(canvas.getLocalToDevice(), [ + 1, 0, 0, 4, + 0, 1, 0, 5, + 0, 0, 1, 0, + 0, 0, 0, 1, + ]); }); test('drawPicture', () { From 06efd181f0f5d6ecf0678f1a0b5e7370e875058a Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Fri, 10 Jun 2022 16:11:14 -0700 Subject: [PATCH 10/11] add web functional tests (both CK and Dom) --- .../lib/src/engine/canvaskit/canvas.dart | 10 +- .../engine/canvaskit/canvaskit_canvas.dart | 2 +- lib/web_ui/lib/src/engine/html/canvas.dart | 11 +- .../lib/src/engine/html/recording_canvas.dart | 2 +- lib/web_ui/test/canvaskit/canvas_test.dart | 30 ++ .../test/canvaskit/canvaskit_api_test.dart | 10 +- lib/web_ui/test/engine/canvas_test.dart | 280 ++++++++++++++++++ testing/dart/canvas_test.dart | 6 +- 8 files changed, 338 insertions(+), 13 deletions(-) create mode 100644 lib/web_ui/test/canvaskit/canvas_test.dart create mode 100644 lib/web_ui/test/engine/canvas_test.dart diff --git a/lib/web_ui/lib/src/engine/canvaskit/canvas.dart b/lib/web_ui/lib/src/engine/canvaskit/canvas.dart index 194bbf68dc0dc..c6bf1c45fda72 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/canvas.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/canvas.dart @@ -319,11 +319,13 @@ class CkCanvas { Float32List getLocalToDevice() { final List list = skCanvas.getLocalToDevice(); - final Float32List matrix = Float32List(16); - for (int i = 0; i < 16; i++) { - matrix[i] = list[i]; + final Float32List matrix4 = Float32List(16); + for (int r = 0; r < 4; r++) { + for (int c = 0; c < 4; c++) { + matrix4[c * 4 + r] = list[r * 4 + c].toDouble(); + } } - return matrix; + return matrix4; } CkPictureSnapshot? get pictureSnapshot => null; diff --git a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_canvas.dart b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_canvas.dart index e21cd2f655557..197dc04e63189 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/canvaskit_canvas.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/canvaskit_canvas.dart @@ -149,7 +149,7 @@ class CanvasKitCanvas implements ui.Canvas { // non-invertible transforms collapse space to a line or point return ui.Rect.zero; } - return transformRect(transform, getDestinationClipBounds()); + return transformRect(transform, _canvas.getDeviceClipBounds()); } @override diff --git a/lib/web_ui/lib/src/engine/html/canvas.dart b/lib/web_ui/lib/src/engine/html/canvas.dart index 0b3c1a65409bf..2ec8507ac0b9c 100644 --- a/lib/web_ui/lib/src/engine/html/canvas.dart +++ b/lib/web_ui/lib/src/engine/html/canvas.dart @@ -142,6 +142,15 @@ class SurfaceCanvas implements ui.Canvas { return _canvas.getDestinationClipBounds() ?? ui.Rect.largest; } + ui.Rect _roundOut(ui.Rect rect) { + return ui.Rect.fromLTRB( + rect.left.floorToDouble(), + rect.top.floorToDouble(), + rect.right.ceilToDouble(), + rect.bottom.ceilToDouble(), + ); + } + @override ui.Rect getLocalClipBounds() { final ui.Rect? destBounds = _canvas.getDestinationClipBounds(); @@ -153,7 +162,7 @@ class SurfaceCanvas implements ui.Canvas { // non-invertible transforms collapse space to a line or point return ui.Rect.zero; } - return transformRect(transform, destBounds); + return transformRect(transform, _roundOut(destBounds)); } @override diff --git a/lib/web_ui/lib/src/engine/html/recording_canvas.dart b/lib/web_ui/lib/src/engine/html/recording_canvas.dart index ed62343165ac4..8ed7282a3c7c4 100644 --- a/lib/web_ui/lib/src/engine/html/recording_canvas.dart +++ b/lib/web_ui/lib/src/engine/html/recording_canvas.dart @@ -1722,7 +1722,7 @@ class _PaintBounds { if (sx != 1.0 || sy != 1.0) { _currentMatrixIsIdentity = false; } - _currentMatrix.scale(sx, sy); + _currentMatrix.scale(sx, sy, 1.0); } void rotateZ(double radians) { diff --git a/lib/web_ui/test/canvaskit/canvas_test.dart b/lib/web_ui/test/canvaskit/canvas_test.dart new file mode 100644 index 0000000000000..d8cbb1f0a6d15 --- /dev/null +++ b/lib/web_ui/test/canvaskit/canvas_test.dart @@ -0,0 +1,30 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +@TestOn('chrome || safari || firefox') + +import 'dart:async'; + +import 'package:test/bootstrap/browser.dart'; +import 'package:test/test.dart'; +import 'package:ui/src/engine/browser_detection.dart'; + +import '../engine/canvas_test.dart'; +import 'common.dart'; + +void main() { + internalBootstrapBrowserTest(() => testMain); +} + +// Run the same semantics tests in CanvasKit mode because as of today we do not +// yet share platform view logic with the HTML renderer, which affects +// semantics. +Future testMain() async { + group('CanvasKit semantics', () { + setUpCanvasKitTest(); + + runCanvasTests(deviceClipRoundsOut: true); + // TODO(hterkelsen): https://github.com/flutter/flutter/issues/60040 + }, skip: isIosSafari); +} diff --git a/lib/web_ui/test/canvaskit/canvaskit_api_test.dart b/lib/web_ui/test/canvaskit/canvaskit_api_test.dart index 21b6313fb859c..d6427cdc137c6 100644 --- a/lib/web_ui/test/canvaskit/canvaskit_api_test.dart +++ b/lib/web_ui/test/canvaskit/canvaskit_api_test.dart @@ -1033,7 +1033,11 @@ void _canvasTests() { test('clipPath', () { canvas.clipPath( - _testClosedSkPath(), + SkPath() + ..moveTo(10.9, 10.9) + ..lineTo(19.1, 10.9) + ..lineTo(19.1, 19.1) + ..lineTo(10.9, 19.1), canvasKit.ClipOp.Intersect, true, ); @@ -1042,7 +1046,7 @@ void _canvasTests() { test('clipRRect', () { canvas.clipRRect( - Float32List.fromList([0, 0, 100, 100, 1, 2, 3, 4, 5, 6, 7, 8]), + Float32List.fromList([0.9, 0.9, 99.1, 99.1, 1, 2, 3, 4, 5, 6, 7, 8]), canvasKit.ClipOp.Intersect, true, ); @@ -1051,7 +1055,7 @@ void _canvasTests() { test('clipRect', () { canvas.clipRect( - Float32List.fromList([0, 0, 100, 100]), + Float32List.fromList([0.9, 0.9, 99.1, 99.1]), canvasKit.ClipOp.Intersect, true, ); diff --git a/lib/web_ui/test/engine/canvas_test.dart b/lib/web_ui/test/engine/canvas_test.dart new file mode 100644 index 0000000000000..9eb32f57ef039 --- /dev/null +++ b/lib/web_ui/test/engine/canvas_test.dart @@ -0,0 +1,280 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:math'; +import 'dart:typed_data'; + +import 'package:test/bootstrap/browser.dart'; +import 'package:test/test.dart'; +import 'package:ui/src/engine.dart'; + +import 'package:ui/ui.dart' as ui; + +void main() { + internalBootstrapBrowserTest(() => testMain); +} + +Future testMain() async { + await ui.webOnlyInitializePlatform(); + runCanvasTests(deviceClipRoundsOut: false); +} + +void runCanvasTests({required bool deviceClipRoundsOut}) { + setUp(() { + EngineSemanticsOwner.debugResetSemantics(); + }); + + group('ui.Canvas transform tests', () { + void transformsClose(Float64List value, Float64List expected) { + expect(expected.length, equals(16)); + expect(value.length, equals(16)); + for (int r = 0; r < 4; r++) { + for (int c = 0; c < 4; c++) { + expect(value[r*4 + c], closeTo(expected[r*4 + c], 1e-10)); + } + } + } + + void transformsNotClose(Float64List value, Float64List expected) { + // We check the lengths here even though [transformsClose] will + // check them so that the [TestFailure] we catch below can only + // be due to a difference in matrix values. + expect(expected.length, equals(16)); + expect(value.length, equals(16)); + try { + transformsClose(value, expected); + } on TestFailure { + return; + } + throw TestFailure('transforms were too close to equal'); + } + + test('ui.Canvas.translate affects canvas.getTransform', () { + final ui.PictureRecorder recorder = ui.PictureRecorder(); + final ui.Canvas canvas = ui.Canvas(recorder); + canvas.translate(12, 14.5); + final Float64List matrix = Matrix4.translationValues(12, 14.5, 0).toFloat64(); + final Float64List curMatrix = canvas.getTransform(); + transformsClose(curMatrix, matrix); + canvas.translate(10, 10); + final Float64List newCurMatrix = canvas.getTransform(); + transformsNotClose(newCurMatrix, matrix); + transformsClose(curMatrix, matrix); + }); + + test('ui.Canvas.scale affects canvas.getTransform', () { + final ui.PictureRecorder recorder = ui.PictureRecorder(); + final ui.Canvas canvas = ui.Canvas(recorder); + canvas.scale(12, 14.5); + final Float64List matrix = Matrix4.diagonal3Values(12, 14.5, 1).toFloat64(); + final Float64List curMatrix = canvas.getTransform(); + transformsClose(curMatrix, matrix); + canvas.scale(10, 10); + final Float64List newCurMatrix = canvas.getTransform(); + transformsNotClose(newCurMatrix, matrix); + transformsClose(curMatrix, matrix); + }); + + test('Canvas.rotate affects canvas.getTransform', () async { + final ui.PictureRecorder recorder = ui.PictureRecorder(); + final ui.Canvas canvas = ui.Canvas(recorder); + canvas.rotate(pi); + final Float64List matrix = Matrix4.rotationZ(pi).toFloat64(); + final Float64List curMatrix = canvas.getTransform(); + transformsClose(curMatrix, matrix); + canvas.rotate(pi / 2); + final Float64List newCurMatrix = canvas.getTransform(); + transformsNotClose(newCurMatrix, matrix); + transformsClose(curMatrix, matrix); + }); + + test('Canvas.skew affects canvas.getTransform', () async { + final ui.PictureRecorder recorder = ui.PictureRecorder(); + final ui.Canvas canvas = ui.Canvas(recorder); + canvas.skew(12, 14.5); + final Float64List matrix = (Matrix4.identity()..setEntry(0, 1, 12)..setEntry(1, 0, 14.5)).toFloat64(); + final Float64List curMatrix = canvas.getTransform(); + transformsClose(curMatrix, matrix); + canvas.skew(10, 10); + final Float64List newCurMatrix = canvas.getTransform(); + transformsNotClose(newCurMatrix, matrix); + transformsClose(curMatrix, matrix); + }); + + test('Canvas.transform affects canvas.getTransform', () async { + final ui.PictureRecorder recorder = ui.PictureRecorder(); + final ui.Canvas canvas = ui.Canvas(recorder); + final Float64List matrix = (Matrix4.identity()..translate(12.0, 14.5)..scale(12.0, 14.5)).toFloat64(); + canvas.transform(matrix); + final Float64List curMatrix = canvas.getTransform(); + transformsClose(curMatrix, matrix); + canvas.translate(10, 10); + final Float64List newCurMatrix = canvas.getTransform(); + transformsNotClose(newCurMatrix, matrix); + transformsClose(curMatrix, matrix); + }); + }); + + void rectsClose(ui.Rect value, ui.Rect expected) { + expect(value.left, closeTo(expected.left, 1e-6)); + expect(value.top, closeTo(expected.top, 1e-6)); + expect(value.right, closeTo(expected.right, 1e-6)); + expect(value.bottom, closeTo(expected.bottom, 1e-6)); + } + + void rectsNotClose(ui.Rect value, ui.Rect expected) { + try { + rectsClose(value, expected); + } on TestFailure { + return; + } + throw TestFailure('transforms were too close to equal'); + } + + group('ui.Canvas clip tests', () { + test('Canvas.clipRect affects canvas.getClipBounds', () async { + final ui.PictureRecorder recorder = ui.PictureRecorder(); + final ui.Canvas canvas = ui.Canvas(recorder, const ui.Rect.fromLTRB(0, 0, 100, 100)); + const ui.Rect clipRawBounds = ui.Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); + const ui.Rect clipExpandedBounds = ui.Rect.fromLTRB(10, 11, 21, 26); + final ui.Rect clipDestBounds = deviceClipRoundsOut ? clipExpandedBounds : clipRawBounds; + canvas.clipRect(clipRawBounds); + + // Save initial return values for testing restored values + final ui.Rect initialLocalBounds = canvas.getLocalClipBounds(); + final ui.Rect initialDestinationBounds = canvas.getDestinationClipBounds(); + rectsClose(initialLocalBounds, clipExpandedBounds); + rectsClose(initialDestinationBounds, clipDestBounds); + + canvas.save(); + canvas.clipRect(const ui.Rect.fromLTRB(0, 0, 15, 15)); + // Both clip bounds have changed + rectsNotClose(canvas.getLocalClipBounds(), clipExpandedBounds); + rectsNotClose(canvas.getDestinationClipBounds(), clipDestBounds); + // Previous return values have not changed + rectsClose(initialLocalBounds, clipExpandedBounds); + rectsClose(initialDestinationBounds, clipDestBounds); + canvas.restore(); + + // save/restore returned the values to their original values + expect(canvas.getLocalClipBounds(), initialLocalBounds); + expect(canvas.getDestinationClipBounds(), initialDestinationBounds); + + canvas.save(); + canvas.scale(2, 2); + const ui.Rect scaledExpandedBounds = ui.Rect.fromLTRB(5, 5.5, 10.5, 13); + rectsClose(canvas.getLocalClipBounds(), scaledExpandedBounds); + // Destination bounds are unaffected by transform + rectsClose(canvas.getDestinationClipBounds(), clipDestBounds); + canvas.restore(); + + // save/restore returned the values to their original values + expect(canvas.getLocalClipBounds(), initialLocalBounds); + expect(canvas.getDestinationClipBounds(), initialDestinationBounds); + }); + + test('Canvas.clipRRect affects canvas.getClipBounds', () async { + final ui.PictureRecorder recorder = ui.PictureRecorder(); + final ui.Canvas canvas = ui.Canvas(recorder, const ui.Rect.fromLTRB(0, 0, 100, 100)); + const ui.Rect clipRawBounds = ui.Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); + const ui.Rect clipExpandedBounds = ui.Rect.fromLTRB(10, 11, 21, 26); + final ui.Rect clipDestBounds = deviceClipRoundsOut ? clipExpandedBounds : clipRawBounds; + final ui.RRect clip = ui.RRect.fromRectAndRadius(clipRawBounds, const ui.Radius.circular(3)); + canvas.clipRRect(clip); + + // Save initial return values for testing restored values + final ui.Rect initialLocalBounds = canvas.getLocalClipBounds(); + final ui.Rect initialDestinationBounds = canvas.getDestinationClipBounds(); + rectsClose(initialLocalBounds, clipExpandedBounds); + rectsClose(initialDestinationBounds, clipDestBounds); + + canvas.save(); + canvas.clipRect(const ui.Rect.fromLTRB(0, 0, 15, 15)); + // Both clip bounds have changed + rectsNotClose(canvas.getLocalClipBounds(), clipExpandedBounds); + rectsNotClose(canvas.getDestinationClipBounds(), clipDestBounds); + // Previous return values have not changed + rectsClose(initialLocalBounds, clipExpandedBounds); + rectsClose(initialDestinationBounds, clipDestBounds); + canvas.restore(); + + // save/restore returned the values to their original values + expect(canvas.getLocalClipBounds(), initialLocalBounds); + expect(canvas.getDestinationClipBounds(), initialDestinationBounds); + + canvas.save(); + canvas.scale(2, 2); + const ui.Rect scaledExpandedBounds = ui.Rect.fromLTRB(5, 5.5, 10.5, 13); + rectsClose(canvas.getLocalClipBounds(), scaledExpandedBounds); + // Destination bounds are unaffected by transform + rectsClose(canvas.getDestinationClipBounds(), clipDestBounds); + canvas.restore(); + + // save/restore returned the values to their original values + expect(canvas.getLocalClipBounds(), initialLocalBounds); + expect(canvas.getDestinationClipBounds(), initialDestinationBounds); + }); + + test('Canvas.clipPath affects canvas.getClipBounds', () async { + final ui.PictureRecorder recorder = ui.PictureRecorder(); + final ui.Canvas canvas = ui.Canvas(recorder, const ui.Rect.fromLTRB(0, 0, 100, 100)); + const ui.Rect clipRawBounds = ui.Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); + const ui.Rect clipExpandedBounds = ui.Rect.fromLTRB(10, 11, 21, 26); + final ui.Rect clipDestBounds = deviceClipRoundsOut ? clipExpandedBounds : clipRawBounds; + final ui.Path clip = ui.Path()..addRect(clipRawBounds)..addOval(clipRawBounds); + canvas.clipPath(clip); + + // Save initial return values for testing restored values + final ui.Rect initialLocalBounds = canvas.getLocalClipBounds(); + final ui.Rect initialDestinationBounds = canvas.getDestinationClipBounds(); + rectsClose(initialLocalBounds, clipExpandedBounds); + rectsClose(initialDestinationBounds, clipDestBounds); + + canvas.save(); + canvas.clipRect(const ui.Rect.fromLTRB(0, 0, 15, 15)); + // Both clip bounds have changed + rectsNotClose(canvas.getLocalClipBounds(), clipExpandedBounds); + rectsNotClose(canvas.getDestinationClipBounds(), clipDestBounds); + // Previous return values have not changed + rectsClose(initialLocalBounds, clipExpandedBounds); + rectsClose(initialDestinationBounds, clipDestBounds); + canvas.restore(); + + // save/restore returned the values to their original values + expect(canvas.getLocalClipBounds(), initialLocalBounds); + expect(canvas.getDestinationClipBounds(), initialDestinationBounds); + + canvas.save(); + canvas.scale(2, 2); + const ui.Rect scaledExpandedBounds = ui.Rect.fromLTRB(5, 5.5, 10.5, 13); + rectsClose(canvas.getLocalClipBounds(), scaledExpandedBounds); + // Destination bounds are unaffected by transform + rectsClose(canvas.getDestinationClipBounds(), clipDestBounds); + canvas.restore(); + + // save/restore returned the values to their original values + expect(canvas.getLocalClipBounds(), initialLocalBounds); + expect(canvas.getDestinationClipBounds(), initialDestinationBounds); + }); + + test('Canvas.clipRect(diff) does not affect canvas.getClipBounds', () async { + final ui.PictureRecorder recorder = ui.PictureRecorder(); + final ui.Canvas canvas = ui.Canvas(recorder, const ui.Rect.fromLTRB(0, 0, 100, 100)); + const ui.Rect clipRawBounds = ui.Rect.fromLTRB(10.2, 11.3, 20.4, 25.7); + const ui.Rect clipExpandedBounds = ui.Rect.fromLTRB(10, 11, 21, 26); + final ui.Rect clipDestBounds = deviceClipRoundsOut ? clipExpandedBounds : clipRawBounds; + canvas.clipRect(clipRawBounds); + + // Save initial return values for testing restored values + final ui.Rect initialLocalBounds = canvas.getLocalClipBounds(); + final ui.Rect initialDestinationBounds = canvas.getDestinationClipBounds(); + rectsClose(initialLocalBounds, clipExpandedBounds); + rectsClose(initialDestinationBounds, clipDestBounds); + + canvas.clipRect(const ui.Rect.fromLTRB(0, 0, 15, 15), clipOp: ui.ClipOp.difference); + expect(canvas.getLocalClipBounds(), initialLocalBounds); + expect(canvas.getDestinationClipBounds(), initialDestinationBounds); + }); + }); +} diff --git a/testing/dart/canvas_test.dart b/testing/dart/canvas_test.dart index 809e16145803d..50b499f11bc9a 100644 --- a/testing/dart/canvas_test.dart +++ b/testing/dart/canvas_test.dart @@ -489,7 +489,7 @@ void main() { final Float64List matrix = Matrix4.diagonal3Values(12, 14.5, 1).storage; final Float64List curMatrix = canvas.getTransform(); expect(curMatrix, closeToTransform(matrix)); - canvas.translate(10, 10); + canvas.scale(10, 10); final Float64List newCurMatrix = canvas.getTransform(); expect(newCurMatrix, notCloseToTransform(matrix)); expect(curMatrix, closeToTransform(matrix)); @@ -502,7 +502,7 @@ void main() { final Float64List matrix = Matrix4.rotationZ(pi).storage; final Float64List curMatrix = canvas.getTransform(); expect(curMatrix, closeToTransform(matrix)); - canvas.translate(10, 10); + canvas.rotate(pi / 2); final Float64List newCurMatrix = canvas.getTransform(); expect(newCurMatrix, notCloseToTransform(matrix)); expect(curMatrix, closeToTransform(matrix)); @@ -515,7 +515,7 @@ void main() { final Float64List matrix = (Matrix4.identity()..setEntry(0, 1, 12)..setEntry(1, 0, 14.5)).storage; final Float64List curMatrix = canvas.getTransform(); expect(curMatrix, closeToTransform(matrix)); - canvas.translate(10, 10); + canvas.skew(10, 10); final Float64List newCurMatrix = canvas.getTransform(); expect(newCurMatrix, notCloseToTransform(matrix)); expect(curMatrix, closeToTransform(matrix)); From 793acab5059fe21ce711c737a3a8f628407bb46e Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Tue, 14 Jun 2022 17:04:06 -0700 Subject: [PATCH 11/11] use switch statements for enums --- display_list/display_list_builder.cc | 51 ++++++++++++++++------------ display_list/display_list_utils.cc | 24 +++++++++---- 2 files changed, 48 insertions(+), 27 deletions(-) diff --git a/display_list/display_list_builder.cc b/display_list/display_list_builder.cc index c0f83e4d37927..408e8e9a18b04 100644 --- a/display_list/display_list_builder.cc +++ b/display_list/display_list_builder.cc @@ -599,13 +599,16 @@ void DisplayListBuilder::transform(const SkM44* m44) { void DisplayListBuilder::clipRect(const SkRect& rect, SkClipOp clip_op, bool is_aa) { - if (clip_op == SkClipOp::kIntersect) { - Push(0, 1, rect, is_aa); - if (!current_layer_->clip_bounds.intersect(rect)) { - current_layer_->clip_bounds.setEmpty(); - } - } else { - Push(0, 1, rect, is_aa); + switch (clip_op) { + case SkClipOp::kIntersect: + Push(0, 1, rect, is_aa); + if (!current_layer_->clip_bounds.intersect(rect)) { + current_layer_->clip_bounds.setEmpty(); + } + break; + case SkClipOp::kDifference: + Push(0, 1, rect, is_aa); + break; } } void DisplayListBuilder::clipRRect(const SkRRect& rrect, @@ -614,13 +617,16 @@ void DisplayListBuilder::clipRRect(const SkRRect& rrect, if (rrect.isRect()) { clipRect(rrect.rect(), clip_op, is_aa); } else { - if (clip_op == SkClipOp::kIntersect) { - Push(0, 1, rrect, is_aa); - if (!current_layer_->clip_bounds.intersect(rrect.getBounds())) { - current_layer_->clip_bounds.setEmpty(); - } - } else { - Push(0, 1, rrect, is_aa); + switch (clip_op) { + case SkClipOp::kIntersect: + Push(0, 1, rrect, is_aa); + if (!current_layer_->clip_bounds.intersect(rrect.getBounds())) { + current_layer_->clip_bounds.setEmpty(); + } + break; + case SkClipOp::kDifference: + Push(0, 1, rrect, is_aa); + break; } } } @@ -644,13 +650,16 @@ void DisplayListBuilder::clipPath(const SkPath& path, return; } } - if (clip_op == SkClipOp::kIntersect) { - Push(0, 1, path, is_aa); - if (!current_layer_->clip_bounds.intersect(path.getBounds())) { - current_layer_->clip_bounds.setEmpty(); - } - } else { - Push(0, 1, path, is_aa); + switch (clip_op) { + case SkClipOp::kIntersect: + Push(0, 1, path, is_aa); + if (!current_layer_->clip_bounds.intersect(path.getBounds())) { + current_layer_->clip_bounds.setEmpty(); + } + break; + case SkClipOp::kDifference: + Push(0, 1, path, is_aa); + break; } } SkRect DisplayListBuilder::getLocalClipBounds() { diff --git a/display_list/display_list_utils.cc b/display_list/display_list_utils.cc index ccdd2b65cc15a..88a82c28a1166 100644 --- a/display_list/display_list_utils.cc +++ b/display_list/display_list_utils.cc @@ -175,22 +175,34 @@ void SkMatrixDispatchHelper::reset() { void ClipBoundsDispatchHelper::clipRect(const SkRect& rect, SkClipOp clip_op, bool is_aa) { - if (clip_op == SkClipOp::kIntersect) { - intersect(rect, is_aa); + switch (clip_op) { + case SkClipOp::kIntersect: + intersect(rect, is_aa); + break; + case SkClipOp::kDifference: + break; } } void ClipBoundsDispatchHelper::clipRRect(const SkRRect& rrect, SkClipOp clip_op, bool is_aa) { - if (clip_op == SkClipOp::kIntersect) { - intersect(rrect.getBounds(), is_aa); + switch (clip_op) { + case SkClipOp::kIntersect: + intersect(rrect.getBounds(), is_aa); + break; + case SkClipOp::kDifference: + break; } } void ClipBoundsDispatchHelper::clipPath(const SkPath& path, SkClipOp clip_op, bool is_aa) { - if (clip_op == SkClipOp::kIntersect) { - intersect(path.getBounds(), is_aa); + switch (clip_op) { + case SkClipOp::kIntersect: + intersect(path.getBounds(), is_aa); + break; + case SkClipOp::kDifference: + break; } } void ClipBoundsDispatchHelper::intersect(const SkRect& rect, bool is_aa) {