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

glsl-in: support for storage image types #1723

Merged
merged 3 commits into from
Feb 16, 2022

Conversation

JCapucho
Copy link
Collaborator

@JCapucho JCapucho commented Feb 10, 2022

Adds support for the gimageDim family of types and the imageSize, imageLoad and imageStore functions.

Missing

Unresolved

  • glsl supports multisampled storage images, naga doesn't, how do we handle it?
  • glsl allows for writeonly storage images to not have their format specified, naga doesn't like it, once again how do we handle it?
  • there are some texture formats to which I don't know the proper naga equivalent
  • glsl allows for cube images which naga doesn't allow in imageLoad/Store, how do we handle it?

Closes #1456

@JCapucho JCapucho marked this pull request as draft February 10, 2022 21:07
@JCapucho JCapucho marked this pull request as ready for review February 11, 2022 23:29
@JCapucho JCapucho requested a review from kvark February 11, 2022 23:29
@JCapucho JCapucho changed the title WIP: glsl-in: support for storage image types glsl-in: support for storage image types Feb 12, 2022
@kvark
Copy link
Member

kvark commented Feb 13, 2022

glsl supports multisampled storage images, naga doesn't, how do we handle it

let's support it in the IR, but have a capability in the validator to check for it. Relevant SPIR-V capability - https://www.khronos.org/registry/vulkan/specs/1.2/html/chap31.html#features-shaderStorageImageMultisample

glsl allows for writeonly storage images to not have their format specified, naga doesn't like it, once again how do we handle it?

Not sure what to do with it. I suggest just not supporting this in Naga, at least for now.
SPIR-V has capabilities for either reads or writes to be unspecified with formats, but that would complicate our IR and backend logic possibly unnecessarily.

there are some texture formats to which I don't know the proper naga equivalent

We'll probably just need to add them.

glsl allows for cube images which naga doesn't allow in imageLoad/Store, how do we handle it?

This doesn't appear useful in modern APIs, since one would just create a 2D view instead. It looks like a GLSL artifact, so we shouldn't try supporting it.

src/front/glsl/builtins.rs Outdated Show resolved Hide resolved
src/front/glsl/parser/types.rs Outdated Show resolved Hide resolved
@kvark
Copy link
Member

kvark commented Feb 13, 2022

Suggestions are optional, please land when you feel ready.

@JCapucho
Copy link
Collaborator Author

I've applied your suggestions, I'll merge this as an initial implementation and work on the missing bits that require some IR changes later when I have more time.

@JCapucho JCapucho enabled auto-merge (rebase) February 16, 2022 21:53
@JCapucho JCapucho merged commit 9e4f678 into gfx-rs:master Feb 16, 2022
@JCapucho JCapucho deleted the glsl-image-decl branch February 24, 2022 16:51
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.

[glsl-in] Support image variables
2 participants