google-code-export / gpick

Automatically exported from code.google.com/p/gpick
2 stars 0 forks source link

RGB linearization is broken #105

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
This is simple to reproduce -- drop black and white into the 'Blend Colors' 
dialog, reduce the first number of steps so there is only one inbetween color, 
and check the 'middle color'. This should be no darker than #777777, but not 
only is it much darker (#3b... or thereabouts), it's also not a shade of actual 
gray, it's greenish.

Not sure when this change was introduced, but the 
color_rgb_get_linear/color_linear_get_rgb functions seem to be using incorrect 
coefficients (2.1 vs 2.2, or ideally (most accurately) a 2-part function that's 
linear below a certain point and gamma 2.4 above that point).. and also a 
different coefficient(2.0) for the green channel.  I haven't had the time yet 
to track down when this change occurred, but it makes RGB blends much worse in 
quality (overall, way too dark, and as I mentioned, discolored.)

Original issue reported on code.google.com by fintic...@gmail.com on 10 Jun 2013 at 3:27

GoogleCodeExporter commented 9 years ago
It's not clear that this was ever correct, but something recent seems to have 
made it vastly worse (probably something outside of the above mentioned 
functions)

Original comment by fintic...@gmail.com on 11 Jun 2013 at 12:41

GoogleCodeExporter commented 9 years ago
I take that back: the attached patch to color_rgb_get_linear, 
color_linear_get_rgb,
implementing the companding recommended by Bruce Lindbloom, 
http://www.brucelindbloom.com/Eqn_RGB_to_XYZ.html + 
http://www.brucelindbloom.com/Eqn_XYZ_to_RGB.html, seems to have fixed the 
excessive darkness. These equations were already in use for the existing RGB 
<-> XYZ transforms, but for some reason (speed?) were not used in the linear 
<-> nonlinear RGB transform functions).

Not pushing this patch yet because I'm sure there was a reason for the use of 
unconventional and asymmetric coefficients in the previous RGB de/linearization 
code, and hope to find out what those reasons were.

Original comment by fintic...@gmail.com on 11 Jun 2013 at 1:41

Attachments:

GoogleCodeExporter commented 9 years ago
The original de/linearization functions are a total fail. Worst of all things, 
is that the linearization code is in color_linear_get_rgb, and delinearization 
code is in color_rgb_get_linear.

I do not think there was any reason to do anything differently, and not use a 
proper sRGB to linear conversion.

Original comment by thezbyg on 11 Jun 2013 at 3:50

GoogleCodeExporter commented 9 years ago
Okay, then I'll reverse the meaning of those two functions so 
color_rgb_get_linear returns linearized results and color_linear_get_rgb 
returns delinearized, adjust references appropriately, and push.

Original comment by fintic...@gmail.com on 12 Jun 2013 at 1:43

GoogleCodeExporter commented 9 years ago
Never mind the 'reverse the meaning'. It seems that they are currently (with my 
patch) already correct and the code using them is already correct too. I'll 
just check everything and push the patch as-is. 

Original comment by fintic...@gmail.com on 12 Jun 2013 at 2:18

GoogleCodeExporter commented 9 years ago
This issue was closed by revision 9664b3ff15f6.

Original comment by 00a...@gmail.com on 12 Jun 2013 at 2:30

GoogleCodeExporter commented 9 years ago
BTW, I've noticed that for some reason I'm obligated to do 'merge commits' 
every time I 'hg pull;hg update' from Google code and there are actual changes 
transferred. I'd like to avoid clogging up the commit log with these things but 
'hg push' doesn't seem to work without first 'hg merge'ing, despite the fact 
that the merge commits report 'no changes'.
Currently trying to fix this, any suggestions will be gratefully received.

Original comment by fintic...@gmail.com on 12 Jun 2013 at 2:38

GoogleCodeExporter commented 9 years ago
Before doing 'hg push' to Google Code repository, you can run 'hg pull 
--rebase', or 'hg rebase' if you already have pulled some remote changes while 
having local commits. This will move your local commits after the most recent 
remote commit.

The '--rebase' flag and 'hg rebase' command requires 'rebase' extension. which 
is included with default Mercurial distribution, but must be enabled by editing 
your '.hgrc' file.

More information about 'rebase' extension:
http://mercurial.selenic.com/wiki/RebaseExtension

Original comment by thezbyg on 12 Jun 2013 at 9:06