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

Drop texture clear_views in surface_texture_discard #4057

Merged
merged 1 commit into from
Aug 30, 2023
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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ By @Valaphee in [#3402](https://github.com/gfx-rs/wgpu/pull/3402)

- Derive storage bindings via `naga::StorageAccess` instead of `naga::GlobalUse`. By @teoxoy in [#3985](https://github.com/gfx-rs/wgpu/pull/3985).
- `Queue::on_submitted_work_done` callbacks will now always be called after all previous `BufferSlice::map_async` callbacks, even when there are no active submissions. By @cwfitzgerald in [#4036](https://github.com/gfx-rs/wgpu/pull/4036).
- Fix `clear` texture views being leaked when `wgpu::SurfaceTexture` is dropped before it is presented. By @rajveermalviya in [#4057](https://github.com/gfx-rs/wgpu/pull/4057).

#### Vulkan
- Fix enabling `wgpu::Features::PARTIALLY_BOUND_BINDING_ARRAY` not being actually enabled in vulkan backend. By @39ali in[#3772](https://github.com/gfx-rs/wgpu/pull/3772).
Expand All @@ -99,7 +100,7 @@ This release was fairly minor as breaking changes go.

#### `wgpu` types now `!Send` `!Sync` on wasm

Up until this point, wgpu has made the assumption that threads do not exist on wasm. With the rise of libraries like [`wasm_thread`](https://crates.io/crates/wasm_thread) making it easier and easier to do wasm multithreading this assumption is no longer sound. As all wgpu objects contain references into the JS heap, they cannot leave the thread they started on.
Up until this point, wgpu has made the assumption that threads do not exist on wasm. With the rise of libraries like [`wasm_thread`](https://crates.io/crates/wasm_thread) making it easier and easier to do wasm multithreading this assumption is no longer sound. As all wgpu objects contain references into the JS heap, they cannot leave the thread they started on.

As we understand that this change might be very inconvenient for users who don't care about wasm threading, there is a crate feature which re-enables the old behavior: `fragile-send-sync-non-atomic-wasm`. So long as you don't compile your code with `-Ctarget-feature=+atomics`, `Send` and `Sync` will be implemented again on wgpu types on wasm. As the name implies, especially for libraries, this is very fragile, as you don't know if a user will want to compile with atomics (and therefore threads) or not.

Expand Down
16 changes: 7 additions & 9 deletions wgpu-core/src/present.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,15 +300,7 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {

let (texture, _) = hub.textures.unregister(texture_id.value.0, &mut token);
if let Some(texture) = texture {
if let resource::TextureClearMode::RenderPass { clear_views, .. } =
texture.clear_mode
{
for clear_view in clear_views {
unsafe {
hal::Device::destroy_texture_view(&device.raw, clear_view);
}
}
}
texture.clear_mode.destroy_clear_views(&device.raw);

let suf = A::get_surface_mut(surface);
match texture.inner {
Expand Down Expand Up @@ -386,10 +378,16 @@ impl<G: GlobalIdentityHandlerFactory> Global<G> {

// The texture ID got added to the device tracker by `submit()`,
// and now we are moving it away.
log::debug!(
"Removing swapchain texture {:?} from the device tracker",
texture_id.value
);
device.trackers.lock().textures.remove(texture_id.value);

let (texture, _) = hub.textures.unregister(texture_id.value.0, &mut token);
if let Some(texture) = texture {
texture.clear_mode.destroy_clear_views(&device.raw);

let suf = A::get_surface_mut(surface);
match texture.inner {
resource::TextureInner::Surface {
Expand Down
12 changes: 12 additions & 0 deletions wgpu-core/src/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,18 @@ pub enum TextureClearMode<A: hal::Api> {
None,
}

impl<A: hal::Api> TextureClearMode<A> {
pub(crate) fn destroy_clear_views(self, device: &A::Device) {
if let TextureClearMode::RenderPass { clear_views, .. } = self {
for clear_view in clear_views {
unsafe {
hal::Device::destroy_texture_view(device, clear_view);
}
}
}
}
}

#[derive(Debug)]
pub struct Texture<A: hal::Api> {
pub(crate) inner: TextureInner<A>,
Expand Down
Loading