fix(daemon): match D3D12 adapter on multi-GPU hosts (#225)#226
Conversation
_match_capture_gpu previously only matched Vulkan vkEnumeratePhysicalDevices chunks; D3D12 captures fell through to gpus[0], picking the integrated GPU on multi-GPU Windows hosts and failing heap creation with E_INVALIDARG. - Walk D3D12 DriverInit / EnumAdapters / CreateDXGIFactory chunks for AdapterDesc.Description / DeviceName - Vendor fallback prefers discrete (nVidia > AMD > Intel) and skips Software / WARP via renderdoc.GPUVendor (defensive getattr with int fallbacks) - Remote replay call site now also passes structured data and the renderdoc module to the matcher - Adds 8 unit tests covering single-GPU, Vulkan, D3D12, fallback, and remote-call-site paths
Pins the contract that when a Vulkan chunk is present but its deviceName does not match any available GPU, _match_capture_gpu falls through to the vendor-priority fallback instead of returning the first GPU.
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.
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR implements GPU selection improvements for multi-GPU D3D12 capture replay, addressing a failure where replay falls back to integrated/WARP GPUs lacking sufficient VRAM. The fix adds D3D12 structured-data matching, vendor-priority fallback, and ensures both local and remote replay paths pass required parameters consistently. ChangesMulti-GPU D3D12 Capture Replay
Sequence Diagram(s)sequenceDiagram
participant ReplaySetup as Replay Setup
participant MatchGPU as _match_capture_gpu()
participant FindAdapter as _find_adapter_description()
participant GPUList as Available GPUs
ReplaySetup->>MatchGPU: call with gpus, sd, rd
MatchGPU->>MatchGPU: check single GPU short-circuit
MatchGPU->>MatchGPU: try Vulkan vkEnumeratePhysicalDevices match
MatchGPU->>FindAdapter: extract D3D12 adapter description
FindAdapter->>FindAdapter: traverse DriverInit/EnumAdapters chunks
FindAdapter->>MatchGPU: return adapter description string
MatchGPU->>GPUList: case-insensitive substring match vs gpu.name
MatchGPU->>MatchGPU: vendor-priority fallback: NVIDIA > AMD > Intel
MatchGPU->>MatchGPU: exclude WARP from fallback candidates
MatchGPU->>ReplaySetup: return selected GPU
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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: 1
🤖 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-fix-225-d3d12-multi-gpu/proposal.md`:
- Around line 56-65: The fenced code block under the header "Scenario: Multi-GPU
capture replay" is missing a language identifier; update that fenced block to
include a language tag (e.g., markdown) so linters/renderers recognize it—locate
the triple-backtick block immediately before "#### Scenario: Multi-GPU capture
replay" in proposal.md and change it from ``` to ```markdown (or another
appropriate language) ensuring the rest of the block content remains unchanged.
🪄 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: fa6a3ff4-2af6-4920-81d7-b4453672ece5
📒 Files selected for processing (6)
openspec/changes/2026-05-15-fix-225-d3d12-multi-gpu/proposal.mdopenspec/changes/2026-05-15-fix-225-d3d12-multi-gpu/tasks.mdopenspec/changes/2026-05-15-fix-225-d3d12-multi-gpu/test-plan.mdopenspec/specs/daemon/spec.mdsrc/rdc/daemon_server.pytests/unit/test_daemon_server_unit.py
| ``` | ||
| #### Scenario: Multi-GPU capture replay | ||
| - WHEN the capture was taken on a multi-GPU system | ||
| - AND multiple GPUs are available for replay | ||
| - THEN the daemon selects the GPU whose name matches structured-data adapter | ||
| metadata extracted from the capture | ||
| - AND if no structured-data match exists, Software/WARP adapters are excluded | ||
| and the highest-ranked discrete GPU is preferred (nVidia > AMD > Intel) | ||
| - AND a single available GPU is always returned directly without inspection | ||
| ``` |
There was a problem hiding this comment.
Add language identifier to fenced code block.
The fenced code block lacks a language identifier. Add markdown or another appropriate identifier to satisfy linting rules and improve rendering.
📝 Proposed fix
-```
+```markdown
#### Scenario: Multi-GPU capture replay📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ``` | |
| #### Scenario: Multi-GPU capture replay | |
| - WHEN the capture was taken on a multi-GPU system | |
| - AND multiple GPUs are available for replay | |
| - THEN the daemon selects the GPU whose name matches structured-data adapter | |
| metadata extracted from the capture | |
| - AND if no structured-data match exists, Software/WARP adapters are excluded | |
| and the highest-ranked discrete GPU is preferred (nVidia > AMD > Intel) | |
| - AND a single available GPU is always returned directly without inspection | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 56-56: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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-fix-225-d3d12-multi-gpu/proposal.md` around lines
56 - 65, The fenced code block under the header "Scenario: Multi-GPU capture
replay" is missing a language identifier; update that fenced block to include a
language tag (e.g., markdown) so linters/renderers recognize it—locate the
triple-backtick block immediately before "#### Scenario: Multi-GPU capture
replay" in proposal.md and change it from ``` to ```markdown (or another
appropriate language) ensuring the rest of the block content remains unchanged.
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.
…#225) (#230) PR #226's structured-data adapter guard never fired on real hardware. Defects fixed here: 1. Chunk-name guard: the real renderdoc 1.42 chunk is "Internal::Driver Initialisation Parameters" (with a space), which none of the legacy markers ("DriverInit"/"EnumAdapters"/ "CreateDXGIFactory") match. Add "Driver Initialisation Parameters" to d3d_markers; keep the legacy markers (commented as version-fallback) for other versions/paths. 2. Tree depth: the real Description lives at depth 3 (chunk -> InitParams -> AdapterDesc -> Description), but the old walker only inspected depth 1/2. Replace it with a depth-bounded recursive descent (cap 5) returning an AdapterMatch carrying (description, device_id); DeviceId read via AsInt(). VendorId is deliberately not parsed (PCI id vs GPUVendor enum are unrelated, so it has no consumer). 3. Disambiguation: name-substring matching is fragile for same-vendor multi-GPU. _match_capture_gpu now selects in three tiers: exact DeviceId == g.deviceID (primary), name-substring (secondary), vendor-priority (tertiary, unchanged). 4. WARP/software sentinel: DXGI_ADAPTER_DESC.DeviceId == 0 on software/WARP/virtualized captures. Tier-1 is gated on truthiness (not "is not None") and skips any candidate with deviceID == 0, so a zero id can never bind the WARP adapter. 5. Version-drift diagnostic: a distinct _log.warning fires when a multi-GPU capture has structured data but no recognized vk/d3d adapter chunk, surfacing future renderdoc chunk renames instead of silently falling back to vendor priority. Adds 14 unit tests (5 red-first defect proofs + 9 regression guards, incl. the WARP-sentinel guard).
1 participant
Fixes #225.
Problem
_match_capture_gpu()indaemon_server.pypreviously matched GPUs only via the VulkanvkEnumeratePhysicalDevicesstructured-data chunk. D3D12 captures don't emit that chunk, so the function fell through toreturn gpus[0]. On the reporter's multi-GPU Windows machine (NVIDIA RTX 4500 Ada discrete + AMD Radeon iGPU + WARP)gpus[0]was the AMD iGPU with ~2 GB VRAM, which is insufficient for the D3D12 heaps the capture needs — replay failed withE_INVALIDARGon heap creation.A second latent bug was discovered alongside it: the remote-replay call site never passed structured data to
_match_capture_gpu, so even Vulkan matching was silently bypassed in remote mode.Fix
src/rdc/daemon_server.py_match_capture_gpu(cap, sd=None, rd=None)short-circuits on 0/1 GPUs, then walkssd.chunks:vkEnumeratePhysicalDevices→physProps→deviceNamematch).DriverInit,EnumAdapters, orCreateDXGIFactoryare searched via a new_find_adapter_descriptionhelper that walks one or two levels deep looking forAdapterDesc/pAdapter/adaptercontainers andDescription/DeviceNamestrings, then matches by substring againstgpu.name.renderdoc.GPUVendor(accessed defensively withgetattr+ integer fallback so missing enum members don't crash).OpenCapturepath, remote replay at the metadata-load path) now passcap.GetStructuredData()andrd.Test plan
tests/unit/test_daemon_server_unit.py— newTestMatchCaptureGpuclass with 9 cases:deviceName(regression guard for existing path)deviceNamematches nothing → vendor fallback (pins contract for swapped/uninstalled hardware)DriverInit/AdapterDescNoneSpec
Adds a "Multi-GPU capture replay" scenario under
### Requirement: Replay lifecycleinopenspec/specs/daemon/spec.md. OpenSpec change docs underopenspec/changes/2026-05-15-fix-225-d3d12-multi-gpu/.Cannot verify locally
This is a Linux dev box without a Windows multi-GPU + D3D12 setup. Logic is exercised end-to-end via mocked structured data, but the exact D3D12 chunk name emitted by renderdoc 1.42 (
DriverInitvsSystemChunk::DriverInitvs version variants) and theAdapterDescchild layout are educated guesses based on the renderdoc source. The matcher uses substring matching and a two-tier helper as defensive measures, but a real capture from the reporter is the only way to confirm the field names actually line up.@YunHsiao — would you mind grabbing this branch (or waiting for the merged build) and re-running
rdc openon the original D3D12 capture? If it still picks the iGPU, the chunk-name guess is wrong and we can iterate on the_find_adapter_descriptionwalk. Thanks for the very detailed bug report —forceGPUVendor/forceGPUDeviceIDevidence made the root cause obvious.Commits
ec40b10fix(daemon): match D3D12 adapter on multi-GPU hosts (_match_capture_gpu() falls back to integrated GPU on multi-GPU D3D12 systems, causing OpenCapture heap failure #225)45543e9test(daemon): cover vulkan-no-match fallback path (_match_capture_gpu() falls back to integrated GPU on multi-GPU D3D12 systems, causing OpenCapture heap failure #225)Summary by CodeRabbit
Bug Fixes
Documentation