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

Nodes: Struct extension proposal #29760

Open
Spiri0 opened this issue Oct 29, 2024 · 6 comments
Open

Nodes: Struct extension proposal #29760

Spiri0 opened this issue Oct 29, 2024 · 6 comments

Comments

@Spiri0
Copy link
Contributor

Spiri0 commented Oct 29, 2024

Description

I'm ready to get to work on this myself, but I'd like to hear your opinion on my ideas. Better I ask before I Investing energy and time in a way that you don't consider good.

After the initial euphoria about Aardgoose's bundler expansion, disillusionment followed in the days that followed.
The extension is great because it takes care of all the uniforms that I as a user don't want to take care of myself.
But it is not intended for storageBuffers. So far we only use the storageBuffers in analogy to attributes. In WebGPU you would like to use entire parameter sets with storage buffers in compute shaders. If uniform had an additional flag that we would call struct, then that would be a clean distinctive feature. Then at this point the WGSLNodeBuilder would make a case distinction as to whether it is a struct or not and at if false, the previous code is executed and if true, we can run a structbuilder without any risks.

} else if ( uniform.type === 'buffer' || uniform.type === 'storageBuffer' || uniform.type === 'indirectStorageBuffer' ) {

    if ( uniform.struct ) {

    }
    else {

        const bufferNode = uniform.node;
        const bufferType = this.getType( bufferNode.bufferType );
        const bufferCount = bufferNode.bufferCount;

        const bufferCountSnippet = bufferCount > 0 && uniform.type === 'buffer' ? ', ' + bufferCount : '';
        const bufferTypeSnippet = bufferNode.isAtomic ? `atomic<${bufferType}>` : `${bufferType}`;
        const bufferSnippet = `\t${ uniform.name } : array< ${ bufferTypeSnippet }${ bufferCountSnippet } >\n`;
        const bufferAccessMode = bufferNode.isStorageBufferNode ? `storage, ${ this.getStorageAccess( bufferNode ) }` : 'uniform';

        bufferSnippets.push( this._getWGSLStructBinding( 'NodeBuffer_' + bufferNode.id, bufferSnippet, bufferAccessMode, uniformIndexes.binding ++, uniformIndexes.group ) );
    
    }
}

What I don't yet see clearly is how the struct content is best transported into the WGSLNodeBuilder using the uniform.
But with this you could store worldMatrices, boundingSphere, ... of every instance of a mesh in a storageBuffer.
This means that almost anything can be brought into the compute shader for processing in order to exploit the big potential of WebGPU.

A new node is necessary for this, e.g. struct and I imagine it like this.

const StructExample = struct({
    someVector4: someVector4, 
    someVector3: someVector3,
    someScalar: someScalar
});
or struct.push() to add many of such datasets for different meshes or instances.

//in case of wgsl, not neccessary for Fn
const computeShader = wgslFn(`
    fn compute( 
        structBuffer: ptr<storage, StructExample, read_write>,   //StructExample is the name from the struct
    ) -> void {

    }
`);       

this.compute = computeShader({
    structBuffer: StructExample.buffer 
}).compute(length);

Maybe just assign the struct and the shader builder will recognize from the struct flag that it should use the buffer and all the names?

The struct built by the WGSLNodeBuilder would then look like this:

struct StructExample {
    someVector4: vec4<f32>
    someVector3: vec3<f32>
    someScalar: f32
};

If you see it as a good way, I'm willing to invest a lot of time and energy myself.

Solution

Described in the description

Alternatives

Lots of individual storage buffers and in the case of matrices no sensible solution.

Additional context

No response

@gkjohnson gkjohnson changed the title My idea of ​​a struct extension Nodes: Struct extension proposal Oct 29, 2024
@Spiri0
Copy link
Contributor Author

Spiri0 commented Oct 30, 2024

@sunag I noticed that there is already a method in the WGSLNodeBuilder getStructs and getStructMembers. Maybe I'm thinking to complex because it's already fully functional and I just haven't found it in the examples yet. Is there an example with structs?

PS: matrices already work with storage buffers so I withdrew the PR

@aardgoose
Copy link
Contributor

the structs methods were added for rendering to multiple render targets in a single pass, so aren't a complete implementation of structs in WGSL. For example an assumption that all elements are vec4() is built in at the moment.

@Spiri0
Copy link
Contributor Author

