fix(wgc): report adapter LUIDs, because the descriptions can be identical - #342
fix(wgc): report adapter LUIDs, because the descriptions can be identical#342EtienneLescot wants to merge 1 commit into
Conversation
…ical
The `capture-adapter` diagnostic compared adapters by LUID and printed only
their descriptions. On the exact configuration it exists to catch, those two
descriptions are the same string.
An IddCx virtual display driver renders through the physical GPU and inherits
its description while being a separate DXGI adapter with its own LUID. Measured
on a rented multi-adapter box:
adapter[0] NVIDIA Quadro RTX 4000 LUID 0:24084 -> \.\DISPLAY1
adapter[1] NVIDIA Quadro RTX 4000 LUID 0:12889146 -> the Parsec IDD
So a machine with the divergence would have reported two identical names beside
`sameAdapter:false` — which reads as a bug in the reporting rather than as the
finding it is, and would most likely have been dismissed as one. The comparison
was never wrong; the output was unreadable at the only moment it matters.
Adds `deviceLuid` and `monitorLuid` to the event, and one `[adapters]` line per
enumerated adapter on stderr with its LUID and its outputs, marking the captured
one. Two adapters sharing a description is something a reader has to see before
they will believe `sameAdapter` over the names, and an adapter with no output at
all is how an inactive virtual display presents -- which is why the rented box
reported `sameAdapter:true` despite having a virtual display driver installed.
Formatted HighPart:LowPart to match QueryDisplayConfig traces, so a LUID from a
bug report can be grepped straight against one.
📝 WalkthroughWalkthroughThe Windows capture helper now reports capture and monitor adapter LUIDs, enumerates DXGI adapters and outputs at startup, and documents the new diagnostics and cross-adapter detection. ChangesGPU adapter diagnostics
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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: 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 `@electron/native/wgc-capture/src/main.cpp`:
- Around line 307-311: Update the DXGI monitor/output enumeration loops in main
so they stop only when the enumeration call returns DXGI_ERROR_NOT_FOUND; treat
DXGI_ERROR_NOT_CURRENTLY_AVAILABLE as an indeterminate enumeration failure and
report that distinct status instead of emitting monitorLuid:null or
outputs=none. Preserve the existing no-output handling for genuinely completed
enumeration, and run the WGC smoke test on a real Windows system.
🪄 Autofix
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 Plus
Run ID: 472a463e-0f04-4098-8c6a-86f2b70a5cc8
📒 Files selected for processing (2)
electron/native/README.mdelectron/native/wgc-capture/src/main.cpp
| } else { | ||
| // No output claims this monitor: it is driven by something DXGI does not | ||
| // enumerate, which on the machines in #252 means a virtual display | ||
| // adapter. Worth seeing in a report in its own right. | ||
| std::cout << "null,\"sameAdapter\":null"; | ||
| std::cout << "null,\"monitorLuid\":null,\"sameAdapter\":null"; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- file outline ---'
ast-grep outline electron/native/wgc-capture/src/main.cpp
printf '%s\n' '--- target section ---'
sed -n '240,370p' electron/native/wgc-capture/src/main.cpp
printf '%s\n' '--- HRESULT and enumeration references ---'
rg -n -C 4 'EnumAdapters|EnumOutputs|DXGI_ERROR_NOT_FOUND|DXGI_ERROR_NOT_CURRENTLY_AVAILABLE|outputs=|monitorLuid|sameAdapter' electron/native/wgc-capture/src/main.cpp electron/nativeRepository: getopenscreen/openscreen
Length of output: 25062
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- function context and callers ---'
sed -n '190,355p' electron/native/wgc-capture/src/main.cpp
rg -n -C 3 'reportCaptureAdapters|capture-adapter|schemaVersion.*2|monitorAdapter|outputs=none' --glob '!node_modules/**' .
printf '%s\n' '--- relevant project guidance and native test references ---'
rg -n -C 3 'manual smoke|smoke test|wgc-capture|Windows|macOS|native capture' README.md CONTRIBUTING.md docs electron .github 2>/dev/null || true
printf '%s\n' '--- change summary ---'
git diff --stat
git status --shortRepository: getopenscreen/openscreen
Length of output: 50381
🌐 Web query:
Microsoft DXGI IDXGIAdapter1::EnumAdapters1 IDXGIAdapter::EnumOutputs documentation DXGI_ERROR_NOT_FOUND DXGI_ERROR_NOT_CURRENTLY_AVAILABLE
💡 Result:
In the Microsoft DirectX Graphics Infrastructure (DXGI) API, these methods and error codes are used for enumerating graphics hardware and displays. IDXGIFactory1::EnumAdapters1 This method is used to enumerate available graphics adapters (video cards) on the system [1][2]. It is part of the IDXGIFactory1 interface, which provides improvements over the original DXGI 1.0 IDXGIFactory by explicitly supporting the enumeration of adapters that do not have monitors attached [1][2]. - Usage: You pass an index (UINT) to the method to retrieve the corresponding IDXGIAdapter1 interface [1][3]. - Enumeration Order: It first returns the adapter associated with the desktop primary display, followed by other adapters with outputs, and finally adapters without outputs [1][4]. IDXGIAdapter::EnumOutputs This method is used to enumerate the outputs (monitors/displays) connected to a specific graphics adapter [5][6]. - Usage: You pass an index (UINT) to the method to retrieve the corresponding IDXGIOutput interface [5][7]. DXGI_ERROR_NOT_FOUND This error code indicates that the requested index is out of range [8]. - For IDXGIFactory1::EnumAdapters1: It is returned when the provided index is greater than or equal to the total number of adapters in the system [1][3]. - For IDXGIAdapter::EnumOutputs: It is returned when the index is greater than the number of available outputs [5][7]. It is also returned if the adapter is a WARP (Windows Advanced Rasterization Platform) software adapter, as these have no physical outputs [5][7]. DXGI_ERROR_NOT_CURRENTLY_AVAILABLE This error code indicates that the requested resource or operation is currently unavailable, though it may become available later [8][9]. - Specifically for IDXGIAdapter::EnumOutputs: This error is frequently returned if the method is called from a Session 0 process (such as a Windows service), where interactive display access is restricted [5][7]. Summary of Return Values - IDXGIFactory1::EnumAdapters1: Returns S_OK on success, or DXGI_ERROR_NOT_FOUND if the index is invalid [1][3]. - IDXGIAdapter::EnumOutputs: Returns S_OK on success, DXGI_ERROR_NOT_FOUND if the output index is out of range, or DXGI_ERROR_NOT_CURRENTLY_AVAILABLE if called from an inappropriate context like Session 0 [5][7].
Citations:
- 1: https://learn.microsoft.com/en-us/windows/win32/api/dxgi/nf-dxgi-idxgifactory1-enumadapters1
- 2: https://learn.microsoft.com/en-us/windows/win32/direct3ddxgi/d3d10-graphics-programming-guide-dxgi
- 3: https://github.com/MicrosoftDocs/sdk-api/blob/docs/sdk-api-src/content/dxgi/nf-dxgi-idxgifactory1-enumadapters1.md
- 4: https://learn.microsoft.com/en-us/windows/win32/api/dxgi/nf-dxgi-createdxgifactory1
- 5: https://learn.microsoft.com/en-us/windows/win32/api/dxgi/nf-dxgi-idxgiadapter-enumoutputs
- 6: https://learn.microsoft.com/en-us/windows/win32/direct3d11/overviews-direct3d-11-devices-get-adapter-info
- 7: https://github.com/MicrosoftDocs/sdk-api/blob/docs/sdk-api-src/content/dxgi/nf-dxgi-idxgiadapter-enumoutputs.md
- 8: https://learn.microsoft.com/en-us/windows/win32/direct3ddxgi/dxgi-error
- 9: [DXGI_ERROR_NOT_CURRENTLY_AVAILABLE/NotCurrentlyAvailable] On each start-up FakeFishGames/Barotrauma#4916
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
path = Path("electron/native/wgc-capture/src/main.cpp")
text = path.read_text()
# Read-only structural verifier for the two enumeration loops.
patterns = [
("monitor lookup", r"for \(UINT adapterIndex = 0;; \+\+adapterIndex\).*?monitorAdapterFound = true;",
r"EnumAdapters1\(adapterIndex, &adapter\).*?FAILED\(\).*?break;",
r"EnumOutputs\(outputIndex, &output\).*?FAILED\(\).*?break;"),
("full diagnostic", r"for \(UINT adapterIndex = 0;; \+\+adapterIndex\).*?std::cerr << std::endl;",
r"EnumAdapters1\(adapterIndex, &adapter\).*?FAILED\(\).*?break;",
r"EnumOutputs\(outputIndex, &output\).*?FAILED\(\).*?break;"),
]
for name, scope_pattern, adapter_pattern, output_pattern in patterns:
scope = re.search(scope_pattern, text, re.S)
if not scope:
raise SystemExit(f"{name}: scope not found")
body = scope.group(0)
print(f"{name}:")
print(f" adapter_failed_hresult_break = {bool(re.search(adapter_pattern, body, re.S))}")
print(f" output_failed_hresult_break = {bool(re.search(output_pattern, body, re.S))}")
# Verify the state assignments made when no matching output is found.
json_else = re.search(
r"if \(monitorAdapterFound\).*?else \{.*?std::cout << (.*?);",
text, re.S)
if not json_else:
raise SystemExit("JSON fallback not found")
print("JSON fallback:")
print(" monitorAdapterFound_false_emits_null_monitorLuid = "
+ str('"monitorLuid":null' in json_else.group(1)))
print(" monitorAdapterFound_false_emits_null_sameAdapter = "
+ str('"sameAdapter":null' in json_else.group(1)))
stderr_scope = text[text.index(" // The full enumeration, to stderr"):text.index("\n}\n\nbool hasVisibleBgraContent")]
print("stderr fallback:")
print(" outputCount_zero_emits_outputs_none = "
+ str('if (outputCount == 0)' in stderr_scope and 'outputs=none' in stderr_scope))
print(" outputCount_increments_only_after_success = "
+ str(stderr_scope.index("++outputCount") > stderr_scope.index("EnumOutputs")))
PY
printf '%s\n' '--- narrow contract references ---'
rg -n -C 2 'capture-adapter|monitorLuid|sameAdapter|outputs=none|schemaVersion' \
electron/native electron/native-bridge scripts .github/workflows \
-g '*.ts' -g '*.js' -g '*.mjs' -g '*.json' -g '*.md' 2>/dev/null | head -n 240
printf '%s\n' '--- main call site ---'
sed -n '600,710p' electron/native/wgc-capture/src/main.cppRepository: getopenscreen/openscreen
Length of output: 11660
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re
source = Path("electron/native/wgc-capture/src/main.cpp").read_text()
# Extract the two relevant regions by their stable comments.
lookup_start = source.index(" // Both loops end on FAILED()")
lookup_end = source.index(" // The LUIDs are reported", lookup_start)
lookup = source[lookup_start:lookup_end]
diagnostic_start = source.index(" // The full enumeration, to stderr")
diagnostic_end = source.index("\n}\n\nbool hasVisibleBgraContent", diagnostic_start)
diagnostic = source[diagnostic_start:diagnostic_end]
for name, region in (("monitor lookup", lookup), ("full diagnostic", diagnostic)):
adapter_guard = re.search(
r"EnumAdapters1\s*\([^;]+;\s*if\s*\(FAILED\s*\([^)]*\)\s*\|\|\s*!adapter\)\s*\{\s*break;",
region, re.S)
output_guard = re.search(
r"EnumOutputs\s*\([^;]+;\s*if\s*\(FAILED\s*\([^)]*\)\s*\|\|\s*!output\)\s*\{\s*break;",
region, re.S)
print(f"{name}: adapter_guard={bool(adapter_guard)} output_guard={bool(output_guard)}")
print("monitor lookup fallback:")
print(" monitorAdapterFound_false_branch =",
"if (monitorAdapterFound)" in source and "} else {" in source[lookup_end:source.index(" std::cout << \"}\"", lookup_end)])
json_tail = source[source.index('std::cout << "null', lookup_end):source.index('std::cout << "null', lookup_end)+100]
print(" emits_monitorLuid_null =", '\\"monitorLuid\\":null' in json_tail)
print(" emits_sameAdapter_null =", '\\"sameAdapter\\":null' in json_tail)
print("full diagnostic fallback:")
print(" outputCount_starts_at_zero =", "UINT outputCount = 0;" in diagnostic)
print(" outputCount_increments_after_enum_success =",
diagnostic.index("++outputCount") > diagnostic.index("EnumOutputs"))
print(" zero_count_emits_outputs_none =",
"if (outputCount == 0)" in diagnostic and 'outputs=none' in diagnostic)
# Behavioral state model: the current code treats every FAILED HRESULT as the
# same loop terminator, so NOT_CURRENTLY_AVAILABLE produces the zero/unknown
# fallbacks instead of a distinct indeterminate state.
HRESULTS = {
"S_OK": 0x00000000,
"DXGI_ERROR_NOT_FOUND": 0x887A0002,
"DXGI_ERROR_NOT_CURRENTLY_AVAILABLE": 0x887A0022,
}
def failed(hr):
return bool(hr & 0x80000000)
for name, hr in HRESULTS.items():
print(f"FAILED({name})={failed(hr)}")
print("non-terminal failure is distinct from normal end marker =",
HRESULTS["DXGI_ERROR_NOT_CURRENTLY_AVAILABLE"] != HRESULTS["DXGI_ERROR_NOT_FOUND"])
PY
printf '%s\n' '--- exact README diagnostic contract ---'
sed -n '45,105p' electron/native/README.mdRepository: getopenscreen/openscreen
Length of output: 9414
Distinguish DXGI_ERROR_NOT_FOUND from enumeration failures.
Stop only on DXGI_ERROR_NOT_FOUND. Report DXGI_ERROR_NOT_CURRENTLY_AVAILABLE as an indeterminate enumeration failure. The current loops emit monitorLuid:null and outputs=none for both cases. Run the WGC smoke test on a real Windows system because CI does not exercise native capture.
🤖 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 `@electron/native/wgc-capture/src/main.cpp` around lines 307 - 311, Update the
DXGI monitor/output enumeration loops in main so they stop only when the
enumeration call returns DXGI_ERROR_NOT_FOUND; treat
DXGI_ERROR_NOT_CURRENTLY_AVAILABLE as an indeterminate enumeration failure and
report that distinct status instead of emitting monitorLuid:null or
outputs=none. Preserve the existing no-output handling for genuinely completed
enumeration, and run the WGC smoke test on a real Windows system.
1 participant
The
capture-adapterdiagnostic from #332 compared adapters by LUID and printed only their descriptions. On the exact configuration it exists to catch, those two descriptions are the same string.Measured on a rented multi-adapter VM:
An IddCx virtual display driver renders through the physical GPU and inherits its description string, while being a separate DXGI adapter with its own LUID.
QueryDisplayConfigconfirmed the virtual monitor sits under0:12889146.So a machine that actually has the divergence would have printed:
{"deviceAdapter":"NVIDIA Quadro RTX 4000","monitorAdapter":"NVIDIA Quadro RTX 4000","sameAdapter":false}Two identical names next to
sameAdapter:false— which reads as a bug in the reporting rather than as the finding it is, and would most likely have been dismissed as one. The comparison was never wrong. The output was unreadable at the only moment it matters, and we were days away from asking four reporters to run it.What changed
deviceLuidandmonitorLuidon the event, formattedHighPart:LowPartto matchQueryDisplayConfigtraces so a LUID from a bug report greps straight against one[adapters]line per enumerated adapter on stderr, with its LUID and its outputs, marking the captured oneThe stderr dump matters for the negative case too: an adapter with no output is how an inactive virtual display presents, and that is exactly why the rented box reported
sameAdapter:truedespite having a virtual display driver installed. Having one is not enough — it has to be driving the captured display.Not in scope
Making
createD3DDeviceselect the adapter that owns the captured HMONITOR instead of taking the default. That is a real correctness gap — nothing in DXGI promises adapter 0 drives the display, and nothing in the code checks — but it is a behaviour change and belongs in its own PR. This one only makes the existing diagnostic legible.Refs #252, #292, #327.
Summary by CodeRabbit
New Features
Documentation