fix(texture): call SetFrameEvent before SaveTexture/GetTextureData#207
fix(texture): call SetFrameEvent before SaveTexture/GetTextureData#207BANANASJIM merged 11 commits intomasterfrom
Conversation
…e_push to AUR git job
Curly braces in jq examples inside <pre><code> were parsed as Astro JSX
expressions, causing esbuild to fail with "triangles is not defined" and
"{}" parse errors. Wrap affected jq object literals in {"..."} string
expressions. Also add force_push: true to aur-git job to fix non-fast-forward
push rejections from AUR.
pixi run rdc previously fell back to /usr/bin/rdc (stale system install) because no explicit task existed. Adding `rdc = "uv run rdc"` ensures the development version is always used.
D3D11 replay requires SetFrameEvent(eid) to populate internal texture tracking maps before SaveTexture or GetTextureData can succeed. Without this call, WrappedID3D11Texture*::m_TextureList remains empty and SaveTexture always fails. Vulkan replay does not enforce this ordering, so the bug only manifests on Windows D3D11 captures. Fixes #206.
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 (1)
📝 WalkthroughWalkthroughHandlers for texture export/raw now accept an optional Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Daemon as Daemon/RPC
participant Handler as TextureHandler
participant Adapter as RenderDocAdapter
participant Replay as RenderDoc/Replay
Client->>Daemon: tex_export / tex_raw request (params include optional eid)
Daemon->>Handler: dispatch request
Handler->>Handler: parse eid (or use state.current_eid) and int()
Handler->>Adapter: _set_frame_event(state, eid)
Adapter-->>Handler: ok / error
alt ok
Handler->>Adapter: controller.save_texture/write_raw(...)
Adapter->>Replay: SaveTexture / WriteTextureData
Replay-->>Adapter: result/data
Adapter-->>Handler: saved / written
Handler-->>Daemon: success response
Daemon-->>Client: success
else error
Handler-->>Daemon: error (-32002)
Daemon-->>Client: error (-32002)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
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.
|
Note: this fix can't be verified locally (no Windows D3D11 environment). @Junlin-Yin could you test with your setup and confirm? |
|
Can't verify locally — no Windows/D3D11 setup. @Junlin-Yin could you test this? |
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.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
59-60: The comment already includes an explicit removal trigger; consider adding a tracking issue or monitoring schedule for additional clarity.The code correctly documents the suppression condition ("ignore until upstream ships a patch"). Verification confirms no patched release exists as of now, so the suppression is necessary. To prevent this from lingering indefinitely, consider adding a link to the upstream issue or a specific check-in date (e.g., "revisit 2026-06-01") in the comment so the team actively monitors for the next Pygments release.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 59 - 60, Update the CI suppression comment for the pip-audit step that ignores CVE-2026-4539 (the commented line and the run: uv run --with pip-audit pip-audit --ignore-vuln CVE-2026-4539) to include a tracking link or revisit date; specifically, append either an upstream Pygments issue/PR URL or a "revisit YYYY-MM-DD" note (e.g., revisit 2026-06-01) to the existing comment so reviewers know when to re-evaluate the suppression and where to monitor for a patch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 59-60: Update the CI suppression comment for the pip-audit step
that ignores CVE-2026-4539 (the commented line and the run: uv run --with
pip-audit pip-audit --ignore-vuln CVE-2026-4539) to include a tracking link or
revisit date; specifically, append either an upstream Pygments issue/PR URL or a
"revisit YYYY-MM-DD" note (e.g., revisit 2026-06-01) to the existing comment so
reviewers know when to re-evaluate the suppression and where to monitor for a
patch.
On Windows D3D11, cap.GetStructuredData() creates a temporary WrappedID3D11Device whose destructor clears the static m_TextureList, causing all subsequent SaveTexture calls to fail. Use controller.GetStructuredFile() instead — same return type, no destructive side effect. Reported-by: Junlin-Yin Fixes #206
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.
Summary
_handle_tex_exportand_handle_tex_rawwere missing a_set_frame_event()call before invokingSaveTexture/GetTextureDataSetFrameEvent(eid)to populate internal texture tracking maps (WrappedID3D11Texture*::m_TextureList); without it,SaveTexturealways failsFixes #206. Reported by @Junlin-Yin.
Test plan
rdc texture <id> -o out.pngshould succeedSummary by CodeRabbit
Bug Fixes
Tests
Refactor
Chores