Spiri0 commented Oct 31, 2024

Thanks for the explanation aardgoose.
At the moment I'm using mat4 to bundle several variables. This may be a little more unusual but it also works quite well.
I think structs would be a good extension because as calculations increasingly move from the CPU to the GPU, they are very useful for specifically bundling datas for processing in compute shaders. This is nothing urgent, but would be a nice addition to your current backend bundling. I have no idea whether the terms frontend / backend bundling are appropriate, but I'm sure you know what I mean.

@sunag
Copy link
Collaborator

sunag commented Nov 1, 2024

I also see advantages in having a struct() for buffers, in fact I missed it when I was doing the tiled lighting to organize the buffer alignment, maybe we will need to type it before compilation so that it can also generate array buffers. I imagined something like:

const example = struct({
    someVector4: 'vec4', 
    someVector3: 'vec3',
    someScalar: 'float'
});

const structBuffer = structArray( example, count );

@Spiri0
Copy link
Contributor Author

Spiri0 commented Nov 1, 2024

I like the idea of ​​the structArray. I had also seen the struct optionally as a structArray, hence the .push() to add elements. But two cleanly separated nodes with struct and structArray is better.
I noticed that “array” is also available. What do you think of the idea of ​​using this as a node to implement my idea with the array<vec4, x> i.e. an array of x vec4.
I had this in mind for the camera frustum planes.

@Spiri0
Copy link
Contributor Author

Spiri0 commented Nov 6, 2024

I thought about the topic a little again. I think I have an idea. I would like to explain this using this code that I work with:

getObjectsInfo(objects) {
    const objectCount = objects.length;
    const countPerObject = 28;
  
    const objectInfoArray = new Float32Array(objectCount * countPerObject);
  
    for (let i = 0; i < objectCount; i++) {
        const object = objects[i];
        const offset = i * countPerObject;
  
        const bv = object.boundingVolume;
        const pbv = object.boundingVolume;
  
        objectInfoArray.set([
            bv.center.x, bv.center.y, bv.center.z, bv.radius,
            pbv.center.x, pbv.center.y, pbv.center.z, pbv.radius,
            object.lod, 0, 0, 0,
            object.error, 0, 0, 0,
            object.parentError, 0, 0, 0,
            ...object.bounds.min.toArray(), 0,
            ...object.bounds.max.toArray(), 0,
        ], offset);
    }

    return objectInfoArray;
}

Using node elements in it would be more cumbersome. The node system is intended to be a help and not to complicate a simple way to summarize data

I then use this function like this:

const objectInfo = this.getObjectsInfo(this.objects);

this.objectInfoStorage = new THREE.StorageBufferAttribute( objectInfo , 4 );

This way I can easily bundle any datas in a storage. But what is missing now is the struct. But this can be solved much more easily and elegantly with a new method for the storageBufferArrays.

this.objectInfoStorage.name = "ObjecInfo";
this.objectInfoStorage.setStructNames = [
    "boundingSphere",
    "parentBoundingSphere",
    "lod",
    "error",
    "parentError",
    "bboxMin",
    "bboxMax"
];

Without set structNames then the WGSLNodeBuilder works normally and treats the storage as vec4 elements because of the 4 in my StorageBufferAttribute example. So everything is as usual and fine. However, if the WGSLNodeBuilder sees that structNames are given to the storageBufferAttribute, then the node builder does not build arrays as it does by default but simply struct arrays. Setting the name list can then also set a flag isStructStorage = true;.
I don't want to argue against a possible structArray node. But the elegant thing with the setStructNames function for the storageBufferArrays would be that a structArray node then simply needs to set this list of names in the storageBufferArray for the storageBufferArray creaed by the struct or structArray node. So no more complex struct management extensions are necessary in the WGSLNodeBuilder.
Giving the storageBuffers a structNames list that can be accessed anywhere in the WGSLNodeBuilder seems much more natural to me than having to carry the struct informations with extra code next to the storageBuffers.

Another option is a setStuct function. With this you could also specify the data types, which makes it possible to use mixed types.

this.objectInfoStorage.setStruct = {
    boundingSphere: "vec4",
    parentBoundingSphere: "vec4",
    lod: "vec4",
    error: "vec4",
    parentError: "vec4",
    bboxMin: "vec4",
    bboxMax: "vec4"
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants
@sunag @Mugen87 @aardgoose @Spiri0 and others