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

Should IDXGI* types be marked as [Agile]? #1588

Closed
MarijnS95 opened this issue Jun 7, 2023 · 12 comments · Fixed by #1591
Closed

Should IDXGI* types be marked as [Agile]? #1588

MarijnS95 opened this issue Jun 7, 2023 · 12 comments · Fixed by #1591

Comments

@MarijnS95
Copy link
Contributor

#740 and #861 marked the ID3D* types [Agile] to have Send and Sync in Rust because all these types are (supposedly) synchronized internally and thus thread-safe. Is the same true for DXGI types, and if so can we add the same attribute to them too?

@riverar
Copy link
Collaborator

riverar commented Jun 7, 2023

Sounds reasonable, thoughts @damyanp?

@damyanp
Copy link
Member

damyanp commented Jun 7, 2023

I'm working on getting an answer to this, but it may take a few days.

@damyanp
Copy link
Member

damyanp commented Jun 14, 2023

Word I've got back is that the IDXGI* interfaces are as agile as the D3D ones are.

@MarijnS95
Copy link
Contributor Author

@damyanp lovely, thanks for getting back to me so quickly! (and concidentally right when I remembered I asked this question)

Will you submit a PR adding this or should I?

@riverar
Copy link
Collaborator

riverar commented Jun 14, 2023

Happy to sign off on this as soon as your PR lands. Or Mike and I can take care of it too.

@damyanp
Copy link
Member

damyanp commented Jun 14, 2023

Thanks! I'm unlikely to find time to work on this any time soon I'm afraid.

MarijnS95 added a commit to MarijnS95/win32metadata that referenced this issue Jun 14, 2023
Fixes microsoft#1588

Just like microsoft#740/microsoft#861 the `IDXGI*` interface classes should be marked as
`Agile` so that `windows-rs` implements `Sync` and `Send` marker
`trait`s for these `struct`s in Rust, allowing these types to be
fearlessly shared across threads, and accessed from multiple threads at
once: their implementations [are thread-safe].

[are thread-safe]: microsoft#1588 (comment)
@MarijnS95
Copy link
Contributor Author

Done in #1591: .\DoAll.ps1 -Clean (which takes ages, by the way!) didn't report any changes, and the [Agile] attribute is not visible (on anything) in ILSpy, fwiw.

@MarijnS95
Copy link
Contributor Author

and the [Agile] attribute is not visible (on anything) in ILSpy

After patching up windows-rs, still no Send/Sync for this:

https://github.com/MarijnS95/windows-rs/compare/regen

@riverar
Copy link
Collaborator

riverar commented Jun 14, 2023

[Agile] seems to be broken at the moment, standby. (cc: @mikebattista for awareness)

@MarijnS95
Copy link
Contributor Author

Strange, the regenerated branch has it working for ID3D types for example, as before.

@riverar
Copy link
Collaborator

riverar commented Jun 14, 2023

@marignS95 And missing from IDWrite* interfaces, which is strange. Running through bisect now.

@riverar
Copy link
Collaborator

riverar commented Jun 14, 2023

OK bisect shows it broke in 62a25c4, we'll get it patched up.

mikebattista added a commit that referenced this issue Jun 29, 2023
* Mark `IDXGI*` interfaces as `[Agile]`

Fixes #1588

Just like #740/#861 the `IDXGI*` interface classes should be marked as
`Agile` so that `windows-rs` implements `Sync` and `Send` marker
`trait`s for these `struct`s in Rust, allowing these types to be
fearlessly shared across threads, and accessed from multiple threads at
once: their implementations [are thread-safe].

[are thread-safe]: #1588 (comment)

* Updated the baseline.

---------

Co-authored-by: Mike Battista <13860912+mikebattista@users.noreply.github.com>
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 a pull request may close this issue.

3 participants