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

Put raw texture access behind snatch guards #4954

Merged
merged 2 commits into from
Jan 5, 2024
Merged
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
10 changes: 4 additions & 6 deletions wgpu-core/src/command/clear.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,11 +252,9 @@ pub(crate) fn clear_texture<A: HalApi>(
alignments: &hal::Alignments,
zero_buffer: &A::Buffer,
) -> Result<(), ClearError> {
let dst_inner = dst_texture.inner();
let dst_raw = dst_inner
.as_ref()
.unwrap()
.as_raw()
let snatch_guard = dst_texture.device.snatchable_lock.read();
let dst_raw = dst_texture
.raw(&snatch_guard)
.ok_or_else(|| ClearError::InvalidTexture(dst_texture.as_info().id()))?;

// Issue the right barrier.
Expand Down Expand Up @@ -296,7 +294,7 @@ pub(crate) fn clear_texture<A: HalApi>(
let dst_barrier = texture_tracker
.set_single(dst_texture, selector, clear_usage)
.unwrap()
.map(|pending| pending.into_hal(dst_inner.as_ref().unwrap()));
.map(|pending| pending.into_hal(dst_raw));
unsafe {
encoder.transition_textures(dst_barrier.into_iter());
}
Expand Down
4 changes: 2 additions & 2 deletions wgpu-core/src/command/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,11 +226,11 @@ impl<A: HalApi> CommandBuffer<A> {
profiling::scope!("drain_barriers");

let buffer_barriers = base.buffers.drain_transitions(snatch_guard);
let (transitions, textures) = base.textures.drain_transitions();
let (transitions, textures) = base.textures.drain_transitions(snatch_guard);
let texture_barriers = transitions
.into_iter()
.enumerate()
.map(|(i, p)| p.into_hal(textures[i].as_ref().unwrap()));
.map(|(i, p)| p.into_hal(textures[i].unwrap().raw().unwrap()));

unsafe {
raw.transition_buffers(buffer_barriers);
Expand Down
38 changes: 14 additions & 24 deletions wgpu-core/src/command/transfer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -811,18 +811,15 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
.textures
.set_single(&dst_texture, dst_range, hal::TextureUses::COPY_DST)
.ok_or(TransferError::InvalidTexture(destination.texture))?;
let dst_inner = dst_texture.inner();
let dst_raw = dst_inner
.as_ref()
.unwrap()
.as_raw()
let dst_raw = dst_texture
.raw(&snatch_guard)
.ok_or(TransferError::InvalidTexture(destination.texture))?;
if !dst_texture.desc.usage.contains(TextureUsages::COPY_DST) {
return Err(
TransferError::MissingCopyDstUsageFlag(None, Some(destination.texture)).into(),
);
}
let dst_barrier = dst_pending.map(|pending| pending.into_hal(dst_inner.as_ref().unwrap()));
let dst_barrier = dst_pending.map(|pending| pending.into_hal(dst_raw));

if !dst_base.aspect.is_one() {
return Err(TransferError::CopyAspectNotOne.into());
Expand Down Expand Up @@ -951,11 +948,8 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
.textures
.set_single(&src_texture, src_range, hal::TextureUses::COPY_SRC)
.ok_or(TransferError::InvalidTexture(source.texture))?;
let src_inner = src_texture.inner();
let src_raw = src_inner
.as_ref()
.unwrap()
.as_raw()
let src_raw = src_texture
.raw(&snatch_guard)
.ok_or(TransferError::InvalidTexture(source.texture))?;
if !src_texture.desc.usage.contains(TextureUsages::COPY_SRC) {
return Err(TransferError::MissingCopySrcUsageFlag.into());
Expand All @@ -973,7 +967,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
}
.into());
}
let src_barrier = src_pending.map(|pending| pending.into_hal(src_inner.as_ref().unwrap()));
let src_barrier = src_pending.map(|pending| pending.into_hal(src_raw));

let (dst_buffer, dst_pending) = {
let buffer_guard = hub.buffers.read();
Expand Down Expand Up @@ -1077,6 +1071,8 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
return Err(TransferError::InvalidDevice(cmd_buf.device.as_info().id()).into());
}

let snatch_guard = device.snatchable_lock.read();

let mut cmd_buf_data = cmd_buf.data.lock();
let cmd_buf_data = cmd_buf_data.as_mut().unwrap();

Expand All @@ -1101,12 +1097,10 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
.textures
.get(source.texture)
.map_err(|_| TransferError::InvalidTexture(source.texture))?;
let src_inner = src_texture.inner();
let dst_texture = hub
.textures
.get(destination.texture)
.map_err(|_| TransferError::InvalidTexture(source.texture))?;
let dst_inner = dst_texture.inner();

// src and dst texture format must be copy-compatible
// https://gpuweb.github.io/gpuweb/#copy-compatible
Expand Down Expand Up @@ -1168,10 +1162,8 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
.textures
.set_single(&src_texture, src_range, hal::TextureUses::COPY_SRC)
.ok_or(TransferError::InvalidTexture(source.texture))?;
let src_raw = src_inner
.as_ref()
.unwrap()
.as_raw()
let src_raw = src_texture
.raw(&snatch_guard)
.ok_or(TransferError::InvalidTexture(source.texture))?;
if !src_texture.desc.usage.contains(TextureUsages::COPY_SRC) {
return Err(TransferError::MissingCopySrcUsageFlag.into());
Expand All @@ -1180,26 +1172,24 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
//TODO: try to avoid this the collection. It's needed because both
// `src_pending` and `dst_pending` try to hold `trackers.textures` mutably.
let mut barriers: ArrayVec<_, 2> = src_pending
.map(|pending| pending.into_hal(src_inner.as_ref().unwrap()))
.map(|pending| pending.into_hal(src_raw))
.collect();

let dst_pending = cmd_buf_data
.trackers
.textures
.set_single(&dst_texture, dst_range, hal::TextureUses::COPY_DST)
.ok_or(TransferError::InvalidTexture(destination.texture))?;
let dst_raw = dst_inner
.as_ref()
.unwrap()
.as_raw()
let dst_raw = dst_texture
.raw(&snatch_guard)
.ok_or(TransferError::InvalidTexture(destination.texture))?;
if !dst_texture.desc.usage.contains(TextureUsages::COPY_DST) {
return Err(
TransferError::MissingCopyDstUsageFlag(None, Some(destination.texture)).into(),
);
}

barriers.extend(dst_pending.map(|pending| pending.into_hal(dst_inner.as_ref().unwrap())));
barriers.extend(dst_pending.map(|pending| pending.into_hal(dst_raw)));

let hal_copy_size = hal::CopyExtent {
width: src_copy_size.width.min(dst_copy_size.width),
Expand Down
26 changes: 12 additions & 14 deletions wgpu-core/src/device/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -745,21 +745,19 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {

let last_submit_index = texture.info.submission_index();

if let resource::TextureInner::Native { ref raw } = *texture.inner().as_ref().unwrap() {
if !raw.is_none() {
let temp = queue::TempResource::Texture(texture.clone());
let mut guard = device.pending_writes.lock();
let pending_writes = guard.as_mut().unwrap();
if pending_writes.dst_textures.contains_key(&texture_id) {
pending_writes.temp_resources.push(temp);
} else {
drop(guard);
device
.lock_life()
.schedule_resource_destruction(temp, last_submit_index);
}
let snatch_guard = texture.device.snatchable_lock.read();

if let Some(resource::TextureInner::Native { .. }) = texture.inner.get(&snatch_guard) {
let temp = queue::TempResource::Texture(texture.clone());
let mut guard = device.pending_writes.lock();
let pending_writes = guard.as_mut().unwrap();
if pending_writes.dst_textures.contains_key(&texture_id) {
pending_writes.temp_resources.push(temp);
} else {
return Err(resource::DestroyError::AlreadyDestroyed);
drop(guard);
device
.lock_life()
.schedule_resource_destruction(temp, last_submit_index);
}
}

Expand Down
51 changes: 23 additions & 28 deletions wgpu-core/src/device/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,8 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
}
}

let snatch_guard = device.snatchable_lock.read();

// Re-get `dst` immutably here, so that the mutable borrow of the
// `texture_guard.get` above ends in time for the `clear_texture`
// call above. Since we've held `texture_guard` the whole time, we know
Expand All @@ -810,11 +812,8 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
dst.info
.use_at(device.active_submission_index.load(Ordering::Relaxed) + 1);

let dst_inner = dst.inner();
let dst_raw = dst_inner
.as_ref()
.unwrap()
.as_raw()
let dst_raw = dst
.raw(&snatch_guard)
.ok_or(TransferError::InvalidTexture(destination.texture))?;

let bytes_per_row = data_layout
Expand Down Expand Up @@ -897,9 +896,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
.set_single(&dst, selector, hal::TextureUses::COPY_DST)
.ok_or(TransferError::InvalidTexture(destination.texture))?;
unsafe {
encoder.transition_textures(
transition.map(|pending| pending.into_hal(dst_inner.as_ref().unwrap())),
);
encoder.transition_textures(transition.map(|pending| pending.into_hal(dst_raw)));
encoder.transition_buffers(iter::once(barrier));
encoder.copy_buffer_to_texture(inner_buffer.as_ref().unwrap(), dst_raw, regions);
}
Expand Down Expand Up @@ -1076,11 +1073,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
dst.info
.use_at(device.active_submission_index.load(Ordering::Relaxed) + 1);

let dst_inner = dst.inner();
let dst_raw = dst_inner
.as_ref()
.unwrap()
.as_raw()
let snatch_guard = device.snatchable_lock.read();
let dst_raw = dst
.raw(&snatch_guard)
.ok_or(TransferError::InvalidTexture(destination.texture))?;

let regions = hal::TextureCopy {
Expand All @@ -1100,9 +1095,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
.textures
.set_single(&dst, selector, hal::TextureUses::COPY_DST)
.ok_or(TransferError::InvalidTexture(destination.texture))?;
encoder.transition_textures(
transitions.map(|pending| pending.into_hal(dst_inner.as_ref().unwrap())),
);
encoder.transition_textures(transitions.map(|pending| pending.into_hal(dst_raw)));
encoder.copy_external_image_to_texture(
source,
dst_raw,
Expand Down Expand Up @@ -1238,12 +1231,12 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
}
for texture in cmd_buf_trackers.textures.used_resources() {
let id = texture.info.id();
let should_extend = match *texture.inner().as_ref().unwrap() {
TextureInner::Native { raw: None } => {
let should_extend = match texture.inner.get(&snatch_guard) {
None => {
return Err(QueueSubmitError::DestroyedTexture(id));
}
TextureInner::Native { raw: Some(_) } => false,
TextureInner::Surface { ref has_work, .. } => {
Some(TextureInner::Native { .. }) => false,
Some(TextureInner::Surface { ref has_work, .. }) => {
has_work.store(true, Ordering::Relaxed);
true
}
Expand Down Expand Up @@ -1399,11 +1392,12 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
trackers
.textures
.set_from_usage_scope(&used_surface_textures);
let (transitions, textures) = trackers.textures.drain_transitions();
let (transitions, textures) =
trackers.textures.drain_transitions(&snatch_guard);
let texture_barriers = transitions
.into_iter()
.enumerate()
.map(|(i, p)| p.into_hal(textures[i].as_ref().unwrap()));
.map(|(i, p)| p.into_hal(textures[i].unwrap().raw().unwrap()));
let present = unsafe {
baked.encoder.transition_textures(texture_barriers);
baked.encoder.end_encoding().unwrap()
Expand All @@ -1429,12 +1423,12 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
{
used_surface_textures.set_size(hub.textures.read().len());
for (&id, texture) in pending_writes.dst_textures.iter() {
match *texture.inner().as_ref().unwrap() {
TextureInner::Native { raw: None } => {
match texture.inner.get(&snatch_guard) {
None => {
return Err(QueueSubmitError::DestroyedTexture(id));
}
TextureInner::Native { raw: Some(_) } => {}
TextureInner::Surface { ref has_work, .. } => {
Some(TextureInner::Native { .. }) => {}
Some(TextureInner::Surface { ref has_work, .. }) => {
has_work.store(true, Ordering::Relaxed);
unsafe {
used_surface_textures
Expand All @@ -1451,11 +1445,12 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
trackers
.textures
.set_from_usage_scope(&used_surface_textures);
let (transitions, textures) = trackers.textures.drain_transitions();
let (transitions, textures) =
trackers.textures.drain_transitions(&snatch_guard);
let texture_barriers = transitions
.into_iter()
.enumerate()
.map(|(i, p)| p.into_hal(textures[i].as_ref().unwrap()));
.map(|(i, p)| p.into_hal(textures[i].unwrap().raw().unwrap()));
unsafe {
pending_writes
.command_encoder
Expand Down
15 changes: 6 additions & 9 deletions wgpu-core/src/device/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -587,9 +587,7 @@ impl<A: HalApi> Device<A> {
debug_assert_eq!(self.as_info().id().backend(), A::VARIANT);

Texture {
inner: RwLock::new(Some(resource::TextureInner::Native {
raw: Some(hal_texture),
})),
inner: Snatchable::new(resource::TextureInner::Native { raw: hal_texture }),
device: self.clone(),
desc: desc.map_label(|_| ()),
hal_usage,
Expand Down Expand Up @@ -907,15 +905,14 @@ impl<A: HalApi> Device<A> {
texture: &Arc<Texture<A>>,
desc: &resource::TextureViewDescriptor,
) -> Result<TextureView<A>, resource::CreateTextureViewError> {
let inner = texture.inner();
let texture_raw = inner
.as_ref()
.unwrap()
.as_raw()
let snatch_guard = texture.device.snatchable_lock.read();

let texture_raw = texture
.raw(&snatch_guard)
.ok_or(resource::CreateTextureViewError::InvalidTexture)?;

// resolve TextureViewDescriptor defaults
// https://gpuweb.github.io/gpuweb/#abstract-opdef-resolving-gputextureviewdescriptor-defaults

let resolved_format = desc.format.unwrap_or_else(|| {
texture
.desc
Expand Down
19 changes: 11 additions & 8 deletions wgpu-core/src/present.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use crate::{
identity::{GlobalIdentityHandlerFactory, Input},
init_tracker::TextureInitTracker,
resource::{self, ResourceInfo},
snatch::Snatchable,
track,
};

Expand Down Expand Up @@ -215,11 +216,11 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
let mut presentation = surface.presentation.lock();
let present = presentation.as_mut().unwrap();
let texture = resource::Texture {
inner: RwLock::new(Some(resource::TextureInner::Surface {
inner: Snatchable::new(resource::TextureInner::Surface {
raw: Some(ast.texture),
parent_id: surface_id,
has_work: AtomicBool::new(false),
})),
}),
device: device.clone(),
desc: texture_desc,
hal_usage,
Expand Down Expand Up @@ -325,8 +326,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {

let texture = hub.textures.unregister(texture_id);
if let Some(texture) = texture {
let mut exclusive_snatch_guard = device.snatchable_lock.write();
let suf = A::get_surface(&surface);
let mut inner = texture.inner_mut();
let mut inner = texture.inner_mut(&mut exclusive_snatch_guard);
let inner = inner.as_mut().unwrap();

match *inner {
Expand Down Expand Up @@ -420,13 +422,14 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
let texture = hub.textures.unregister(texture_id);
if let Some(texture) = texture {
let suf = A::get_surface(&surface);
match *texture.inner_mut().as_mut().take().as_mut().unwrap() {
&mut resource::TextureInner::Surface {
ref mut raw,
ref parent_id,
let exclusive_snatch_guard = device.snatchable_lock.write();
match texture.inner.snatch(exclusive_snatch_guard).unwrap() {
resource::TextureInner::Surface {
mut raw,
parent_id,
has_work: _,
} => {
if surface_id == *parent_id {
if surface_id == parent_id {
unsafe { suf.unwrap().raw.discard_texture(raw.take().unwrap()) };
} else {
log::warn!("Surface texture is outdated");
Expand Down
Loading