Fix spurious StorageImageReadWithoutFormat capability for write-only textures#10023
Fix spurious StorageImageReadWithoutFormat capability for write-only textures#10023jhelferty-nv wants to merge 4 commits intoshader-slang:masterfrom
Conversation
…textures Fixes shader-slang#9997 WTexture* types with unknown format were emitting both StorageImageReadWithoutFormat and StorageImageWriteWithoutFormat SPIR-V capabilities, even though write-only textures cannot be read. This caused pipeline rejection on hardware that doesn't support unformatted storage image reads (e.g. pre-Xe Intel GPUs). Check the texture access mode when deciding which capabilities to emit: write-only textures now only emit StorageImageWriteWithoutFormat. Add tests for both write-only (WTexture2D) and read-write (RWTexture2D) capability emission to prevent regressions.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdjusts SPIR‑V capability emission for storage images with unknown format: require StorageImageReadWithoutFormat only when the texture access permits reads; write-only textures require only StorageImageWriteWithoutFormat. Adds tests for RWTexture2D (both capabilities) and WTexture2D (write-only capability, NonReadable decoration). Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 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)
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.
Pull request overview
This PR fixes a bug where write-only textures (WTexture2D) were incorrectly emitting the StorageImageReadWithoutFormat SPIR-V capability, even though they only support write operations. This caused pipeline rejection on hardware that doesn't support unformatted storage image reads (e.g., pre-Xe Intel GPUs). The fix adds a conditional check to only emit the read capability for non-write-only textures.
Changes:
- Modified SPIR-V emission logic to check texture access mode before emitting
StorageImageReadWithoutFormatcapability - Added regression test for write-only textures (
WTexture2D) to ensure only write capability is emitted - Added companion test for read-write textures (
RWTexture2D) to ensure both capabilities are still emitted correctly
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| source/slang/slang-emit-spirv.cpp | Added access mode check to conditionally emit StorageImageReadWithoutFormat capability only for non-write-only textures |
| tests/spirv/write-only-image-capability.slang | Test verifying WTexture2D only emits StorageImageWriteWithoutFormat and NonReadable decoration |
| tests/spirv/read-write-image-capability.slang | Test verifying RWTexture2D emits both read and write capabilities |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/spirv/write-only-image-capability.slang`:
- Line 13: The test declares a write-only texture without an explicit type
parameter; update the declaration from WTexture2D to WTexture2D<float4> so it
matches the companion read-write test and the Store call writing float4; locate
the WTexture2D declaration in write-only-image-capability.slang and add the
<float4> type parameter to the WTexture2D symbol.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Code Review: Fix spurious StorageImageReadWithoutFormat capability for write-only texturesSummaryClean, well-targeted fix that resolves #9997 — Correctness ✅The fix is correct and directly addresses the existing TODO comment in the code:
Test Coverage ✅Good regression tests:
Nit (non-blocking)A test for LGTM — minimal, well-scoped fix with good test coverage. |
Fixes #9997
WTexture* types with unknown format were emitting both StorageImageReadWithoutFormat and StorageImageWriteWithoutFormat SPIR-V capabilities, even though write-only textures cannot be read. This caused pipeline rejection on hardware that doesn't support unformatted storage image reads (e.g. pre-Xe Intel GPUs).
Check the texture access mode when deciding which capabilities to emit: write-only textures now only emit StorageImageWriteWithoutFormat.
Add tests for both write-only (WTexture2D) and read-write (RWTexture2D) capability emission to prevent regressions.