Skip to content

cubic root calculation in dt_XYZ_to_Lab()#20649

Open
jenshannoschwalm wants to merge 1 commit intodarktable-org:masterfrom
jenshannoschwalm:precise_lab_conversions
Open

cubic root calculation in dt_XYZ_to_Lab()#20649
jenshannoschwalm wants to merge 1 commit intodarktable-org:masterfrom
jenshannoschwalm:precise_lab_conversions

Conversation

@jenshannoschwalm
Copy link
Collaborator

In dt_XYZ_to_Lab() we have to calculate a cubic root.

For performance this was changed in 2010 for CPU code to halleys approximation in e81648c

The OpenCL code started using halleys but for precision this was changed to pow() in 26341e0 and later to cbrt() in 4fb7b15

  1. Since 2010 cpu floating point performance has increased drastically so it seems ok now to use cbrt() for precision.
  2. We used to do most modules in Lab space then, this has changed over the years so we often do multiple to/from Lab conversions resulting in accumulated errors in CPU path. Using cbrt() and cubicf() while to/from Lab conversions results in minimising errors.

Integration test logs confirm much less differences.
lab_cbrt.log

In dt_XYZ_to_Lab() we have to calculate a cubic root.

For performance this was changed in 2010 for CPU code to halleys approximation in e81648c

The OpenCL code started using halleys but for precision this was changed to pow() in 26341e0
and later to cbrt() 4fb7b15

1. Since 2010 cpu floating point performance has increased drastically so it seems ok now to use cbrt() for precision.
2. We used to do most modules in Lab space then, this has changed over the years so we often do multiple to/from Lab
   conversions resulting in accumulated errors in CPU path.
   Using cbrt() and cubicf() while to/from Lab conversions results in minimising errors.

Integration test logs confirm much less differences.
@jenshannoschwalm jenshannoschwalm added this to the 5.6 milestone Mar 23, 2026
@jenshannoschwalm jenshannoschwalm added bugfix pull request fixing a bug scope: image processing correcting pixels labels Mar 23, 2026
@jenshannoschwalm
Copy link
Collaborator Author

Just mentioning, if we go this way there is not much SSE specific code remaining (old colorbalance)

@TurboGit
Copy link
Member

Looks fine by me. One question the integration test log above is the result with only the PR, right?

Asking because there is some regressions, so maybe I should merge this before I push the changes in the integration test expected output that I have prepared today.

@jenshannoschwalm
Copy link
Collaborator Author

Yes, the log was only with this PR. So good to merge soon :-) Also there is #20639 doing no maths, just CL interface corrections.

@jenshannoschwalm
Copy link
Collaborator Author

Asking because there is some regressions, so maybe I should merge this before I push the changes in the integration test expected output that I have prepared today.

Yes, there is quite a lot of CL differences that would need some love :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix pull request fixing a bug scope: image processing correcting pixels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants