Problem with hue color shift

[Carbonate]

I am working on a project within PIXI that requires me to shift the hue of a saturated rainbow gradient in a loop so it looks like its moving. I figured this would be cut and dry, since changing the hue of said image in Photoshop make it appear as if its moving.

When starting out and getting the filter in place and animating, I was perplexed by the results. The colors looked nothing like I expected nor could have predicted. You can see a little demo of what I'm talking about and seeing on codepen here.

I started digging to see what could be the problem, trying different image formats and color depths to no avail.

I gave a shot at another canvas solution, GLFX, and can get the proper results I expected, as also seen on codepen here.

 

My question to you guys is whats going on here? I am suspecting either an error in PIXI or my understanding of how hue rotation should work. I tried a few different PIXI revisions and didn't spot a difference. I dug a little further and came across an article explaining the difference between HSL and CSS hue rotations and its inaccuracies here. Though, that doesn't help me fix the issue with PIXI. Anyway to fix this on my end while staying within PIXI?

Thank you.

[george]

Yes! I think this is plain wrong. You should definitely create an issue on github for this http://github.com/pixijs/pixi.js

This is the currently wrong version in pixi:
https://github.com/pixijs/pixi.js/blob/364c2457c6693d6e3ab68786539c53d566503e7b/src/filters/color/ColorMatrixFilter.js#L189

This is the sane version from glfx:
https://github.com/evanw/glfx.js/blob/58841c23919bd59787effc0333a4897b43835412/src/filters/adjust/huesaturation.js

On the first sight they are both doing the same, which is rotating an imaginary rgb cube. But the lum* weights in the expanded rotation matrix look wrong.

Here is a nice answer on hue rotation.
http://stackoverflow.com/a/8510751

Regards
George

[ivan.popelyshev]

Ok, make a PR please?

[george]

Hello again, here we go. 

First of all the demo: 
http://codepen.io/gkey/pen/yJeVVy

Test Cases:

1.  `old` is the current wrong version
2. `better` is a fixed matrix
3. "new shader" is the shader from the glfx integrated as a new filter for comparison and as our reference.

Number 2 looks pretty good in comparison to 3.

You can see the error in the current implementation pretty well when you exaggerate the saturation.  To do so activate the checkbox 'overdrive'. It looks like the image attached.

 

Here is my current branch for the changes:
https://github.com/georgiee/pixi.js/tree/fix-hue
Here the adjusted matrix:

https://github.com/georgiee/pixi.js/blob/7f33ca6bcaae305d55783751248ee96812b1d189/src/filters/color/ColorMatrixFilter.js#L194
 

And the new saturation filter and shader:
https://github.com/georgiee/pixi.js/blob/fix-hue/src/filters/color/HueSaturationFilter.js
https://github.com/georgiee/pixi.js/blob/7f33ca6bcaae305d55783751248ee96812b1d189/src/filters/color/hue-saturation.frag
 

I see no better quality and no other advantages in using a separate hue saturation filter so I would prepare a PR for the updated matrix (Nr. 2)

Regards
George

[george]

This is ready to get merged: https://github.com/pixijs/pixi.js/pull/2638

Have a nice day
Regards George

[ivan.popelyshev]

This branch has conflicts that must be resolved

May be you have an idea what triggered that? I cant merge it myself, but I can help to prepare it

[george]

Hello,

I have no clue, I mean it's a single commit on a single file which haven't been changed for weeks plus I based it on the most recent state of the dev branch. But this can be merged manually with little effort. So it's fine for the maintainers I guess.

Regards

[ivan.popelyshev]

@george the name of directory is "colormatrix" and not "color", that's why

[george]

That was it! Thanks. This happened because I worked on master all the time and quick fixed this after completing the fix.
I updated the existing PR, now it's find. Phew, what a trip, thanks Ivan.