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

Add a way to notify with Queue::submit() to Vulkan's vk::Semaphore allocated outside of wgpu #6813

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

sotaroikeda
Copy link
Contributor

Connections
Fixes #6812

Description
The following changes are done.

  • Add Global::queue_as_hal() for accessing hal queue.
  • Add Queue::add_signal_semahore() for adding signal semaphore to Queue, the semaphore is submitted by next Queue::submit().
  • Add Queue::raw_device(&self). It could be used to allocate vk::Semaphore.

@sotaroikeda sotaroikeda requested a review from a team as a code owner December 23, 2024 06:52
&self.device.raw
}

pub fn add_signal_semahore(&self, semaphore: vk::Semaphore) {
Copy link
Member

Choose a reason for hiding this comment

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

typo: s/semahore/semaphore

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you!

/// # Safety
///
/// - The raw queue handle must not be manually destroyed
pub unsafe fn queue_as_hal<A: HalApi, F: FnOnce(Option<&A::Queue>) -> R, R>(
Copy link
Member

Choose a reason for hiding this comment

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

Nit: lets use a where here, as the expressions are complex.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you!

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit on top of nit: all other as_hal() functions have the bound directly on the type definition, it would be more consistent to keep it or replace it across the code-base atomically?

@@ -1210,6 +1211,11 @@ impl crate::Queue for Queue {
signal_values.push(!0);
}

let mut pending_signal_semaphores = self.signal_semaphores.lock();
if !pending_signal_semaphores.is_empty() {
signal_semaphores.append(&mut pending_signal_semaphores);
Copy link
Member

Choose a reason for hiding this comment

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

This also needs to push to signal_values or you'll throw a validation error as the values/semaphores won't be matched up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am going to apply the comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

What if the caller wants to signal a timeline semaphore? Perhaps additional semaphore values should be exposed as well?

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.

Add a way to notify with Queue::submit() to Vulkan's vk::Semaphore allocated outside of wgpu
5 participants