Bring `d3d12` crate in-tree #3987

ErichDonGubler · 2023-07-28T14:32:40Z

I've noted in Matrix chat that d3d12, as a crate, sits in an awkward place. It does not realize the potential it was apparently envisioned to have of being a robust abstraction that others can use, like with metal or ash. It remains totally coupled to WGPU's needs. And yet, d3d12 also reaps no benefits of its coupling. Many D3D12 APIs used WGPU's D3D backends are bound only in wgpu-hal, which defeats the purpose of having a dedicated layer for D3D12 interop. For those APIs that are captured by the d3d12 crate, effective development requires two compilation feedback loops across repositories. ~~The crate barrier also inhibits Clippy and rustc lints that would surely be useful for code that could otherwise be a module tree in wgpu-hal.~~

@cwfitzgerald noted that he'd be open to inlining d3d12 code into wgpu-hal. I also took time to get some Mozilla consensus; @jimblandy laughed in apparent catharsis when I declared that d3d12 is, proverbially, a child who either needs to get a job and pay its rent, or move back in with its parents (i.e., wgpu-hal). So, here's the PR making moving the entirety of the d3d12 crate from there to here!

This PR copies the subtree-merges d3d12 repo into a new d3d12 top-level folder. ~~It also fixes up some integration issues that appear unique to the current HEAD of the d3d12 repo.~~

Note that while this is inlining d3d12 for now, there's no reason that we can't or shouldn't eventually try to grow the in-tree crate into a well-formed separate repository again. No plans for that are established here, but it's a definite future possibility.

I've taken pains to ensure that actual changes to code are easy to distinguish as small commits, while keeping mechanical movement of code to larger ones. Therefore, recommended review experience is by commit.

Post-merge checklist:

Archive the d3d12 crate?

Checklist

Run cargo clippy.
Run cargo clippy --target wasm32-unknown-unknown if applicable.
Add change to CHANGELOG.md. See simple instructions inside file.

Connections
Link to the issues addressed by this PR, or dependent PRs in other repositories

Linked in prose above.

Description
Describe what problem this is solving, and how it's solved.

See my prose above.

Testing
Explain how this change is tested.

No behavior should change because of this PR. I consider successful compilation to be a sufficient test for it.

1: Initialize crate r=kvark a=msiglreith The crate is supposed to be a Rust wrapper around D3D12. Motivated by trying to hide the FFI behind a nicer to use layer (part of BAL). It aims at providing a zero-cost abstraction without any overhead at all (no transition costs between Rust structs and D3D12 structs or allocations). The API closely ressambles the d3d12 interfaces and is strucutered accordingly. It currently uses a custom `ComPtr` implementation which does not do any refcounting, therefore requires manual destruction when dropping. Co-authored-by: msiglreith <m.siglreith@gmail.com>

2: Fix command signature uuid r=msiglreith a=msiglreith Co-authored-by: msiglreith <m.siglreith@gmail.com>

3: Add struct for descriptor binding r=grovesNL a=msiglreith A bit more safety for API users to pass the correct values. Co-authored-by: msiglreith <m.siglreith@gmail.com>

4: Factory, swapchain and more r=kvark a=msiglreith Additionally renaming the crate to `d3d12` Co-authored-by: msiglreith <m.siglreith@gmail.com>

5: Add rects length check for clears r=msiglreith a=dati91 Co-authored-by: Attila Dusnoki <attila.dusnoki@h-lab.eu>

6: Expose event internal and Event in root level r=msiglreith a=msiglreith Co-authored-by: msiglreith <m.siglreith@gmail.com>

7: Add missing cargo metadata for the release r=msiglreith a=kvark Co-authored-by: Dzmitry Malyshau <dmalyshau@mozilla.com>

