-
-
Notifications
You must be signed in to change notification settings - Fork 35.5k
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
NodeMaterial: Use materialReference()
for env maps.
#28982
Conversation
📦 Bundle sizeFull ESM build, minified and gzipped.
🌳 Bundle size after tree-shakingMinimal build including a renderer, camera, empty scene, and dependencies.
|
A few time ago I started this approach #28422 avoiding classes and directing everything to the TSL approach, there are certainly a lot of classes to review with this. |
let envNode = this.envNode; | ||
|
||
if ( envNode.isTextureNode ) { | ||
if ( envNode.isTextureNode || envNode.isMaterialReferenceNode ) { |
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.
FYI: I had to add this check otherwise PMREM generation was broken.
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.
Exchanging the env map for basic, lambert and phong works now as expected. Great!
However, EnvironmentNode
needs an enhancement since the PMREM isn't regenerated when envMap
is changed.
updateBefore()
might be a good spot for checking the status but I'm not sure how to access the material without the builder reference. Besides, ideally we only update the PMREM node without regenerating the radiance
and irradiance
node. Is that even possible?
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.
It seems like having a reference()/materialReference()
supporting pmrem
we could be removed this cacheEnv using MeshStandardNodeMaterial.setupEnvironment()
for create this.
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.
Let's tackle this in a different PR.
Related issue: -
Description
It is currently not possible to dynamically update
material.envMap
: https://jsfiddle.net/br8ezgfw/1/That works in
WebGLRenderer
and it also works to exchange normal textures (e.g.material.map
) withWebGPURenderer
.The issue can be resolved by using
materialReference()
when setting up the env node inNodeMaterial
but that leads unfortunately to a cyclic dependency:@sunag I wonder if you have any ideas to solve this issue. What I find not optimal is the fact that
CubeTextureNode.getDefaultUV()
has a material dependency sincerefractVector
relies on the refraction ratio of the material.Would it be an option to move the uv computation into the place where the sampling actually happens (that is
BasicLightingModel
)? However, without proper default uvs, it might be inconvenient to usecubeTexture()
elsewhere in the code base and on app level.