From 3ab4a2a15dd7ecf765b9f4aa9855b7b60ae273aa Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 14 Sep 2022 11:14:45 -0700 Subject: [PATCH] Fix fill type for applyOp (flutter/engine#36149) --- .../tools/path_ops/dart/lib/path_ops.dart | 13 ++++- .../path_ops/dart/test/path_ops_test.dart | 48 +++++++++++++++++-- engine/src/flutter/tools/path_ops/path_ops.cc | 4 ++ engine/src/flutter/tools/path_ops/path_ops.h | 2 + 4 files changed, 60 insertions(+), 7 deletions(-) diff --git a/engine/src/flutter/tools/path_ops/dart/lib/path_ops.dart b/engine/src/flutter/tools/path_ops/dart/lib/path_ops.dart index d64b9a9cebc..5fa84818e06 100644 --- a/engine/src/flutter/tools/path_ops/dart/lib/path_ops.dart +++ b/engine/src/flutter/tools/path_ops/dart/lib/path_ops.dart @@ -131,7 +131,7 @@ class SvgPathProxy implements PathProxy { /// [dispose] has been called, this class must not be used again. class Path implements PathProxy { /// Creates an empty path object with the specified fill type. - Path([this.fillType = FillType.nonZero]) + Path([FillType fillType = FillType.nonZero]) : _path = _createPathFn(fillType.index); /// Creates a copy of this path. @@ -142,7 +142,10 @@ class Path implements PathProxy { } /// The [FillType] of this path. - final FillType fillType; + FillType get fillType { + assert(_path != null); + return FillType.values[_getFillTypeFn(_path!)]; + } ffi.Pointer<_SkPath>? _path; ffi.Pointer<_PathData>? _pathData; @@ -388,3 +391,9 @@ typedef _destroy_data_type = ffi.Void Function(ffi.Pointer<_PathData>); final _DestroyDataType _destroyDataFn = _dylib.lookupFunction<_destroy_data_type, _DestroyDataType>('DestroyData'); + +typedef _GetFillTypeType = int Function(ffi.Pointer<_SkPath>); +typedef _get_fill_type_type = ffi.Int32 Function(ffi.Pointer<_SkPath>); + +final _GetFillTypeType _getFillTypeFn = + _dylib.lookupFunction<_get_fill_type_type, _GetFillTypeType>('GetFillType'); diff --git a/engine/src/flutter/tools/path_ops/dart/test/path_ops_test.dart b/engine/src/flutter/tools/path_ops/dart/test/path_ops_test.dart index 0aefc35472d..d12dcdf2de2 100644 --- a/engine/src/flutter/tools/path_ops/dart/test/path_ops_test.dart +++ b/engine/src/flutter/tools/path_ops/dart/test/path_ops_test.dart @@ -24,11 +24,17 @@ void main() { PathVerb.moveTo, // Skia inserts a moveTo here. PathVerb.cubicTo, ]); - expect(path.points, [0, 0, 10, 0, 10, 10, 0, 10, 0, 0, 30, 30, 40, 40, 50, 50]); + expect( + path.points, + [0, 0, 10, 0, 10, 10, 0, 10, 0, 0, 30, 30, 40, 40, 50, 50], + ); final SvgPathProxy proxy = SvgPathProxy(); path.replay(proxy); - expect(proxy.toString(), 'M0.0,0.0L10.0,0.0L10.0,10.0L0.0,10.0ZM0.0,0.0C30.0,30.0 40.0,40.0 50.0,50.0'); + expect( + proxy.toString(), + 'M0.0,0.0L10.0,0.0L10.0,10.0L0.0,10.0ZM0.0,0.0C30.0,30.0 40.0,40.0 50.0,50.0', + ); path.dispose(); }); @@ -61,11 +67,11 @@ void main() { ]); expect(intersection.points, [ 34.06542205810547, 128.0, // move - 48.90797424316406, 48.59233856201172, // line - 57.80497360229492, 39.73065185546875, 68.189697265625, 32.3614387512207, 79.66168212890625, 26.885154724121094, // cubic + 48.90797424316406, 48.59233856201172, // line + 57.80497360229492, 39.73065185546875, 68.189697265625, 32.3614387512207, 79.66168212890625, 26.885154724121094, // cubic 151.7936248779297, 58.72270584106445, // line 150.66123962402344, 59.74142837524414, 149.49365234375, 60.752471923828125, 148.32867431640625, 61.76123809814453, // cubic - 132.3506317138672, 75.59684753417969, 116.86703491210938, 89.0042953491211, 199.52090454101562, 115.93260192871094, // cubic + 132.3506317138672, 75.59684753417969, 116.86703491210938, 89.0042953491211, 199.52090454101562, 115.93260192871094, // cubic 199.36000061035156, 128.0, // line 34.06542205810547, 128.0, // line // close @@ -74,4 +80,36 @@ void main() { quad.dispose(); intersection.dispose(); }); + + test('Ops where fill type changes', () { + final Path a = Path(FillType.evenOdd) + ..moveTo(9.989999771118164, 20.0) + ..cubicTo(4.46999979019165, 20.0, 0.0, 15.520000457763672, 0.0, 10.0) + ..cubicTo(0.0, 4.480000019073486, 4.46999979019165, 0.0, 9.989999771118164, 0.0) + ..cubicTo(15.520000457763672, 0.0, 20.0, 4.480000019073486, 20.0, 10.0) + ..cubicTo(20.0, 15.520000457763672, 15.520000457763672, 20.0, 9.989999771118164, 20.0) + ..close() + ..moveTo(10.0, 18.0) + ..cubicTo(5.579999923706055, 18.0, 2.0, 14.420000076293945, 2.0, 10.0) + ..cubicTo(2.0, 5.579999923706055, 5.579999923706055, 2.0, 10.0, 2.0) + ..cubicTo(14.420000076293945, 2.0, 18.0, 5.579999923706055, 18.0, 10.0) + ..cubicTo(18.0, 14.420000076293945, 14.420000076293945, 18.0, 10.0, 18.0) + ..close() + ..moveTo(11.0, 5.0) + ..lineTo(11.0, 11.0) + ..lineTo(9.0, 11.0) + ..lineTo(9.0, 5.0) + ..lineTo(11.0, 5.0) + ..close() + ..moveTo(11.0, 13.0) + ..lineTo(11.0, 15.0) + ..lineTo(9.0, 15.0) + ..lineTo(9.0, 13.0) + ..lineTo(11.0, 13.0) + ..close(); + final Path b = Path()..moveTo(0, 0)..lineTo(0, 20)..lineTo(20, 20)..lineTo(20, 0)..close(); + + final Path intersection = a.applyOp(b, PathOp.intersect); + expect(intersection.fillType, a.fillType); + }); } diff --git a/engine/src/flutter/tools/path_ops/path_ops.cc b/engine/src/flutter/tools/path_ops/path_ops.cc index 78b52eabe86..be27e213cdf 100644 --- a/engine/src/flutter/tools/path_ops/path_ops.cc +++ b/engine/src/flutter/tools/path_ops/path_ops.cc @@ -45,6 +45,10 @@ void Op(SkPath* one, SkPath* two, SkPathOp op) { Op(*one, *two, op, one); } +int GetFillType(SkPath* path) { + return static_cast(path->getFillType()); +} + struct PathData* Data(SkPath* path) { int point_count = path->countPoints(); int verb_count = path->countVerbs(); diff --git a/engine/src/flutter/tools/path_ops/path_ops.h b/engine/src/flutter/tools/path_ops/path_ops.h index ec393e303c1..2565055a362 100644 --- a/engine/src/flutter/tools/path_ops/path_ops.h +++ b/engine/src/flutter/tools/path_ops/path_ops.h @@ -43,6 +43,8 @@ API void DestroyPath(SkPath* path); API void Op(SkPath* one, SkPath* two, SkPathOp op); +API int GetFillType(SkPath* path); + struct API PathData { uint8_t* verbs; size_t verb_count;