playcanvas
It seems reflectionSource do not respect gamma ?
I am sure my standard materials do not need useGammaTonemap, so I set it to false. All textures are sampled correctly except for material.cubeMap, which appears darker than it should. Here is what I found:
In standard.js, we have the following logic:
subCode = subCode.replace(/\$DECODE/g, ChunkUtils.decodeFunc((!options.litOptions.gamma && encoding === 'srgb')
? 'linear'
: encoding)
);
It will check litOptions.gamma to pick linear or encoding.
But in lit-shader.js, the reflectionSource will not check gamma:
if (options.reflectionSource === 'envAtlasHQ') {
func.append(options.fixSeams ? chunks.fixCubemapSeamsStretchPS : chunks.fixCubemapSeamsNonePS);
func.append(chunks.envAtlasPS);
func.append(chunks.reflectionEnvHQPS
.replace(/\$DECODE_CUBEMAP/g, ChunkUtils.decodeFunc(options.reflectionCubemapEncoding))
.replace(/\$DECODE/g, ChunkUtils.decodeFunc(options.reflectionEncoding))
);
} else if (options.reflectionSource === 'envAtlas') {
func.append(chunks.envAtlasPS);
func.append(chunks.reflectionEnvPS.replace(/\$DECODE/g, ChunkUtils.decodeFunc(options.reflectionEncoding)));
} else if (options.reflectionSource === 'cubeMap') {
func.append(options.fixSeams ? chunks.fixCubemapSeamsStretchPS : chunks.fixCubemapSeamsNonePS);
func.append(chunks.reflectionCubePS.replace(/\$DECODE/g, ChunkUtils.decodeFunc(options.reflectionEncoding)));
} else if (options.reflectionSource === 'sphereMap') {
func.append(chunks.reflectionSpherePS.replace(/\$DECODE/g, ChunkUtils.decodeFunc(options.reflectionEncoding)));
}
This means all of the $DECODE will be replaced to decodeGamma.
Even though I set app.scene.gammaCorrection = 0, the cubeMap still use decodeGamma, but what I want is decodeLinear... 😢
Oh that's interesting. We can investigate updating the reflection decode to respect scene.gammaCorrect, but that may break existing projects. @mvaligursky perhaps we can consider for engine v2?
We probably won't be able to push a change like this out quickly though, so I suggest you patch reflectionCubePS shader chunk to do what you need till we have a solution.
@slimbuck I think you should respect material.useGammaTonemap instead of scene.gammaCorrection, because it is a material level setting.
We probably won't be able to push a change like this out quickly though, so I suggest you patch reflectionCubePS shader chunk to do what you need till we have a solution.
Don't worry, I can use a custom build of PlayCanvas engine. 😃
Typically, the decode functions on texture sampling is purely based on the texture format. The lighting part of the shaders needs all data in linear space, and so if the texture is sRGB it needs to be decoded to linear space. If the texture is linear, it should do pass-through decoding.
useGammaTonemap is not related to the way textures are sampled, but only to the way the final pixel is written to the framebuffer.
In engine v2 the useGammaTonemap was removed, and we only have useTonemap. Gamma is automaticaly based on the scene / camera gamma settings.
By the way, from the use of useGammaTonemap it seems you're using engine v1. This has changed considerably in v2. See https://github.com/playcanvas/engine/pull/6702 and https://github.com/playcanvas/engine/pull/6714 and https://github.com/playcanvas/engine/pull/6736
Typically, the decode functions on texture sampling is purely based on the texture format. The lighting part of the shaders needs all data in linear space, and so if the texture is sRGB it needs to be decoded to linear space. If the texture is linear, it should do pass-through decoding.
Yes, but the texture.encoding is just a getter:
get encoding() {
switch (this.type) {
case TEXTURETYPE_RGBM:
return 'rgbm';
case TEXTURETYPE_RGBE:
return 'rgbe';
case TEXTURETYPE_RGBP:
return 'rgbp';
default:
return (this.format === PIXELFORMAT_RGB16F ||
this.format === PIXELFORMAT_RGB32F ||
this.format === PIXELFORMAT_RGBA16F ||
this.format === PIXELFORMAT_RGBA32F ||
isIntegerPixelFormat(this.format)) ? 'linear' : 'srgb';
}
}
Even though I know the decodeLinear is correct, I can not change it. 😢
Why decodeLinear is correct ?
because my scenes are very old. In the old engine, they will be sampled by textureCubeSRGB:
vec4 textureCubeSRGB(samplerCube tex, vec3 uvw) {
return textureCube(tex, uvw);
}
It is in linear space.
In order to keep the same result, I need to replace $DECODE to decodeLinear. But now we have no way to do this.
it seems you're using engine v1.
Yes, I want to upgrade to v2 too, but I still need to support WebGL 1. So I may stick to 1.73.4 until our product drop support iOS 15-...
In case you use engine only, you should be able to change the type of the texture like this. Not sure specifically about cubemaps or what options are supported there, @slimbuck might know more
helipad: new pc.Asset(
'helipad-env-atlas',
'texture',
{ url: rootPath + '/static/assets/cubemaps/helipad-env-atlas.png' },
{ type: pc.TEXTURETYPE_RGBP, mipmaps: false }
),
I'll leave this issue opened for now, but I strongly suspect we won't be addressing this for engine v1. In engine v2, we have 'srgb' option for textures, and this should work.
I'll close this. My opinion here is that this behaves as expected. If the texture is HDR, it can be linear. But for LDR formats, it's always gamma correct. That's the engine expectations. This is how linear / gamma correct workflow works, and in engine v2 we enforce this even more.
I'll close this. My opinion here is that this behaves as expected. If the texture is HDR, it can be linear. But for LDR formats, it's always gamma correct. That's the engine expectations. This is how linear / gamma correct workflow works, and in engine v2 we enforce this even more.
I'd like to give a counterexample here — not all textures require gamma correction. Take this example: https://gamemcu.com/su7/
The environment reflection here is generated using noise, and we can sample it directly. If we apply gamma correction to all LDR textures, then we'd need to additionally encode gamma when generating the noise.
I'm not saying there is no use case where people would prefer different behavior, but this is the behavior the engine officially supports. If different behavior is needed, they'd need to override shader chunks I think.