Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[naga] Let constant evaluation handle Compose of Splat. #4695

Merged
merged 1 commit into from
Nov 16, 2023

Conversation

jimblandy
Copy link
Member

@jimblandy jimblandy commented Nov 16, 2023

When consuming a Compose expression that constructs a vector, flatten Splat subexpressions out into their components.

Fixes #4581.

cc @cwfitzgerald

Checklist

  • Run cargo clippy.
  • Run cargo clippy --target wasm32-unknown-unknown if applicable.
  • Add change to CHANGELOG.md. See simple instructions inside file.

@jimblandy jimblandy added naga Shader Translator lang: SPIR-V Vulkan's Shading Language area: naga processing Passes over IR in the middle labels Nov 16, 2023
@jimblandy jimblandy requested a review from a team as a code owner November 16, 2023 01:18
Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Nice!

When consuming a `Compose` expression that constructs a vector,
flatten `Splat` subexpressions out into their components.

Fixes gfx-rs#4581.
Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Thinking out loud: will this code still have issues when spirv/glsl in is presented with a complex matrix splat (like mat3(vec4, float, vec4))?

@teoxoy
Copy link
Member

teoxoy commented Nov 16, 2023

Thinking out loud: will this code still have issues when spirv/glsl in is presented with a complex matrix splat (like mat3(vec4, float, vec4))?

The GLSL frontend will take care of the desugaring, so I think it should work fine.

TypeInner::Matrix {

SPIR-V doesn't support those complex compose expressions as far as I know.

Copy link
Member

@teoxoy teoxoy left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for looking into it!

@teoxoy teoxoy merged commit b7dd59e into gfx-rs:trunk Nov 16, 2023
27 checks passed
cwfitzgerald pushed a commit that referenced this pull request Dec 6, 2023
@jimblandy jimblandy deleted the const-flatten-splat branch December 13, 2023 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: naga processing Passes over IR in the middle lang: SPIR-V Vulkan's Shading Language naga Shader Translator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

With Restricted Image Loads, Invalid Spirv Generated
3 participants