Improve the ownership story of textures returned by get_current_texture

`present` and `discard` will no longer automatically remove the texture from the registry.
This commit is contained in:
teoxoy 2024-10-11 16:25:04 +02:00 committed by Teodor Tanasoaia
parent e86330977b
commit 39629d0de0
8 changed files with 36 additions and 108 deletions

View File

@ -87,7 +87,6 @@ pub fn op_webgpu_surface_get_current_texture(
let rid = state.resource_table.add(crate::texture::WebGpuTexture { let rid = state.resource_table.add(crate::texture::WebGpuTexture {
instance: instance.clone(), instance: instance.clone(),
id, id,
owned: false,
}); });
Ok(WebGpuResult::rid(rid)) Ok(WebGpuResult::rid(rid))
} }

View File

@ -13,7 +13,6 @@ use super::error::WebGpuResult;
pub(crate) struct WebGpuTexture { pub(crate) struct WebGpuTexture {
pub(crate) instance: crate::Instance, pub(crate) instance: crate::Instance,
pub(crate) id: wgpu_core::id::TextureId, pub(crate) id: wgpu_core::id::TextureId,
pub(crate) owned: bool,
} }
impl Resource for WebGpuTexture { impl Resource for WebGpuTexture {
@ -22,11 +21,9 @@ impl Resource for WebGpuTexture {
} }
fn close(self: Rc<Self>) { fn close(self: Rc<Self>) {
if self.owned {
let instance = &self.instance; let instance = &self.instance;
instance.texture_drop(self.id); instance.texture_drop(self.id);
} }
}
} }
pub(crate) struct WebGpuTextureView( pub(crate) struct WebGpuTextureView(
@ -85,7 +82,6 @@ pub fn op_webgpu_create_texture(
let rid = state.resource_table.add(WebGpuTexture { let rid = state.resource_table.add(WebGpuTexture {
instance: instance.clone(), instance: instance.clone(),
id: val, id: val,
owned: true,
}); });
Ok(WebGpuResult::rid_err(rid, maybe_err)) Ok(WebGpuResult::rid_err(rid, maybe_err))

View File

@ -17,8 +17,7 @@ use crate::{
conv, conv,
device::{Device, DeviceError, MissingDownlevelFlags, WaitIdleError}, device::{Device, DeviceError, MissingDownlevelFlags, WaitIdleError},
global::Global, global::Global,
hal_label, id, hal_label, id, resource,
resource::{self, Trackable},
}; };
use thiserror::Error; use thiserror::Error;
@ -30,7 +29,7 @@ const FRAME_TIMEOUT_MS: u32 = 1000;
pub(crate) struct Presentation { pub(crate) struct Presentation {
pub(crate) device: Arc<Device>, pub(crate) device: Arc<Device>,
pub(crate) config: wgt::SurfaceConfiguration<Vec<wgt::TextureFormat>>, pub(crate) config: wgt::SurfaceConfiguration<Vec<wgt::TextureFormat>>,
pub(crate) acquired_texture: Option<id::TextureId>, pub(crate) acquired_texture: Option<Arc<resource::Texture>>,
} }
#[derive(Clone, Debug, Error)] #[derive(Clone, Debug, Error)]
@ -212,12 +211,12 @@ impl Global {
.textures .textures
.insert_single(&texture, hal::TextureUses::UNINITIALIZED); .insert_single(&texture, hal::TextureUses::UNINITIALIZED);
let id = fid.assign(resource::Fallible::Valid(texture));
if present.acquired_texture.is_some() { if present.acquired_texture.is_some() {
return Err(SurfaceError::AlreadyAcquired); return Err(SurfaceError::AlreadyAcquired);
} }
present.acquired_texture = Some(id); present.acquired_texture = Some(texture.clone());
let id = fid.assign(resource::Fallible::Valid(texture));
let status = if ast.suboptimal { let status = if ast.suboptimal {
Status::Suboptimal Status::Suboptimal
@ -249,8 +248,6 @@ impl Global {
pub fn surface_present(&self, surface_id: id::SurfaceId) -> Result<Status, SurfaceError> { pub fn surface_present(&self, surface_id: id::SurfaceId) -> Result<Status, SurfaceError> {
profiling::scope!("SwapChain::present"); profiling::scope!("SwapChain::present");
let hub = &self.hub;
let surface = self.surfaces.get(surface_id); let surface = self.surfaces.get(surface_id);
let mut presentation = surface.presentation.lock(); let mut presentation = surface.presentation.lock();
@ -269,35 +266,21 @@ impl Global {
device.check_is_valid()?; device.check_is_valid()?;
let queue = device.get_queue().unwrap(); let queue = device.get_queue().unwrap();
let result = { let texture = present
let texture_id = present
.acquired_texture .acquired_texture
.take() .take()
.ok_or(SurfaceError::AlreadyAcquired)?; .ok_or(SurfaceError::AlreadyAcquired)?;
// The texture ID got added to the device tracker by `submit()`, let result = match texture
// and now we are moving it away.
let texture = hub.textures.remove(texture_id).get();
if let Ok(texture) = texture {
device
.trackers
.lock()
.textures
.remove(texture.tracker_index());
let suf = surface.raw(device.backend()).unwrap();
match texture
.inner .inner
.snatch(&mut device.snatchable_lock.write()) .snatch(&mut device.snatchable_lock.write())
.unwrap() .unwrap()
{ {
resource::TextureInner::Surface { raw } => unsafe { resource::TextureInner::Surface { raw } => unsafe {
queue.raw().present(suf, raw) let raw_surface = surface.raw(device.backend()).unwrap();
queue.raw().present(raw_surface, raw)
}, },
_ => unreachable!(), _ => unreachable!(),
}
} else {
Err(hal::SurfaceError::Outdated) //TODO?
}
}; };
match result { match result {
@ -319,8 +302,6 @@ impl Global {
pub fn surface_texture_discard(&self, surface_id: id::SurfaceId) -> Result<(), SurfaceError> { pub fn surface_texture_discard(&self, surface_id: id::SurfaceId) -> Result<(), SurfaceError> {
profiling::scope!("SwapChain::discard"); profiling::scope!("SwapChain::discard");
let hub = &self.hub;
let surface = self.surfaces.get(surface_id); let surface = self.surfaces.get(surface_id);
let mut presentation = surface.presentation.lock(); let mut presentation = surface.presentation.lock();
let present = match presentation.as_mut() { let present = match presentation.as_mut() {
@ -337,35 +318,22 @@ impl Global {
device.check_is_valid()?; device.check_is_valid()?;
{ let texture = present
let texture_id = present
.acquired_texture .acquired_texture
.take() .take()
.ok_or(SurfaceError::AlreadyAcquired)?; .ok_or(SurfaceError::AlreadyAcquired)?;
// The texture ID got added to the device tracker by `submit()`,
// and now we are moving it away.
let texture = hub.textures.remove(texture_id).get();
if let Ok(texture) = texture {
device
.trackers
.lock()
.textures
.remove(texture.tracker_index());
let suf = surface.raw(device.backend());
match texture match texture
.inner .inner
.snatch(&mut device.snatchable_lock.write()) .snatch(&mut device.snatchable_lock.write())
.unwrap() .unwrap()
{ {
resource::TextureInner::Surface { raw } => { resource::TextureInner::Surface { raw } => {
unsafe { suf.unwrap().discard_texture(raw) }; let raw_surface = surface.raw(device.backend()).unwrap();
unsafe { raw_surface.discard_texture(raw) };
} }
_ => unreachable!(), _ => unreachable!(),
} }
}
}
Ok(()) Ok(())
} }

View File

@ -27,11 +27,6 @@ impl<T: Clone> ResourceMetadata<T> {
} }
} }
/// Returns the number of indices we can accommodate.
pub(super) fn size(&self) -> usize {
self.owned.len()
}
pub(super) fn set_size(&mut self, size: usize) { pub(super) fn set_size(&mut self, size: usize) {
self.resources.resize(size, None); self.resources.resize(size, None);
resize_bitvec(&mut self.owned, size); resize_bitvec(&mut self.owned, size);

View File

@ -18,7 +18,7 @@
//! is known to be in some undefined state. Any transition away from UNINITIALIZED //! is known to be in some undefined state. Any transition away from UNINITIALIZED
//! will treat the contents as junk. //! will treat the contents as junk.
use super::{range::RangedStates, PendingTransition, PendingTransitionList, TrackerIndex}; use super::{range::RangedStates, PendingTransition, PendingTransitionList};
use crate::{ use crate::{
resource::{Texture, TextureInner, TextureView, Trackable}, resource::{Texture, TextureInner, TextureView, Trackable},
snatch::SnatchGuard, snatch::SnatchGuard,
@ -826,32 +826,6 @@ impl DeviceTextureTracker {
pending.into_hal(tex) pending.into_hal(tex)
}) })
} }
/// Unconditionally removes the given resource from the tracker.
///
/// Returns true if the resource was removed.
///
/// If the index is higher than the length of internal vectors,
/// false will be returned.
pub fn remove(&mut self, index: TrackerIndex) -> bool {
let index = index.as_usize();
if index >= self.metadata.size() {
return false;
}
self.tracker_assert_in_bounds(index);
unsafe {
if self.metadata.contains_unchecked(index) {
self.current_state_set.complex.remove(&index);
self.metadata.remove(index);
return true;
}
}
false
}
} }
impl TextureTrackerSetSingle for DeviceTextureTracker { impl TextureTrackerSetSingle for DeviceTextureTracker {

View File

@ -252,7 +252,6 @@ impl Device {
Texture { Texture {
context: Arc::clone(&self.context), context: Arc::clone(&self.context),
data, data,
owned: true,
descriptor: TextureDescriptor { descriptor: TextureDescriptor {
label: None, label: None,
view_formats: &[], view_formats: &[],
@ -291,7 +290,6 @@ impl Device {
Texture { Texture {
context: Arc::clone(&self.context), context: Arc::clone(&self.context),
data: Box::new(texture), data: Box::new(texture),
owned: true,
descriptor: TextureDescriptor { descriptor: TextureDescriptor {
label: None, label: None,
view_formats: &[], view_formats: &[],

View File

@ -142,7 +142,6 @@ impl Surface<'_> {
texture: Texture { texture: Texture {
context: Arc::clone(&self.context), context: Arc::clone(&self.context),
data, data,
owned: false,
descriptor, descriptor,
}, },
suboptimal, suboptimal,

View File

@ -12,7 +12,6 @@ use crate::*;
pub struct Texture { pub struct Texture {
pub(crate) context: Arc<C>, pub(crate) context: Arc<C>,
pub(crate) data: Box<Data>, pub(crate) data: Box<Data>,
pub(crate) owned: bool,
pub(crate) descriptor: TextureDescriptor<'static>, pub(crate) descriptor: TextureDescriptor<'static>,
} }
#[cfg(send_sync)] #[cfg(send_sync)]
@ -138,7 +137,7 @@ impl Texture {
impl Drop for Texture { impl Drop for Texture {
fn drop(&mut self) { fn drop(&mut self) {
if self.owned && !thread::panicking() { if !thread::panicking() {
self.context.texture_drop(self.data.as_ref()); self.context.texture_drop(self.data.as_ref());
} }
} }