-
-
Notifications
You must be signed in to change notification settings - Fork 35.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow textures to be encoded/decoded in Linear,sRGB,RGBE,RGBM, etc #8117
Conversation
…issiveMap that can contain unbounded intensities.
Very nice! Incidentally, I ended up abandoning this approach of decoding these formats in the shader every frame. As you probably know, you can't use bilinear filtering on RGBE or LogLuv images and I found that I really needed mipmapping and filtering for optimal quality for IBL. I've ended up using RGBE but I unpack it into a floating-point texture at load time. Support is in the high 90's for float textures according to WebGL stats with the exception of IE11, which is at 92%. Still, those are pretty good numbers. |
That makes sense. :) I'd still advocate pulling this in because it does introduce the ability to encode/decode textures, and if you want you can use these decoders/encoders to write to floating point textures and then access thus via ENCODING_Linear. So I view most of this code as necessary for pre-decoding as well. |
I found the artifacts minimal when using RGBM7 and RGBM16 formats though, in part because M interpolates better than E: https://clara.io/view/d59d26ab-c26e-4fa0-8907-d52c0c8acc1a BTW we had this discussion a year ago here: #6593 That said I still think we should get this merged in because it enables decoding-based pipelines, whether pre or inline. |
Also I agree with you that pre-decoding is better, just more work, as I described here: #6383 (comment) |
Totally agree. I didn't mean to argue against the pull request. Only sharing my experience. RGBM definitely works better but I was having quality issues with some HDR maps. I think I remember having better luck with RGBD. |
Heh. We had the RGBD discussion as well last year: #6593 (comment) Because of that, I added RGBD in this PR but commented out until someone wants to add formal support. |
I found that RGBM's main limitation was its limited dynamic range. RGBM7 is limited to [0-7], and RGBM16 just goes from [0-16], which is better than LDR but far from the RGBE range of [0,2^127]. |
//THREE.LogLuv = 3003; TODO | ||
THREE.RGBM7 = 3004; | ||
THREE.RGBM16 = 3005; | ||
//THREE.RGBD = 3006; TODO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably do:
THREE.LinearTextureEncoding = 3000;
THREE.sRGBTextureEncoding = 3001; // AKA gamma 2.2.
THREE.RGBETextureEncoding = 3002; // AKA Radiance
THREE.RGBM7TextureEncoding = 3004;
THREE.RGBM16TextureEncoding = 3005;
or finally start doing this:
THREE.TEXTURE_ENCODINGS = {
LINEAR: 3000,
SRGB: 3001, // AKA gamma 2.2.
RGBE: 3002, // AKA Radiance
RGBM7: 3004,
RGBM16: 3005
};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll make this change now. Somehow I missed this comment.
Added support for Material.map to have an encoding to deal with @mattdesl's sRGB usecase. |
@mattdesl I've updated this PR to move away from the uniform-based selection of encodings in the glsl (which used up precious uniforms and also required a lot of if tests) to a preprocessor define system that is actually texture-specific rather than material specific. This will allow us to add as many texture-specific encodings as we want. I've also written a shader preprocessor so that I can use "#include" statements in my shaders. |
@WestLangley @mrdoob @MiiBond @tschw any thoughts on this PR? I would like to get this PR merged so that @spidersharma03 and myself can get the PMREM generator, which is dependent upon encodings, finished. I think with the new preprocessor define-based system there is no GLSL run-time cost to this PR's increased flexibility with regards to texture encodings, although the cost to compile is slightly higher. It also includes a useful but short WebGLShaderPreProcessor that can be further expanded - right now it supports only top-level "#include" statements. |
…it into texture_encoding
…_encoding # Conflicts: # src/renderers/shaders/ShaderLib.js
1fb11ae
to
0d45254
Compare
@mrdoob I have updated this PR with two changes in response to your feedback: (1) I've replaced the include-based function definition with JavaScript-based function generation here, per your request: (2) I've merged in your dev branch that has the new regex-based include system, and I have removed the more verbose include-system WebGLShaderPreprocessor. |
Great work! |
Allow textures to be encoded/decoded in Linear,sRGB,RGBE,RGBM, etc
Thanks! |
Thanks for being open to the change! Now onto the PMREM! |
Seems like it produced some breakage though...
|
I'll have a look now. BTW there will be some minor look changes because texture decoding no longer pays attention to WebGLRenderer.inputGamma = true/false, instead you have to set each texture.encoding = THREE.GammaEncoding to have similar gamma result. |
I see... I'm thinking whether it would be better to break backwards compatibility and show a warning when the user sets/access |
Also, out of curiosity... how come we're not using gl.UNPACK_COLORSPACE_CONVERSION_WEBGL? |
In the other PR I added perfect backwards compatibility with inputGamma. Best regards,
|
This PR implements the idea discussed here a year ago:
#6593
To understand the theory behind this, please read this page:
http://lousodrome.net/blog/light/2013/05/26/gamma-correct-and-hdr-rendering-in-a-32-bits-buffer/
I have created functions for texelEncode and texelDecode. I have not filled out all the formats in both situations, but I think that is really easy for others to contribute there on a need basis.
Right now I only use the encoding when accessing textures that are expected to be outside of the standard [0-1] range, which are generally light generating textures, either EnvMaps or Emissivemaps. I applied the encoding sparingly for now because I didn't want to unnecessarily use uniforms (all other maps generaly do not use such encodings) when they may be at a premium on mobile devices.
Clara.io uses this change in its custom ThreeJS in order to support HDR IBL maps without requiring the float texture extension. I believe SketchFab and Marmoset do similar things as well. Thus this is an industry standard change.
All existing code works as it did, so this should be a safe merge. I think it is also something that can easily be improved upon by others on this project as it is a very clean design.
/ping @WestLangley @spidersharma03