`D3D12_HEAP_TYPE, D3D12_CPU_PAGE_PROPERTY, D3D12_MEMORY_POOL, D3D12_HEAP_FLAGS, D3D12_HEAP_PROPERTIES, and D3D12_HEAP_DESC ID3D12Heap Wrap ID3D12Device::CreateHeap Made a tricky enum because DX12 doesn't follow Rust's rules bump version to 0.2.0 Use bitflags for heap::Flags Fix typo Another typo fix

9: Add structs and enums for ID3D12Heap r=msiglreith a=DethRaid I've added the structs and enums needed to create an `ID3D12Heap`. I've also added the method `create_heap` to `Device`, and and added the module imports and reexports to the appropriate places The weird thing here is `heap::Flags`. Thing is, D3D12 uses the value 0 for two of the enum constants in `D3D12_HEAP_FLAGS`. Rust does not allow the same value to be used by multiple enum constants (that I know of). I made an enum without any useful values, then gave it constants with the values from `D3D12_HEAP_FLAGS`. This lets my code compile, and the syntax to use the enum is the same, but the compiler complains about capitalization and I don't really like it. I'd welcome any ideas about a better way to implement this Co-authored-by: David Dubois <dexcelstraun7@gmail.com>

11: Add root descriptor r=kvark a=msiglreith Co-authored-by: msiglreith <m.siglreith@gmail.com>

12: Add root descriptor commands and add trait derives r=kvark a=msiglreith Co-authored-by: msiglreith <m.siglreith@gmail.com>

13: Bump version to 0.2.2 r=kvark a=msiglreith As far as I can see there were no breaking changes to the API only additions Co-authored-by: msiglreith <m.siglreith@gmail.com>

16: Remove one layer of modules and Queue::execute_command_lists r=kvark a=msiglreith Co-authored-by: msiglreith <m.siglreith@gmail.com>

d3d12/src/com.rs

cwfitzgerald · 2023-09-26T19:42:36Z

I think we should split this change - one MR to bring in exactly what we depended on previously. Then follow up MRs to improve/iterate on the apis.

ErichDonGubler · 2023-09-26T20:01:01Z

I will plan on changing this PR to “just” bring 0.7 (or a fixed d3d12 upstream) in-tree.

cwfitzgerald · 2023-09-26T20:06:52Z

I think we should leave d3d12 completely untouched from the commit we were depending on. That way we can land the organizational change without testing on Firefox, and reducing the pressure to land all of the subsequent changes

Addressed feedback by "just" using the Crates.io commit for 0.7.0 (which, notably, is not the same as the v0.7.0 tag in gfx-rs/d3d12).

git-subtree-dir: d3d12 git-subtree-mainline: ca7ac86 git-subtree-split: 661dcee

Cargo.toml

d3d12/bors.toml

d3d12/rustfmt.toml

wgpu-hal/Cargo.toml

Cargo.toml

msiglreith and others added 30 commits August 26, 2018 00:47

Initial commit

b1d6410

Setup bors and CI

a0c95c9

Setup crate structure and wrap first interface

4da4b1e

Fix appveyor build

0d34879

Fix command signature uuid

5e4b1b8

Merge #2

124f9fc

2: Fix command signature uuid r=msiglreith a=msiglreith Co-authored-by: msiglreith <m.siglreith@gmail.com>

Add struct for descriptor binding

56770dc

Merge #3

4097a10

3: Add struct for descriptor binding r=grovesNL a=msiglreith A bit more safety for API users to pass the correct values. Co-authored-by: msiglreith <m.siglreith@gmail.com>

Add debug interface, factory creation and adapter enumeration

2aacc3b

Add swapchain handling and 2d RTV creation

908c29e

Extend event and fence support

e6ba80c

Address review comments

bf7f2e6

Merge #4

6369df2

4: Factory, swapchain and more r=kvark a=msiglreith Additionally renaming the crate to `d3d12` Co-authored-by: msiglreith <m.siglreith@gmail.com>

Add rects length check for clears

4a938bd

Merge #5

7d8a7d3

5: Add rects length check for clears r=msiglreith a=dati91 Co-authored-by: Attila Dusnoki <attila.dusnoki@h-lab.eu>

Expose event internal and Event in root level

09d68b3

Merge #6

ee01154

6: Expose event internal and Event in root level r=msiglreith a=msiglreith Co-authored-by: msiglreith <m.siglreith@gmail.com>

Add missing cargo metadata for the release

3e15f7c

Merge #7

5324fa6

7: Add missing cargo metadata for the release r=msiglreith a=kvark Co-authored-by: Dzmitry Malyshau <dmalyshau@mozilla.com>

Add root descriptor

0a4a86e

Merge #11

f40c804

11: Add root descriptor r=kvark a=msiglreith Co-authored-by: msiglreith <m.siglreith@gmail.com>

Add root descriptor commands and add trait derives

f4043cd

Merge #12

ee7ec68

12: Add root descriptor commands and add trait derives r=kvark a=msiglreith Co-authored-by: msiglreith <m.siglreith@gmail.com>

Bump version to 0.2.2

241a638

Merge #13

e84b691

13: Bump version to 0.2.2 r=kvark a=msiglreith As far as I can see there were no breaking changes to the API only additions Co-authored-by: msiglreith <m.siglreith@gmail.com>

Remove one layer of modules and Queue::execute_command_lists

8bac8b5

Merge #16

a4c5943

16: Remove one layer of modules and Queue::execute_command_lists r=kvark a=msiglreith Co-authored-by: msiglreith <m.siglreith@gmail.com>

kdashg suggested changes Sep 26, 2023

View reviewed changes

d3d12/src/com.rs Outdated Show resolved Hide resolved

This comment was marked as duplicate.

Sign in to view

ErichDonGubler marked this pull request as draft September 26, 2023 20:00

This comment was marked as outdated.

Sign in to view

This comment was marked as resolved.

Sign in to view

This comment was marked as outdated.

Sign in to view

This comment was marked as resolved.

Sign in to view

cwfitzgerald removed the PR: waiting for author label Oct 4, 2023

ErichDonGubler force-pushed the inline-d3d12-crate branch from 9cf23d7 to dabe457 Compare October 25, 2023 15:44

ErichDonGubler force-pushed the inline-d3d12-crate branch from dabe457 to d16c7fc Compare October 25, 2023 15:49

ErichDonGubler requested review from kdashg and cwfitzgerald and removed request for kdashg October 25, 2023 15:51

ErichDonGubler marked this pull request as ready for review October 25, 2023 15:53

ErichDonGubler changed the title ~~Bring d3d12 crate in-tree~~ Bring d3d12 crate in-tree (squashed) Oct 25, 2023

Add 'd3d12/' from commit '661dcee3f9b72c787b7aac33e4e492d9f887dc7b'

5663caa

git-subtree-dir: d3d12 git-subtree-mainline: ca7ac86 git-subtree-split: 661dcee

ErichDonGubler force-pushed the inline-d3d12-crate branch from d16c7fc to b3a4ed6 Compare October 26, 2023 14:39

ErichDonGubler changed the title ~~Bring d3d12 crate in-tree (squashed)~~ Bring d3d12 crate in-tree Oct 26, 2023

cwfitzgerald requested changes Oct 26, 2023

View reviewed changes

Cargo.toml Show resolved Hide resolved

d3d12/bors.toml Show resolved Hide resolved

d3d12/rustfmt.toml Outdated Show resolved Hide resolved

wgpu-hal/Cargo.toml Show resolved Hide resolved

ErichDonGubler commented Oct 26, 2023

View reviewed changes

Cargo.toml Show resolved Hide resolved

move(hal,dx11,dx12): use local path dep. for d3d12

9e2d73d

ErichDonGubler force-pushed the inline-d3d12-crate branch from 8878689 to 9e2d73d Compare October 26, 2023 17:30

ErichDonGubler requested a review from cwfitzgerald October 26, 2023 17:31

cwfitzgerald approved these changes Oct 26, 2023

View reviewed changes

cwfitzgerald merged commit af4a97f into gfx-rs:trunk Oct 26, 2023
25 of 27 checks passed

ErichDonGubler deleted the inline-d3d12-crate branch October 30, 2023 20:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bring `d3d12` crate in-tree #3987

Bring `d3d12` crate in-tree #3987

ErichDonGubler commented Jul 28, 2023 •

edited

Loading

cwfitzgerald commented Sep 26, 2023

This comment was marked as duplicate.

ErichDonGubler commented Sep 26, 2023

This comment was marked as outdated.

cwfitzgerald commented Sep 26, 2023

This comment was marked as resolved.

This comment was marked as outdated.

This comment was marked as resolved.

Bring d3d12 crate in-tree #3987

Bring d3d12 crate in-tree #3987

Conversation

ErichDonGubler commented Jul 28, 2023 • edited Loading

cwfitzgerald commented Sep 26, 2023

This comment was marked as duplicate.

ErichDonGubler commented Sep 26, 2023

This comment was marked as outdated.

cwfitzgerald commented Sep 26, 2023

This comment was marked as resolved.

This comment was marked as outdated.

This comment was marked as resolved.

Bring `d3d12` crate in-tree #3987

Bring `d3d12` crate in-tree #3987

ErichDonGubler commented Jul 28, 2023 •

edited

Loading