Fix shader-coverage demo launch on Windows#11625
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughBoth ChangesWindows DLL PATH fix in coverage scripts
🚥 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. 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
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d996c92d-b15a-47af-a534-dacec386394d
📒 Files selected for processing (3)
examples/shader-coverage-bvh-traversal/run_coverage.pyexamples/shader-coverage-image-pipeline/run_coverage.pytools/coverage-html/slang-coverage-html.py
There was a problem hiding this comment.
♻️ Duplicate comments (1)
examples/shader-coverage-bvh-traversal/run_coverage.py (1)
97-99:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe Windows preset-layout gap is shared by both wrappers. Both files claim generalized
binaryDirsupport, but binary discovery remains rooted toslang_root/build, so non-default Windows preset layouts can still fail before PATH augmentation.
examples/shader-coverage-bvh-traversal/run_coverage.py#L97-L99: either broaden_candidate_paths()/ discovery to include presetbinaryDirvariants (e.g.,build/windows-vs2022-dev/...) or narrow the doc claim to current support.examples/shader-coverage-image-pipeline/run_coverage.py#L97-L99: apply the same discovery fix (or same doc narrowing) to keep behavior and docs consistent between wrappers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 38989f37-af28-49e1-a805-9694f07c2e59
📒 Files selected for processing (2)
examples/shader-coverage-bvh-traversal/run_coverage.pyexamples/shader-coverage-image-pipeline/run_coverage.py
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
jvepsalainen-nv
force-pushed
the
fix/coverage-demo-dll-search-and-ascii-output
branch
5 times, most recently
from
31a2ac7 to
276beb8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8b0c9776-152d-43dd-bfbf-abc215f02296
📒 Files selected for processing (2)
examples/shader-coverage-bvh-traversal/run_coverage.pyexamples/shader-coverage-image-pipeline/run_coverage.py
There was a problem hiding this comment.
Verdict: 🔴 Has issues — 1 bug, 1 question
The PR prepends a derived bin/ directory to the child process PATH so the Windows loader can find slang.dll when launching the shader-coverage demo. The derivation uses positional parents[3] / parent.name arithmetic that is only correct for multi-config CMake layouts; it silently produces a wrong (non-existent) DLL directory for the single-config layout that _candidate_paths itself enumerates. The PR title also advertises an "ASCII console output" change that is not present in the diff.
Changes Overview
Windows DLL search path for demo wrappers (examples/shader-coverage-bvh-traversal/run_coverage.py, examples/shader-coverage-image-pipeline/run_coverage.py)
- Before:
subprocess.run(binary_cmd)— child process inherits parentPATH; on Windows the loader cannot findslang.dll/etc. because they live inbuild/<config>/bin/rather than next to the demo.exe, so the demo aborts withSTATUS_DLL_NOT_FOUND(0xC0000135). - After: when
platform.system() == "Windows"andlen(binary.parents) >= 4, prependbinary.parents[3] / binary.parent.name / "bin"to a copiedPATHand pass it viaenv=to the single demosubprocess.run. POSIX behavior is unchanged (rpath handles DLL resolution there). File mode also flipped 100644 → 100755 on both scripts (consistent with the existing#!/usr/bin/env python3shebang).
Findings (2 total)
| Severity | Location | Finding |
|---|---|---|
| 🔴 Bug | examples/shader-coverage-bvh-traversal/run_coverage.py:190, examples/shader-coverage-image-pipeline/run_coverage.py:199 |
binary.parents[3] / binary.parent.name / "bin" only matches the multi-config layout; for the single-config layout enumerated in _candidate_paths it computes a non-existent directory and the DLL load still fails. |
| 🔵 Question | PR title | Title advertises an "ASCII console output" fix that is not in the diff — was a commit dropped, or is the title stale? |
jvepsalainen-nv
force-pushed
the
fix/coverage-demo-dll-search-and-ascii-output
branch
from
276beb8 to
c275f3f
Compare
jvepsalainen-nv
changed the title
The demo exe is built into build/examples/<target>/<config>/ but the Slang runtime DLLs live in build/<config>/bin/. The Windows loader only searches the exe's own directory, so the process aborts with STATUS_DLL_NOT_FOUND (0xC0000135). Derive the DLL directory from the already-located binary path and prepend it to the child process PATH before launching the demo. POSIX is unaffected (rpath handles DLL resolution there). Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
jvepsalainen-nv
force-pushed
the
fix/coverage-demo-dll-search-and-ascii-output
branch
from
c275f3f to
99faf9a
Compare
jvepsalainen-nv
changed the title
jvepsalainen-nv
changed the title
## Motivation
`run_coverage.py` in both shader-coverage demos fails on Windows because
the demo `.exe` lives in `build/examples/<target>/<config>/` but the
Slang runtime DLLs (`slang.dll`, etc.) live in `build/<config>/bin/`.
The Windows loader only searches the exe's own directory, so the process
aborts immediately with `STATUS_DLL_NOT_FOUND` (0xC0000135).
## Changes
Prepend the runtime DLL directory to the child process `PATH` before
launching the demo binary. The directory is derived from the
already-known
binary path and handles both CMake generator layouts:
```python
_WIN_CONFIGS = {"Release", "Debug", "RelWithDebInfo"}
demo_env = os.environ.copy()
if platform.system() == "Windows" and len(binary.parents) >= 4:
config = (binary.parent.name if binary.parent.name in _WIN_CONFIGS
else binary.parents[2].name)
dll_dir = binary.parents[3] / config / "bin"
demo_env["PATH"] = str(dll_dir) + os.pathsep + demo_env.get("PATH", "")
result = subprocess.run(binary_cmd, env=demo_env)
```
- **Multi-config** (Ninja Multi-Config / Visual Studio): exe at
`<build>/examples/<target>/<config>/<exe>` — config =
`binary.parent.name`.
- **Single-config** (plain Ninja / Makefiles): exe at
`<build>/<config>/examples/<target>/<exe>` — config =
`binary.parents[2].name`.
- In both cases `binary.parents[3]` is the build root and the DLL
directory
is `<build-root>/<config>/bin/`.
- No change on POSIX — `rpath` handles DLL resolution there.
Both `examples/shader-coverage-bvh-traversal/run_coverage.py` and
`examples/shader-coverage-image-pipeline/run_coverage.py` receive the
same fix.
Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
2 participants
Motivation
run_coverage.pyin both shader-coverage demos fails on Windows becausethe demo
.exelives inbuild/examples/<target>/<config>/but theSlang runtime DLLs (
slang.dll, etc.) live inbuild/<config>/bin/.The Windows loader only searches the exe's own directory, so the process
aborts immediately with
STATUS_DLL_NOT_FOUND(0xC0000135).Changes
Prepend the runtime DLL directory to the child process
PATHbeforelaunching the demo binary. The directory is derived from the already-known
binary path and handles both CMake generator layouts:
<build>/examples/<target>/<config>/<exe>— config =binary.parent.name.<build>/<config>/examples/<target>/<exe>— config =binary.parents[2].name.binary.parents[3]is the build root and the DLL directoryis
<build-root>/<config>/bin/.rpathhandles DLL resolution there.Both
examples/shader-coverage-bvh-traversal/run_coverage.pyandexamples/shader-coverage-image-pipeline/run_coverage.pyreceive thesame fix.