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

WebGLRenderer: Merge update ranges before issuing updates to the GPU. #29189

Merged
merged 2 commits into from
Sep 2, 2024

Conversation

HunterLarco
Copy link
Contributor

@HunterLarco HunterLarco commented Aug 20, 2024

TLDR: this PR achieves up to 3 orders of magnitude performance improvement when updating a large number of adjacent ranges within InstancedBufferAttribute which is a common use case for projects heavily leveraging instancing.

Description

BufferAttribute#addUpdateRange can be used with needsUpdate so that three only transfers subsections of data to the GPU. This is a powerful feature which allows clients to better manage CPU<>GPU bandwidth. For example, in cases where a BufferAttribute may be several MB large and only a few bytes change per frame, clients can transfer only the changed bytes instead of the entire buffer.

In our product we've seen large improvement gains using update ranges, but frame drops in cases where many update ranges are present in a single frame. This can easily be observed with InstancedBufferAttribute. In a project which heavily leverages InstancedMesh and therefore InstancedBufferAttribute to represent instance data, it's commonly required that individual instances are updated using addUpdateRange. In a frame where all instances need to be updated, this can create a large number of update ranges which are nearly all adjacent. As a result we observe a large number of avoidable gl.bufferSubData calls and frame drops (I imagine due to GPU command overhead).

This PR automatically merges overlapping / adjacent update ranges before calling gl.bufferSubData and results in up to a 99.78% wall time reduction rendering our project (see below for details)

Impact

In a toy example within our company, I created a scene with 10k plane geometries (via InstancedMesh) which are positioned by vec3's interleaved via InstancedBufferAttribute. Updating all 10k positions in a single frame on a 2021 M1 Macbook Pro takes 112.21ms in three.js today, when run on this this PR it takes 0.25ms instead.

Design Notes

  1. I added this logic in the renderers code as opposed to addUpdateRange where we could amortize the merging costs because clients are allowed to directly manipulate the updateRanges array. Adding this logic to the renderers ensures robustness regardless of how clients interact with update ranges.
  2. I'd be happy to apply this change to WebGPU and WebGL-Fallback if there's interest.
  3. I'd love to add unit tests, but there are currently no unit tests for WebGLAttributes making it challenging to envision how we'd mock gl when instantiating WebGLAttributes. If there's a suggestion here, I'd love to hear it.

This contribution is funded by SOOT

Copy link

github-actions bot commented Aug 20, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
685.4 kB (169.7 kB) 685.6 kB (169.8 kB) +192 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Filesize dev Filesize PR Diff
462 kB (111.4 kB) 462.2 kB (111.5 kB) +192 B

@HunterLarco HunterLarco marked this pull request as ready for review August 20, 2024 18:35
@HunterLarco
Copy link
Contributor Author

bump~ anything I can do to help with review?

Copy link
Collaborator

@Mugen87 Mugen87 left a comment

Choose a reason for hiding this comment

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

LGTM, but it would be great if @gkjohnson could have a look at this change as well.

@Mugen87 Mugen87 added this to the r168 milestone Aug 26, 2024
@Mugen87 Mugen87 modified the milestones: r168, r169 Aug 29, 2024
@Mugen87 Mugen87 merged commit 08626b4 into mrdoob:dev Sep 2, 2024
12 checks passed
@Mugen87 Mugen87 changed the title [performance] merge WebGL BufferAttribute update ranges before issuing updates to the GPU WebGLRenderer: Merge update ranges before issuing updates to the GPU. Sep 2, 2024
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