Skip to content
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.

Sign up for GitHub

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

Jump to bottom

TSL: Fix label() usage in uniform() #1023

Merged
merged 11 commits into from
Jun 19, 2024
Merged

TSL: Fix label() usage in uniform() #1023

merged 11 commits into from
Jun 19, 2024

Conversation

Methuselah96
Copy link
Contributor

@Methuselah96 Methuselah96 mentioned this pull request Jun 18, 2024
Closed
36 tasks
@Methuselah96 Methuselah96 merged commit 9a08e07 into master Jun 19, 2024
4 checks passed
@Methuselah96 Methuselah96 deleted the update-three branch June 19, 2024 03:14
@0b5vr
Copy link
Contributor

0b5vr commented Jul 23, 2024 •
edited
Loading

@Methuselah96 What is the intention of the commit 4819e9c ?
I assume that there was some inconvenience in term of maintainability?

I'm using methods on nodes of NodeMaterials (NodeMaterial.positionNode.add() for example). This commit made us unable to use node methods.

0b5vr added a commit to pixiv/three-vrm that referenced this pull request Jul 23, 2024
@0b5vr
BREAKING feature: preparing for Three.js r167
150d637 
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)
@0b5vr 0b5vr mentioned this pull request Jul 23, 2024
Merged
@Methuselah96 Methuselah96 restored the update-three branch July 23, 2024 12:50
@Methuselah96 Methuselah96 deleted the update-three branch July 23, 2024 12:54
@Methuselah96
Copy link
Contributor Author

Methuselah96 commented Jul 23, 2024 •
edited
Loading

@0b5vr The issue is that currently Typescript does not allow a ShaderNodeObject<SubTypeNode> to be assigned to ShaderNodeObject<Node> because of methods that return this like UniformNode.label(). I'm not sure what the best solution is. I've created #1123 to track this issue. You can see that the text-examples build is failing (test is failing for unrelated reasons).

@0b5vr
Copy link
Contributor

0b5vr commented Jul 25, 2024

@0b5vr The issue is that currently Typescript does not allow a ShaderNodeObject<SubTypeNode> to be assigned to ShaderNodeObject<Node> because of methods that return this like UniformNode.label().

Thank you for the explanation. hm...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Methuselah96 @0b5vr