feat(mesh): add vs-in stage for input-assembler mesh export (#224)#227
Merged
Conversation
Expose vertex shader input (VS-In, MeshDataStage.VSIn=0) geometry through the rdc mesh command and the daemon mesh_data handler: - buffer.py: add "vs-in": 0 to _MESH_STAGE_MAP; update invalid-stage error string to list vs-in, vs-out, gs-out - mesh.py: add "vs-in" to the --stage Click Choice - mock_renderdoc.py: add set_mesh_data helper to key GetPostVSData by stage int; unconfigured stages keep the -32001 non-draw contract Non-draw events inherit the existing -32001 "no PostVS data at this event" contract unchanged. Includes unit tests for handler decode, non-draw error, invalid-stage message, CLI forwarding, plus a gpu-marked Vulkan integration test.
…ref (#224) - Rewrite test_vs_in_decodes_geometry to use MockReplayController + set_mesh_data(0, MeshFormat(...)) instead of monkeypatching GetPostVSData directly, so the real helper is exercised by the test suite. - Correct tasks.md Phase A verification path from tests/mocks/test_mock_api_sync.py to tests/integration/test_mock_api_sync.py (the file exists there).
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR extends the ChangesVS-In Mesh Stage Support
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openspec/changes/2026-05-15-issue-224-vsin-mesh/proposal.md`:
- Around line 48-49: The markdown at the proposal mentions "Triangle*
topologies" which treats the asterisk as emphasis; update the text in
proposal.md to prevent MD037 by either escaping the star (Triangle\*) or
wrapping the term in a code span (`Triangle*`), e.g., change the phrase near the
OBJ face generator reference to use Triangle\* or `Triangle*` so the markdown
linter no longer flags malformed emphasis; keep the surrounding sentence intact
and ensure other occurrences of "Triangle*" in the paragraph are updated
similarly.
In `@openspec/changes/2026-05-15-issue-224-vsin-mesh/test-plan.md`:
- Line 58: The code span "`v `" in the test-plan assertion includes a trailing
space and triggers markdownlint MD038; update the sentence to remove the space
inside the code span and rephrase to something like “Assert OBJ contains lines
starting with `v`; count matches reported vertex count from `MeshFormat`” so the
code span is just `v` and the meaning remains clear. Ensure the change touches
the test-plan.md assertion that mentions `MeshFormat` and replaces the
backticked `v ` with a non-space code span or descriptive wording such as "lines
starting with `v`".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a99286fd-d7a4-4d4e-be73-d1cabdac9b82
📒 Files selected for processing (10)
openspec/changes/2026-05-15-issue-224-vsin-mesh/proposal.mdopenspec/changes/2026-05-15-issue-224-vsin-mesh/specs/daemon/spec.mdopenspec/changes/2026-05-15-issue-224-vsin-mesh/tasks.mdopenspec/changes/2026-05-15-issue-224-vsin-mesh/test-plan.mdsrc/rdc/commands/mesh.pysrc/rdc/handlers/buffer.pytests/integration/test_daemon_handlers_real.pytests/mocks/mock_renderdoc.pytests/unit/test_buffer_decode.pytests/unit/test_mesh_commands.py
| - Analogous to `test_mesh_data_real` (~line 1573 in `test_daemon_handlers_real.py`). | ||
| - Open a vkcube Vulkan capture; pick a draw eid known to have vertex data. | ||
| - Call handler with `stage=vs-in`; assert OBJ output is non-empty and vertex count > 0. | ||
| - Assert OBJ contains `v ` lines; count matches reported vertex count from `MeshFormat`. |
There was a problem hiding this comment.
Fix code-span whitespace at Line 58.
v inside backticks includes a trailing space and can trigger markdownlint MD038. Rephrase to avoid space inside the code span (e.g., “lines starting with v”).
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 58-58: Spaces inside code span elements
(MD038, no-space-in-code)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openspec/changes/2026-05-15-issue-224-vsin-mesh/test-plan.md` at line 58, The
code span "`v `" in the test-plan assertion includes a trailing space and
triggers markdownlint MD038; update the sentence to remove the space inside the
code span and rephrase to something like “Assert OBJ contains lines starting
with `v`; count matches reported vertex count from `MeshFormat`” so the code
span is just `v` and the meaning remains clear. Ensure the change touches the
test-plan.md assertion that mentions `MeshFormat` and replaces the backticked `v
` with a non-space code span or descriptive wording such as "lines starting with
`v`".
…tri topology (#224) Cross-check against RenderDoc decode_mesh.py reference revealed the OpenSpec premise 'no changes needed below the map' was invalid for vs-in: - Apply mesh.baseVertex to decoded indices uniformly (no-op for vs-out/gs-out where baseVertex==0), matching the reference. - Read the position attribute at i*stride + mesh.vertexByteOffset instead of i*stride, so vs-in positions are correct when POSITION is not the first interleaved element. - Index width follows mesh.indexByteStride (16/32-bit); regression tested explicitly. - mesh CLI now emits a clear stderr warning naming the topology when a non-triangle topology produces 0 OBJ faces (no silent loss). - Retract the invalid premise in the OpenSpec proposal and document vs-in support and limitations.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
BANANASJIM
added a commit
that referenced
this pull request
May 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part 1 of #224.
What
Exposes the VS input (input-assembler) mesh through
rdc mesh --stage vs-in. Previously only post-transform geometry (vs-out/gs-out) was reachable; debugging original model geometry, vertex layout, skinning inputs, or index ranges required a custom RenderDoc Python script.Changes
handlers/buffer.py: add"vs-in": 0(MeshDataStage.VSIn) to_MESH_STAGE_MAP; invalid-stage error string now listsvs-in, vs-out, gs-out.commands/mesh.py:--stageClickChoiceacceptsvs-in.vs-ininherits the existing-32001 "no PostVS data at this event"contract unchanged (same asvs-out/gs-out) — no silent-empty path was added.Tests
set_mesh_datamock helper), non-draw-32001, invalid-stage message.--stage vs-inaccepted & forwarded; unknown stage rejected.@pytest.mark.gpuVulkan integration test (vkcube).Verification
Verified end-to-end on Linux/Vulkan. D3D12-specific behavior cannot be verified on the dev box (Linux replay is Vulkan-only) — reporter verification on a real D3D12 capture is requested before merge.
OpenSpec:
openspec/changes/2026-05-15-issue-224-vsin-mesh/. Completes part of the unshipped CLI plan from the archived2026-02-19-phase2-buffer-decodechange.Summary by CodeRabbit
New Features
vs-instage option to retrieve VS-In mesh output.Bug Fixes
Tests
vs-in, decoding, error cases, and CLI behavior.Documentation
vs-inbehavior and error contracts.