Add support transform feedback separate buffer mode#9109
Conversation
|
It'd be great to add an example. It can be a hidden test examples in test folder as well, that's just tests the functionality without having to look particularly good. |
There was a problem hiding this comment.
Pull request overview
Adds support for transform feedback “separate” buffer mode and multi-slot transform feedback buffer binding on the WebGL backend, extending the existing transform feedback plumbing to better match WebGL2’s indexed TF buffer bindings (Issue #9031).
Changes:
- Added
feedbackVaryingsModeshader definition option to choose betweenINTERLEAVED_ATTRIBSandSEPARATE_ATTRIBSwhen configuring transform feedback varyings. - Extended
WebglGraphicsDevice#setTransformFeedbackBufferto support a bufferslotand updated draw-time TF buffer binding/unbinding to iterate slots. - Updated JSDoc to document the new shader option and added a runtime warning in the
TransformFeedbackhelper when used with separate mode.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/platform/graphics/webgl/webgl-shader.js | Selects TF varying buffer mode (interleaved vs separate) during program link. |
| src/platform/graphics/webgl/webgl-graphics-device.js | Adds multi-slot TF buffer tracking and binds/unbinds all active TF slots around draws. |
| src/platform/graphics/transform-feedback.js | Warns when the TransformFeedback helper is used with separate varyings mode. |
| src/platform/graphics/shader.js | Documents new definition.feedbackVaryingsMode shader option. |
| src/platform/graphics/shader-definition-utils.js | Plumbs feedbackVaryingsMode through shader definition creation and documents it. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const gl = this.gl; | ||
| const buffers = this.transformFeedbackBuffers; | ||
|
|
||
| let len = buffers.length; | ||
|
|
||
| // Let's expand the array up to the slot index. | ||
| if (len <= slot) { | ||
| for (let i = len; i <= slot; i++) { | ||
| buffers.push(null); | ||
| } | ||
| len = slot + 1; | ||
| } | ||
|
|
||
| buffers[slot] = tf; | ||
|
|
||
| // Let's take the last slot available in the array. | ||
| let numSlots = 0; | ||
| for (let i = len - 1; i >= 0; i--) { | ||
| if (buffers[i]) { | ||
| numSlots = i + 1; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| this.transformFeedbackNumSlots = numSlots; |
There was a problem hiding this comment.
The old API works with slot 0, so there shouldn't be any issues; the added loop, conversely, might mask the error, making it impossible for the user to debug it.
It looks like there is no WebGL2 mock implemented yet—I just noticed this. I created my own version, but perhaps it would be better to submit it as a separate PR? |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
src/platform/graphics/webgl/webgl-graphics-device.js:2057
- Debug.assert(bufId, ...) treats any falsy value as invalid. While WebGLBuffer objects are truthy, this can still produce false assertions (and a vague message) with mocks or alternative implementations. Also, accessing buf.impl without optional chaining can throw if a non-VertexBuffer is accidentally stored.
const buf = tfb[i];
const bufId = buf?.impl.bufferId ?? null;
Debug.assert(bufId, 'Transform feedback buffer slot value is null.');
gl.bindBufferBase(gl.TRANSFORM_FEEDBACK_BUFFER, i, bufId);
|
As mentioned initially, I would really like to see either engine example for public, or engine example in test folder for internal testing. Unit test does not proof this actually executes on WebGL. |
This example wore me out a bit—I couldn't think of anything simpler than a series. I hope it turned out to be clear. |
…engine into alex-tf-separate-slots
Description
Adding
separate buffer modefortransform feedbackwould expand the engine’s capabilities and make GPU pipelines more flexible.Interleavedmode is great for compact data packing, butseparateprovides cleaner separation of concerns and works better when different outputs should be stored and consumed independently.Key benefits:
This change adds a new extension to the existing API:
Fixes #9031
Checklist