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

Add reflection bindings #75

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

aclysma
Copy link

@aclysma aclysma commented Sep 8, 2024

I think this covers most of the currently available reflection API exposed by ID3D12ShaderReflection.

I've added code comments about this in the change but want to call it out explicitly:

  • There's a new reflection example and an HLSL file that came from a D3D12 sample for it to load
  • I've added #[rustfmt::skip] in places where I felt it was clearer without rustfmt
  • The reflection API I'm adding bindings for is COM-like, in that it returns objects with a vtable pointer, but they don't inherit from Unknown. The com crate doesn't like this, and I've had to implement bindings for these by hand. The lifetime for these pseudo com objects is probably the lifetime of the owning ID3D12ShaderReflection com object. So the safe wrappers contain both the COM-like object and the ref-counted ID3D12ShaderReflection.
  • I added the enums that are used by the reflection. I used the machine-generated code from the windows crate and did a human pass on them to clean them up. They aren't strictly needed and could be removed from this change. (The constants, not the new types for the enums.

I've used code from the following MIT-licensed sources:

@Jasper-Bekkers
Copy link
Member

Thanks for the contribution, I haven't looked too deeply yet at the setup / implementation, however, I think the wrapper structs at least should be snake_case rather then how they are defined now.

@Jasper-Bekkers
Copy link
Member

Im also ok with bumping this to the 2021 edition but we should do that in a separate PR from this one.

@aclysma
Copy link
Author

aclysma commented Sep 8, 2024

I'm not strongly opinionated about it, but I kept the original member names verbatim so that they are easier to locate in documentation.

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