-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add node input type support for f32 to enable usage on GPU
#3095
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
Conversation
|
I have fixed the UI. You just have to hook up the number input inside the |
8545df8 to
3a49a47
Compare
|
will have a look tomorrow ❤️ |
3a49a47 to
6917695
Compare
Firestar99
left a comment
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.
@0HyperCube Thanks for your help, the f32 nodes now work perfectly! ❤️
| assert_eq!( | ||
| ids, | ||
| vec![NodeId(13743208144182721472), NodeId(4607569396187877965), NodeId(16950305885390329527), NodeId(15151181027373658932)] | ||
| vec![NodeId(2791689253855410677), NodeId(11246167042277902310), NodeId(1014827049498980779), NodeId(4864562752646903491)] |
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.
Is this comment I left correct? Based on git history, I see them being updated frequently
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.
Yes, I think the test network contains a U32(u32), so moving the position of that will change the hash.
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.
@0HyperCube what about the comment right above, which github is hiding?
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 fair. I don't really see why it matters if the node ids aren't stable across different commits. However most modifications to the TaggedValue take place under the U32 so it probably shouldn't break too much.
f32 for node paramsf32 to enable usage on GPU
| // ============ | ||
| // STRUCT TYPES | ||
| // ============ | ||
| Vec2(Vec2), |
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 please call this FVec2 because the user-facing name for DVec2 is already "Vec2" and we need to avoid the confusion of the internal version being given the opposite name of the user-facing version. And probably the same for Affine2 -> FAffine2 for consistency. This also lets us discriminate between them in the future for rename purposes if we figure out how to clean this up once there is more Graphene infrastructure in place to handle numeric precision and units.
f32 params breaks the UI, I have no idea why. Feel free to take over this PR and fix that UI.