Fix wrong fromRGBO alpha value calculation #13777
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
KNawm
changed the title
KNawm
changed the title
Hixie
reviewed
Dec 9, 2019
lib/ui/painting.dart
Outdated
| ((r & 0xff) << 16) | | ||
| ((g & 0xff) << 8) | | ||
| ((b & 0xff) << 0)) & 0xFFFFFFFF; | ||
| value = (((((opacity * 0xff + 0.5) ~/ 1) & 0xff) << 24) | // Round the alpha value for accuracy |
There was a problem hiding this comment.
How about just ((opacity * 0xff).round() & 0xff) < 24 ? Seems like that would be more idiomatic.
There was a problem hiding this comment.
It is a constant which is why I think it was that way originally
There was a problem hiding this comment.
can you add a comment explaining that? thanks
|
Please add a test. |
|
Thanks for adding the test. I've re-triggered the failed |
|
I've rebased this against head of |
chingjun
added a commit
to chingjun/flutter_svg
that referenced
this pull request
Dec 18, 2019
flutter/engine#13777 changed the rendering, update the goldens to fix the tests
chingjun
added a commit
that referenced
this pull request
Dec 18, 2019
filmil
pushed a commit
to filmil/engine
that referenced
this pull request
Mar 13, 2020
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.
filmil
pushed a commit
to filmil/engine
that referenced
this pull request
Mar 13, 2020
…" (flutter#14548) This reverts commit 9f2daad.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Constructing colors using
fromRGBOshould return the same values as the CSSrgba()notation.rgba(0, 0, 255, 0.5)is the same as#0000ff80but
fromRGBOsometimes creates a color with an off by one alpha valueif we use
withOpacityto create the same color it returns the correct color:it should be changed in lib/web_ui/lib/src/ui/painting.dart too and I think it would break golden tests so I don't know where to go from here.