fix: Support for PrimitiveOffsetInfo in render pipeline draw...Indirect methods#2337
fix: Support for PrimitiveOffsetInfo in render pipeline draw...Indirect methods#2337cieplypolar wants to merge 11 commits intomainfrom
PrimitiveOffsetInfo in render pipeline draw...Indirect methods#2337Conversation
|
pkg.pr.new packages benchmark commit |
📊 Bundle Size Comparison
👀 Notable resultsStatic test results:No major changes. Dynamic test results:No major changes. 📋 All resultsClick to reveal the results table (349 entries).
If you wish to run a comparison for other, slower bundlers, run the 'Tree-shake test' from the GitHub Actions menu. |
There was a problem hiding this comment.
Pull request overview
This PR addresses missing support for PrimitiveOffsetInfo-based offsets in render pipeline indirect draw methods, and extends indirect dispatch to accept raw GPUBuffers (aligning compute + render indirect APIs and adding runtime validation/warnings).
Changes:
- Add
PrimitiveOffsetInfo | numberoffset support (and stricterIndirectFlagtyping) fordrawIndirect/drawIndexedIndirect. - Allow
dispatchWorkgroupsIndirectto accept rawGPUBuffers, with consistent validation/warning behavior. - Centralize indirect offset/size validation into a new
resolveIndirectOffsetutility and add/extend tests + mocks.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/typegpu/src/core/pipeline/renderPipeline.ts | Updates indirect draw APIs to accept `PrimitiveOffsetInfo |
| packages/typegpu/src/core/pipeline/computePipeline.ts | Extends dispatchWorkgroupsIndirect to accept GPUBuffer and delegates offset validation to shared utility. |
| packages/typegpu/src/core/pipeline/pipelineUtils.ts | New shared helper for indirect offset resolution, alignment/size validation, and padding-contiguity warnings. |
| packages/typegpu/tests/renderPipeline.test.ts | Adds coverage for indirect draw buffer usage/offset validation and padding warnings. |
| packages/typegpu/tests/computePipeline.test.ts | Adds coverage for raw GPUBuffer indirect dispatch validation and warning behaviors. |
| packages/typegpu-testing-utility/src/extendedIt.ts | Extends render pass encoder mocks with indirect draw methods for new tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
PrimitiveOffsetInfo in render pipeline draw...Indirect methodsPrimitiveOffsetInfo in render pipeline draw...Indirect methods
aleksanderkatan
left a comment
There was a problem hiding this comment.
Neat! 🐛🌊 I would just reconsider these warns, maybe they can just be a part of JSDocs?
| pipeline.dispatchWorkgroupsIndirect(buffer, 4); | ||
|
|
||
| expect(warnSpy.mock.calls[0]![0]).toMatchInlineSnapshot( | ||
| `"dispatchWorkgroupsIndirect: Using raw GPUBuffer. Offset validation is limited."`, |
There was a problem hiding this comment.
If somebody uses this api, they will get 60 warns per second. I think it would be fine if it only appeared once, but we don't yet have the logging infrastructure to do this.
Maybe we could suggest using typegpu buffers instead? As a way to silence this warn
There was a problem hiding this comment.
Good point, I will put this in jsdocs
There was a problem hiding this comment.
Or just like we do with raw number, I will provide quick fix how to suppress warns
Closes #2336.
Also I added
GPUBuffertodispatchWorkgroupsIndirectsignature.TODO: