-
Notifications
You must be signed in to change notification settings - Fork 82
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
[apps/gamut-mapping] Add edge seeker algorithm #448
Conversation
✅ Deploy Preview for colorjs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Just an FYI, playing with it in the Netlify preview, the algorithm seems to have some failing scenarios. I haven't investigated closely to determine the precise issues, but those would need to be addressed. I'll comment on specific cases when I have a chance unless they solved before I get around to it. |
Thank you for taking a look! |
I mean errors getting thrown. It may be an issue when you ported it to this library, but when you are incrementing through colors, it starts to not update. Looking in the browser console, you can see errors. I'm not in front of my computer right now, but I'd run it through some ranges and see for yourself. |
@ardov, it's a lot of errors like this:
|
@facelessuser oh I see, thank you. I forgot to add hue normalization here so it fails when the hue is outside 0-360 range |
Makes sense. It seems to be working now. |
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.
This seems to work well. As it is an app, and not going into the core library, I don't intend on extensively going through every line of the code, so generally, it LGTM. I'll let other reviewers chime in if they have specific issues.
LGTM. We should probably change the UI at some point to facilitate comparing across so many gamut mapping algorithms. |
@LeaVerou It would also be nice to compare some practical cases with some of these algorithms, like interpolation, etc. Do you get color banding? Are they uniform? etc. I think ∆E is a fine metric, but sometimes clipping has great ∆E and the colors are still not great. |
My approach to testing was a bit different. I used this flow:
There are ~390K hex colors on the surface of the cube. I tested each of them and it helped identify weaknesses. Of course, this method only works if we agree that chroma clamping is the way. |
Yeah, agree, real-world scenarios should be the priority. I think there is no need to implement my algorithm to test the colors. It's easier to tweak CSS a bit. |
Yep, I'm not suggesting it should match CSS. A pre-calculated LUT is always going to be tough to beat as you don't have to approximate the data as you have it all there 🙂. |
I think this just needs a rebase. I'd be happy to merge after that. |
26158ab
to
fa66a2c
Compare
Damn, tried to rebase and now there are conflicts 😬 I'll figure them out later today. |
fa66a2c
to
85e535f
Compare
Ok, now it seems to be fine. The first time I rebased to the wrong branch 🙈 |
* main: Audit function parameter and return types (2/2) (#457) Always show delta display to help confirm it is in gamut (#458) Final clip is required in Ray tracing [apps/gamut-mapping] Add edge seeker algorithm (#448) Audit function parameter and return types (1/2) (#456) Rework raytracing limiting hue shift to no more than 3 degrees at worst
Added an algorithm that generates a lookup table as a first step and then uses it to accurately predict highest chroma for color.