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

macros/class: Propagate cfg annotations to COM class methods #234

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

MarijnS95
Copy link
Contributor

@MarijnS95 MarijnS95 commented Feb 1, 2022

We have a very peculiar workaround for DXC (pending upstream fix solution microsoft/DirectXShaderCompiler#3793) that requires us to conditionally provide function implementations for extra vtable "spacers": https://github.com/Traverse-Research/hassle-rs/blob/f5a090c70bbcf66f3bafd3d549716a414e873838/src/wrapper.rs#L130-L139. The spacers are conditionally defined in: https://github.com/Traverse-Research/hassle-rs/blob/f5a090c70bbcf66f3bafd3d549716a414e873838/src/unknown.rs.

These need to be forwarded otherwise the initialization of these vtable members will happen unconditionally even when they don't exist: https://github.com/Traverse-Research/hassle-rs/actions/runs/1777800733

Fixes #166

@MarijnS95
Copy link
Contributor Author

Alternatively we could have that pub(crate) type IDxcUnknownShim = IUnknown; but it requires propagating the attribute on the impl instead.

@jelmansouri
Copy link

Hi! Is there any update on this PR, our game engine has a dependency in our shader building infra which is Linux only to the DirectXShaderCompiler and we use hassle-rs. Legacy non Microsoft Com library is unmaintained and still causing us issues. Having DirectXShaderCompiler + Rust working properly is important for the professional game development community which uses HLSL most of the time, especially that Rust is taking more and more it's place in the tooling side games!

@MarijnS95
Copy link
Contributor Author

MarijnS95 commented Feb 28, 2022

@rylev can we get a review, merge and release for #235 and this PR (#234)? That would make us able to start using com-rs in production on hassle-rs 🎉

We have a very peculiar workaround for DXC (pending upstream fix
[microsoft/DirectXShaderCompiler#3793]) that requires us to
conditionally provide function implementations for extra vtable
"spacers": https://github.com/Traverse-Research/hassle-rs/blob/f5a090c70bbcf66f3bafd3d549716a414e873838/src/wrapper.rs#L130-L139.
The spacers are conditionally defined in:
https://github.com/Traverse-Research/hassle-rs/blob/f5a090c70bbcf66f3bafd3d549716a414e873838/src/unknown.rs.

These need to be forwarded otherwise the initialization of these vtable
members will happen unconditionally even when they don't exist:
https://github.com/Traverse-Research/hassle-rs/actions/runs/1777800733

Fixes microsoft#166
[microsoft/DirectXShaderCompiler#3793]: microsoft/DirectXShaderCompiler#3793
@MarijnS95 MarijnS95 force-pushed the propagate-cfg-to-class-methods branch from c8256cb to 07148b2 Compare March 28, 2023 20:28
@MarijnS95
Copy link
Contributor Author

@rylev not sure if you still intend to at least merge outstanding PRs and make another release (and at also push the missing tags to this repo), but I don't really care about following up on this anymore as the upstream fix has finally made its way in, so I can use published com 0.6 now rather than this branch on a fork.

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.

Propagate cfg annotations on COM class methods
2 participants