-
Notifications
You must be signed in to change notification settings - Fork 17
Forward fix for wide gamut changes in colors #10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
friendly ping @james-alex @isaacroberts |
Add an implementation of Color.toARGB32 to subclasses of Color
|
FYI, after upgrading my project to the new stable release Flutter 3.27.0, the underlying issue here resulted in build errors in my project. In my tests, @gaaclarke's PR here resolves these errors. Thus, this PR (or another similar fix) needs to be merged to resolve build incompatibilities between color_models and Flutter 3.27.0. In case it might help anyone in the meantime, I resolved my project's build errors with a dependency override pointing to gaaclarke's PR code (but substituting with my own fork), like so: dependencies:
color_models:
git:
url: https://github.com/gaaclarke/color_models.git
ref: wide-gamut
path: color_models
flutter_color_models:
git:
url: https://github.com/gaaclarke/color_models.git
ref: wide-gamut
path: flutter_color_models
dependency_overrides:
color_models:
git:
url: https://github.com/gaaclarke/color_models.git
ref: wide-gamut
path: color_models
flutter_color_models:
git:
url: https://github.com/gaaclarke/color_models.git
ref: wide-gamut
path: flutter_color_models |
| double get a => alpha / 255.0; | ||
|
|
||
| double get r => red / 255.0; | ||
|
|
||
| double get g => green / 255.0; | ||
|
|
||
| double get b => blue / 255.0; |
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.
Hi! Just notifying something that happened to me.
I ran into an issue after the renaming of the a and b fields in LabColor with this extension. The code still compiled but produced incorrect behavior because I was accessing the wrong fields in LabColor.
For example, in the following function calculating Euclidean distance between two LAB colors, it caused a wrong result and still compiled:
// Euclidean distance between two LAB colors
double euclideanDistance(LabColor labColor, LabColor baseColor) {
return sqrt(pow(labColor.lightness - baseColor.lightness, 2) +
pow(labColor.a - baseColor.a, 2) +
pow(labColor.b - baseColor.b, 2));
}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.
I'm sorry @tivorreda64. Unfortunately this package is abandoned and has minimal testing. I'm not going to invest any more time on an abandoned package with sparse tests. If you can come up with a test that demonstrates the problem we can post it here and maybe someone who needs this package can find the time to fix it. I'm happy to append any patches to this PR to keep the info all in one place.
|
The equivalent has landed, no need for this pr anymore. |
fixes #9
changes:
I chose
fromValuesinstead ofwithValuessince "with*" is used to mean tweaking an existing color, whereas that method is creating completely new color.