Sized PMREM #23322
Sized PMREM #23322
Conversation
| @@ -205,8 +222,18 @@ class PMREMGenerator { | |||
|
|
|||
| _fromTexture( texture, renderTarget ) { | |||
|
|
|||
| if ( texture.mapping === CubeReflectionMapping || texture.mapping === CubeRefractionMapping ) { | |||
|
|
|||
| this._setSize( texture.image.length === 0 ? 16 : texture.image[ 0 ].width ?? texture.image[ 0 ].image.width ); | |||
There was a problem hiding this comment.
@Mugen87 This is hideous and hacky, and I'd love for there to be a better way. I really wish textures had their own width and height members, because as it stands, finding that info is totally different for textures, cubemaps, data textures, textures created by renderTargets, etc. I'm not sure what the best path forward for three.js is, but this was the best I could manage for now to get this working. Also, I'm falling back to 16 because that's what the WebXR cubemap is. I couldn't find its size anywhere at all; am I missing something?
There was a problem hiding this comment.
I couldn't find its size anywhere at all; am I missing something?
I did not find a note about the size in the documentation.
I'm not sure what the best path forward for three.js is, but this was the best I could manage for now to get this working.
Yeah, it's inconvenient right now to cover all possible texture permutations. The main problem is that cube textures can hold an array of images or an array of (data) textures. I'm not sure yet but one solution could be to wrap the images of a cube texture in instances of Texture to unify the access.
There was a problem hiding this comment.
Maybe, but I was thinking just add width and height members to the base texture class and fill them in appropriately so that we don't have to dig into the .image at all.
And my point with the WebXR environment cube map is that the three.js code knows the size (it gets an array back from the API), but I don't think it saves that size anywhere that we as users of three.js can retrieve. That feels like a bug, but a bug in the API shape that a texture width and height could solve.
|
@mikkoh This is the PMREM update I told you about to make small environment maps work even better. |
|
It seems the "old" node material needs an update so it works with this PR. /cc @sunag |
|
Thanks! |
|
@sunag could you take a look at the nodes examples that broke (
|
|
@elalish seems like
|
How long do we plan to support the "old" node material? Maybe this is a good opportunity to remove it so we can focus on the new system? |
Yes, I agree too. |
* new PMREM format works; #defines functional * regenerate internals when envMap size changes * hooked up variable size * added disposal, cleanup * fixed size fallback * fix linting * mrdoob approves? * removed envMapCubeUV * mrdoob approves?
At long last, I've finally refactored PMREM to have variable size, based on the size of the input environment map. This helps in several ways:
These are all now fixed. I also changed the internal PMREM format a bit to make it scale more easily. @mrdoob @Mugen87 @WestLangley
It should be noted that there is a direct relationship between the minimum roughness value you want to resolve on your materials and the required environment size. For instance, the old default of 1024x512 (or a cubemap of 256x256) handled down to roughness 0.05. For 0.1, only a 64x64 cubemap is required (which can save a lot of bandwidth on your HDR). The WebXR lighting cubemap is 16x16, so it can only resolve roughness down to 0.21. Now, you may also be able to get away with less environment map resolution if you are rendering to a small canvas or your shiny parts are more curved. However, keep in mind that in the limit of roughness => 0, you may need infinite environment resolution to avoid seeing pixels (this is just the physics of optics).
As an example, here is the old code with a 1024x512 envMap (and zero roughness on the visor):
And here is the new way with the HDR downsampled to 256x128 (130kb vs 450kb).
