search

vasturiano / three-globe

WebGL Globe Data Visualization as a ThreeJS reusable 3D object
https://vasturiano.github.io/three-globe/example/links/
MIT License
1.19k stars 145 forks source link

arcColor: Color replication issue since 2.29.0 #88

Closed marconett closed 9 months ago

marconett commented 9 months ago

I recently updated this library in my project from 2.25.4 to 2.30.0 and noticed an issue with the color of arcs not being replicated correctly.

Trying .arcColor(() => '#ff0000'), the color gets displayed correctly, but something like .arcColor(() => '#33CC66'), the color gets a white tint and looks different.

I boiled it down to the change of the utility function color2ShaderArr in https://github.com/vasturiano/three-globe/blob/master/src/utils/color-utils.js in the following commit: https://github.com/vasturiano/three-globe/commit/7b628e0952b63440cc4f9934e61f3d8cbf145917#diff-2683afbd9f919b557d9e16672ab05d332ce50a2f2a9d0cd6304ccde2cbe3086bR8 which was publicised in 2.29.0.

Changing color2ShaderArr back to it's state in 2.28.0, the color is replicated correctly again.

reference color #33CC66 screenshot 2.29.0 (wrong) screenshot 2.28.0 (correct)
ref wrong correct
vasturiano commented 9 months ago

@marconett thanks for reaching out.

This certainly looks related to the color space used inside ThreeJS. It's typical of color mismatches with values in the middle (between 00 and FF), as the transfer function changes.

https://threejs.org/docs/#manual/en/introduction/Color-management

However, I cannot reproduce your case with the latest version of both this lib and ThreeJS itself. I know ThreeJS has changed their internal color space logic a few versions back, which prompts me to ask which version of ThreeJS are you using, as this may play a factor.

marconett commented 9 months ago

My project is currently on three 0.150.1

vasturiano commented 9 months ago

@marconett that would be the reason why then. In a following version the color spaces were updated. 153 if I'm not mistaken.

https://github.com/mrdoob/three.js/pull/26162

marconett commented 9 months ago

Thanks! For now I'm going to release with the older version but will keep in mind updating threejs later on.

I'd suggest updating the threejs peer dependency version.

vasturiano commented 9 months ago

@marconett yeah I thought about updating the peer dep version. But the fact is that the lib is actually fully compatible with older versions of ThreeJS, so it seemed to me it would be overly restrictive to do so. These are internal changes to the ThreeJS behaviour, which I would rather assume consumers are aware of.

But thanks for the good suggestion in any case. 👍