Skip to content

Commit

Permalink
Fix off-by-one fromRGBO alpha value calculation (#13777)
Browse files Browse the repository at this point in the history
Constructing colors using `fromRGBO` should return the same values as the CSS
`rgba()` notation.  rgba(0, 0, 255, 0.5) is the same as `#0000ff80`

However `fromRGBO` sometimes creates a color with an off-by-one alpha value:

    expect(Color.fromRGBO(0, 0, 255, 0.5), Color(0x800000ff));

Expected: Color:<Color(0x800000ff)>
  Actual: Color:<Color(0x7f0000ff)>

If we use `withOpacity` to create the same color, it returns the correct color:

    expect(Color.fromRGBO(0, 0, 255, 1).withOpacity(0.5), Color(0x800000ff));

This should also be changed in lib/web_ui/lib/src/ui/painting.dart in a
followup change.
  • Loading branch information
KNawm authored and cbracken committed Dec 17, 2019
1 parent ee92285 commit 9f2daad
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 5 deletions.
10 changes: 5 additions & 5 deletions lib/ui/painting.dart
Original file line number Diff line number Diff line change
Expand Up @@ -129,17 +129,17 @@ class Color {
/// * `r` is [red], from 0 to 255.
/// * `g` is [green], from 0 to 255.
/// * `b` is [blue], from 0 to 255.
/// * `opacity` is alpha channel of this color as a double, with 0.0 being
/// * `opacity` is the alpha channel of this color as a double, with 0.0 being
/// transparent and 1.0 being fully opaque.
///
/// Out of range values are brought into range using modulo 255.
///
/// See also [fromARGB], which takes the opacity as an integer value.
const Color.fromRGBO(int r, int g, int b, double opacity) :
value = ((((opacity * 0xff ~/ 1) & 0xff) << 24) |
((r & 0xff) << 16) |
((g & 0xff) << 8) |
((b & 0xff) << 0)) & 0xFFFFFFFF;
value = (((((opacity * 0xff + 0.5) ~/ 1) & 0xff) << 24) | // Since colors are canonicalized we need to round the alpha value manually
((r & 0xff) << 16) |
((g & 0xff) << 8) |
((b & 0xff) << 0)) & 0xFFFFFFFF;

/// A 32 bit value representing this color.
///
Expand Down
6 changes: 6 additions & 0 deletions testing/dart/color_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ void main() {
expect(const NotAColor(123), equals(const NotAColor(123)));
});

test('Color.fromRGBO', () {
expect(const Color.fromRGBO(0, 0, 0, 1.0), const Color(0xFF000000));
expect(const Color.fromRGBO(0, 0, 0, 0.5), const Color(0x80000000));
expect(const Color.fromRGBO(0, 0, 0, 0.0), const Color(0x00000000));
});

test('Color.lerp', () {
expect(
Color.lerp(const Color(0x00000000), const Color(0xFFFFFFFF), 0.0),
Expand Down

0 comments on commit 9f2daad

Please sign in to comment.