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

Is there a way to inspect uniforms (e.g. sizes, strides, offsets) within a uniform buffer? #43

Closed
StrayLightning opened this issue Jan 5, 2018 · 13 comments

Comments

@StrayLightning
Copy link
Contributor

I can see how to query the uniform buffers, but at the moment I can't see how to enumerate the fields within the buffer. I am wanting to use reflection on some SPIR-V from within a Rust project build.

Nice project, BTW.

@grovesNL
Copy link
Owner

grovesNL commented Jan 5, 2018

Hi! This is possible in the C++ SPIRV-Cross API that we use (see example usage), but I don't think we've created a Rust wrapper for the member methods yet. Some of the C++ API hasn't been wrapped yet (at the moment we've usually expanded the wrappers whenever gfx-rs needs them).

The relevant part of the C++ API should be the *member* methods listed on the base Compiler class (or whichever methods are interesting for your use case). Feel free to submit a pull request if you'd like to try adding a wrapper for any of these methods (following the pattern we use for similar methods), otherwise I can try to take a look soon.

@StrayLightning
Copy link
Contributor Author

StrayLightning commented Jan 5, 2018

Thanks for the prompt reply. If I get a chance I'll take a closer look at the internals. If I start looking in earnest I'll assign this issue to myself to avoid duplication. Cheers!

EDIT: Ah, yes, I won't be able to assign it. I'll just comment here instead.

@grovesNL
Copy link
Owner

grovesNL commented Jan 7, 2018

I started adding some member reflection in #44 (the diff has some unrelated changes, you can ignore them).

For now I have get member names and get/set member decorations for uniform buffer members, so I believe you should be able to get strides/offsets with these changes. Is that sufficient for what you're trying to do?

@StrayLightning
Copy link
Contributor Author

Thanks. I shall have a go at getting the strides etc out of my shaders.

@StrayLightning
Copy link
Contributor Author

Thanks again. I can query uniform block members and their decorations now, but it's not clear to me how to get the number of members in the block.

The other thing I would like to get is the overall size of the uniform block, i.e. get_declared_struct_size(type).

I shall try and take another look when I get a chance.

@grovesNL
Copy link
Owner

grovesNL commented Jan 18, 2018

To get the number of members, I believe we will just need to expose member_types on Type::Struct (mapped directly from the C++ side), so member_types.len() will give us the member count (see comment in KhronosGroup/SPIRV-Cross#49). This should be relatively straightforward.

For the second point, we'll just need to add the binding for get_declared_struct_size which should also be straightforward.

bors bot added a commit that referenced this issue Feb 9, 2018
53: Improve struct handling r=grovesNL a=grovesNL

This should address some of the questions in #43.
@grovesNL
Copy link
Owner

@StrayLightning Sorry for the long delay! If you're still interested in this, the changes in #53 should address the points you mentioned

@StrayLightning
Copy link
Contributor Author

Ah, thank you. I shall try to get back to it soon, as I am still interested. I have been distracted by other stuff recently.

@StrayLightning StrayLightning changed the title Is there a way to inpect uniforms (e.g. sizes, strides, offsets) within a uniform buffer? Is there a way to inspect uniforms (e.g. sizes, strides, offsets) within a uniform buffer? Feb 16, 2018
@StrayLightning
Copy link
Contributor Author

StrayLightning commented Feb 17, 2018

Thanks, that great! I've used this in StrayLightning/aeolius@ed0198515444e86b10892e766e50c11455afb7c0. I feel like I should look at adding the array field of the SPIRV-Cross SPIRType, which is the last piece of the puzzle that I currently rely on. I have taken a look at f7e07fa: Pattern matching this ought to be fairly easy. But unlike member_types, which only applies to structs, array applies to everything apart from Unknown and Void.

So, for example, my proposed from_raw() implementation in compiler.rs might look like:

impl spirv::Type {
    pub fn from_raw(ty: spirv_cross::SPIRType_BaseType, member_types: Vec<u32>, array: Vec<u32>) -> Type {
        use bindings::root::spirv_cross::SPIRType_BaseType as b;
        use spirv::Type::*;
        match ty {
            b::Unknown => Unknown,
            b::Void => Void,
            b::Boolean => Boolean { array },
            b::Char => Char { array },
            b::Int => Int { array },
            b::UInt => UInt { array },
            b::Int64 => Int64 { array },
            b::UInt64 => UInt64 { array },
            b::AtomicCounter => AtomicCounter { array },
            b::Float => Float { array },
            b::Double => Double { array },
            b::Struct => Struct { member_types, array },
            b::Image => Image { array },
            b::SampledImage => SampledImage { array },
            b::Sampler => Sampler { array },
        }
    }
}

Is that the correct approach to take?

@grovesNL
Copy link
Owner

Yeah, sounds good to me! The enum is just meant to limit the visibility of fields from SPIRType which don't apply to all of the enum variants. But we could always expose a struct instead if it becomes an issue.

@StrayLightning
Copy link
Contributor Author

Just a quick update on progress: I've got a commit that surfaces SPIRType array. It's a breaking change, and I'd like to think of ways around this. Although I've updated the unit tests, it requires some more coverage. I might push to my fork this evening, but possibly won't submit it as a formal PR just yet.

@grovesNL
Copy link
Owner

grovesNL commented Mar 1, 2018

Great, glad you were able to figure it out!

I'm fine with breaking changes at this point, especially if they're easy to fix -- I'll just release another minor version (this crate isn't used too widely yet, mostly with gfx).

@StrayLightning
Copy link
Contributor Author

The merge of PR #58 addresses the remaining desired functionality: Closing this issue.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants