This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Removes the int storage from Color #54714
Merged
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
gaaclarke
added a commit
that referenced
this pull request
Aug 22, 2024
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 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: #54714 Here are follow up PRs: 1) #54473 - changes DlColor so the wide gamut colors are rendered 1) #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 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat
gaaclarke
force-pushed
the
framework-wide-color-float
branch
from
c423af9 to
c8058e6
Compare
gaaclarke
added a commit
to gaaclarke/engine
that referenced
this pull request
Aug 23, 2024
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 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat
8 tasks
gaaclarke
added a commit
to gaaclarke/engine
that referenced
this pull request
Aug 29, 2024
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 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat
auto-submit bot
pushed a commit
that referenced
this pull request
Aug 29, 2024
[This PR](#54415) was reverted because it requires a manual roll into the framework. 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 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: #54714 Here are follow up PRs: 1) #54473 - changes DlColor so the wide gamut colors are rendered 1) #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
8 tasks
auto-submit bot
added a commit
that referenced
this pull request
Aug 30, 2024
Reverts: #54737 Initiated by: chingjun Reason for reverting: Breaking internal tests. See b/363125155 Original PR Author: gaaclarke Reviewed By: {matanlurey, jonahwilliams} This change reverts the following previous change: [This PR](#54415) was reverted because it requires a manual roll into the framework. 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 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: #54714 Here are follow up PRs: 1) #54473 - changes DlColor so the wide gamut colors are rendered 1) #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
gaaclarke
added a commit
to gaaclarke/engine
that referenced
this pull request
Aug 30, 2024
[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
8 tasks
gaaclarke
added a commit
that referenced
this pull request
Aug 30, 2024
[This PR](#54415) 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 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: #54714 Here are follow up PRs: 1) #54473 - changes DlColor so the wide gamut colors are rendered 1) #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 ## Reland 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 - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat
gaaclarke
force-pushed
the
framework-wide-color-float
branch
from
c8058e6 to
3fe7306
Compare
This comment was marked as outdated.
This comment was marked as outdated.
9 tasks
auto-submit bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Sep 5, 2024
This is a forward fix for the failures seen in https://github.com/flutter/engine/pull/54714/checks?check_run_id=29742599916 on flutter/engine#54714 issue: #127855
gaaclarke
force-pushed
the
framework-wide-color-float
branch
from
5b84e61 to
8633e6d
Compare
jonahwilliams
approved these changes
Sep 6, 2024
Yea, it happens in the next PR: #54981 I've decided that equality and hash will look exactly at the float values. Custom matchers can be used if people want something less precise. |
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Sep 6, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Sep 6, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Sep 6, 2024
zanderso
added a commit
to flutter/flutter
that referenced
this pull request
Sep 6, 2024
flutter/engine@c50eb8a...419fb8c 2024-09-06 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[engine] always force platform channel responses to schedule a task. (#54975)" (flutter/engine#55000) 2024-09-06 [[email protected]](mailto:[email protected]) Roll Skia from b6bab0fde426 to 6ad117bd2efe (2 revisions) (flutter/engine#54999) 2024-09-06 [[email protected]](mailto:[email protected]) Roll Fuchsia Test Scripts from D9INMR2u4wcyiZ750... to 5dqcFlKzRjJb6V95W... (flutter/engine#54998) 2024-09-06 [[email protected]](mailto:[email protected]) Roll Skia from a09312b70d37 to b6bab0fde426 (3 revisions) (flutter/engine#54997) 2024-09-06 [[email protected]](mailto:[email protected]) Roll Skia from 368f209ccca5 to a09312b70d37 (1 revision) (flutter/engine#54995) 2024-09-06 [[email protected]](mailto:[email protected]) Roll Skia from aec11ae18bb6 to 368f209ccca5 (3 revisions) (flutter/engine#54992) 2024-09-06 [[email protected]](mailto:[email protected]) Roll Fuchsia Linux SDK from xNv47d1TZmK9XgTxu... to PBeI0gGvgFdXV6hCg... (flutter/engine#54990) 2024-09-06 [[email protected]](mailto:[email protected]) Roll Skia from 809f868ded1c to aec11ae18bb6 (22 revisions) (flutter/engine#54988) 2024-09-06 [[email protected]](mailto:[email protected]) Removes the int storage from Color (flutter/engine#54714) 2024-09-06 [[email protected]](mailto:[email protected]) iOS,macOS: Add logging of duplicate codesign binaries (flutter/engine#54987) 2024-09-06 [[email protected]](mailto:[email protected]) Roll Fuchsia Test Scripts from k4lKsecg0pdIp-U7c... to D9INMR2u4wcyiZ750... (flutter/engine#54984) 2024-09-05 [[email protected]](mailto:[email protected]) Manual roll of Dart. (flutter/engine#54983) 2024-09-05 [[email protected]](mailto:[email protected]) iOS,macOS: add unsigned_binaries.txt (flutter/engine#54977) 2024-09-05 [[email protected]](mailto:[email protected]) Manual Skia roll to 809f868ded1c (flutter/engine#54972) 2024-09-05 [[email protected]](mailto:[email protected]) [canvaskit] Fix incorrect calculation of ImageFilter paint bounds (flutter/engine#54980) 2024-09-05 [[email protected]](mailto:[email protected]) [engine] always force platform channel responses to schedule a task. (flutter/engine#54975) 2024-09-05 [[email protected]](mailto:[email protected]) Fix unexpected ViewFocus events when Text Editing utilities change focus in the middle of a blur call. (flutter/engine#54965) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from xNv47d1TZmK9 to PBeI0gGvgFdX --------- Co-authored-by: Christopher Fujino <[email protected]> Co-authored-by: Zachary Anderson <[email protected]>
copybara-service bot
pushed a commit
to dart-lang/sdk
that referenced
this pull request
Sep 25, 2024
…s changes In flutter/engine#54714 the "int value" that was used for storing the color values for a Flutter color was removed and replaced with `double red, double green` etc. The color computer used the `value` field so this stopped it from computing any colors for previews in LSP clients. No tests broke because the tests here use a mock version of the Color class that still had `value`. This change updates the mock Color class to match the new Flutter implementation, and updates the computer to use the red/green/blue doubles instead of parsing from an int. It also adds support for the new `Color.from()` constructor that was also recently added. Fixes Dart-Code/Dart-Code#5289 Change-Id: I3ff20bda0cc848e028822cfcb5a14e6a8f6934d5 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/386802 Commit-Queue: Phil Quitslund <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]> Reviewed-by: Phil Quitslund <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
issue: flutter/flutter#127855
This depends on flutter/flutter#153938 being landed.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.