Skip to content

Conversation

@cwfitzgerald
Copy link
Member

Small thing I found that if you write Texture2D<uint> in slang, it will add a format of R32Uint to the image, which tripped up our detection. This makes the behavior better. If anyone runs into this and doesn't have this patch, add [[vk::image_format("unknown")]] and it will generate the spirv we expect.

@Wumpf Wumpf self-assigned this Jan 7, 2026
@cwfitzgerald cwfitzgerald force-pushed the cw/improve-optypeimage-detection branch from 40a43b5 to b01e594 Compare January 9, 2026 03:08
@cwfitzgerald cwfitzgerald force-pushed the cw/improve-optypeimage-detection branch from b01e594 to ff3764f Compare January 9, 2026 16:21
@Wumpf Wumpf requested a review from Copilot January 10, 2026 16:34
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves the SPIR-V input frontend's detection of ImageClass to correctly handle cases where textures have explicit formats specified (e.g., Texture2D<uint> in Slang generating R32Uint format). The fix refines the logic for determining whether an image is a storage texture, sampled texture, or depth texture based on the combination of the is_sampled flag and explicit format specification.

Changes:

  • Enhanced ImageClass detection logic to properly handle unknown sampling information with explicit formats
  • Improved error messages to distinguish between different multisampling-related validation failures
  • Added validation to prevent depth images from being storage textures

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
naga/src/front/spv/mod.rs Updated ImageClass detection logic to infer storage class from explicit format when sampling info is unknown
naga/src/front/spv/error.rs Renamed error variant and added new error for depth+storage combination
naga/src/valid/expression.rs Split InvalidImageOtherIndex into two specific errors and added debug statement

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

},
_ => {
e => {
std::dbg!(e, class);
Copy link

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

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

Debug print statement (std::dbg!) should be removed before merging. This appears to be leftover debugging code that will pollute stderr output in production.

Suggested change
std::dbg!(e, class);

Copilot uses AI. Check for mistakes.
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

nice, great errors. but leftover dbg needs to go. Pre-approving

},
_ => {
e => {
std::dbg!(e, class);
Copy link
Member

Choose a reason for hiding this comment

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

leftover dbg

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.

2 participants