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

tracking for EXTERNAL texture use #3019

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions wgpu-core/src/device/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use crate::{
};

use arrayvec::ArrayVec;
use hal::{CommandEncoder as _, Device as _};
use hal::{CommandEncoder as _, Device as _, Texture};
use parking_lot::{Mutex, MutexGuard};
use smallvec::SmallVec;
use thiserror::Error;
Expand Down Expand Up @@ -4043,6 +4043,8 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
Err(error) => break error,
};

let is_external = hal_texture.is_external();

let mut texture = device.create_texture_from_hal(
hal_texture,
conv::map_texture_usage(desc.usage, desc.format.into()),
Expand All @@ -4062,11 +4064,17 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
let id = fid.assign(texture, &mut token);
log::info!("Created texture {:?} with {:?}", id, desc);

device.trackers.lock().textures.insert_single(
id.0,
ref_count,
hal::TextureUses::UNINITIALIZED,
);
let mut uses = hal::TextureUses::UNINITIALIZED;

if is_external {
uses |= hal::TextureUses::EXTERNAL;
}

device
.trackers
.lock()
.textures
.insert_single(id.0, ref_count, uses);

return (id.0, None);
};
Expand Down
63 changes: 60 additions & 3 deletions wgpu-core/src/device/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@ use crate::{
id,
init_tracker::{has_copy_partial_init_tracker_coverage, TextureInitRange},
resource::{BufferAccessError, BufferMapState, StagingBuffer, TextureInner},
track, FastHashSet, SubmissionIndex,
track::{self, TextureUsageScope},
FastHashSet, SubmissionIndex,
};

use hal::{CommandEncoder as _, Device as _, Queue as _};
use hal::{CommandEncoder as _, Device as _, Queue as _, Texture as _};
use parking_lot::Mutex;
use smallvec::SmallVec;
use std::{iter, mem, ptr};
use std::{collections::HashSet, iter, mem, ptr};
use thiserror::Error;

/// Number of command buffers that we generate from the same pool
Expand Down Expand Up @@ -1218,6 +1219,62 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
baked
.initialize_texture_memory(&mut *trackers, &mut *texture_guard, device)
.map_err(|err| QueueSubmitError::DestroyedTexture(err.0))?;

// Insert synthetic barriers to insert EXTERNAL barriers for any used external textures.
{
let mut used_external_textures = TextureUsageScope::new();
i509VCB marked this conversation as resolved.
Show resolved Hide resolved
let mut visited_ids = HashSet::new();

let external_textures = baked
.trackers
.textures
.pending()
// Iterate in reverse to find the last transition state.
.rev()
// We only care about external textures
.filter(|transition| {
// SAFETY: The texture must be known by the tracker if it was used during
// command submission or is pending.
let texture =
unsafe { texture_guard.get_unchecked(transition.id) };

texture
.inner
.as_raw()
.map(<A::Texture>::is_external)
.unwrap_or(false)
})
.filter(|transition| {
// Insert returns false if the element was already added.
visited_ids.insert(&transition.id)
});

external_textures.for_each(|transition| {
// Create and record a synthetic transition state to EXTERNAL based on the last usage.
unsafe {
let id = texture_guard
.get_valid_unchecked(transition.id, A::VARIANT);
let ref_count = baked.trackers.textures.get_ref_count(id);
used_external_textures
.merge_single(
&*texture_guard,
id,
Some(transition.selector.clone()),
ref_count,
transition.usage.end | hal::TextureUses::EXTERNAL,
)
.unwrap();
}
});

if !used_external_textures.is_empty() {
baked
.trackers
.textures
.set_from_usage_scope(&*texture_guard, &used_external_textures);
}
}

//Note: stateless trackers are not merged:
// device already knows these resources exist.
CommandBuffer::insert_barriers_from_tracker(
Expand Down
10 changes: 10 additions & 0 deletions wgpu-core/src/hub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,16 @@ impl<T, I: id::TypedId> Storage<T, I> {
}
}

pub(crate) unsafe fn get_valid_unchecked(&self, id: u32, backend: Backend) -> id::Valid<I> {
let epoch = match self.map[id as usize] {
Element::Occupied(_, epoch) => epoch,
Element::Vacant => panic!("{}[{}] does not exist", self.kind, id),
Element::Error(_, _) => panic!(""),
};

id::Valid(I::zip(id, epoch, backend))
}

