dyne / frei0r

A large collection of free and portable video plugins
https://frei0r.dyne.org/
GNU General Public License v2.0
420 stars 90 forks source link

select0r hue term for cylindrical space not handled correctly(?) #87

Closed Undoundoundo closed 4 years ago

Undoundoundo commented 4 years ago

Still verifying with original author, but it appears to me that the hue term in HCI cylindrical space distance calculations for the select0r plugin is not handled correctly.

The rgb2hci function appears to work correctly, returning a hue term in the range of (0.5,0.5] going from cyan (-0.5, exclusive) on the counter-clockwise side through blue (-1/3rd), magenta (-1/6th), red (0), yellow (1/6th), green (1/3rd), to cyan (0.5, inclusive).

In the cylindrical color space distance functions, however, the hue term, crossing across zero, is handled via these lines:

    ax=fabsf(hue-chue);
    if (ax>0.5) ax=ax-0.5;

Where hue and chue are the two colors' hue components, fabsf takes the absolute value, and ax is ostensibly the angular distance,

This causes an issue with e.g. comparing magenta (-0.1666) and cyan (0.5), nearly opposite colors on a hue wheel:

    ax = fabsf(0.5 - -0.1666) // -> 0.6666...
    if (ax > 0.5) { // true
      ax = ax - 0.5; // 0.6666 - 0.5 -> 0.16666
    }

Making the hue distance between them equivalent to, say, red and yellow, which are neighboring on a hue wheel.

If plotted, it becomes more readily apparent that instead of the distance function being smooth, it is discontinuous. imgur.com/XUpvf6f

There are many ways to handle this, two common ones employed will find favor depending on which ends up compiling to faster execution. A branched approach:

ax = hue - chue;
if (ax > 0.5) { ax -= 1; }
else if (ax < -0.5) { ax += 1; }

A non-branched approach:

ax = 0.5 - fabsf(fabsf(hue - chue) - 0.5);

Either should result in an appropriate hue difference (angle difference) and a continuous function. imgur.com/6TNOJXW ( both assume the angles to be normalized in the -0.5 .. 0.5 range and should not be used as general purpose angle difference formula )

Note that proposed changes to this code will affect its results when using one of the cylindrical color spaces, and dependent projects may wish to add a compatibility flag or advise end-users of such a change.

ddennedy commented 4 years ago

Marko sent an e-mail to the developer mailing list asking someone to help you because he is having problems with git-ssh. He agrees with your finding, and I volunteered to help. Have you decided on a preference between your 2 changes? If so, can you make a pull request or at least run git diff and post it here? Thanks

Undoundoundo commented 4 years ago

Bit rusty on my github routine myself, but the above PR should be it. I ended up choosing the non-branching version based solely on online benchmarks where it had a slight edge ( < 0.01% ). Exact build configuration may well cause the branching version to have the edge, but it's unlikely to affect performance enough to matter.