mrdoob
MeshPhysicalMaterial: Match behavior of attenuationDistance to KHR_materials_volume
Related issue: N/A
Description
This is an alternative approach to https://github.com/mrdoob/three.js/pull/24620 See also https://github.com/mrdoob/three.js/pull/24620#issuecomment-1242754206
The default attenuationDistance in MeshPhysicalMaterial is 0.0, which is functionally equivalent to Infinity (c.f. https://github.com/mrdoob/three.js/blob/f160d03a91b77cb711bdef561a8dd960a7c9cc47/src/renderers/shaders/ShaderChunk/transmission_pars_fragment.glsl.js#L74 )
The KHR_materials_volume spec requires that attenuationDistance be greater than zero if present. It defaults to Infinity if not present. (c.f. https://github.com/KhronosGroup/glTF/blob/main/extensions/2.0/Khronos/KHR_materials_volume/README.md#properties )
The current GTLFExporter produces incorrect gltf files for MeshPhysicalMaterial if transmission is set greater than zero, but attenuationDistance is set to the default of zero. These incorrect files cannot be read in some popular GLTF software (like Blender 3.3 GLTF Importer)
This PR changes the default value of attenuationDistance to Infinity, and the behavior of 0 and Infinity to more closely align with the KHR_materials_volume specification.
Note: Only one of https://github.com/mrdoob/three.js/pull/24620 or this PR should be accepted. They are different approaches to the same problem.
I do not think we can use Infinity...
JSON doesn't support Infinity so we wouldn't be able to de/serialize the value.
I do not think we can use Infinity... JSON doesn't support Infinity so we wouldn't be able to de/serialize the value.
Why do we serialize default values sometimes -- and sometimes not?
Why do we serialize default values sometimes -- and sometimes not?
Oh! That is a good point. The fact that Infinity is the default means we can ignore it when de/serializing it 👍
Right. But my larger point is the toJSON() method needs to be revisited. It is not at all consistent.
It seems
webgl_nodes_loader_gltf_transmissionis failing with this change.
This should be fixed now. (Required an additional change to GLTFLoader to use the new default)