-
Notifications
You must be signed in to change notification settings - Fork 110
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
BREAKING feature: preparing for Three.js r167 #1446
Conversation
import WebGPU / NodeMaterial stuff from `three/webgpu` See: mrdoob/three.js#28650 Three.js r167 is not released yet! I tested the behavior by using `yarn link` on `three` and `@types/three` importmaps for Three.js in webgpu examples are temporarily replaced to node_modules, please change them back before merging Some codes inside MToonNodeMaterial emit type errors because of recent @types/three changes I'm asking Methuselar96 why the change is made See: three-types/three-ts-types#1023 (comment)
The failing test says |
r167 just released! looking forward to 3.0 release |
package.json, readme, examples the build is failing
See about the type assertion: three-types/three-ts-types#1123
`indirectDiffuse`, `indirectSpecular` are no longer interface functions, replaced by `indirect` - See: https://github.com/mrdoob/three.js/blob/4cc8fdfa944181c4274c91061574bcf033315bee/src/nodes/functions/PhysicalLightingModel.js#L510 - See: https://github.com/mrdoob/three.js/blob/4cc8fdfa944181c4274c91061574bcf033315bee/src/nodes/lighting/LightsNode.js#L102
), | ||
); | ||
} | ||
|
||
indirectDiffuse({ irradiance, reflectedLight }: Nodes.LightingModelIndirectInput) { | ||
indirect(context: THREE.LightingModelIndirectInput) { |
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.
not mentioned in https://github.com/mrdoob/three.js/wiki/Migration-Guide#166--r167 ..
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.
Three.js devs don't assume users to extend NodeMaterial
😅
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.
So we will rename exports in another PR?
No, we're going to go |
This PR adapts usage of WebGPU stuff to the r167 way.
import WebGPU / NodeMaterial stuff from
three/webgpu
.See: mrdoob/three.js#28650
Three.js r167 is not released yet!I tested the behavior by usingyarn link
onthree
and@types/three
.importmaps for Three.js in webgpu examples are temporarily replaced to node_modules, please change them back before merging.This PR also bumps Three.js to r167.
Several changes are made even outside the WebGPU scope in order to follow r167 changes.
MToonNodeMaterial now requires r167 or higher.
The main WebGL module should still work in r137-r166.