fixed cubemap dynamic example and small PMREMs #23428
Conversation
|
Thanks! |
#23433 should fix this issue for cube render targets in general. |
| @@ -73,19 +73,23 @@ | |||
|
|
|||
| // | |||
|
|
|||
| cubeRenderTarget1 = new THREE.WebGLCubeRenderTarget( 256 ); | |||
| const envSize = 64; // minimum size for roughness >= 0.1 | |||
There was a problem hiding this comment.
I guess 64 was used so we have at least one use case with a small sized environment map. TBH, I'm not sure which value is better. Both setups makes sense.
There was a problem hiding this comment.
Since users copy these patterns, I suggest we set it to a recommended size -- not to an edge case for testing.
There was a problem hiding this comment.
Actually, I changed it here on purpose. Yes, this did catch an error, but more importantly, there's nothing special about 256. We've actually found most of our users preferring smaller envMaps to get reasonable HDR downloads (since they're PNGs internally). 256 is required for roughness >= 0.05, while 64 is for roughness >= 0.1; I consider that to be a very worthwhile trade in performance (both GPU cache and download) for the vast majority of users.
There was a problem hiding this comment.
I think it would be good to document this.
@elalish what are the roughness ranges for 32, 128 and 512?
| envmap size | roughness |
|---|---|
| 32 | ??? |
| 64 | >= 0.1 |
| 128 | ??? |
| 256 | >= 0.05 |
| 512 | ??? |
Once we complete the table we could add it to the docs:
https://threejs.org/docs/?q=pmrem#api/en/extras/PMREMGenerator
There was a problem hiding this comment.
roughness = ( 2 ^ ( -0.5 * mip ) ) / 1.16, where dimension is 2 ^ mip. So:
- 16: mip 4: roughness 0.21
- 32: mip 5: roughness 0.15
- 64: mip 6: roughness 0.11
- 128: mip 7: roughness 0.076
- 256: mip 8: roughness 0.054
- 512: mip 9: roughness 0.038
- 1024: mip 10: roughness 0.027
Related issue: #23322 (comment)
The example was yet another victim of textures not containing their dimensions; a simple fix, but I'd really like to see three's API improved here. I also optimized it a bit, which revealed an actual PMREM bug for smaller input sizes, which is here fixed as well.