fix(daemon): match D3D12 adapter via real chunk name + exact DeviceId (#225)#230
Conversation
…#225) 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).
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? |
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 (6)
✨ Finishing Touches🧪 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.
Summary
Fix-forward on #226 (squash
3735f2b). Issue reporter @YunHsiao hardware-verified that #226 resolved the user-facing D3D12 crash, but the daemon log showed the structured-data adapter match never fires on a real renderdoc-1.42 D3D12 capture — it only works by coasting on the vendor-priority fallback. @YunHsiao provided a canonical structured-data dump that pinpointed three defects in the matcher shipped by #226:Internal::Driver Initialisation Parameters(spaces); the oldd3d_markerssubstring"DriverInit"never occurs in it._find_adapter_descriptionwalked only 2 levels. The realDescriptionis at depth 3:chunk → InitParams → AdapterDesc → Description.AdapterDescexposes a PCIDeviceIdthat gives an exact key.Changes
d3d_markersgains"Driver Initialisation Parameters"(renderdoc-1.42 D3D12); legacy markers retained for other versions._find_adapter_description→ depth-bounded recursive descent (cap 5), returningAdapterMatch(description, device_id)._match_capture_gputhree-tier selection: exactDeviceId(primary) → name-substring (secondary) → vendor-priority (tertiary, unchanged).deviceID == 0is treated as a sentinel: a present-but-zero parsedDeviceIdfalls through to name-substring, and zero-deviceIDcandidates are skipped in the exact loop, so a software/WARP capture can never force-bind the WARP adapter.Tests
14 new unit tests (5 red-first defect proofs built from the real dump tree + 9 regression guards incl. the WARP-sentinel guard). Full suite green: ruff + ruff format, mypy strict,
2906 passed, 6 skipped, coverage 92.82% (daemon_server.py94%).Hardware-unverifiable assumptions (need @YunHsiao re-verification)
Linux replay is Vulkan-only, so the D3D12 path cannot be exercised here. Two load-bearing assumptions are documented in the OpenSpec proposal as known risks:
GPUDevice.deviceIDis the same integer as PCIDXGI_ADAPTER_DESC.DeviceId. A mismatch silently demotes to name-substring (correct, but non-deterministic). Verification ask: printGetAvailableGPUs()[i].deviceIDfor the NVIDIA RTX 4500 entry andAdapterDesc.DeviceIdfrom the same capture; confirm they are bitwise-equal (expected 10161)."Driver Initialisation Parameters"substring is renderdoc-1.42-specific; a version rename would revert to vendor priority — now mitigated by the diagnostic log above.Issue #225 stays open until @YunHsiao re-verifies on the multi-GPU box (structured-data match must now fire, not the fallback).
OpenSpec:
openspec/changes/2026-05-17-fix-225-d3d12-chunk-name/.