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

Examples - Update WebGPU Compute Water #30440

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

cmhhelgeson
Copy link
Contributor

@cmhhelgeson cmhhelgeson commented Feb 1, 2025

Related issue: #30426

Description

Updates WebGPU Compute Water to demonstrate an example of returning a struct from a defined function. DOES NOT solve the issue mentioned by this comment in this initial struct implementation, which is still under investigation: #30394 (comment)

@sunag sunag added this to the r174 milestone Feb 2, 2025
@mrdoob
Copy link
Owner

mrdoob commented Feb 5, 2025

@sunag looks good?

@sunag
Copy link
Collaborator

sunag commented Feb 5, 2025

@sunag looks good?

@cmhhelgeson did a great job, I just would like to think a little more about destructureStruct() and maybe make something easier natively in TSL.

@cmhhelgeson
Copy link
Contributor Author

cmhhelgeson commented Feb 6, 2025

@sunag looks good?

@cmhhelgeson did a great job, I just would like to think a little more about destructureStruct() and maybe make something easier natively in TSL.

Dirty version I was able to come up with in StructNode, though it currently has only limited use cases (i.e it can destructure right after creating a struct but not a struct that has been returned from a funtion). I'll try experimenting from here.

destructure() {

     const structMembers = this.structLayoutNode.membersLayout.map( member => member.name );

     const memberNodes = {};

    for ( const member of structMembers ) {

	memberNodes[ member ] = this.get( member );

     }

    return memberNodes;

}

// Use Case

const neighborValues = NeighborValuesStruct();
const { north, south, east, west } = neighborValues.destructure();
north.assign( store.element( northIndex ) );
south.assign( store.element( southIndex ) );
east.assign( store.element( eastIndex ) );
west.assign( store.element( westIndex ) );
return neighborValues;

Would we prefer destructure(StructNodeInstance) or StructNodeInstance.destructure()

@sunag
Copy link
Collaborator

sunag commented Feb 6, 2025

Initially I did the assignment directly from the declaration of struct, so we could use it with the properties. e.g:

const neighborValues = NeighborValuesStruct();
neighborValues.north.assign( ... )

The problem is that this doesn't work when we work with TSL function returns. So my solution was to create get() so that it could generate a MemberNode that would look for the reference at the time of setup() or generate().

Then I thought about generating a member for properties that had the $ prefix, for example.

const neighborValues = NeighborValuesStruct();
neighborValues.$north.assign( ... )

This would solve the case, since with the prefix you could still access undefined properties of the Nodes. But I would be extremely dependent on the Proxy class, and to be honest, I hope we can do everything with prototype in the future. So I was thinking about using .member() similar to what you presented, but handling Proxy only in the scope of members, so that it would also work on returns from TSL functions.

@mrdoob
Copy link
Owner

mrdoob commented Feb 6, 2025

I hope we can do everything with prototype in the future

👍

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.

3 participants