Skip to content

Fix macro to allow disabling dx12 feature.#3590

Merged
teoxoy merged 3 commits intogfx-rs:masterfrom
toastmod:master
Mar 15, 2023
Merged

Fix macro to allow disabling dx12 feature.#3590
teoxoy merged 3 commits intogfx-rs:masterfrom
toastmod:master

Conversation

@toastmod
Copy link
Copy Markdown
Contributor

Checklist

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

Connections
N/A

Description
Couldn't compile when disabling dx12 feature from core in wgpu/Cargo.toml. Fixed by adding:

#[cfg(all(target_os = "windows", feature = "dx12"))]

My only concerns are if the functions I affected are also supposed to be used with dx11? Also might need some guidance on fixing the docs deployment test (if that was my fault).

Testing
Compiled without dx12 feature in wgpu/Cargo.toml

@toastmod
Copy link
Copy Markdown
Contributor Author

I was able to compile and use wgpu without dx12 using these changes but I realize I should probably add this conditional macro over the wgpu::Backends::DX12 enum.

Copy link
Copy Markdown
Member

@teoxoy teoxoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR!

Just a nit: we seem to be using the style of guard that I suggested everywhere else.

@teoxoy teoxoy merged commit 110e62a into gfx-rs:master Mar 15, 2023
teoxoy added a commit that referenced this pull request Apr 4, 2023
teoxoy added a commit that referenced this pull request Apr 4, 2023
toastmod added a commit to toastmod/wgpu that referenced this pull request Apr 20, 2023
* Fix macro for DX12 conditional compilation

* Update CHANGELOG.md

* Apply suggestions from code review

---------

Co-authored-by: Teodor Tanasoaia <28601907+teoxoy@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 this pull request may close these issues.

2 participants