any information available regarding the upcoming breaking changes in 137 WebGLRenderer: Remove inline sRGB decode. by Mugen87 · Pull Request #23129 · mrdoob/three.js · GitHub ? from what i’ve overheard shaders could be affected (sRGBToLinear etc)?
2 Likes
The migration guide for r137 is available here: Migration Guide · mrdoob/three.js Wiki · GitHub
The general rule of thumb is all sRGB encoded textures have to be defined in RGBA8. In three.js that is the combination of THREE.RGBAFormat and THREE.UnsingedByteType. Custom shaders have to remove the inline GLSL decode (the invocation of sRGBToLinear()).
5 Likes
THREE.RGBAFormat and THREE.UnsingedByteType
i have to admit, i know nothing about shaders, for instance, i’ve noticed that 135 > 136 also changes practically all shaders i’ve been using, is this related? i tried to look for any mention of srgb but there seems to be none but still colors are drastically different.
135: Grass shader - CodeSandbox
136: https://codesandbox.io/s/grass-shader-forked-urlor?file=/src/GrassMaterial.js
i think it is related to the textures. this article Color management in three.js by @donmccurdy states
Textures with color data (
.map,.emissiveMap, etc.) should be configured with.encoding = sRGBEncoding; all other textures useLinearEncoding.
is this no longer the case? because as of 136 this breaks. textures receiving sRGBEncoding are washed out and dark. if i set them to linear (albeit the general encoding is srgb) it looks good again, like 135. is this a bug perhaps?
No, this approach is still correct.
I’m afraid you did not handle color spaces correctly in your shader. If you sample a sRGB texture, it was previously necessary to decode texels in GLSL with sRGBToLinear() into linear color space. This is not necessary anymore since all RGBA8 sRGB encoded textures are automatically decoded by WebGL.
Since your custom material decodes texels values now, you also have to encode to sRGB at the end of your fragment shader. Change the last line in your shader code from:
gl_FragColor = col;
to
gl_FragColor = LinearTosRGB( col );
or even better add right below gl_FragColor = col;.
#include <encodings_fragment>
With this approach, you have a proper color space workflow.
1 Like
do you mean in the upcoming 137? because if i do what you said and just add
#include <encodings_fragment>
then i still get broken colors. it’s not as dark but it lost all its dynamic range, the colors are now bright but washed out.
i can get the same result as 135 if i specifically encode the textures as THREE.LinearEncoding. react auto encodes them to SRGB by default, because that was always suggested. i wonder if that’s not accurate any longer. should i not encode to srgb and leave textures linear from 136 onwards?
You should still mark sRGB encoded color textures via texture.encoding = THREE.sRGBEncoding;
With r135 the code should actually look like so: Grass shader (forked) - CodeSandbox
When upgrading to r136, you have to remove the inline decode: Grass shader (forked) - CodeSandbox
So if the shader would be correct right from the beginning, you could just apply the task from the migration guide (the removal of sRGBToLinear()).
what i do not understand is, after you have made the correct changes, your result, the box you have posted, is still color incorrect, it completely blows out all the colors. the only way to fix it is to set the texture to linear, now colors don’t blow or band.
on the poimandres discord where srgb encoding is the norm i hear people complaining all day now after they have switched to 137, even for non custom shader stuff, regular meshstandardmaterials etc and more and more i see them go linear against all recommendations.
i believe there is either a bug in threejs, or the new recommended normal for textures has switched from srgb to linear.
Sorry, but except from this thread we don’t get this feedback at the forum, stackoverflow or GitHub.
I suggest you create a fiddle without react that shows the visual variations between different three.js versions.
it could be that vanilla srgb is not so common, projects seem to be linear. i assume most people using srgb are either on aframe and r3f at this point. i will try to clamp down on simpler examples, and/or express them in vanilla. i agree that these complex shader things make it needlessly complex to track down the root issue.
i think i found something, the original shader defines colors inside the shader, this is what breaks when jumping to 137:
//Add more green towards root
col = mix(vec4(0.0, 0.6, 0.0, 1.0), col, frc);
//Add a shadow towards root
col = mix(vec4(0.0, 0.1, 0.0, 1.0), col, frc);
when i do this it’s fixed:
uniforms: {
tipColor: { value: new THREE.Color(0.0, 0.6, 0.0).convertSRGBToLinear() },
bottomColor: { value: new THREE.Color(0.0, 0.1, 0.0).convertSRGBToLinear() },
}
...
uniform vec3 tipColor;
uniform vec3 bottomColor;
...
//Add more green towards root
col = mix(vec4(tipColor, 1.0), col, frc);
//Add a shadow towards root
col = mix(vec4(bottomColor, 1.0), col, frc);
is there a recommendation how we should handle colors inside the shader, for instance LinearTosRGB(col), or is uniforms + convert better?
Your new code is the correct fix. Vertex colors or uniform color values have to be converted to linear since they are usually defined in sRGB. You want to perform this conversion in JS/TS before the values are passed to the GPU.
If you embed a color right in the shader, keep in mind that this value is supposed to be in linear color space. So no gamma is applied on it.
makes sense, thank you for clearing it up!
ps @donmccurdy could in-shader color manipulation and #include <tonemapping_fragment> +
#include <encodings_fragment> perhaps be added to Color management in three.js this was the missing 
Comments on Color Management: Documentation by donmccurdy · Pull Request #23430 · mrdoob/three.js · GitHub would be welcome, but I don’t think sRGB → Linear-sRGB conversion in a shader is the approach I’d recommend. Vertex colors in three.js should always be Linear-sRGB before GPU upload, and any colors attached to a material must be Linear-sRGB as well. The only thing that should be left for GPU conversion is textures.
The two #include statements for output encoding would make sense to include in that documentation for now, although they could also be moved out of the shader eventually.