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 a way to convert from an Interface to Option<Interface> #1339

Open
jrmuizel opened this issue Nov 18, 2021 · 11 comments · Fixed by #1562 or #2233
Open

Add a way to convert from an Interface to Option<Interface> #1339

jrmuizel opened this issue Nov 18, 2021 · 11 comments · Fixed by #1562 or #2233
Labels
enhancement New feature or request

Comments

@jrmuizel
Copy link
Contributor

Some APIs like VSSetConstantBuffers take an *const Option<ID3D11Buffer>. Given a &ID3D11Buffer I'd like to be able to construct a &Option<ID3D11Buffer> to pass to a function like this.

@fowenix
Copy link

fowenix commented Nov 19, 2021

Doing &buffer.clone().into() works fine for turning &ID3D11Buffer into &Option<ID3D11Buffer>

@kennykerr
Copy link
Collaborator

This API is difficult to use because it really expects an array but the Rust bindings aren't yet adapted to accept a slice.

This is a duplicate of #479 that I hope to get to soon.

@kennykerr
Copy link
Collaborator

Sorry for the delay, #1562 adds support for Win32 array parameters.

@chyyran
Copy link

chyyran commented Feb 28, 2022

Unfortunately #1562 does not solve the core issue. Even for functions that take slices, they expect slices of &[Option<T>], which makes it difficult when you have a &[T]. The .clone().into() trick works well if you have a slice of size 1, but for slices without known size, we either have to map over the slices of T to Option<T>::Some, or transmute &[T] to &[Option<T>]

@kennykerr kennykerr reopened this Feb 28, 2022
@kennykerr
Copy link
Collaborator

Reopening to address this.

@kennykerr
Copy link
Collaborator

kennykerr commented Mar 8, 2022

This function is generated as follows:

pub unsafe fn VSSetConstantBuffers(&self, startslot: u32, ppconstantbuffers: &[::core::option::Option<ID3D11Buffer>]) {
    (::windows::core::Interface::vtable(self).VSSetConstantBuffers)(::core::mem::transmute_copy(self), ::core::mem::transmute(startslot), ppconstantbuffers.len() as _, ::core::mem::transmute(::windows::core::as_ptr_or_null(ppconstantbuffers)))
}

This is in line with the metadata:

unsafe void VSSetConstantBuffers([In] uint StartSlot, [In] uint NumBuffers, [Optional][In][NativeArrayInfo(CountParamIndex = 1)] ID3D11Buffer* ppConstantBuffers);

Unfortunately, there's nothing in the metadata that says that the interface pointers in the array may not be null, hence the Option. Such a (non-null) option could be explored here: https://github.com/microsoft/win32metadata

@chyyran
Copy link

chyyran commented Mar 8, 2022

What if an IntoParam<Option<T>> implementation is generated for every T that is exposed by a function as [In][Optional]? The issue here is that it's difficult to pass &[T] into functions that accept &[Option<T>], and likewise for the non-slice variants without transmute. This is a sound-ish transmute in the context of COM and COM-lite APIs but only because Rust guarantees so (kind of) for #[repr(transparent)], and because the input to the transmute is non-null, contract wise it is still wildly unsafe (if the wrapped COM ptr is null, the transmute is UB). Expressing it as IntoParam would codifying this relation in the function contract, but I'm not familiar enough to know if this is feasible codegen wise.

This wouldn't work for [Out] params for obvious reasons but for [In] params it would eliminate the need to use transmute here.

@kennykerr
Copy link
Collaborator

Ah that's a good point, thanks for the reminder! Yes, I'll experiment with using IntoParam for input arrays.

@aconverse
Copy link

I know this issue is very old. I can open a new issue if that is preferred.

But this was closed with a change (#2233) that had to be reverted and never re-opened.

Going back to the original use case: User has &ID3D11Buffer and wants to call VSSetConstantBuffers().

Can we get an AsRef to take us from &ID3D11Buffer to a &Option<ID3D11Buffer>? (Any more generically
this transformation for any COM object).

Right now the options seem to be:

  • Call transmute from application code
  • Clone the ID3D11Buffer churning ref counts

See also: #2930 (comment)

@kennykerr kennykerr reopened this Jan 10, 2025
@zopsicle
Copy link

If VSSetConstantBuffers took Option<Ref<ID3D11Buffer>> instead, neither clone nor transmute would be necessary. That would be ideal IMO.

@kennykerr
Copy link
Collaborator

Yes, I added Ref to deal with such cases. Just need to apply it more broadly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
6 participants