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

dx12: Adopt BAL approch #2266

Closed
wants to merge 13 commits into from
Closed

Conversation

msiglreith
Copy link
Contributor

PoC for switching towards the BAL concept suggested in #2249

  • New /src structure:
- /bal
--- /dx12 (gfx-bal-dx12)
- /hal
--- /dx12 (gfx-hal-dx12)
--- /src (gfx-hal)
  • backend/dx12 moved to hal/dx12
  • hal/dx12 has a new dependency on bal/dx12

Source code from HAL will be gradually refactored into the corresponding BAL backend in order to slim down the HAL implementation and move 'unsafe' portions into BAL (ie zero cost wrappers over d3d12 structs)

Upcoming PR should be a rustfmt as the backend code doesnt seem to be formatted at all (?)

@kvark
Copy link
Member

kvark commented Jul 24, 2018

Hmm, I'm not sure it's worth it for us to split the backends into HAL/BAL sections. It would introduce longer dependency chains for us and clients. Couldn't the backend crates just provide both? Assuming HAL implementation is mostly just wrapping around BAL primitives.

Copy link

@zakarumych zakarumych left a comment

Choose a reason for hiding this comment

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

Safety concerns.

}

#[derive(Clone)]
pub struct Copy {

Choose a reason for hiding this comment

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

Naming conflict with std::prelude::Copy is undesirable.

}

impl<T: Interface> WeakPtr<T> {
pub fn as_unknown(&self) -> &IUnknown {

Choose a reason for hiding this comment

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

This function must be marked unsafe.
Otherwise this "safe" expression WeakPtr::from_raw(1usize as *mut u32).as_unknown() will trigger UB.

}

// Cast creates a new WeakPtr requiring explicit destroy call.
pub fn cast<U>(&self) -> D3DResult<WeakPtr<U>>

Choose a reason for hiding this comment

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

unsafe function


// Destroying one instance of the WeakPtr will invalidate all
// copies and clones.
pub fn destroy(&self) {

Choose a reason for hiding this comment

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

unsafe function

impl<T> Deref for WeakPtr<T> {
type Target = T;
fn deref(&self) -> &T {
debug_assert!(!self.is_null());

Choose a reason for hiding this comment

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

This type is pub so debug_assert doesn't seem enough.

@msiglreith
Copy link
Contributor Author

The native part is moved out into the d3d12-rs crate.
The transition of the current HAL backend is partially done in #2374

Moving out common pieces like blitting will done in a separate PR.

@msiglreith msiglreith closed this Sep 1, 2018
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.

3 participants