From 9ba500236be4d9dec0dff7e72ee0edd95bd251ab Mon Sep 17 00:00:00 2001 From: Jackson Gardner Date: Thu, 28 Mar 2024 17:40:43 -0700 Subject: [PATCH] [skwasm] Fix `toString` methods on Paint and ImageFilter/ColorFilter This fixes https://github.com/flutter/flutter/issues/141639 Most of this was previously unimplemented. It turns out the reason for the hang described in the github issue was that there was a typo in the name of the `getMiterLimit` C function, so if the client actually called that method the Wasm module failed to compile, as it couldn't find an import with the misspelled name. --- .../engine/skwasm/skwasm_impl/filters.dart | 182 +++++++++++++++--- .../src/engine/skwasm/skwasm_impl/paint.dart | 79 +++++++- lib/web_ui/skwasm/paint.cpp | 2 +- lib/web_ui/test/ui/paint_test.dart | 32 ++- 4 files changed, 244 insertions(+), 51 deletions(-) diff --git a/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/filters.dart b/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/filters.dart index adc8026ef1d5b..d85668e61f374 100644 --- a/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/filters.dart +++ b/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/filters.dart @@ -2,42 +2,38 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:ffi'; import 'dart:typed_data'; import 'package:ui/src/engine.dart'; import 'package:ui/src/engine/skwasm/skwasm_impl.dart'; import 'package:ui/ui.dart' as ui; -class SkwasmImageFilter extends SkwasmObjectWrapper implements SceneImageFilter { - SkwasmImageFilter._(ImageFilterHandle handle) : super(handle, _registry); +abstract class SkwasmImageFilter extends SkwasmObjectWrapper implements SceneImageFilter { + SkwasmImageFilter(ImageFilterHandle handle) : super(handle, _registry); factory SkwasmImageFilter.blur({ double sigmaX = 0.0, double sigmaY = 0.0, ui.TileMode tileMode = ui.TileMode.clamp, - }) => SkwasmImageFilter._(imageFilterCreateBlur(sigmaX, sigmaY, tileMode.index)); + }) => SkwasmBlurFilter(sigmaX, sigmaY, tileMode); factory SkwasmImageFilter.dilate({ double radiusX = 0.0, double radiusY = 0.0, - }) => SkwasmImageFilter._(imageFilterCreateDilate(radiusX, radiusY)); + }) => SkwasmDilateFilter(radiusX, radiusY); factory SkwasmImageFilter.erode({ double radiusX = 0.0, double radiusY = 0.0, - }) => SkwasmImageFilter._(imageFilterCreateErode(radiusX, radiusY)); + }) => SkwasmErodeFilter(radiusX, radiusY); factory SkwasmImageFilter.matrix( Float64List matrix4, { ui.FilterQuality filterQuality = ui.FilterQuality.low - }) => withStackScope((StackScope scope) => SkwasmImageFilter._(imageFilterCreateMatrix( - scope.convertMatrix4toSkMatrix(matrix4), - filterQuality.index - ))); + }) => SkwasmMatrixFilter(matrix4, filterQuality); factory SkwasmImageFilter.fromColorFilter(SkwasmColorFilter filter) => - SkwasmImageFilter._(imageFilterCreateFromColorFilter(filter.handle)); + SkwasmColorImageFilter(filter); factory SkwasmImageFilter.fromUiFilter(ui.ImageFilter filter) { if (filter is ui.ColorFilter) { @@ -54,11 +50,10 @@ class SkwasmImageFilter extends SkwasmObjectWrapper implements S factory SkwasmImageFilter.compose( ui.ImageFilter outer, ui.ImageFilter inner, - ) { - final SkwasmImageFilter nativeOuter = SkwasmImageFilter.fromUiFilter(outer); - final SkwasmImageFilter nativeInner = SkwasmImageFilter.fromUiFilter(inner); - return SkwasmImageFilter._(imageFilterCompose(nativeOuter.handle, nativeInner.handle)); - } + ) => SkwasmComposedImageFilter( + SkwasmImageFilter.fromUiFilter(outer), + SkwasmImageFilter.fromUiFilter(inner), + ); static final SkwasmFinalizationRegistry _registry = SkwasmFinalizationRegistry(imageFilterDispose); @@ -71,32 +66,161 @@ class SkwasmImageFilter extends SkwasmObjectWrapper implements S }); } -class SkwasmColorFilter extends SkwasmObjectWrapper { - SkwasmColorFilter._(ColorFilterHandle handle) : super(handle, _registry); +class SkwasmBlurFilter extends SkwasmImageFilter { + SkwasmBlurFilter( + this.sigmaX, + this.sigmaY, + this.tileMode, + ) : super(imageFilterCreateBlur(sigmaX, sigmaY, tileMode.index)); + + final double sigmaX; + final double sigmaY; + ui.TileMode tileMode; + + @override + String toString() => 'ImageFilter.blur($sigmaX, $sigmaY, ${tileModeString(tileMode)})'; +} + +class SkwasmDilateFilter extends SkwasmImageFilter { + SkwasmDilateFilter( + this.radiusX, + this.radiusY, + ) : super(imageFilterCreateDilate(radiusX, radiusY)); + + final double radiusX; + final double radiusY; + + @override + String toString() => 'ImageFilter.dilate($radiusX, $radiusY)'; +} + +class SkwasmErodeFilter extends SkwasmImageFilter { + SkwasmErodeFilter( + this.radiusX, + this.radiusY, + ) : super(imageFilterCreateErode(radiusX, radiusY)); + + final double radiusX; + final double radiusY; + + @override + String toString() => 'ImageFilter.erode($radiusX, $radiusY)'; +} + +class SkwasmMatrixFilter extends SkwasmImageFilter { + SkwasmMatrixFilter( + this.matrix4, + this.filterQuality, + ) : super(withStackScope((StackScope scope) => imageFilterCreateMatrix( + scope.convertMatrix4toSkMatrix(matrix4), + filterQuality.index, + ))); + + final Float64List matrix4; + final ui.FilterQuality filterQuality; + + @override + String toString() => 'ImageFilter.matrix($matrix4, $filterQuality)'; +} + +class SkwasmColorImageFilter extends SkwasmImageFilter { + SkwasmColorImageFilter( + this.filter, + ) : super(imageFilterCreateFromColorFilter(filter.handle)); + + final SkwasmColorFilter filter; + + @override + String toString() => filter.toString(); +} + +class SkwasmComposedImageFilter extends SkwasmImageFilter { + SkwasmComposedImageFilter( + this.outer, + this.inner, + ) : super(imageFilterCompose(outer.handle, inner.handle)); + + final SkwasmImageFilter outer; + final SkwasmImageFilter inner; + + @override + String toString() => 'ImageFilter.compose($outer, $inner)'; +} + +abstract class SkwasmColorFilter extends SkwasmObjectWrapper { + SkwasmColorFilter(ColorFilterHandle handle) : super(handle, _registry); factory SkwasmColorFilter.fromEngineColorFilter(EngineColorFilter colorFilter) => switch (colorFilter.type) { - ColorFilterType.mode => SkwasmColorFilter._(colorFilterCreateMode( - colorFilter.color!.value, - colorFilter.blendMode!.index, - )), - ColorFilterType.linearToSrgbGamma => SkwasmColorFilter._(colorFilterCreateLinearToSRGBGamma()), - ColorFilterType.srgbToLinearGamma => SkwasmColorFilter._(colorFilterCreateSRGBToLinearGamma()), - ColorFilterType.matrix => withStackScope((StackScope scope) { - final Pointer nativeMatrix = scope.convertDoublesToNative(colorFilter.matrix!); - return SkwasmColorFilter._(colorFilterCreateMatrix(nativeMatrix)); - }), + ColorFilterType.mode => SkwasmModeColorFilter(colorFilter.color!, colorFilter.blendMode!), + ColorFilterType.linearToSrgbGamma => SkwasmLinearToSrgbGammaColorFilter(), + ColorFilterType.srgbToLinearGamma => SkwasmSrgbToLinearGammaColorFilter(), + ColorFilterType.matrix => SkwasmMatrixColorFilter(colorFilter.matrix!), }; factory SkwasmColorFilter.composed( SkwasmColorFilter outer, SkwasmColorFilter inner, - ) => SkwasmColorFilter._(colorFilterCompose(outer.handle, inner.handle)); + ) => SkwasmComposedColorFilter(outer, inner); static final SkwasmFinalizationRegistry _registry = SkwasmFinalizationRegistry(colorFilterDispose); } +class SkwasmModeColorFilter extends SkwasmColorFilter { + SkwasmModeColorFilter( + this.color, + this.blendMode, + ) : super(colorFilterCreateMode( + color.value, + blendMode.index, + )); + + final ui.Color color; + final ui.BlendMode blendMode; + + @override + String toString() => 'ColorFilter.mode($color, $blendMode)'; +} + +class SkwasmLinearToSrgbGammaColorFilter extends SkwasmColorFilter { + SkwasmLinearToSrgbGammaColorFilter() : super(colorFilterCreateLinearToSRGBGamma()); + + @override + String toString() => 'ColorFilter.linearToSrgbGamma()'; +} + +class SkwasmSrgbToLinearGammaColorFilter extends SkwasmColorFilter { + SkwasmSrgbToLinearGammaColorFilter() : super(colorFilterCreateSRGBToLinearGamma()); + + @override + String toString() => 'ColorFilter.srgbToLinearGamma()'; +} + +class SkwasmMatrixColorFilter extends SkwasmColorFilter { + SkwasmMatrixColorFilter(this.matrix) : super(withStackScope((StackScope scope) => + colorFilterCreateMatrix(scope.convertDoublesToNative(matrix)) + )); + + final List matrix; + + @override + String toString() => 'ColorFilter.matrix($matrix)'; +} + +class SkwasmComposedColorFilter extends SkwasmColorFilter { + SkwasmComposedColorFilter( + this.outer, + this.inner, + ) : super(colorFilterCompose(outer.handle, inner.handle)); + + final SkwasmColorFilter outer; + final SkwasmColorFilter inner; + + @override + String toString() => 'ColorFilter.compose($outer, $inner)'; +} + class SkwasmMaskFilter extends SkwasmObjectWrapper { SkwasmMaskFilter._(MaskFilterHandle handle) : super(handle, _registry); diff --git a/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/paint.dart b/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/paint.dart index 26e91164134df..507bf94c63ca6 100644 --- a/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/paint.dart +++ b/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/paint.dart @@ -11,6 +11,15 @@ import 'package:ui/ui.dart' as ui; class SkwasmPaint extends SkwasmObjectWrapper implements ui.Paint { SkwasmPaint() : super(paintCreate(), _registry); + // Must be kept in sync with the default in paint.cc. + static const double _kStrokeMiterLimitDefault = 4.0; + + // Must be kept in sync with the default in paint.cc. + static const int _kColorDefault = 0xFF000000; + + // Must be kept in sync with the default in paint.cc. + static final int _kBlendModeDefault = ui.BlendMode.srcOver.index; + static final SkwasmFinalizationRegistry _registry = SkwasmFinalizationRegistry(paintDispose); @@ -173,7 +182,73 @@ class SkwasmPaint extends SkwasmObjectWrapper implements ui.Paint { _setEffectiveColorFilter(); } - // TODO(yjbanov): https://github.com/flutter/flutter/issues/141639 @override - String toString() => 'Paint()'; + String toString() { + String resultString = 'Paint()'; + + assert(() { + final StringBuffer result = StringBuffer(); + String semicolon = ''; + result.write('Paint('); + if (style == ui.PaintingStyle.stroke) { + result.write('$style'); + if (strokeWidth != 0.0) { + result.write(' ${strokeWidth.toStringAsFixed(1)}'); + } else { + result.write(' hairline'); + } + if (strokeCap != ui.StrokeCap.butt) { + result.write(' $strokeCap'); + } + if (strokeJoin == ui.StrokeJoin.miter) { + if (strokeMiterLimit != _kStrokeMiterLimitDefault) { + result.write(' $strokeJoin up to ${strokeMiterLimit.toStringAsFixed(1)}'); + } + } else { + result.write(' $strokeJoin'); + } + semicolon = '; '; + } + if (!isAntiAlias) { + result.write('${semicolon}antialias off'); + semicolon = '; '; + } + if (color != const ui.Color(_kColorDefault)) { + result.write('$semicolon$color'); + semicolon = '; '; + } + if (blendMode.index != _kBlendModeDefault) { + result.write('$semicolon$blendMode'); + semicolon = '; '; + } + if (colorFilter != null) { + result.write('${semicolon}colorFilter: $colorFilter'); + semicolon = '; '; + } + if (maskFilter != null) { + result.write('${semicolon}maskFilter: $maskFilter'); + semicolon = '; '; + } + if (filterQuality != ui.FilterQuality.none) { + result.write('${semicolon}filterQuality: $filterQuality'); + semicolon = '; '; + } + if (shader != null) { + result.write('${semicolon}shader: $shader'); + semicolon = '; '; + } + if (imageFilter != null) { + result.write('${semicolon}imageFilter: $imageFilter'); + semicolon = '; '; + } + if (invertColors) { + result.write('${semicolon}invert: $invertColors'); + } + result.write(')'); + resultString = result.toString(); + return true; + }()); + + return resultString; + } } diff --git a/lib/web_ui/skwasm/paint.cpp b/lib/web_ui/skwasm/paint.cpp index d75264dad42a8..8bba9a5f8a819 100644 --- a/lib/web_ui/skwasm/paint.cpp +++ b/lib/web_ui/skwasm/paint.cpp @@ -82,7 +82,7 @@ SKWASM_EXPORT void paint_setMiterLimit(SkPaint* paint, SkScalar miterLimit) { paint->setStrokeMiter(miterLimit); } -SKWASM_EXPORT SkScalar paint_getMiterLImit(SkPaint* paint) { +SKWASM_EXPORT SkScalar paint_getMiterLimit(SkPaint* paint) { return paint->getStrokeMiter(); } diff --git a/lib/web_ui/test/ui/paint_test.dart b/lib/web_ui/test/ui/paint_test.dart index 4518d08c4d3ec..2c34c77c020c6 100644 --- a/lib/web_ui/test/ui/paint_test.dart +++ b/lib/web_ui/test/ui/paint_test.dart @@ -7,7 +7,6 @@ import 'package:test/test.dart'; import 'package:ui/ui.dart' as ui; import '../common/test_initialization.dart'; -import 'utils.dart'; void main() { internalBootstrapBrowserTest(() => testMain); @@ -49,23 +48,18 @@ Future testMain() async { tileMode: ui.TileMode.mirror, ); - if (!isSkwasm) { - expect( - paint.toString(), - 'Paint(' - 'Color(0xaabbccdd); ' - 'BlendMode.darken; ' - 'colorFilter: ColorFilter.linearToSrgbGamma(); ' - 'maskFilter: MaskFilter.blur(BlurStyle.normal, 1.7); ' - 'filterQuality: FilterQuality.high; ' - 'shader: Gradient(); ' - 'imageFilter: ImageFilter.blur(1.9, 2.1, mirror); ' - 'invert: true' - ')', - ); - } else { - // TODO(yjbanov): https://github.com/flutter/flutter/issues/141639 - expect(paint.toString(), 'Paint()'); - } + expect( + paint.toString(), + 'Paint(' + 'Color(0xaabbccdd); ' + 'BlendMode.darken; ' + 'colorFilter: ColorFilter.linearToSrgbGamma(); ' + 'maskFilter: MaskFilter.blur(BlurStyle.normal, 1.7); ' + 'filterQuality: FilterQuality.high; ' + 'shader: Gradient(); ' + 'imageFilter: ImageFilter.blur(1.9, 2.1, mirror); ' + 'invert: true' + ')', + ); }); }