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.

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

Example webgpu_tsl_editor: Simple uv.x animation #26368

Merged
merged 1 commit into from Jul 4, 2023

Conversation

sunag
Copy link
Collaborator

@sunag sunag commented Jul 4, 2023

Description

Improving the example a bit.

@sunag sunag added this to the r155 milestone Jul 4, 2023

const timer = timerLocal( .5 ); // .5 is speed
const uv0 = uv();
const animateUv = vec2( uv0.x.add( oscSine( timer ) ), uv0.y );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way... Could we try adding support for something like vectorVariable.x.addAssign( something ), based on how we have that for VarNodes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting you think this, because I was drafting something similar but using setXYZ(), like node.setX( x ), maybe node.addX( node ) too. I think that node.x.addAssign() seems more complicated.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like node.x.assign/addAssign more because it better mirrors the shader code -- i.e. var.x = something or var.x += something. It also feels more understandable for me.
(And to add all the *Assign things we should only add assign support for SplitNode -- other *Assigns will be supported automatically)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging now, anyway better to create a new PR for it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you rather work on it, or I work on it?

Copy link
Collaborator Author

@sunag sunag Jul 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... worries me a bit the users use just uv0.x.addAssign( oscSine( timer ) ); instead of const animateUv = uv0.x.addAssign( oscSine( timer ) );

const animateUv = uv0.x.addAssign( oscSine( timer ) );

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should just note somewhere that they should either do const animateUv = temp( uv() ); animateUv.x.addAssign( oscSine( timer ) ) or const uv0 = uv(); const animateUv = uv0.x.addAssign( oscSine( timer ) ).

Would you rather work on it, or I work on it?

I think you can implement this better than me 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// Example 1
const animateUv = temp( uv() ); 
animateUv.x.addAssign( oscSine( timer ) );

// Example 2
const uv0 = uv(); 
const animateUv = uv0.x.addAssign( oscSine( timer ) );

// Example 3
const uv0 = uv(); 
const animateUv = uv0.addX( oscSine( timer ) );

It seems that we have 3 options at now, .set*(), .add*() still seems more simple for me.

I think you can implement this better than me 👍

Thanks, but you'd do a great job all the same.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that we have 3 options at now, .set*(), .add*() still seems more simple for me.

I think we can try to support both first two options -- we can check in SplitNode whether the node it's acting on is VarNode, and if it is not it automatically makes it a such.

@sunag sunag merged commit 775ff28 into mrdoob:dev Jul 4, 2023
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants