Implement CAM16 #342
Merged
Implement CAM16 #342
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
|
The precision seem strangely low on my computer (3-4 digits are the same before it diverges). I can't find what causes it, but it's probably in |
|
The accuracy difference may be some trigonometry function that's different. Converting back to RGB yields near perfect results, so the round trip seem to be working as it should. |
Benchmark for 899f822Click to view benchmark
|
Benchmark for 76ea61dClick to view benchmark
|
Benchmark for 7f44c63Click to view benchmark
|
Benchmark for 8f8ed28Click to view benchmark
|
Benchmark for 3ee5a58Click to view benchmark
|
|
Getting UCS to work well is pretty challenging. The main issue is that it's based on brightness, while the "main" metric and default for The good news is that this solution works relatively well with the derive macro for |
Benchmark for 95b77b8Click to view benchmark
|
CodSpeed Performance ReportMerging #342 will degrade performances by 11.42%Comparing Summary
Benchmarks breakdown
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #342 +/- ##
==========================================
+ Coverage 82.01% 83.48% +1.47%
==========================================
Files 115 125 +10
Lines 18287 20145 +1858
Branches 18287 20145 +1858
==========================================
+ Hits 14998 16818 +1820
- Misses 3131 3170 +39
+ Partials 158 157 -1 ☔ View full report in Codecov by Sentry. |
bors bot
added a commit
that referenced
this pull request
Sep 9, 2023
355: Add traits for delta E and the improvemed formulas from Huang et al r=Ogeon a=Ogeon Adds the `DeltaE`, `ImprovedDeltaE` and `ImprovedCiede200` traits. The first one represents the "default" or canonical delta E formula for a color space as it was originally specified. If there is no specification for delta E, it's not implemented. It's also not necessarily the best. The other two implement the enhancements described in *Power functions improving the performance of color-difference formulas* by Huang et al. They are provided as separate traits so the user can choose which one is the most appropriate for their use case. This is the result of me trying to sort out where different formulas come from and prepare for adding a delta E for CAM16-UCS in #342. The recommended formula for CAM16 turns out to be the "improved" one. Co-authored-by: Erik Hedvall <[email protected]>
|
Splitting the colors into groups, and forcing XYZ <-> CAM16 conversion to be explicit, avoids the issue with the default parameters. Now it's up to the user to make sure they are set as expected. |
|
I think this latest change solved the last major challenge with this addition. It's nice that splitting the spaghetti of conversations into groups actually seems to work so well, since I really didn't like the idea of having default parameters. It never felt right, but this is better. Also, in case anyone wonders what's taking so long, it's both because of the tricky problems like these (I also made some other changes earlier, such as the macros, to lay a better foundation) and because I haven't been feeling like programming as much in my free time lately. This comes and goes over time, so nothing to worry about. It's just how I manage my time and energy. 🙂 |
Ogeon
force-pushed
the
cam16
branch
2 times, most recently
from
c0e4538
to
af713ef
Compare
|
For anyone who's watching this, feel free to start testing it. I would appreciate any input on the API and of course possible bugs. |
|
I ended up changing the approach for partial CAM16. The generic struct complicated trait implementations to the degree that it didn't really save any effort, compared to a cookie cutter macro solution. That and the awkward naming and newtype structs didn't sit right with me. This new approach is easier to treat as any other type, at the price of more repetition in some places. Still not too bad. |
Ogeon
force-pushed
the
cam16
branch
2 times, most recently
from
123de6f
to
f2dc15c
Compare
Ogeon
force-pushed
the
cam16
branch
3 times, most recently
from
7498ce7
to
285cb2a
Compare
|
The |
|
That's the largest squash I have ever done. I think it's ready for merging now. |
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.
Implements support for CIE CAM 16 and surrounding parts. This is a work in progress that is put up for early review, in case anyone has feedback or want to play Find The Typo. Based on https://observablehq.com/@jrus/cam16.
Surroundrepresent percent (20% instead of 2.0) and rename theCustomvariant toPercent.discountingto an enum to make room for a custom discounting degree later.Closed Issues