GNOME Bugzilla – Bug 789695
CIE: Make RGB to Lab fast paths accurate; add conversion from "RGBA float" to "CIE Lab float"
Last modified: 2017-11-07 09:41:24 UTC
The RGB to Lab fast paths in extensions/CIE.c need a BABL_TOLERANCE of 0.000006 on an Intel i7 CPU. Even though the fast paths are pretty naive, they do make a substantial difference in some cases. For example this one: commit b4535d6efc205c790c8263ba3f9568f9b442027d Author: Debarshi Ray <debarshir@gnome.org> Date: Sat Jan 9 19:41:29 2016 +0100 CIE: Add conversion from "RGB float" to "CIE Lab float" and vice versa Conversions from "RGB float" to "CIE Lab float" are 10 times slower without this. One use case is passing a "RGB float" buffer to a saturation operation that works in the "CIE Lab" colour space. https://bugzilla.gnome.org/show_bug.cgi?id=760310 This inaccuracy is caused by the fast paths using slightly different matrices than the reference matrices in the Babl space to convert from RGB to XYZ.
Created attachment 362608 [details] [review] CIE: Add conversion from "RGBA float" to "CIE Lab float"
Created attachment 362609 [details] [review] CIE: Use cbrt instead of pow for the reference XYZ to LAB conversion
Created attachment 362610 [details] [review] CIE: Make the RGB to Lab fast paths as accurate as the reference
Just for completeness of the bug, as mentioned on IRC - the XYZ <-> RGB conversions should no longer be using constants, but values taken from babl_space, since we now support more than just sRGB primaries.
Created attachment 362617 [details] [review] CIE: Make the RGB to Lab fast paths work with non-sRGB primaries
All patches pushed, thanks for clearing out these warnings about missing fast paths =) commit 72a074df6545fcb35b82ecc2636864afecaa9294 Author: Debarshi Ray <debarshir@gnome.org> Date: Tue Oct 31 12:20:45 2017 +0100 CIE: Make the RGB to Lab fast paths work with non-sRGB primaries The older constants were tied to sRGB primaries. Using the matrices from the Babl space removes this restriction. https://bugzilla.gnome.org/show_bug.cgi?id=789695 commit e17807f235557d5d7b5d2dff3e4c96a7386d5d36 Author: Debarshi Ray <debarshir@gnome.org> Date: Tue Oct 31 09:43:53 2017 +0100 CIE: Make the RGB to Lab fast paths as accurate as the reference The matrices used by the fast paths to convert from RGB to XYZ didn't exactly match the reference matrices in the Babl space. This caused a measurable error in their output. https://bugzilla.gnome.org/show_bug.cgi?id=789695 commit ab883c55fa74dfaeb27d488f0bd5de53fd1b1afb Author: Debarshi Ray <debarshir@gnome.org> Date: Tue Oct 31 09:36:39 2017 +0100 CIE: Use cbrt instead of pow for the reference XYZ to LAB conversion The fast-paths use an inlining-friendly version of cbrt(3). Using something similar removes superficial differences between the two conversion paths. It's not like the C library's cbrt(3) will perform any worse than its own pow(3). https://bugzilla.gnome.org/show_bug.cgi?id=789695 commit ea00661dd8986b9a1380c9c500122dec8ca5a0db Author: Debarshi Ray <debarshir@gnome.org> Date: Sun Oct 29 13:52:26 2017 +0100 CIE: Add conversion from "RGBA float" to "CIE Lab float" Conversions from "RaGaBaA float" to "CIE Lab float", as seen when using gegl:shadows-highlights" go via: "RaGaBaA float" to "RGBA float" "RGBA float" to "RGB float" "RGB float" to "CIE Lab float" A direct conversion from "RaGaBaA float" to "CIE Lab float" in simple C is hindered by the need to check every pixel's alpha value to avoid dividing by zero. The pipeline stalls make it lose out to the look-up table and SIMD based conversions to unassociated alpha. However, we can trivially cut out the second step and still reduce some memory traffic. https://bugzilla.gnome.org/show_bug.cgi?id=789695
(In reply to Øyvind Kolås (pippin) from comment #6) > All patches pushed, thanks for clearing out these warnings about missing > fast paths =) > At least these 2 need the same adjustment https://git.gnome.org/browse/babl/tree/extensions/CIE.c#n750 https://git.gnome.org/browse/babl/tree/extensions/CIE.c#n784
(In reply to Massimo from comment #7) > (In reply to Øyvind Kolås (pippin) from comment #6) > > All patches pushed, thanks for clearing out these warnings about missing > > fast paths =) > > > > At least these 2 need the same adjustment > > https://git.gnome.org/browse/babl/tree/extensions/CIE.c#n750 > > https://git.gnome.org/browse/babl/tree/extensions/CIE.c#n784 Yes, as I mentioned on IRC, I have only looked at the RGB to Lab conversions and not the other way round. I'll do that once I have some tests to see what Lab to RGB fast-paths are missing, etc.. One small step at a time. :P
(In reply to Debarshi Ray from comment #8) > (In reply to Massimo from comment #7) > > (In reply to Øyvind Kolås (pippin) from comment #6) > > > All patches pushed, thanks for clearing out these warnings about missing > > > fast paths =) > > > > > > > At least these 2 need the same adjustment > > > > https://git.gnome.org/browse/babl/tree/extensions/CIE.c#n750 > > > > https://git.gnome.org/browse/babl/tree/extensions/CIE.c#n784 > > Yes, as I mentioned on IRC, I have only looked at the RGB to Lab conversions > and not the other way round. I'll do that once I have some tests to see what > Lab to RGB fast-paths are missing, etc.. One small step at a time. :P Going to attach some patches against bug 790011