Skip to content

Commit

Permalink
Put raw texture access behind snatch guards
Browse files Browse the repository at this point in the history
  • Loading branch information
@nical
nical committed Jan 3, 2024
1 parent 6fdec5c commit 4df12cf
Show file tree
Hide file tree
Showing 11 changed files with 104 additions and 124 deletions.
10 changes: 4 additions & 6 deletions wgpu-core/src/command/clear.rs
View file
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
.as_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
View file
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().as_raw().unwrap()));

unsafe {
raw.transition_buffers(buffer_barriers);
Expand Down
38 changes: 14 additions & 24 deletions wgpu-core/src/command/transfer.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -822,18 +822,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
.as_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 @@ -963,11 +960,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
.as_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 @@ -985,7 +979,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 @@ -1089,6 +1083,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 @@ -1114,11 +1110,9 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {
let src_texture = texture_guard
.get(source.texture)
.map_err(|_| TransferError::InvalidTexture(source.texture))?;
let src_inner = src_texture.inner();
let dst_texture = texture_guard
.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 @@ -1180,10 +1174,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
.as_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 @@ -1192,26 +1184,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
.as_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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -746,21 +746,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
View file
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
.as_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
.as_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().as_raw().unwrap()));
let present = unsafe {
baked.encoder.transition_textures(texture_barriers);
baked.encoder.end_encoding().unwrap()
Expand Down Expand Up @@ -1431,12 +1425,12 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {

used_surface_textures.set_size(texture_guard.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 @@ -1453,11 +1447,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().as_raw().unwrap()));
unsafe {
pending_writes
.command_encoder
Expand Down
15 changes: 6 additions & 9 deletions wgpu-core/src/device/resource.rs
View file
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
.as_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
View file
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

0 comments on commit 4df12cf

Please sign in to comment.