Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Update Color to do all calculations with floating point components #54981

Merged
merged 15 commits into from
Sep 12, 2024
Merged

Update Color to do all calculations with floating point components #54981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
36ca48a
Makes Color take advantage of the wider bit depth.
gaaclarke Sep 5, 2024
66f8393
updated lerp
gaaclarke Sep 5, 2024
138f388
migrated alphaBlend
gaaclarke Sep 5, 2024
7493253
moved equality and toString
gaaclarke Sep 5, 2024
745a8a7
brought over web changes
gaaclarke Sep 6, 2024
94e4b23
fixed fragile "toString" test
gaaclarke Sep 6, 2024
2aebceb
fixed fragile text test
gaaclarke Sep 6, 2024
26595ac
removed unused functions
gaaclarke Sep 6, 2024
aa290db
fixed front end test, moved tests to abstract away color.tostring
gaaclarke Sep 6, 2024
4466d89
removed verbotin import
gaaclarke Sep 11, 2024
e45bb0c
fixed some web tests
gaaclarke Sep 11, 2024
dadd6c9
fixed more web tests
gaaclarke Sep 11, 2024
72dca37
removed num clamp
gaaclarke Sep 11, 2024
19adc88
doh
gaaclarke Sep 11, 2024
512d0a1
jim feedback
gaaclarke Sep 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion flutter_frontend_server/test/to_string_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ void main() {
]));
final runResult = io.Process.runSync(dart, <String>[regularDill]);
checkProcessResult(runResult);
var paintString = '"Paint.toString":"Paint(Color(0xffffffff))"';
var paintString =
'"Paint.toString":"Paint(Color(alpha: 1.0000, red: 1.0000, green: 1.0000, blue: 1.0000, colorSpace: ColorSpace.sRGB))"';
Copy link
Member Author

@gaaclarke gaaclarke Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typically I'd call Color.toString() here but I'm unable to import dart:ui here for some reason.

Copy link
Contributor

@jonahwilliams jonahwilliams Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a VM test, so that is why.

Copy link
Member Author

@gaaclarke gaaclarke Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But it's weird that I'm asserting behavior for dart:ui, so it's there, it's just not visible to me.

Copy link
Contributor

@jonahwilliams jonahwilliams Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its basically a compiler test though

if (buildDir.contains('release')) {
paintString = '"Paint.toString":"Instance of \'Paint\'"';
}
Expand Down
12 changes: 0 additions & 12 deletions lib/ui/lerp.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
96 changes: 48 additions & 48 deletions lib/ui/painting.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: clampDouble(x.a * factor, 0, 1));
}

/// An immutable 32 bit color value in ARGB format.
Expand Down Expand Up @@ -310,10 +310,11 @@ class Color {
///
/// See <https://en.wikipedia.org/wiki/Relative_luminance>.
double computeLuminance() {
assert(colorSpace != ColorSpace.extendedSRGB);
// See <https://www.w3.org/TR/WCAG20/#relativeluminancedef>
final double R = _linearizeColorComponent(red / 0xFF);
final double G = _linearizeColorComponent(green / 0xFF);
final double B = _linearizeColorComponent(blue / 0xFF);
final double R = _linearizeColorComponent(r);
Copy link
Contributor

@flar flar Sep 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we have colorspaces, should these be taught to clamp or convert to a non-extended CS as well?

Copy link
Member Author

@gaaclarke gaaclarke Sep 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added an assert that extended srgb is in't used. I don't think it will make sense with that.

final double G = _linearizeColorComponent(g);
final double B = _linearizeColorComponent(b);
return 0.2126 * R + 0.7152 * G + 0.0722 * B;
}

Expand All @@ -339,28 +340,26 @@ class Color {
///
/// Values for `t` are usually obtained from an [Animation<double>], 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: 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,
);
}
}
Expand All @@ -377,32 +376,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,
);
}
}
Expand All @@ -423,16 +420,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() =>
Copy link
Member Author

@gaaclarke gaaclarke Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 sig figs was chosen since hdmi 1.3 can support 12 bits per color component and 1/2^12 = 0.00024

Copy link
Member Author

@gaaclarke gaaclarke Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scientific notation could have been another option if we wanted ( 2.4e-4 ).

'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.
Expand Down
7 changes: 0 additions & 7 deletions lib/web_ui/lib/lerp.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
92 changes: 48 additions & 44 deletions lib/web_ui/lib/painting.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ void _validateColorStops(List<Color> colors, List<double>? 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 {
Expand Down Expand Up @@ -141,32 +141,32 @@ class Color {

double computeLuminance() {
// See <https://www.w3.org/TR/WCAG20/#relativeluminancedef>
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,
);
}
}
Expand All @@ -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,
);
}
}
Expand All @@ -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 {
Expand Down
Loading