Skip to content

fix(daemon): match D3D12 adapter via real chunk name + exact DeviceId (#225)#230

Merged
BANANASJIM merged 2 commits into
masterfrom
fix/225-d3d12-chunk-name
May 17, 2026
Merged

fix(daemon): match D3D12 adapter via real chunk name + exact DeviceId (#225)#230
BANANASJIM merged 2 commits into
masterfrom
fix/225-d3d12-chunk-name

Conversation

@BANANASJIM
Copy link
Copy Markdown
Owner

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:

  1. Chunk guard never matched. Real adapter chunk is Internal::Driver Initialisation Parameters (spaces); the old d3d_markers substring "DriverInit" never occurs in it.
  2. _find_adapter_description walked only 2 levels. The real Description is at depth 3: chunk → InitParams → AdapterDesc → Description.
  3. Name-substring was the only disambiguation. Fragile for same-vendor multi-GPU; AdapterDesc exposes a PCI DeviceId that gives an exact key.

Changes

  • d3d_markers gains "Driver Initialisation Parameters" (renderdoc-1.42 D3D12); legacy markers retained for other versions.
  • _find_adapter_description → depth-bounded recursive descent (cap 5), returning AdapterMatch(description, device_id).
  • _match_capture_gpu three-tier selection: exact DeviceId (primary) → name-substring (secondary) → vendor-priority (tertiary, unchanged).
  • WARP/deviceID == 0 is treated as a sentinel: a present-but-zero parsed DeviceId falls through to name-substring, and zero-deviceID candidates are skipped in the exact loop, so a software/WARP capture can never force-bind the WARP adapter.
  • A distinct diagnostic warning is logged when multiple GPUs are present, structured data exists, but no recognized adapter chunk was seen at all — so a future renderdoc chunk-name change is diagnosable from the log instead of silently reverting to vendor priority.

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.py 94%).

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:

  • RISK-A: tier-1 assumes renderdoc GPUDevice.deviceID is the same integer as PCI DXGI_ADAPTER_DESC.DeviceId. A mismatch silently demotes to name-substring (correct, but non-deterministic). Verification ask: print GetAvailableGPUs()[i].deviceID for the NVIDIA RTX 4500 entry and AdapterDesc.DeviceId from the same capture; confirm they are bitwise-equal (expected 10161).
  • RISK-B: the "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/.

…#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-code-review
Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

Copy link
Copy Markdown

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 17, 2026

Warning

Rate limit exceeded

@BANANASJIM has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 15 minutes and 13 seconds before requesting another review.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fcde04c9-31fb-472d-9e6d-ef7b9ac4b3b6

📥 Commits

Reviewing files that changed from the base of the PR and between 88781c2 and 94f3d1f.

📒 Files selected for processing (6)
  • openspec/changes/2026-05-17-fix-225-d3d12-chunk-name/proposal.md
  • openspec/changes/2026-05-17-fix-225-d3d12-chunk-name/tasks.md
  • openspec/changes/2026-05-17-fix-225-d3d12-chunk-name/test-plan.md
  • openspec/specs/daemon/spec.md
  • src/rdc/daemon_server.py
  • tests/unit/test_daemon_server_unit.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/225-d3d12-chunk-name

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@BANANASJIM BANANASJIM merged commit 056c02f into master May 17, 2026
17 checks passed
@BANANASJIM BANANASJIM deleted the fix/225-d3d12-chunk-name branch May 17, 2026 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant