This repository was archived by the owner on Feb 25, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 6k
Reland: Framework wide color (#54415) (#54737) #54905
Merged
gaaclarke
merged 3 commits into
flutter:main
from
gaaclarke:framework-wide-color-reland-2
Aug 30, 2024
Merged
Reland: Framework wide color (#54415) (#54737) #54905
gaaclarke
merged 3 commits into
flutter:main
from
gaaclarke:framework-wide-color-reland-2
Aug 30, 2024
Conversation
This file contains hidden or 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
[This PR](flutter#54415) was reverted because it requires a manual roll into the framework. issue: flutter/flutter#127855 integration test: flutter#54415 This does the preliminary work for implementing wide gamut colors in the Flutter framework. Here are the following changes: 1) colors now specify a colorspace with which they are to be interpreted 1) colors now store their components as floats to accommodate bit depths more than 8 The storage of this Color class is weird with float/int storage but that is a temporary solution to support a smooth transition. Here is the plan for landing this: 1) Land this PR 1) Wait for it to roll into the Framework 1) Land flutter/flutter#153938 which will make CupertinoDynamicColor implement Color 1) Land another engine PR that rips out the int storage: flutter#54714 Here are follow up PRs: 1) flutter#54473 - changes DlColor so the wide gamut colors are rendered 1) flutter#54567 - Hooks up these changes to take advantage of wide DlColor 1) flutter/flutter#153319 - the integration test for the framework repo There are some things that have been left as follow up PRs since they are technically breaking: 1) The math on `lerp` hasn't been updated to take advantage of the higher bit depth 1) `operator==` hasn't been updated to take advantage of the higher bit depth 1) `hashCode` hasn't been updated to take advantage of the higher bit depth 1) `alphaBlend` hasn't been updated to take advantage of the higher bit depth 1) `toString` hasn't been updated to take advantage of the higher bit depth
@jonahwilliams I got the fix, a comment and a test added. That change to lerp wasn't required in the end. It was needed in a previous iteration and I kept it since it was less ops. |
8 tasks
jonahwilliams
approved these changes
Aug 30, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
auto-submit bot
pushed a commit
that referenced
this pull request
Aug 30, 2024
relands #54567 depends on #54905 This was reverted because the PR that it depends on was reverted. There is nothing to be addressed here. [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Aug 30, 2024
auto-submit bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Aug 30, 2024
…154435) flutter/engine@94487cc...eff1b76 2024-08-30 [email protected] Reland: Framework wide color (#54415) (#54737) (flutter/engine#54905) 2024-08-30 [email protected] Roll Skia from f3811180e7df to 2727e4e5d7ec (3 revisions) (flutter/engine#54904) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Buchimi
pushed a commit
to Buchimi/flutter
that referenced
this pull request
Sep 2, 2024
…lutter#154435) flutter/engine@94487cc...eff1b76 2024-08-30 [email protected] Reland: Framework wide color (flutter#54415) (flutter#54737) (flutter/engine#54905) 2024-08-30 [email protected] Roll Skia from f3811180e7df to 2727e4e5d7ec (3 revisions) (flutter/engine#54904) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
autosubmit
Merge PR when tree becomes green via auto submit App
platform-web
Code specifically for the web engine
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.
This PR was reverted because it required customer testing updates.
issue: flutter/flutter#127855
integration test: #54415
This does the preliminary work for implementing wide gamut colors in the Flutter framework. Here are the following changes: 1) colors now specify a colorspace with which they are to be interpreted 1) colors now store their components as floats to accommodate bit depths more than 8
The storage of this Color class is weird with float/int storage but that is a temporary solution to support a smooth transition. Here is the plan for landing this: 1) Land this PR
implements Color
flutter#153938 which will make CupertinoDynamicColor implement Color 1) Land another engine PR that rips out the int storage: Removes the int storage from Color #54714Here are follow up PRs:
There are some things that have been left as follow up PRs since they are technically breaking: 1) The math on
lerp
hasn't been updated to take advantage of the higher bit depth 1)operator==
hasn't been updated to take advantage of the higher bit depth 1)hashCode
hasn't been updated to take advantage of the higher bit depth 1)alphaBlend
hasn't been updated to take advantage of the higher bit depth 1)toString
hasn't been updated to take advantage of the higher bit depthReland 2 notes
This was reverted because it changes the math on
_lerpDouble
. While those changes were mathematcially equivalent, they had different behaviors when working with non-numbers which created unexpected changes. The change has been reverted and a test added.Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.