pub(crate) fn label_for_invalid_id(&self, id: I) -> &str {
let (index, _, _) = id.unzip();
match self.map.get(index as usize) {
Expand Down
2 changes: 1 addition & 1 deletion wgpu-core/src/track/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ use wgt::strict_assert_ne;
/// A structure containing all the information about a particular resource
/// transition. User code should be able to generate a pipeline barrier
/// based on the contents.
#[derive(Debug, PartialEq)]
#[derive(Debug, Clone, PartialEq)]
pub(crate) struct PendingTransition<S: ResourceUses> {
pub id: u32,
pub selector: S::Selector,
Expand Down
5 changes: 5 additions & 0 deletions wgpu-core/src/track/texture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,11 @@ impl<A: hub::HalApi> TextureTracker<A> {
self.metadata.owned_ids()
}

/// Returns all currently pending transitions.
pub fn pending(&self) -> impl DoubleEndedIterator<Item = &PendingTransition<TextureUses>> + '_ {
self.temp.iter()
}

/// Drains all currently pending transitions.
pub fn drain(&mut self) -> Drain<PendingTransition<TextureUses>> {
self.temp.drain(..)
Expand Down
6 changes: 6 additions & 0 deletions wgpu-hal/src/dx11/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,12 @@ impl crate::Queue<super::Api> for super::Queue {
}
}

impl crate::Texture<super::Api> for super::Texture {
fn is_external(&self) -> bool {
false
}
}

impl super::D3D11Device {
#[allow(trivial_casts)] // come on
pub unsafe fn check_feature_support<T>(&self, feature: d3d11::D3D11_FEATURE) -> T {
Expand Down
6 changes: 6 additions & 0 deletions wgpu-hal/src/dx12/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -900,3 +900,9 @@ impl crate::Queue<Api> for Queue {
(1_000_000_000.0 / frequency as f64) as f32
}
}

impl crate::Texture<Api> for Texture {
fn is_external(&self) -> bool {
false
}
}
6 changes: 6 additions & 0 deletions wgpu-hal/src/empty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -411,3 +411,9 @@ impl crate::CommandEncoder<Api> for Encoder {
unsafe fn dispatch(&mut self, count: [u32; 3]) {}
unsafe fn dispatch_indirect(&mut self, buffer: &Resource, offset: wgt::BufferAddress) {}
}

impl crate::Texture<Api> for Resource {
fn is_external(&self) -> bool {
false
}
}
6 changes: 6 additions & 0 deletions wgpu-hal/src/gles/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1321,6 +1321,12 @@ impl crate::Device<super::Api> for super::Device {
}
}

impl crate::Texture<super::Api> for super::Texture {
fn is_external(&self) -> bool {
false
}
}

// SAFE: WASM doesn't have threads
#[cfg(target_arch = "wasm32")]
unsafe impl Sync for super::Device {}
Expand Down
18 changes: 16 additions & 2 deletions wgpu-hal/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ pub trait Api: Clone + Sized {
type CommandBuffer: Send + Sync + fmt::Debug;

type Buffer: fmt::Debug + Send + Sync + 'static;
type Texture: fmt::Debug + Send + Sync + 'static;
type Texture: Texture<Self> + 'static;
type SurfaceTexture: fmt::Debug + Send + Sync + Borrow<Self::Texture>;
type TextureView: fmt::Debug + Send + Sync;
type Sampler: fmt::Debug + Send + Sync;
Expand Down Expand Up @@ -550,6 +550,13 @@ pub trait CommandEncoder<A: Api>: Send + Sync + fmt::Debug {
unsafe fn dispatch_indirect(&mut self, buffer: &A::Buffer, offset: wgt::BufferAddress);
}

pub trait Texture<A: Api>: fmt::Debug + Send + Sync {
/// Whether this texture originates from external memory.
///
/// This indicates whether the texture may have the `EXTERNAL` usage.
fn is_external(&self) -> bool;
}

bitflags!(
/// Instance initialization flags.
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
Expand Down Expand Up @@ -765,9 +772,16 @@ bitflags::bitflags! {

/// Flag used by the wgpu-core texture tracker to say a texture is in different states for every sub-resource
const COMPLEX = 1 << 10;

/// Flag used by the wgpu-core texture tracker to say a texture was imported from external memory.
///
/// In the Vulkan backend, this indicates the texture needs to be transferred from an external queue
/// family to the graphics queue family.
const EXTERNAL = 1 << 11;

/// Flag used by the wgpu-core texture tracker to say that the tracker does not know the state of the sub-resource.
/// This is different from UNINITIALIZED as that says the tracker does know, but the texture has not been initialized.
const UNKNOWN = 1 << 11;
const UNKNOWN = 1 << 12;
}
}

Expand Down
6 changes: 6 additions & 0 deletions wgpu-hal/src/metal/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,12 @@ impl crate::Queue<Api> for Queue {
}
}

impl crate::Texture<Api> for Texture {
fn is_external(&self) -> bool {
false
}
}

#[derive(Debug)]
pub struct Buffer {
raw: metal::Buffer,
Expand Down
34 changes: 24 additions & 10 deletions wgpu-hal/src/vulkan/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,16 +165,30 @@ impl crate::CommandEncoder<super::Api> for super::CommandEncoder {
let dst_layout = conv::derive_image_layout(bar.usage.end, bar.texture.format);
dst_stages |= dst_stage;

vk_barriers.push(
vk::ImageMemoryBarrier::builder()
.image(bar.texture.raw)
.subresource_range(range)
.src_access_mask(src_access)
.dst_access_mask(dst_access)
.old_layout(src_layout)
.new_layout(dst_layout)
.build(),
);
let mut barrier = vk::ImageMemoryBarrier::builder()
.image(bar.texture.raw)
.subresource_range(range)
.src_access_mask(src_access)
.dst_access_mask(dst_access)
.old_layout(src_layout)
.new_layout(dst_layout);

// If the texture is external, we need to specify a queue family ownership transfer.
if bar.usage.start.contains(crate::TextureUses::EXTERNAL) {
barrier = barrier
.src_queue_family_index(bar.texture.external_queue_family_index.unwrap())
.dst_queue_family_index(self.device.queue_index);
}

// If this is the last usage of the texture during this command submission, return the queue to
// it's sentinel queue family.
if bar.usage.end.contains(crate::TextureUses::EXTERNAL) {
barrier = barrier
.src_queue_family_index(self.device.queue_index)
.dst_queue_family_index(bar.texture.external_queue_family_index.unwrap());
}

vk_barriers.push(barrier.build());
}

if !vk_barriers.is_empty() {
Expand Down
20 changes: 20 additions & 0 deletions wgpu-hal/src/vulkan/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -646,12 +646,15 @@ impl super::Device {
/// # Safety
///
/// - `vk_image` must be created respecting `desc`
/// - If [`TextureUses::EXTERNAL`](crate::TextureUses::EXTERNAL) is set, then `external_queue_family_index` must be set.
/// - If `external_queue_family_index` is set, then [`TextureUses::EXTERNAL`](crate::TextureUses::EXTERNAL) must be set.
/// - If `drop_guard` is `Some`, the application must manually destroy the image handle. This
/// can be done inside the `Drop` impl of `drop_guard`.
/// - If the `ImageCreateFlags` does not contain `MUTABLE_FORMAT`, the `view_formats` of `desc` must be empty.
pub unsafe fn texture_from_raw(
vk_image: vk::Image,
desc: &crate::TextureDescriptor,
external_queue_family_index: Option<u32>,
drop_guard: Option<crate::DropGuard>,
) -> super::Texture {
let mut raw_flags = vk::ImageCreateFlags::empty();
Expand All @@ -668,6 +671,20 @@ impl super::Device {
view_formats.push(desc.format)
}

if desc.usage.contains(crate::TextureUses::EXTERNAL) {
wgt::strict_assert!(
external_queue_family_index.is_none(),
"Texture has TextureUse::EXTERNAL, but does not specify the owning queue family"
);
}

if external_queue_family_index.is_none() {
wgt::strict_assert!(
desc.usage.contains(crate::TextureUses::EXTERNAL),
"Texture specifies external queue family ownership but does not have TextureUse::EXTERNAL"
);
}

Choose a reason for hiding this comment

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

Aren't these two assertions wrong ? They don't match the error message or the comments above


super::Texture {
raw: vk_image,
drop_guard,
Expand All @@ -677,6 +694,7 @@ impl super::Device {
raw_flags: vk::ImageCreateFlags::empty(),
copy_size: desc.copy_extent(),
view_formats,
external_queue_family_index,
}
}

Expand Down Expand Up @@ -1018,6 +1036,8 @@ impl crate::Device<super::Api> for super::Device {
raw_flags,
copy_size,
view_formats: wgt_view_formats,
// wgpu's own textures use the exclusive sharing mode.
external_queue_family_index: None,
})
}
unsafe fn destroy_texture(&self, texture: super::Texture) {
Expand Down
1 change: 1 addition & 0 deletions wgpu-hal/src/vulkan/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -820,6 +820,7 @@ impl crate::Surface<super::Api> for super::Surface {
depth: 1,
},
view_formats: sc.view_formats.clone(),
external_queue_family_index: None,
},
};
Ok(Some(crate::AcquiredSurfaceTexture {
Expand Down
20 changes: 20 additions & 0 deletions wgpu-hal/src/vulkan/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,19 @@ pub struct Texture {
raw_flags: vk::ImageCreateFlags,
copy_size: crate::CopyExtent,
view_formats: Vec<wgt::TextureFormat>,
/// The index of the external queue family which owns the image contents.
///
/// When using images imported from external memory in Vulkan, the images belong to a sentinel "external"
/// queue family. In order to use these textures, the texture must be transferred to the graphics queue
/// family using a memory barrier before the texture used, and then returned to the sentinel queue at the
/// end of command execution.
///
/// If this is [`Some`], the value is typically [`QUEUE_FAMILY_EXTERNAL`](ash::vk::QUEUE_FAMILY_EXTERNAL)
/// or [`QUEUE_FAMILY_FOREIGN_EXT`](ash::vk::QUEUE_FAMILY_FOREIGN_EXT) depending on imported memory object
/// and or the type of memory object.
///
/// The value will be [`None`] if the texture was not imported using external memory.
external_queue_family_index: Option<u32>,
}

impl Texture {
Expand Down Expand Up @@ -610,6 +623,13 @@ impl crate::Queue<Api> for Queue {
}
}

impl crate::Texture<Api> for Texture {
fn is_external(&self) -> bool {
self.usage.contains(crate::TextureUses::EXTERNAL)
&& self.external_queue_family_index.is_some()
}
}

impl From<vk::Result> for crate::DeviceError {
fn from(result: vk::Result) -> Self {
match result {
Expand Down