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

WebGPURenderer: recycleBuffer() - reduce buffer creation overhead. #29341

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

Conversation

aardgoose
Copy link
Contributor

Related issue: #XXXX

As noted, the WebGPURenderer.readRenderTargetPixelsAsync() does not take a user provided buffer, but instead created a new buffer for each call. As noted this is a consequence of the WebGPU API.

#29320 (comment)

This PR attempts to provide a solution to this issue by providing a simple mechanism for the user to return used buffers ( and the related WebGPUbuffer object back to the renderer.

This is demonstrated in the webgpu_multiple_rendertargets_readback example.

Brief testing has demonstrated a reduction in CPU usage.

Copy link

github-actions bot commented Sep 6, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Before After Diff
WebGL 685.18
169.62
685.18
169.62
+0 B
+0 B
WebGPU 825.96
221.44
826.82
221.66
+864 B
+225 B
WebGPU Nodes 825.54
221.34
826.4
221.57
+864 B
+225 B

🌳 Bundle size after tree-shaking

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

Before After Diff
WebGL 461.96
111.46
461.96
111.46
+0 B
+0 B
WebGPU 525.27
141.52
526.13
141.76
+864 B
+242 B
WebGPU Nodes 481.93
131.34
482.79
131.58
+864 B
+239 B

@sunag sunag added this to the r169 milestone Sep 6, 2024
@@ -175,6 +174,8 @@

const selection = options.selection;

if ( pixelBuffer !== undefined ) renderer.recycleBuffer( pixelBuffer );
Copy link
Collaborator

Choose a reason for hiding this comment

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

This could be executed automatically in readRenderTargetPixelsAsync? It would be nice if we could hide renderer.recycleBuffer() from users.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't know what the lifetime of the returned buffer is after it is in the hands of the developer, unless you mean they could pass it in as a parameter to readRenderTargetPixelAsync( ..., bufferFromLastCall )?

Copy link
Collaborator

@sunag sunag Sep 6, 2024

Choose a reason for hiding this comment

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

Maybe if the user use RenderTarget.dispose() or reuse readRenderTargetPixelAsync()? Maybe we could link the buffer with the RenderTarget?

@gkjohnson
Copy link
Collaborator

gkjohnson commented Sep 8, 2024

I see what the complexity is with providing a raw buffer to the read pixels function. I'm wondering if it makes sense to have a three.js class that wraps the resulting buffer and provides the ability to release it whenever the user is finished so this caching doesn't have to happen internally. Something like so:

class WebGPUDataBuffer {

  // handle to the underlying WebGPU resource - can be private
  bufferHandle: GPUBuffer;
  
  // the buffer of data - can be readonly
  buffer: TypedArray;

  // flag indicating whether the buffer is mapped and available to read
  mapped: Boolean;

  // unmaps the data so it can be written to by the GPU
  release() : void;
  
  // destroys the GPU resource and fully releases any memory associated with it
  destroy() : void

}

Then the read pixels function can be used with the ability to reuse that buffer by passing it in as an argument like before. If it's not provided then a new resource is created. You can reuse the buffer over multiple reads like so:

let gpuData = null;

// ...

if ( ! isReading ) {

  renderer
    .readRenderTargetPixelsAsync( renderTarget, x, y, width, height, gpuData, /* index, faceIndex */ )
    .then( result => {

      gpuData = result;
      // ... do something with the data

    } );

}

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