Closed Ogeon closed 4 months ago
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 xyz_to_cam16
, since the output of prepare_parameters
was identical to the reference.
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.
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 PartialCam16
is lightness. The current version makes it possible to set the attributes statically, so UCS can require the input to have brightness and chroma. The downside with this is that there are essentially 7 versions of PartialCam16
(more if static and dynamic attributes are mixed), which adds a bunch of extra for a user to wrap their head around.
The good news is that this solution works relatively well with the derive macro for FromColorUnclamped
. It just required a small extension (I also made errors a bit nicer). Part of the "trick" is to bypass Cam16
, so PartialCam16
becomes the "parent" type for UCS. This breaks up the triangle relationship between Xyz
, Cam16
and PartialCam16
, and makes it so the conversions are as direct as possible.
Comparing cam16
(78acbca) with master
(ccc521b)
❌ 1 (👁 1)
regressions
✅ 44
untouched benchmarks
🆕 1
new benchmarks
Benchmark | master |
cam16 |
Change | |
---|---|---|---|---|
🆕 | Lab improved delta E |
N/A | 3.6 ms | N/A |
👁 | matrix_inverse |
215.6 ns | 243.3 ns | -11.42% |
Attention: Patch coverage is 92.42494%
with 164 lines
in your changes are missing coverage. Please review.
Project coverage is 83.48%. Comparing base (
ccc521b
) to head (78acbca
). Report is 4 commits behind head on master.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
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. 🙂
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.
The cam16
module needs some documentation, but I think it will finally be good to go after that. This is enough for today, though.
That's the largest squash I have ever done. I think it's ready for merging now.
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.
Surround
represent percent (20% instead of 2.0) and rename theCustom
variant toPercent
.discounting
to an enum to make room for a custom discounting degree later.Closed Issues