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

BatchedMesh: Add setGeometryAt, addGeometry functions #27078

Merged
merged 21 commits into from
Oct 31, 2023

Conversation

gkjohnson
Copy link
Collaborator

@gkjohnson gkjohnson commented Oct 28, 2023

Related issue: #25059, #22376

Description

  • Removes applyGeometry function.
  • Adds setGeometryAt and addGeometry function.
  • Includes parameter to addGeometry to "reserve" up to a specified length so other geometry can fit later.

These changes allow for geometry to be added and removed from the batched mesh more freely to afford more dynamic batching. I've produced a demo here that demonstrates the utility of this by rendering a 3D Tiles tileset that loads and unloads tiles dynamically based on camera view in a single draw call using this implementation of BatchedMesh. The full repo of the demo is here. Overall this gets the amount of time spent in the "render" function down 20-25x ( ~1.7ms to ~0.07ms ) or more once a lot of tiles are loaded using a low error target.

image

Here's a rough description of what's happening:

  • When a tile is loaded it is added into the batched mesh with a pre-allocated length to accommodate other tile geometry sizes later.
  • Textures are rendered into a texture array and rendered using a modified shader and "texture id" geometry attribute per tile.
  • When tiles are unloaded they're id is released and the buffer range is reused by another tile as-needed.
  • Just the range of modified batched geometry is uploaded as-needed to avoid stalls from reuploading everything. (this deserves another PR to simplify)

The current limitations of the technique given that multidraw isn't used:

  • Hidden tile vertices are always transformed unnecessarily so many vertices are rendered.
  • Frustum culling cannot be done (aside from "hiding" the geometry) (though this is being done already in this case with the 3d tiles algorithm)
  • Tiles cannot be sorted from closest to further to prevent overdraw.

I believe that with the WEBGL_multidraw_arrays extension should be able to help address these limitations.

@gkjohnson gkjohnson added this to the r159 milestone Oct 28, 2023
@mrdoob
Copy link
Owner

mrdoob commented Oct 28, 2023

The current limitations of the technique given that multidraw isn't used

Is multidraw well supported at this point? Or is it too early to make this multidraw-only?

@gkjohnson
Copy link
Collaborator Author

gkjohnson commented Oct 29, 2023

Is multidraw well supported at this point? Or is it too early to make this multidraw-only?

Looks like firefox is the straggler:

https://caniuse.com/mdn-api_webgl_multi_draw

We could probably make a fallback that doesn't support the multi-draw features and is less performant. But we can do that in another PR depending on if you feel it's worth supporting.

@gkjohnson
Copy link
Collaborator Author

Web3dSurvey.com reports 92.5% coverage with its sample set:

https://web3dsurvey.com/webgl2

@mrdoob
Copy link
Owner

mrdoob commented Oct 29, 2023

Then maybe we can make it multidraw-only and give Firefox a reason to implement 🤓

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 29, 2023

Includes parameter to addGeometry to "reserve" up to a specified length so other geometry can fit later.

Do you mind explaining in more detail why you need to reserve space for individual geometries? I understand to reserve space when creating the entire BatchedMesh. But isn't it one benefit of the entire class to just add geometries to the batched mesh without thinking about upcoming additions?

What would it mean for performance if addGeometry() does not has a reserve feature?

Could you please also explain the difference between setGeomrtryAt() and addGeometry()? When do you use each method?

@gkjohnson
Copy link
Collaborator Author

gkjohnson commented Oct 29, 2023

Do you mind explaining in more detail why you need to reserve space for individual geometries?

If you know that most of the geometry you'll be swapping out is similar in size or what the maximum size of your geometry will be you can allocate enough space to ensure any new geometry will be able to cleanly fit in the pre-allocated space making for a faster copy and a small amount of new geometry to be uploaded to the gpu.

But isn't it one benefit of the entire class to just add geometries to the batched mesh without thinking about upcoming additions?

Certainly it's one benefit. Another is that it allows for dynamic batching with transforms (and multidraw soon?) at all. The point of batching is to be able to book-keep a geometry in a way such that it can be rendered in a single draw call to save CPU time. Of course it's nice for BatchedMesh to provide a simple and convenient API for assembling this kind of geometry. But we shouldn't be going out of our way to preclude advanced and clearly useful functionality and flexibility for more advanced use cases.

What would it mean for performance if addGeometry() does not has a reserve feature?

If you don't reserve space you have to "optimize" the subgeometry to make space in the BatchedMesh which in the worst case can mean adjusting every vertex attribute element. For some reference - performing a simple operation like shifting an array of 6,000,000 elements by one element can take over 20 ms for a single BufferAttribute - updating the all the geometry attributes and more complex operations will obviously take a lot more time. Then uploading those vertices to the GPU is taking an additional 4.5+ ms. These are all things that might be happening every few frames - or even multiple times a frame in the tiles use case. It's an unacceptable performance hit in any case let alone when there are alternatives that are afforded by native graphics APIs.

For context I'm running these on a Google Chrome on a 2021 M1 Pro Macbook.

Could you please also explain the difference between setGeomrtryAt() and addGeometry()? When do you use each method?

You use addGeometry to add / append a new geometry to the BatchedMesh geometry - an error will be thrown if the geometry won't fit. setGeometry is used to replace an existing geometry that is no longer needed. The "free ids" are managed by the application in my case above as meshes are added and removed from the object.

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 30, 2023

@gkjohnson Thanks for the additional information. That made everything more clear to me!

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