From 36ca48a23e14f6679ee7763b3b18acaaec39dd3d Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 5 Sep 2024 14:22:35 -0700 Subject: [PATCH 01/15] Makes Color take advantage of the wider bit depth. --- lib/ui/painting.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 202f4991d352f..6c8da22786152 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -311,9 +311,9 @@ class Color { /// See . double computeLuminance() { // See - final double R = _linearizeColorComponent(red / 0xFF); - final double G = _linearizeColorComponent(green / 0xFF); - final double B = _linearizeColorComponent(blue / 0xFF); + final double R = _linearizeColorComponent(r); + final double G = _linearizeColorComponent(g); + final double B = _linearizeColorComponent(b); return 0.2126 * R + 0.7152 * G + 0.0722 * B; } From 66f8393381928cfbdcb9cb180cbd96e188452ca1 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 5 Sep 2024 14:46:30 -0700 Subject: [PATCH 02/15] updated lerp --- lib/ui/painting.dart | 36 +++++++++++++++++------------------- testing/dart/color_test.dart | 15 ++++++++++++++- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 6c8da22786152..daf8302f35cf0 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -48,8 +48,8 @@ bool _radiusIsValid(Radius radius) { return true; } -Color _scaleAlpha(Color a, double factor) { - return a.withAlpha((a.alpha * factor).round().clamp(0, 255)); +Color _scaleAlpha(Color x, double factor) { + return x.withValues(alpha: (x.a * factor).clamp(0, 1)); } /// An immutable 32 bit color value in ARGB format. @@ -339,28 +339,26 @@ class Color { /// /// Values for `t` are usually obtained from an [Animation], such as /// an [AnimationController]. - static Color? lerp(Color? a, Color? b, double t) { - // TODO(gaaclarke): Update math to use floats. This was already attempted - // but it leads to subtle changes that change test results. - assert(a?.colorSpace != ColorSpace.extendedSRGB); - assert(b?.colorSpace != ColorSpace.extendedSRGB); - if (b == null) { - if (a == null) { + static Color? lerp(Color? x, Color? y, double t) { + assert(x?.colorSpace != ColorSpace.extendedSRGB); + assert(y?.colorSpace != ColorSpace.extendedSRGB); + if (y == null) { + if (x == null) { return null; } else { - return _scaleAlpha(a, 1.0 - t); + return _scaleAlpha(x, 1.0 - t); } } else { - if (a == null) { - return _scaleAlpha(b, t); + if (x == null) { + return _scaleAlpha(y, t); } else { - assert(a.colorSpace == b.colorSpace); - return Color._fromARGBC( - _clampInt(_lerpInt(a.alpha, b.alpha, t).toInt(), 0, 255), - _clampInt(_lerpInt(a.red, b.red, t).toInt(), 0, 255), - _clampInt(_lerpInt(a.green, b.green, t).toInt(), 0, 255), - _clampInt(_lerpInt(a.blue, b.blue, t).toInt(), 0, 255), - a.colorSpace, + assert(x.colorSpace == y.colorSpace); + return Color.from( + alpha: _lerpDouble(x.a, y.a, t).clamp(0, 1), + red: _lerpDouble(x.r, y.r, t).clamp(0, 1), + green: _lerpDouble(x.g, y.g, t).clamp(0, 1), + blue: _lerpDouble(x.b, y.b, t).clamp(0, 1), + colorSpace: x.colorSpace, ); } } diff --git a/testing/dart/color_test.dart b/testing/dart/color_test.dart index 56e8766f103b2..dc6b8cae509fb 100644 --- a/testing/dart/color_test.dart +++ b/testing/dart/color_test.dart @@ -6,6 +6,19 @@ import 'dart:ui'; import 'package:litetest/litetest.dart'; +/// Positive result when the Colors will map to the same argb8888 color. +Matcher colorMatches(dynamic o) => (v) { + Expect.isTrue(o is Color); + Expect.isTrue(v is Color); + if (o is Color && v is Color) { + Expect.equals(o.colorSpace, v.colorSpace); + Expect.isTrue((o.a - v.a).abs() <= (1 / 255)); + Expect.isTrue((o.r - v.r).abs() <= (1 / 255)); + Expect.isTrue((o.g - v.g).abs() <= (1 / 255)); + Expect.isTrue((o.b - v.b).abs() <= (1 / 255)); + } +}; + class NotAColor extends Color { const NotAColor(super.value); } @@ -58,7 +71,7 @@ void main() { ); expect( Color.lerp(const Color(0x00000000), const Color(0xFFFFFFFF), 0.5), - const Color(0x7F7F7F7F), + colorMatches(const Color(0x7F7F7F7F)), ); expect( Color.lerp(const Color(0x00000000), const Color(0xFFFFFFFF), 1.0), From 138f388ee12a884fa91a740669a49813febd9ec3 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 5 Sep 2024 15:08:42 -0700 Subject: [PATCH 03/15] migrated alphaBlend --- lib/ui/painting.dart | 42 +++++++++++++++++------------------- testing/dart/color_test.dart | 12 +++++------ 2 files changed, 26 insertions(+), 28 deletions(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index daf8302f35cf0..c13c743816a68 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -375,32 +375,30 @@ class Color { static Color alphaBlend(Color foreground, Color background) { assert(foreground.colorSpace == background.colorSpace); assert(foreground.colorSpace != ColorSpace.extendedSRGB); - // TODO(gaaclarke): Update math to use floats. This was already attempted - // but it leads to subtle changes that change test results. - final int alpha = foreground.alpha; - if (alpha == 0x00) { // Foreground completely transparent. + final double alpha = foreground.a; + if (alpha == 0) { // Foreground completely transparent. return background; } - final int invAlpha = 0xff - alpha; - int backAlpha = background.alpha; - if (backAlpha == 0xff) { // Opaque background case - return Color._fromARGBC( - 0xff, - (alpha * foreground.red + invAlpha * background.red) ~/ 0xff, - (alpha * foreground.green + invAlpha * background.green) ~/ 0xff, - (alpha * foreground.blue + invAlpha * background.blue) ~/ 0xff, - foreground.colorSpace, + final double invAlpha = 1 - alpha; + double backAlpha = background.a; + if (backAlpha == 1) { // Opaque background case + return Color.from( + alpha: 1, + red: alpha * foreground.r + invAlpha * background.r, + green: alpha * foreground.g + invAlpha * background.g, + blue: alpha * foreground.b + invAlpha * background.b, + colorSpace: foreground.colorSpace, ); } else { // General case - backAlpha = (backAlpha * invAlpha) ~/ 0xff; - final int outAlpha = alpha + backAlpha; - assert(outAlpha != 0x00); - return Color._fromARGBC( - outAlpha, - (foreground.red * alpha + background.red * backAlpha) ~/ outAlpha, - (foreground.green * alpha + background.green * backAlpha) ~/ outAlpha, - (foreground.blue * alpha + background.blue * backAlpha) ~/ outAlpha, - foreground.colorSpace, + backAlpha = backAlpha * invAlpha; + final double outAlpha = alpha + backAlpha; + assert(outAlpha != 0); + return Color.from( + alpha: outAlpha, + red: (foreground.r * alpha + background.r * backAlpha) / outAlpha, + green: (foreground.g * alpha + background.g * backAlpha) / outAlpha, + blue: (foreground.b * alpha + background.b * backAlpha) / outAlpha, + colorSpace: foreground.colorSpace, ); } } diff --git a/testing/dart/color_test.dart b/testing/dart/color_test.dart index dc6b8cae509fb..b81c0f0f48e3c 100644 --- a/testing/dart/color_test.dart +++ b/testing/dart/color_test.dart @@ -12,10 +12,10 @@ Matcher colorMatches(dynamic o) => (v) { Expect.isTrue(v is Color); if (o is Color && v is Color) { Expect.equals(o.colorSpace, v.colorSpace); - Expect.isTrue((o.a - v.a).abs() <= (1 / 255)); - Expect.isTrue((o.r - v.r).abs() <= (1 / 255)); - Expect.isTrue((o.g - v.g).abs() <= (1 / 255)); - Expect.isTrue((o.b - v.b).abs() <= (1 / 255)); + Expect.approxEquals(o.a, v.a, 1 / 255); + Expect.approxEquals(o.r, v.r, 1 / 255); + Expect.approxEquals(o.g, v.g, 1 / 255); + Expect.approxEquals(o.b, v.b, 1 / 255); } }; @@ -173,11 +173,11 @@ void main() { ); expect( Color.alphaBlend(const Color(0x11223344), const Color(0xFF000000)), - const Color(0xFF020304), + colorMatches(const Color(0xFF020304)), ); expect( Color.alphaBlend(const Color(0x11223344), const Color(0x80000000)), - const Color(0x88040608), + colorMatches(const Color(0x88040608)), ); }); From 7493253d043136f3c917ff7b584b9d0c1ae50661 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 5 Sep 2024 15:23:10 -0700 Subject: [PATCH 04/15] moved equality and toString --- lib/ui/painting.dart | 11 +++++++---- testing/dart/color_test.dart | 8 ++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index c13c743816a68..2d72569d53107 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -419,16 +419,19 @@ class Color { return false; } return other is Color && - other.value == value && + other.a == a && + other.r == r && + other.g == g && + other.b == b && other.colorSpace == colorSpace; } @override - int get hashCode => Object.hash(value, colorSpace); + int get hashCode => Object.hash(a, r, g, b, colorSpace); - // TODO(gaaclarke): Make toString() print out float values. @override - String toString() => 'Color(0x${value.toRadixString(16).padLeft(8, '0')})'; + String toString() => + 'Color(alpha: ${a.toStringAsFixed(4)}, red: ${r.toStringAsFixed(4)}, green: ${g.toStringAsFixed(4)}, blue: ${b.toStringAsFixed(4)}, colorSpace: $colorSpace)'; } /// Algorithms to use when painting on the canvas. diff --git a/testing/dart/color_test.dart b/testing/dart/color_test.dart index b81c0f0f48e3c..f686bf48f8488 100644 --- a/testing/dart/color_test.dart +++ b/testing/dart/color_test.dart @@ -40,7 +40,7 @@ void main() { const Color c = Color(0x00000000); final Paint p = Paint(); p.color = c; - expect(c.toString(), equals('Color(0x00000000)')); + expect(c, equals(const Color(0x00000000))); }); test('color created with out of bounds value', () { @@ -161,15 +161,15 @@ void main() { ); expect( Color.alphaBlend(const Color(0x80808080), const Color(0xFFFFFFFF)), - const Color(0xFFBFBFBF), + colorMatches(const Color(0xFFBFBFBF)), ); expect( Color.alphaBlend(const Color(0x80808080), const Color(0xFF000000)), - const Color(0xFF404040), + colorMatches(const Color(0xFF404040)), ); expect( Color.alphaBlend(const Color(0x01020304), const Color(0xFF000000)), - const Color(0xFF000000), + colorMatches(const Color(0xFF000000)), ); expect( Color.alphaBlend(const Color(0x11223344), const Color(0xFF000000)), From 745a8a77883fa903e999dbc56cdaa2362ab0e232 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 6 Sep 2024 09:48:46 -0700 Subject: [PATCH 05/15] brought over web changes --- lib/web_ui/lib/painting.dart | 92 +++++++++++++++++++----------------- 1 file changed, 48 insertions(+), 44 deletions(-) diff --git a/lib/web_ui/lib/painting.dart b/lib/web_ui/lib/painting.dart index 113cabadca1e9..7bdd746633328 100644 --- a/lib/web_ui/lib/painting.dart +++ b/lib/web_ui/lib/painting.dart @@ -17,8 +17,8 @@ void _validateColorStops(List colors, List? colorStops) { } } -Color _scaleAlpha(Color a, double factor) { - return a.withAlpha(engine.clampInt((a.alpha * factor).round(), 0, 255)); +Color _scaleAlpha(Color x, double factor) { + return x.withValues(alpha: (x.a * factor).clamp(0, 1)); } class Color { @@ -141,32 +141,32 @@ class Color { double computeLuminance() { // See - final double R = _linearizeColorComponent(red / 0xFF); - final double G = _linearizeColorComponent(green / 0xFF); - final double B = _linearizeColorComponent(blue / 0xFF); + final double R = _linearizeColorComponent(r); + final double G = _linearizeColorComponent(g); + final double B = _linearizeColorComponent(b); return 0.2126 * R + 0.7152 * G + 0.0722 * B; } - static Color? lerp(Color? a, Color? b, double t) { - assert(a?.colorSpace != ColorSpace.extendedSRGB); - assert(b?.colorSpace != ColorSpace.extendedSRGB); - if (b == null) { - if (a == null) { + static Color? lerp(Color? x, Color? y, double t) { + assert(x?.colorSpace != ColorSpace.extendedSRGB); + assert(y?.colorSpace != ColorSpace.extendedSRGB); + if (y == null) { + if (x == null) { return null; } else { - return _scaleAlpha(a, 1.0 - t); + return _scaleAlpha(x, 1.0 - t); } } else { - if (a == null) { - return _scaleAlpha(b, t); + if (x == null) { + return _scaleAlpha(y, t); } else { - assert(a.colorSpace == b.colorSpace); - return Color._fromARGBC( - engine.clampInt(_lerpInt(a.alpha, b.alpha, t).toInt(), 0, 255), - engine.clampInt(_lerpInt(a.red, b.red, t).toInt(), 0, 255), - engine.clampInt(_lerpInt(a.green, b.green, t).toInt(), 0, 255), - engine.clampInt(_lerpInt(a.blue, b.blue, t).toInt(), 0, 255), - a.colorSpace, + assert(x.colorSpace == y.colorSpace); + return Color.from( + alpha: _lerpDouble(x.a, y.a, t).clamp(0, 1), + red: _lerpDouble(x.r, y.r, t).clamp(0, 1), + green: _lerpDouble(x.g, y.g, t).clamp(0, 1), + blue: _lerpDouble(x.b, y.b, t).clamp(0, 1), + colorSpace: x.colorSpace, ); } } @@ -175,30 +175,30 @@ class Color { static Color alphaBlend(Color foreground, Color background) { assert(foreground.colorSpace == background.colorSpace); assert(foreground.colorSpace != ColorSpace.extendedSRGB); - final int alpha = foreground.alpha; - if (alpha == 0x00) { + final double alpha = foreground.a; + if (alpha == 0) { // Foreground completely transparent. return background; } - final int invAlpha = 0xff - alpha; - int backAlpha = background.alpha; - if (backAlpha == 0xff) { - return Color._fromARGBC( - 0xff, - (alpha * foreground.red + invAlpha * background.red) ~/ 0xff, - (alpha * foreground.green + invAlpha * background.green) ~/ 0xff, - (alpha * foreground.blue + invAlpha * background.blue) ~/ 0xff, - foreground.colorSpace, + final double invAlpha = 1 - alpha; + double backAlpha = background.a; + if (backAlpha == 1) { // Opaque background case + return Color.from( + alpha: 1, + red: alpha * foreground.r + invAlpha * background.r, + green: alpha * foreground.g + invAlpha * background.g, + blue: alpha * foreground.b + invAlpha * background.b, + colorSpace: foreground.colorSpace, ); - } else { - backAlpha = (backAlpha * invAlpha) ~/ 0xff; - final int outAlpha = alpha + backAlpha; - assert(outAlpha != 0x00); - return Color._fromARGBC( - outAlpha, - (foreground.red * alpha + background.red * backAlpha) ~/ outAlpha, - (foreground.green * alpha + background.green * backAlpha) ~/ outAlpha, - (foreground.blue * alpha + background.blue * backAlpha) ~/ outAlpha, - foreground.colorSpace, + } else { // General case + backAlpha = backAlpha * invAlpha; + final double outAlpha = alpha + backAlpha; + assert(outAlpha != 0); + return Color.from( + alpha: outAlpha, + red: (foreground.r * alpha + background.r * backAlpha) / outAlpha, + green: (foreground.g * alpha + background.g * backAlpha) / outAlpha, + blue: (foreground.b * alpha + background.b * backAlpha) / outAlpha, + colorSpace: foreground.colorSpace, ); } } @@ -216,15 +216,19 @@ class Color { return false; } return other is Color && - other.value == value && + other.a == a && + other.r == r && + other.g == g && + other.b == b && other.colorSpace == colorSpace; } @override - int get hashCode => Object.hash(value, colorSpace); + int get hashCode => Object.hash(a, r, g, b, colorSpace); @override - String toString() => 'Color(0x${value.toRadixString(16).padLeft(8, '0')})'; + String toString() => + 'Color(alpha: ${a.toStringAsFixed(4)}, red: ${r.toStringAsFixed(4)}, green: ${g.toStringAsFixed(4)}, blue: ${b.toStringAsFixed(4)}, colorSpace: $colorSpace)'; } enum StrokeCap { From 94e4b231baf581dd0de930eef16917ec5a609fb2 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 6 Sep 2024 10:10:42 -0700 Subject: [PATCH 06/15] fixed fragile "toString" test --- testing/dart/image_filter_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/dart/image_filter_test.dart b/testing/dart/image_filter_test.dart index 0200d6f2d45cd..b454b50b58320 100644 --- a/testing/dart/image_filter_test.dart +++ b/testing/dart/image_filter_test.dart @@ -293,7 +293,7 @@ void main() async { ).toString(), contains( 'matrix([10.0, 0.0, 0.0, 0.0, 0.0, 10.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, -0.0, -0.0, 0.0, 1.0], FilterQuality.low) -> ' - 'ColorFilter.mode(Color(0xffabcdef), BlendMode.color) -> ' + 'ColorFilter.mode(Color(alpha: 1.0000, red: 0.6706, green: 0.8039, blue: 0.9373, colorSpace: ColorSpace.sRGB), BlendMode.color) -> ' 'blur(20.0, 20.0, repeated) -> ' 'blur(30.0, 30.0, mirror)' ), From 2aebceb6d54b1e8a6cef36a00b8c77f2e1cc1d65 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 6 Sep 2024 10:14:53 -0700 Subject: [PATCH 07/15] fixed fragile text test --- testing/dart/text_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/dart/text_test.dart b/testing/dart/text_test.dart index cf141892245f9..8cc7e030d5d56 100644 --- a/testing/dart/text_test.dart +++ b/testing/dart/text_test.dart @@ -80,7 +80,7 @@ void testTextStyle() { ); expect( ts1.toString(), - equals('TextStyle(color: Color(0xff00ff00), decoration: unspecified, decorationColor: unspecified, decorationStyle: unspecified, decorationThickness: unspecified, fontWeight: FontWeight.w800, fontStyle: unspecified, textBaseline: unspecified, fontFamily: unspecified, fontFamilyFallback: unspecified, fontSize: 10.0, letterSpacing: unspecified, wordSpacing: unspecified, height: 100.0x, leadingDistribution: unspecified, locale: unspecified, background: unspecified, foreground: unspecified, shadows: unspecified, fontFeatures: unspecified, fontVariations: unspecified)'), + equals('TextStyle(color: Color(alpha: 1.0000, red: 0.0000, green: 1.0000, blue: 0.0000, colorSpace: ColorSpace.sRGB), decoration: unspecified, decorationColor: unspecified, decorationStyle: unspecified, decorationThickness: unspecified, fontWeight: FontWeight.w800, fontStyle: unspecified, textBaseline: unspecified, fontFamily: unspecified, fontFamilyFallback: unspecified, fontSize: 10.0, letterSpacing: unspecified, wordSpacing: unspecified, height: 100.0x, leadingDistribution: unspecified, locale: unspecified, background: unspecified, foreground: unspecified, shadows: unspecified, fontFeatures: unspecified, fontVariations: unspecified)'), ); expect( ts2.toString(), From 26595ac9e611e7d834d7b541b2a944981936e903 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 6 Sep 2024 10:34:08 -0700 Subject: [PATCH 08/15] removed unused functions --- lib/ui/lerp.dart | 12 ------------ lib/web_ui/lib/lerp.dart | 7 ------- 2 files changed, 19 deletions(-) diff --git a/lib/ui/lerp.dart b/lib/ui/lerp.dart index 5b17fe86756e3..b90e8a7738c51 100644 --- a/lib/ui/lerp.dart +++ b/lib/ui/lerp.dart @@ -37,15 +37,3 @@ double _lerpDouble(double a, double b, double t) { double _lerpInt(int a, int b, double t) { return a + (b - a) * t; } - -/// Same as [num.clamp] but specialized for non-null [int]. -int _clampInt(int value, int min, int max) { - assert(min <= max); - if (value < min) { - return min; - } else if (value > max) { - return max; - } else { - return value; - } -} diff --git a/lib/web_ui/lib/lerp.dart b/lib/web_ui/lib/lerp.dart index ff70c354e308f..80a941f7890e8 100644 --- a/lib/web_ui/lib/lerp.dart +++ b/lib/web_ui/lib/lerp.dart @@ -28,10 +28,3 @@ double? lerpDouble(num? a, num? b, double t) { double _lerpDouble(double a, double b, double t) { return a * (1.0 - t) + b * t; } - -/// Linearly interpolate between two integers. -/// -/// Same as [lerpDouble] but specialized for non-null `int` type. -double _lerpInt(int a, int b, double t) { - return a * (1.0 - t) + b * t; -} From aa290dbbe33ef5782ade4933496f4eac6260b3ca Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Fri, 6 Sep 2024 11:04:19 -0700 Subject: [PATCH 09/15] fixed front end test, moved tests to abstract away color.tostring --- flutter_frontend_server/test/to_string_test.dart | 3 ++- testing/dart/image_filter_test.dart | 2 +- testing/dart/text_test.dart | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/flutter_frontend_server/test/to_string_test.dart b/flutter_frontend_server/test/to_string_test.dart index 92a0a2a554584..9dbaea562bf40 100644 --- a/flutter_frontend_server/test/to_string_test.dart +++ b/flutter_frontend_server/test/to_string_test.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'dart:io' as io; +import 'dart:ui' show Color; import 'package:engine_repo_tools/engine_repo_tools.dart'; import 'package:path/path.dart' as path; @@ -60,7 +61,7 @@ void main() { ])); final runResult = io.Process.runSync(dart, [regularDill]); checkProcessResult(runResult); - var paintString = '"Paint.toString":"Paint(Color(0xffffffff))"'; + var paintString = '"Paint.toString":"Paint(${const Color(0xffffffff)})"'; if (buildDir.contains('release')) { paintString = '"Paint.toString":"Instance of \'Paint\'"'; } diff --git a/testing/dart/image_filter_test.dart b/testing/dart/image_filter_test.dart index b454b50b58320..d017ff24be229 100644 --- a/testing/dart/image_filter_test.dart +++ b/testing/dart/image_filter_test.dart @@ -293,7 +293,7 @@ void main() async { ).toString(), contains( 'matrix([10.0, 0.0, 0.0, 0.0, 0.0, 10.0, 0.0, 0.0, 0.0, 0.0, 1.0, 0.0, -0.0, -0.0, 0.0, 1.0], FilterQuality.low) -> ' - 'ColorFilter.mode(Color(alpha: 1.0000, red: 0.6706, green: 0.8039, blue: 0.9373, colorSpace: ColorSpace.sRGB), BlendMode.color) -> ' + 'ColorFilter.mode(${const Color(0xFFABCDEF)}, BlendMode.color) -> ' 'blur(20.0, 20.0, repeated) -> ' 'blur(30.0, 30.0, mirror)' ), diff --git a/testing/dart/text_test.dart b/testing/dart/text_test.dart index 8cc7e030d5d56..95526ea9da177 100644 --- a/testing/dart/text_test.dart +++ b/testing/dart/text_test.dart @@ -80,7 +80,7 @@ void testTextStyle() { ); expect( ts1.toString(), - equals('TextStyle(color: Color(alpha: 1.0000, red: 0.0000, green: 1.0000, blue: 0.0000, colorSpace: ColorSpace.sRGB), decoration: unspecified, decorationColor: unspecified, decorationStyle: unspecified, decorationThickness: unspecified, fontWeight: FontWeight.w800, fontStyle: unspecified, textBaseline: unspecified, fontFamily: unspecified, fontFamilyFallback: unspecified, fontSize: 10.0, letterSpacing: unspecified, wordSpacing: unspecified, height: 100.0x, leadingDistribution: unspecified, locale: unspecified, background: unspecified, foreground: unspecified, shadows: unspecified, fontFeatures: unspecified, fontVariations: unspecified)'), + equals('TextStyle(color: ${const Color(0xFF00FF00)}, decoration: unspecified, decorationColor: unspecified, decorationStyle: unspecified, decorationThickness: unspecified, fontWeight: FontWeight.w800, fontStyle: unspecified, textBaseline: unspecified, fontFamily: unspecified, fontFamilyFallback: unspecified, fontSize: 10.0, letterSpacing: unspecified, wordSpacing: unspecified, height: 100.0x, leadingDistribution: unspecified, locale: unspecified, background: unspecified, foreground: unspecified, shadows: unspecified, fontFeatures: unspecified, fontVariations: unspecified)'), ); expect( ts2.toString(), From 4466d89bf32e39f73f9a28cc4bc435f3621b95f5 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 11 Sep 2024 12:23:23 -0700 Subject: [PATCH 10/15] removed verbotin import --- flutter_frontend_server/test/to_string_test.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/flutter_frontend_server/test/to_string_test.dart b/flutter_frontend_server/test/to_string_test.dart index 9dbaea562bf40..fc21f4cdea34f 100644 --- a/flutter_frontend_server/test/to_string_test.dart +++ b/flutter_frontend_server/test/to_string_test.dart @@ -3,7 +3,6 @@ // found in the LICENSE file. import 'dart:io' as io; -import 'dart:ui' show Color; import 'package:engine_repo_tools/engine_repo_tools.dart'; import 'package:path/path.dart' as path; @@ -61,7 +60,8 @@ void main() { ])); final runResult = io.Process.runSync(dart, [regularDill]); checkProcessResult(runResult); - var paintString = '"Paint.toString":"Paint(${const Color(0xffffffff)})"'; + var paintString = + '"Paint.toString":"Paint(Color(alpha: 1.0000, red: 1.0000, green: 1.0000, blue: 1.0000, colorSpace: ColorSpace.sRGB))"'; if (buildDir.contains('release')) { paintString = '"Paint.toString":"Instance of \'Paint\'"'; } From e45bb0cf1ec75beaad96ae88ddc41ce849ad89ac Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 11 Sep 2024 13:25:44 -0700 Subject: [PATCH 11/15] fixed some web tests --- lib/web_ui/test/ui/color_test.dart | 41 ++++++++++++++++++++----- lib/web_ui/test/ui/filters_test.dart | 4 +-- lib/web_ui/test/ui/paint_test.dart | 2 +- lib/web_ui/test/ui/text_style_test.dart | 4 +-- 4 files changed, 39 insertions(+), 12 deletions(-) diff --git a/lib/web_ui/test/ui/color_test.dart b/lib/web_ui/test/ui/color_test.dart index 2ac2df7595d02..6ee06e7f8a734 100644 --- a/lib/web_ui/test/ui/color_test.dart +++ b/lib/web_ui/test/ui/color_test.dart @@ -8,6 +8,33 @@ import 'package:ui/ui.dart'; import '../common/test_initialization.dart'; + +class _ColorMatcher extends Matcher { + _ColorMatcher(this._target, this._threshold); + + final Color _target; + final double _threshold; + + @override + Description describe(Description description) { + return description.add('matches color "$_target" with threshold "$_threshold".'); + } + + @override + bool matches(dynamic item, Map matchState) { + return item is Color && + item.colorSpace == _target.colorSpace && + (item.a - _target.a).abs() <= _threshold && + (item.r - _target.r).abs() <= _threshold && + (item.g - _target.g).abs() <= _threshold && + (item.b - _target.b).abs() <= _threshold; + } +} + +Matcher isSameColorAs(Color color, {double threshold = 0.004}) { + return _ColorMatcher(color, threshold); +} + void main() { internalBootstrapBrowserTest(() => testMain); } @@ -31,7 +58,7 @@ Future testMain() async { const Color c = Color(0x00000000); final Paint p = Paint(); p.color = c; - expect(c.toString(), equals('Color(0x00000000)')); + expect(c.toString(), equals('${const Color(0x00000000)}')); }); test('color created with out of bounds value', () { @@ -63,7 +90,7 @@ Future testMain() async { ); expect( Color.lerp(const Color(0x00000000), const Color(0xFFFFFFFF), 0.5), - const Color(0x7F7F7F7F), + isSameColorAs(const Color(0x7F7F7F7F)), ); expect( Color.lerp(const Color(0x00000000), const Color(0xFFFFFFFF), 1.0), @@ -102,23 +129,23 @@ Future testMain() async { ); expect( Color.alphaBlend(const Color(0x80808080), const Color(0xFFFFFFFF)), - const Color(0xFFBFBFBF), + isSameColorAs(const Color(0xFFBFBFBF)), ); expect( Color.alphaBlend(const Color(0x80808080), const Color(0xFF000000)), - const Color(0xFF404040), + isSameColorAs(const Color(0xFF404040)), ); expect( Color.alphaBlend(const Color(0x01020304), const Color(0xFF000000)), - const Color(0xFF000000), + isSameColorAs(const Color(0xFF000000)), ); expect( Color.alphaBlend(const Color(0x11223344), const Color(0xFF000000)), - const Color(0xFF020304), + isSameColorAs(const Color(0xFF020304)), ); expect( Color.alphaBlend(const Color(0x11223344), const Color(0x80000000)), - const Color(0x88040608), + isSameColorAs(const Color(0x88040608)), ); }); diff --git a/lib/web_ui/test/ui/filters_test.dart b/lib/web_ui/test/ui/filters_test.dart index d2ca97291548e..204f3e155e2ff 100644 --- a/lib/web_ui/test/ui/filters_test.dart +++ b/lib/web_ui/test/ui/filters_test.dart @@ -126,7 +126,7 @@ Future testMain() async { ); await drawTestImageWithPaint(ui.Paint()..imageFilter = colorFilter); await matchGoldenFile('ui_filter_colorfilter_as_imagefilter.png', region: region); - expect(colorFilter.toString(), 'ColorFilter.mode(Color(0x800000ff), BlendMode.srcOver)'); + expect(colorFilter.toString(), 'ColorFilter.mode(${const ui.Color(0x800000ff)}, BlendMode.srcOver)'); }); test('mode color filter', () async { @@ -136,7 +136,7 @@ Future testMain() async { ); await drawTestImageWithPaint(ui.Paint()..colorFilter = colorFilter); await matchGoldenFile('ui_filter_mode_colorfilter.png', region: region); - expect(colorFilter.toString(), 'ColorFilter.mode(Color(0x800000ff), BlendMode.srcOver)'); + expect(colorFilter.toString(), 'ColorFilter.mode(${const ui.Color(0x800000ff)}, BlendMode.srcOver)'); }); test('linearToSRGBGamma color filter', () async { diff --git a/lib/web_ui/test/ui/paint_test.dart b/lib/web_ui/test/ui/paint_test.dart index 759aa636321d7..253ba3d17d38e 100644 --- a/lib/web_ui/test/ui/paint_test.dart +++ b/lib/web_ui/test/ui/paint_test.dart @@ -69,7 +69,7 @@ Future testMain() async { expect( paint.toString(), 'Paint(' - 'Color(0xaabbccdd); ' + '${const ui.Color(0xaabbccdd)}; ' 'BlendMode.darken; ' 'colorFilter: ColorFilter.linearToSrgbGamma(); ' 'maskFilter: MaskFilter.blur(BlurStyle.normal, 1.7); ' diff --git a/lib/web_ui/test/ui/text_style_test.dart b/lib/web_ui/test/ui/text_style_test.dart index 67eda1dedf007..90ba00ebc02c4 100644 --- a/lib/web_ui/test/ui/text_style_test.dart +++ b/lib/web_ui/test/ui/text_style_test.dart @@ -122,7 +122,7 @@ Future testMain() async { expect( style.toString(), 'TextStyle(' - 'color: Color(0xff000000), ' + 'color: ${const ui.Color(0xff000000)}, ' 'decoration: TextDecoration.none, ' 'decorationColor: Color(0xffaa0000), ' 'decorationStyle: TextDecorationStyle.solid, ' @@ -140,7 +140,7 @@ Future testMain() async { 'locale: en_US, ' 'background: Paint(), ' 'foreground: unspecified, ' - 'shadows: [TextShadow(Color(0xff000000), Offset(0.0, 0.0), ${0.0})], ' + 'shadows: [TextShadow(${const ui.Color(0xff000000)}, Offset(0.0, 0.0), ${0.0})], ' "fontFeatures: [FontFeature('case', 1)], " "fontVariations: [FontVariation('ital', 0.1)]" ')', From dadd6c9da28bd2d6a5fe3233aae787d560ed19b0 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 11 Sep 2024 14:40:56 -0700 Subject: [PATCH 12/15] fixed more web tests --- lib/web_ui/test/ui/filters_test.dart | 4 ++-- lib/web_ui/test/ui/text_style_test.dart | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/web_ui/test/ui/filters_test.dart b/lib/web_ui/test/ui/filters_test.dart index 204f3e155e2ff..fc7ce4aa604aa 100644 --- a/lib/web_ui/test/ui/filters_test.dart +++ b/lib/web_ui/test/ui/filters_test.dart @@ -121,7 +121,7 @@ Future testMain() async { test('color filter as image filter', () async { const ui.ColorFilter colorFilter = ui.ColorFilter.mode( - ui.Color.fromRGBO(0, 0, 255, 128), + ui.Color.fromARGB(128, 0, 0, 255), ui.BlendMode.srcOver, ); await drawTestImageWithPaint(ui.Paint()..imageFilter = colorFilter); @@ -131,7 +131,7 @@ Future testMain() async { test('mode color filter', () async { const ui.ColorFilter colorFilter = ui.ColorFilter.mode( - ui.Color.fromRGBO(0, 0, 255, 128), + ui.Color.fromARGB(128, 0, 0, 255), ui.BlendMode.srcOver, ); await drawTestImageWithPaint(ui.Paint()..colorFilter = colorFilter); diff --git a/lib/web_ui/test/ui/text_style_test.dart b/lib/web_ui/test/ui/text_style_test.dart index 90ba00ebc02c4..d37faa5c9fab9 100644 --- a/lib/web_ui/test/ui/text_style_test.dart +++ b/lib/web_ui/test/ui/text_style_test.dart @@ -165,7 +165,7 @@ Future testMain() async { 'TextStyle(' 'color: unspecified, ' 'decoration: TextDecoration.none, ' - 'decorationColor: Color(0xffaa0000), ' + 'decorationColor: ${const ui.Color(0xffaa0000)}, ' 'decorationStyle: TextDecorationStyle.solid, ' 'decorationThickness: ${1.0}, ' 'fontWeight: FontWeight.w400, ' @@ -181,7 +181,7 @@ Future testMain() async { 'locale: en_US, ' 'background: Paint(), ' 'foreground: Paint(), ' - 'shadows: [TextShadow(Color(0xff000000), Offset(0.0, 0.0), ${0.0})], ' + 'shadows: [TextShadow(${const ui.Color(0xff000000)}, Offset(0.0, 0.0), ${0.0})], ' "fontFeatures: [FontFeature('case', 1)], " "fontVariations: [FontVariation('ital', 0.1)]" ')', From 72dca3798547713c618d8e80da8f17a1fa476b75 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 11 Sep 2024 14:49:01 -0700 Subject: [PATCH 13/15] removed num clamp --- lib/ui/painting.dart | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 2d72569d53107..a51d6f11e3284 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -49,7 +49,7 @@ bool _radiusIsValid(Radius radius) { } Color _scaleAlpha(Color x, double factor) { - return x.withValues(alpha: (x.a * factor).clamp(0, 1)); + return x.withValues(alpha: clampDouble(x.a * factor, 0, 1)); } /// An immutable 32 bit color value in ARGB format. @@ -354,10 +354,10 @@ class Color { } else { assert(x.colorSpace == y.colorSpace); return Color.from( - alpha: _lerpDouble(x.a, y.a, t).clamp(0, 1), - red: _lerpDouble(x.r, y.r, t).clamp(0, 1), - green: _lerpDouble(x.g, y.g, t).clamp(0, 1), - blue: _lerpDouble(x.b, y.b, t).clamp(0, 1), + alpha: clampDouble(_lerpDouble(x.a, y.a, t), 0, 1), + red: clampDouble(_lerpDouble(x.r, y.r, t), 0, 1), + green: clampDouble(_lerpDouble(x.g, y.g, t), 0, 1), + blue: clampDouble(_lerpDouble(x.b, y.b, t), 0, 1), colorSpace: x.colorSpace, ); } From 19adc884a87a948891e29773a4a13d4fffe6d800 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Wed, 11 Sep 2024 15:38:55 -0700 Subject: [PATCH 14/15] doh --- lib/web_ui/test/ui/text_style_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/test/ui/text_style_test.dart b/lib/web_ui/test/ui/text_style_test.dart index d37faa5c9fab9..98b5241946703 100644 --- a/lib/web_ui/test/ui/text_style_test.dart +++ b/lib/web_ui/test/ui/text_style_test.dart @@ -124,7 +124,7 @@ Future testMain() async { 'TextStyle(' 'color: ${const ui.Color(0xff000000)}, ' 'decoration: TextDecoration.none, ' - 'decorationColor: Color(0xffaa0000), ' + 'decorationColor: ${const ui.Color(0xffaa0000)}, ' 'decorationStyle: TextDecorationStyle.solid, ' 'decorationThickness: ${1.0}, ' 'fontWeight: FontWeight.w400, ' From 512d0a1118d76fbbfba6ba7ae87578ea1fbe1369 Mon Sep 17 00:00:00 2001 From: Aaron Clarke Date: Thu, 12 Sep 2024 08:33:35 -0700 Subject: [PATCH 15/15] jim feedback --- lib/ui/painting.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index a51d6f11e3284..6a171e79a94b0 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -310,6 +310,7 @@ class Color { /// /// See . double computeLuminance() { + assert(colorSpace != ColorSpace.extendedSRGB); // See final double R = _linearizeColorComponent(r); final double G = _linearizeColorComponent(g);