From 1e9202e1ee1f3f2bc365045a938ea5ca403b0a7a Mon Sep 17 00:00:00 2001 From: Jackson Gardner Date: Wed, 8 Jan 2025 12:39:21 -0800 Subject: [PATCH] Normalize the translation column of the color matrix. (#161109) `dart:ui` specifies that the translation column of the color matrix should be in the range 0..255. Skia expects a normalized range between 0 and 1. Skwasm was not normalizing this before passing it to Skia, but CanvasKit was. After writing a unit test, it appears that the HTML renderer has the same problem, so I also fixed that as well. This addresses https://github.com/flutter/flutter/issues/159697 --- .../lib/src/engine/html/color_filter.dart | 13 +++- .../engine/skwasm/skwasm_impl/filters.dart | 14 ++++- .../lib/web_ui/test/ui/filters_test.dart | 60 +++++++------------ 3 files changed, 45 insertions(+), 42 deletions(-) diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/html/color_filter.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/html/color_filter.dart index eb04b40bc1..c575fb90bb 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/html/color_filter.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/html/color_filter.dart @@ -312,7 +312,18 @@ class SvgFilter { SvgFilter svgFilterFromColorMatrix(List matrix) { final SvgFilterBuilder builder = SvgFilterBuilder(); - builder.setFeColorMatrix(matrix, result: 'comp'); + + /// Flutter documentation says the translation column of the color matrix + /// is specified in unnormalized 0..255 space. `feColorMatrix` expects the + /// translation values to be normalized to 0..1 space. + /// + /// See [https://api.flutter.dev/flutter/dart-ui/ColorFilter/ColorFilter.matrix.html] + final normalizedMatrix = List.generate( + matrix.length, + (int i) => (i % 5 == 4) ? matrix[i] / 255.0 : matrix[i], + growable: false, + ); + builder.setFeColorMatrix(normalizedMatrix, result: 'comp'); return builder.build(); } diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/filters.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/filters.dart index a49aef28a5..74415a6e0d 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/filters.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/skwasm/skwasm_impl/filters.dart @@ -2,6 +2,7 @@ // 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'; @@ -318,7 +319,18 @@ class SkwasmMatrixColorFilter extends SkwasmColorFilter { @override void withRawColorFilter(ColorFilterHandleBorrow borrow) { withStackScope((scope) { - final rawColorFilter = colorFilterCreateMatrix(scope.convertDoublesToNative(matrix)); + assert(matrix.length == 20); + final Pointer rawMatrix = scope.convertDoublesToNative(matrix); + + /// Flutter documentation says the translation column of the color matrix + /// is specified in unnormalized 0..255 space. Skia expects the + /// translation values to be normalized to 0..1 space. + /// + /// See [https://api.flutter.dev/flutter/dart-ui/ColorFilter/ColorFilter.matrix.html]. + for (final i in [4, 9, 14, 19]) { + rawMatrix[i] /= 255.0; + } + final rawColorFilter = colorFilterCreateMatrix(rawMatrix); borrow(rawColorFilter); colorFilterDispose(rawColorFilter); }); diff --git a/engine/src/flutter/lib/web_ui/test/ui/filters_test.dart b/engine/src/flutter/lib/web_ui/test/ui/filters_test.dart index 3a736c9888..8b9a69d7b9 100644 --- a/engine/src/flutter/lib/web_ui/test/ui/filters_test.dart +++ b/engine/src/flutter/lib/web_ui/test/ui/filters_test.dart @@ -146,32 +146,28 @@ Future testMain() async { test('matrix color filter', () async { const ui.ColorFilter sepia = ui.ColorFilter.matrix([ - 0.393, - 0.769, - 0.189, - 0, - 0, - 0.349, - 0.686, - 0.168, - 0, - 0, - 0.272, - 0.534, - 0.131, - 0, - 0, - 0, - 0, - 0, - 1, - 0, + 0.393, 0.769, 0.189, 0, 0, // row + 0.349, 0.686, 0.168, 0, 0, // row + 0.272, 0.534, 0.131, 0, 0, // row + 0, 0, 0, 1, 0, // row ]); await drawTestImageWithPaint(ui.Paint()..colorFilter = sepia); await matchGoldenFile('ui_filter_matrix_colorfilter.png', region: region); expect(sepia.toString(), startsWith('ColorFilter.matrix([0.393, 0.769, 0.189, ')); }); + test('matrix color filter with 0..255 translation values', () async { + const ui.ColorFilter sepia = ui.ColorFilter.matrix([ + 0.393, 0.769, 0.189, 0, 50.0, // row + 0.349, 0.686, 0.168, 0, 50.0, // row + 0.272, 0.534, 0.131, 0, 50.0, // row + 0, 0, 0, 1, 0, // row + ]); + await drawTestImageWithPaint(ui.Paint()..colorFilter = sepia); + await matchGoldenFile('ui_filter_matrix_colorfilter_with_translation.png', region: region); + expect(sepia.toString(), startsWith('ColorFilter.matrix([0.393, 0.769, 0.189, ')); + }); + test('invert colors', () async { await drawTestImageWithPaint(ui.Paint()..invertColors = true); await matchGoldenFile('ui_filter_invert_colors.png', region: region); @@ -179,26 +175,10 @@ Future testMain() async { test('invert colors with color filter', () async { const ui.ColorFilter sepia = ui.ColorFilter.matrix([ - 0.393, - 0.769, - 0.189, - 0, - 0, - 0.349, - 0.686, - 0.168, - 0, - 0, - 0.272, - 0.534, - 0.131, - 0, - 0, - 0, - 0, - 0, - 1, - 0, + 0.393, 0.769, 0.189, 0, 0, // row + 0.349, 0.686, 0.168, 0, 0, // row + 0.272, 0.534, 0.131, 0, 0, // row + 0, 0, 0, 1, 0, // row ]); await drawTestImageWithPaint(