Skip to content

WEBGPU: make batchMesh._matricesTexture optional #30990

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

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

WEBGPU: make batchMesh._matricesTexture optional #30990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
141a4d6
make batchMesh._matricesTexture optional
VegarRingdalAibel Apr 24, 2025
d4c1585
lint fix
VegarRingdalAibel Apr 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 17 additions & 11 deletions src/nodes/accessors/BatchNode.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,17 +91,23 @@ class BatchNode extends Node {

const matricesTexture = this.batchMesh._matricesTexture;

const size = int( textureSize( textureLoad( matricesTexture ), 0 ).x );
const j = float( indirectId ).mul( 4 ).toInt().toVar();

const x = j.mod( size );
const y = j.div( size );
const batchingMatrix = mat4(
textureLoad( matricesTexture, ivec2( x, y ) ),
textureLoad( matricesTexture, ivec2( x.add( 1 ), y ) ),
textureLoad( matricesTexture, ivec2( x.add( 2 ), y ) ),
textureLoad( matricesTexture, ivec2( x.add( 3 ), y ) )
);
let batchingMatrix = mat4();

if ( matricesTexture !== null ) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@gkjohnson Is BatchedMesh._matricesTexture supposed to be optional?

Copy link
Contributor Author

@vegarringdal vegarringdal Apr 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

more info why I need it to be optional
Im using a custom batched mesh and set it to be .isBatchedMesh = true; and just need to control _multiDrawCounts / _multiDrawStarts / _multiDrawCount mye self.

export class CustomBatchedMesh extends Mesh {
    constructor(
        geometry: BufferGeometry,
        material: MeshStandardMaterialExt,
        multiDrawCounts: Int32Array,
        multiDrawStarts: Int32Array,
        multiDrawCount: number,
        isSelected: boolean,
        isTransformed: boolean,
        opacity_override: number
    ) {
        super(geometry, material as any);

        this.material = material;

        this.isBatchedMesh = true;
      

        this._multiDrawCounts = multiDrawCounts;
        this._multiDrawStarts = multiDrawStarts;
        this._multiDrawCount = multiDrawCount;

        //just to override defaults
        this._matricesTexture = null;
        this._indirectTexture = null;
        this._colorsTexture = null;

Been working very nice to keep drawcalls down on merged mesh, and would be nice to start trying out webgpu in my application without needing to modify threejs on each release

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry, I'm not sure we should support this custom design. Instead, I wonder if we could modify BatchedMesh to accommodate your use case and then provide proper support in the renderers.

Do you mind explaining in more detail why you can't use BatchedMesh and what parts had to be changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I really need to be able to use webgl multidraw. No need for different world transformations/different colors on merged mesh. So basicly dont need any of these:

_matricesTexture
_indirectTexture
_colorsTexture

Batchmesh already do same check for colorsTexture here:

three.js/src/nodes/accessors/BatchNode.js

Line 115 in d4c1585

if ( colorsTexture !== null ) {

and here:

three.js/src/renderers/common/RenderObject.js

Lines 713 to 717 in d4c1585

if ( object._colorsTexture !== null ) {
cacheKey += object._colorsTexture.uuid + ',';
}

So Ive only added a additional check, so Batchmesh can be used without the textures without any modifications to the core. This wont really break anything from what I can tell.
Ofc, @gkjohnson would be the best guy to ask to be sure :-)

Really just want to start trying out webgpu more with minimal changes to my application. Its a lot more stable and faster now 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tried creating an empty texture and using other means for indirection or batched values (e.g., a UBO)?

Note that although the texture will still be created on the GPU, only active uniforms count towards shader resource limits.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@gkjohnson Is BatchedMesh._matricesTexture supposed to be optional?

It's not currently but I'm not against it. It was also requested in #29084.

Sorry, I'm not sure we should support this custom design. Instead, I wonder if we could modify BatchedMesh to accommodate your use case and then provide proper support in the renderers.

I agree that understanding the use cases here would be great if we can fold some of these improvements into the core batched mesh.

@vegarringdal Can you explain what kinds of problems the core BatchedMesh implementation is currently causing you? I understand having textures that are unused seems wasteful but what issues is it causing you in practice?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gkjohnson
Dealing with large bim models.
Mosty do everything to keep memory low, and still keep models loaded on gpu. So have custom way to deal with colors/transformation, use controller "group" per geometry that generates custom meshes.
Only use flatshading, models only on gpu/local postion/indecies are removed. And all drawcall/state in created wasm worker and it updates meshes a lot when they do threeselections/coloring.

For me its just great if batchmesh helps us draw the multidraw part. 😄
Im sure I want to look more into using textures more with tsl, but guess I would end up with more textures, own for colorID, one for transformID, and own textures for transformationMatrix, ColorData(color, opacity...etc). Since those would be shared with many meshes based on user selection/coloring.
Really dont know enough yet, so I migth be very wrong on my ideas here..

Maybe this part should be more dynamic, so its easier to add more textures without core changes ?
String array on names of textures to add ?

three.js/src/renderers/common/RenderObject.js

Lines 705 to 719 in d4c1585

if ( object.isBatchedMesh ) {
if ( object._matricesTexture !== null ) {
cacheKey += object._matricesTexture.uuid + ',';
}
if ( object._colorsTexture !== null ) {
cacheKey += object._colorsTexture.uuid + ',';
}
}

Love threejs and batchedmesh have been very great.
Would just be even greater if we had a check _matricesTexture was null or not for now, so I can use it for my needs without making a branch/build.

Copy link
Collaborator

@gkjohnson gkjohnson Apr 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I understand the use case but can you please provide some numbers and explanations about what real problems it's causing. Eg "before removing the matrix texture I wasn't able to load W model" and "my batched mesh has X number of nodes so it's Y big, taking up Z MB".

I believe this is beneficial - I just know it's tempting to chase down every little thing that seems like it might be a problem (I've done it myself) and I want to have some real data for us to reason about.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dont have any real numbers to show in my app without a lot of rewrites, think it needed a additional attribute for the colors if I dont remember wrong, and GPU had hard hit when _multiDrawStarts/_multiDrawCounts/_multiDrawCount got to high on some of the models when I tried to control all items with own start/count.

Have many items on projects(best is 350K, worst is 4million+), so need to batch each mesh per coloring/transformation.
If I need new material/transformation, I create new meshes and reuse the geometry uploaded on gpu.

Small video when I do selection/coloring & transform with undo/redo.
https://drive.google.com/file/d/1tMQIWWE1A0Odz1BI2sGT7ke6mtG_ujJg/view?usp=sharing

Maybe there is better ways I could have done this I did not see when I made the app. Not the first time 😄
Just trying to make my app work with webgpu without any logic changes, so I can try it out more.
If I dont remember wrong, in webgl I just set batching to false on before compile of material to get around it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since tsl makes it easier to extend/generate shader logic, maybe core could have been a MultiDrawMesh(geometry, material, textures: map<string, texture>)
Then more advanced logic on how to use this just lived in addons ?
But again, i really dont know enough about tsl or core to know if this was a really dumb idea 😂


const size = int( textureSize( textureLoad( matricesTexture ), 0 ).x );
const j = float( indirectId ).mul( 4 ).toInt().toVar();

const x = j.mod( size );
const y = j.div( size );
batchingMatrix = mat4(
textureLoad( matricesTexture, ivec2( x, y ) ),
textureLoad( matricesTexture, ivec2( x.add( 1 ), y ) ),
textureLoad( matricesTexture, ivec2( x.add( 2 ), y ) ),
textureLoad( matricesTexture, ivec2( x.add( 3 ), y ) )
);

}


const colorsTexture = this.batchMesh._colorsTexture;
Expand Down
6 changes: 5 additions & 1 deletion src/renderers/common/RenderObject.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -704,7 +704,11 @@ class RenderObject {

if ( object.isBatchedMesh ) {

cacheKey += object._matricesTexture.uuid + ',';
if ( object._matricesTexture !== null ) {

cacheKey += object._matricesTexture.uuid + ',';

}

if ( object._colorsTexture !== null ) {

Expand Down