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 local variables, interface blocks, and XOR #258

Merged
merged 3 commits into from
Nov 3, 2020

Conversation

kristoff3r
Copy link
Contributor

I did some more GLSL work. With this all the shaders from #210 gets through glsl-in -> glsl-out with what looks like a sensible result - except for a small bug in glsl-out, which I'll open another pull request for.

Otherwise the backends will panic on e.g. `vec4 local = Global;` if the global
isn't used anywere else in the function.
@kvark kvark changed the title Glsl work GLSL local variables, interface blocks, and XOR Nov 3, 2020
} else {
// variables in interface blocks without an instance name are in the global namespace
// https://www.khronos.org/opengl/wiki/Interface_Block_(GLSL)
if let TypeInner::Struct { members } = &extra.module.types[d.ty].inner {
Copy link
Member

Choose a reason for hiding this comment

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

Let's not match on a reference, instead we should just do ref members. This is more explicit and less sugary.

@kvark kvark merged commit bb5c956 into gfx-rs:master Nov 3, 2020
Comment on lines +1079 to +1094
for m in members {
if let Some(name) = &m.name {
let h = extra
.module
.global_variables
.fetch_or_append(GlobalVariable {
name: Some(name.into()),
class,
binding: binding.clone(),
ty: m.ty,
interpolation,
storage_access: StorageAccess::empty(), // TODO
});
extra.lookup_global_variables.insert(name.into(), h);
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't really see how this works - how does it relate to the code here: https://github.com/kristoff3r/naga/blob/c436606d09188efab0057db469802ea01df3230f/src/front/glsl/parser.rs#L971.

Also won't this register the members as 'plain' global vars? What happens when these are referenced - then I don't think it will translate into AccessIndex. (OpAccessChain in spv).

I made a small sample where you can see the spv output for named vs. anon interface block: http://shader-playground.timjones.io/12dc04ae724cbb6aa1ddcf812e638302

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I didn't notice that there was some handling of this already. I'll take a look at it and see if it needs changing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

No worries, we'll sort it out :)

Copy link
Member

Choose a reason for hiding this comment

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

I shall wait for @pjoe before merging the GLSL-in fixes

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.

3 participants