Skip to content
This repository has been archived by the owner on Oct 26, 2023. It is now read-only.

Consider using com-rs #14

Closed
kvark opened this issue Oct 9, 2019 · 11 comments
Closed

Consider using com-rs #14

kvark opened this issue Oct 9, 2019 · 11 comments
Labels
enhancement New feature or request

Comments

@kvark
Copy link
Member

kvark commented Oct 9, 2019

Look at this new thing from MS: https://github.com/microsoft/com-rs/

@kvark kvark added the enhancement New feature or request label Oct 9, 2019
@tangmi
Copy link

tangmi commented Mar 5, 2021

An alternative could be the new windows-rs, adding bindings for d3d12 could be a build.rs along the lines of:

fn main() {
  windows::build!(
    windows::win32::direct3d12::*
    windows::win32::dxgi::*
    // etc
  );
}

These bindings are generated from the Win32 API metadata at build time and should be trivial (for d3d12-rs) to keep up to date.

It picks up API functionality not yet in winapi, like ray tracing, etc.

@Ciantic
Copy link

Ciantic commented Mar 8, 2021

@tangmi your suggestion is probably the reason the most impressive wrapper: https://github.com/LNSEAB/dxplr is now unmaintained. DXPLR is way better wrapper than this d3d12.

Newer code from that same LNSEAB uses Windows-Rs.

Thing is Windows-Rs is so ergonomic it doesn't need wrappers, because it already bakes in the ComPtr etc. (which btw might be the reason not to use it as GFX assumes manual dropping).

However Windows-Rs has lot's of problems, the build time is a long one because the bindings need to be built, rust-analyzer auto complete is spotty, rust-analyzer go to definition stops working, rust-analyzers auto import does not work very well.

@tangmi
Copy link

tangmi commented Mar 9, 2021

FWIW, I've found the windows-rs build time for d3d12 + dxgi (and their dependencies) to be pretty quick. The IDE experience seems to currently be having trouble with rust-analyzer expanding the include! macro, but there's definitely benefit to the generated bindings being just a rust file in target vs generated from macros (as is the case with winapi).

I think the main difference between d3d12-rs and windows-rs bindings is things like the former taking slices where the API wants arrays and returning rust's Result type while the latter takes pointers and returns objects via hresults/out params (although windows-rs provides some nice helpers to make this easier).

If gfx is the only consumer of d3d12-rs, it might be worth it to go directly to windows-rs, simply for the much lower maintenance cost (at the expense of slightly less idiomatically "rust" bindings).

@Ciantic
Copy link

Ciantic commented Mar 11, 2021

@tangmi yes, that's basically what I advocated in #32, because this d3d12-rs library is not particularily useful, it would be good idea to dump this, and use winapi or windows-rs directly.

However I wrote same example in winapi and windows-rs:

https://github.com/Ciantic/dx12-scratch/blob/master/windows-rs/src/main.rs
https://github.com/Ciantic/dx12-scratch/blob/master/winapi-rs/src/main.rs

I have a lot of questions about windows-rs:

  • It bakes in the ComPtr to the interfaces, and it doesn't have the WeakPtr possibility
  • It requires to clone when calling the functions
  • They make pretty big breaking changes to it all the time
  • If multiple different packages use windows-rs, each package takes ~1 minute hit in clean compilation for building the bindings because of the macros they use
  • Rust-analyzer is pretty annoying with it (Go to definition, auto import, occasional freezes as it recompiles bindings for no reason, etc)
  • NOT_YET_SUPPORTED types

Currently as is, the windows-rs is not yet ready to replace winapi crate.

@msiglreith
Copy link
Contributor

@tangmi
Copy link

tangmi commented Mar 16, 2021

It bakes in the ComPtr to the interfaces, and it doesn't have the WeakPtr possibility

I'm not sure I understand why gfx needs to not have "smart" com pointers... I think ephemeral objects created as part of the implementation of the dx12 gfx backend should either be explicitly owned by gfx objects or cleaned up on drop (I don't remember if dx12's implementation is allowed to call AddRef to extend the lifetime of e.g. parameters into functions). Perhaps wrapping the windows-rs IUnknowns with ManuallyDrop might be good enough?

Luckily, the remaining questions (and the outstanding github issues) about windows-rs seem likely to improve over time.

@kvark
Copy link
Member Author

kvark commented Mar 16, 2021

@tangmi it's just to lower the overhead of copying the pointers around within gfx-rs itself. I suppose you are right that we can wrap a smart pointer into a dump pointer on gfx-rs side, it doesn't have to be that pervasive. But that at least requires that all the spots of the low-level d3d12 access would work with &SmartPointer instead of SmartPointer, since otherwise cloning is baked into the API.

@tangmi
Copy link

tangmi commented Mar 17, 2021

It looks like IntoParam might be windows-rs version of AsRef, et all.

windows-rs is probably not yet ready for gfx's purposes, but it seems like it likely will be in the (near?) future, which could make it a candidate to possibly replace winapi.

@Ciantic
Copy link

Ciantic commented Mar 18, 2021

They made a lot of changes, and I managed to compile triangle with windows-rs version that is in the Microsoft's git repository as of today.

Further I'm in this swamp, I have come to conclusion that some helper libraries might still be in order even if we got rid of the overall approach in this D3D12-rs crate and use windows-rs "directly".

For instance many of the Microsoft's examples etc. refer to source only packages such as these functions beginning CD3DX12 defined in d3dx12.h. And none of that code is in windows-rs nor winapi-rs, so major re-invention ensues.

I don't know how useful that d3dx12.h would be for gfx, but I've started to translate parts of it to rust with windows-rs here: https://github.com/Ciantic/dx12-learning/blob/master/common/src/lib.rs

@hwoodiwiss
Copy link

hwoodiwiss commented Nov 7, 2021

@Ciantic I think a level of indirection is still warranted, especially with how some of the constants surface from windows-rs, for example the DXGI_USAGE_ ones, which could definitely benefit from being made into enums, something that winapi does for us at the moment.

Being built from windows-rs would be good, as it allows us to expose functionality as it arrives (raytracing features etc.)

@hwoodiwiss
Copy link

Gave it a little go, it's been okay so far, not currently 100% with a modified WGPU, and the error handling is... garbage atm. But I think windows-rs gives a cleaner interface to COM objects, anything derived from or encapsulating an IUnknown releases on drop which is cool, I don't think WeakPtr would be necessary anymore.

https://github.com/hwoodiwiss/d3d12-rs/tree/switch-to-windows-rs

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

No branches or pull requests

6 participants