WebGLRenderer: Fix feedback loop with RTT read pixels async #29320
Conversation
📦 Bundle sizeFull ESM build, minified and gzipped.
🌳 Bundle size after tree-shakingMinimal build including a renderer, camera, empty scene, and dependencies.
|
| renderer.readRenderTargetPixels( rtt, 0, 0, info.width, info.height, dataBuffer ); | ||
| await renderer.readRenderTargetPixelsAsync( rtt, 0, 0, info.width, info.height, dataBuffer ); |
There was a problem hiding this comment.
I notice that the function signature for readRenderTargetPixelsAsync in WebGPURenderer doesn't take a dataBuffer argument which should probably be fixed to align with the WebGLRenderer implementation. Taking a databuffer was a deliberate decision to avoid memory allocation in cases where this is run ever frame or every few frames and is common pattern use throughout the library, as well, and I think it's worth keeping.
It looks like WebGPURenderer will now create a new typed array on every call.
There was a problem hiding this comment.
Do you mind updating KTX2Exporter as well?
There was a problem hiding this comment.
I've updated KTX2Exporter but haven't tested it since I don't see an example.
Also is it worth creating a new issue to track this function signature difference?
There was a problem hiding this comment.
The WebGPU allocates the buffer returned internally, we just pass the required size etc. We effectively cast it to the correct type on return, but can't provide a buffer to be used, I think to ensure that the buffer alignment and location within memory is suitable for efficient CPU <-> GPU transfers.
The buffer is created at
And made available to JS here:
There was a problem hiding this comment.
Also is it worth creating a new issue to track this function signature difference?
Sounds good. Then we have a dedicated place to discuss this topic.
There was a problem hiding this comment.
If there is a use case that requires constant readback the recommended method would be to create a pool of WebGPU buffer objects in a ring buffer arrangement to avoid repeated allocations, something that doesn't match the current renderer API.
There was a problem hiding this comment.
I have also realized that WebGPURenderer.readRenderTargetPixelsAsync() can not handle cube render targets so far since you can't assign an active cube face index. This is required for a bunch of use cases like e.g. in LightProbeGenerator.
There was a problem hiding this comment.
Relevant line:
Related issue: #29302 (comment)
Description
Fixes the feedback error reported by
readRenderTargetPixelsAsynccaused by modifying the active frame buffer and not resetting it to the actively set one before awaiting the sync fence. I also removed the try / catch statements since we're not so defensive anywhere else in the project and could result in the function silently exiting with no returned data so it may be easiest to look at the diff without whitespace changes. Also throw an error in the branch where we can't return any data.