Rework presentation by inner-daemons · Pull Request #9222 · gfx-rs/wgpu

inner-daemons · 2026-03-14T05:27:08Z

Connections
Closes #6748
Closes #9109

Description
One question: I think surface texture acquisition should also be a queue operation since it may end up waiting for previous presents/whatever. Do you agree?

This was primarily written by Claude, though I looked over everything, tested it, and I generally guided Claude throughout this knowing the issues myself.

Presentation is now a queue operation (as it should've been), and keeps the surface and stuff alive as a result (also makes multi queue easier)
Hal queue now has a wait for complete. This is the only way in standard vulkan to wait for a present to finish
Present is now a queue operation. This fixes an issue with presenting when the queue has been dropped
Queue destruction now waits for presents to finish so that surface textures don't have use after free/UB
Presentation now clears the surface texture and transitions it into present mode if it wasn't used in any command buffers, fixing a bug there (present when not in present layout) and UB there (present an uninitialized texture with garbage data)

Testing
Manual test

Squash or Rebase?
Squash

Checklist

Run cargo fmt.
Run taplo format.
Run cargo clippy --tests. If applicable, add:
- --target wasm32-unknown-unknown
Run cargo xtask test to run tests.
If this contains user-facing changes, add a CHANGELOG.md entry.

atlv24 · 2026-03-15T05:48:49Z

I tested this and it works, and gave it a quick look over and it seems reasonable.

inner-daemons · 2026-03-15T16:44:00Z

wgpu-core/src/device/queue.rs


+        // Wait for all GPU work — submissions *and* any pending presentations —
+        // to complete before releasing resources.
+        match unsafe { self.raw.wait_for_idle() } {


No timeout here may be a problem in some cases?

It could hang if the driver/platform doesn't itself have a timeout that causes device loss. I think this should be ok? cc @cwfitzgerald

It's nice that the new wait_for_idle works the same across all backends; partially resolving #6538.

inner-daemons · 2026-03-15T17:14:30Z

@cwfitzgerald Can you have copilot review this

Copilot

Pull request overview

Reworks surface presentation so that presenting is a queue operation (instead of being driven by SurfaceTexture/surface detail), adds backend support for blocking until presents complete, and fixes Vulkan correctness/UB around presenting unused/uninitialized surface textures and dropping queues with in-flight presents.

Changes:

Move presentation to Queue::present, removing SurfaceTexture::present and backend SurfaceOutputDetailInterface::present.
Add hal::Queue::wait_for_idle across backends and use it to safely wait for present completion (including on queue drop).
Add core-side logic to clear/transition acquired-but-unused surface textures before present, and track present lifetimes until completion.

Reviewed changes

Copilot reviewed 30 out of 32 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
wgpu/src/dispatch.rs	Adds `QueueInterface::present` and removes surface-output-detail present hook.
wgpu/src/backend/wgpu_core.rs	Routes presentation through `CoreQueue::present`; removes now-unused error sink from surface output detail.
wgpu/src/backend/webgpu.rs	Implements no-op `QueueInterface::present` for web; removes output-detail present hook.
wgpu/src/api/surface_texture.rs	Removes `SurfaceTexture::present` API; relies on `Drop` discard when not presented.
wgpu/src/api/surface.rs	Updates presentation docs to reference `Queue::present`.
wgpu/src/api/queue.rs	Introduces `Queue::present(SurfaceTexture) -> SubmissionIndex` API.
wgpu-hal/src/lib.rs	Adds `Queue::wait_for_idle` to hal trait to support waiting for presents.
wgpu-hal/src/dynamic/queue.rs	Plumbs `wait_for_idle` through the dynamic queue abstraction.
wgpu-hal/src/vulkan/mod.rs	Implements `wait_for_idle` via `vkQueueWaitIdle`.
wgpu-hal/src/metal/mod.rs	Implements `wait_for_idle` by committing an empty command buffer and waiting for completion.
wgpu-hal/src/dx12/mod.rs	Implements `wait_for_idle` with a fence signal + wait.
wgpu-hal/src/gles/queue.rs	Implements `wait_for_idle` via `gl.finish()`.
wgpu-hal/src/noop/mod.rs	Implements `wait_for_idle` as a no-op.
wgpu-core/src/present.rs	Moves present logic onto `Queue`, returns a `SubmissionIndex`, and tracks presented textures for safe lifetime.
wgpu-core/src/device/queue.rs	Adds waiting-for-present logic, queue-drop idle wait, and “unused surface texture” prepare/mini-submit path.
wgpu-core/src/device/life.rs	Tracks pending present textures and releases them once completion conditions are met.
wgpu-core/src/device/resource.rs	Adds maintain-time handling intended to resolve pending presents on waits.
wgpu-core/src/device/trace.rs	Extends trace `Action::Present` to include a submission index.
wgpu-core/src/device/trace/record.rs	Updates trace record conversion for the new `Present` action shape.
player/src/lib.rs	Updates pattern match for the new traced `Present` action signature.
player/src/bin/play.rs	Updates replay logic to present via queue/surface and handle new trace action signature.
examples/standalone/custom_backend/src/custom.rs	Extends custom backend queue trait implementation with `present`.
examples/standalone/02_hello_window/src/main.rs	Migrates example to `queue.present(surface_texture)`.
examples/features/src/framework.rs	Migrates example framework to `queue.present(frame)`.
examples/features/src/hello_triangle/mod.rs	Migrates example to `queue.present(frame)`.
examples/features/src/hello_windows/mod.rs	Migrates example to `queue.present(frame)`.
examples/features/src/uniform_values/mod.rs	Migrates example to `queue.present(frame)`.
examples/bug-repro/01_texture_atomic_bug/src/main.rs	Migrates repro to `queue.present(frame)`.
examples/bug-repro/02_present_bugs/src/main.rs	Adds new repro for presenting unused textures and dropping queue after present.
examples/bug-repro/02_present_bugs/Cargo.toml	Adds new repro crate manifest.
Cargo.lock	Adds lockfile entries for the new repro crate.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

wgpu-core/src/device/resource.rs

wgpu-core/src/present.rs

wgpu/src/api/surface.rs

wgpu-core/src/device/queue.rs

cwfitzgerald · 2026-03-18T15:41:12Z

@teoxoy I assigned this to you, if you can't take it, feel free to unassign yourself.

inner-daemons · 2026-03-20T19:42:47Z

@teoxoy I just sent this to a matrix account but you have 2 and I'm not sure if I sent it to the right one, plsu thats probably not the right place to discuss.

Anyway, what do you think about making Surface::get_current_texture be a queue operation instead? I think it can potentially be blocking and depends on the queue timeline (a semaphore). Also we should perhaps consider adding a method to wait for the surface texture to be ready/query if its ready, or even make it an async operation.

P.S. the same probably goes for Surface::configure

teoxoy · 2026-03-21T09:35:28Z

IIRC configure takes a device, maybe it should take a queue instead? Is that really needed though? I guess get_current_texture can get blocked by presentation but I don't think it should be moved on the queue since it's not a queue operation conceptually.

inner-daemons · 2026-03-24T00:46:31Z

@teoxoy Ok, I've undone the changes that move configuration and surface texture acquisition to the queue. Should be good to review.

teoxoy

This looks good overall; my biggest concern is using submission indices for presentation when they have different invariants.

teoxoy · 2026-03-26T16:03:25Z

wgpu-core/src/present.rs

+                    device
+                        .last_successful_submission_index
+                        .store(present_index, Ordering::Release);
+                    self.lock_life().track_present(present_index, texture);
+                    Ok((Status::Timeout, present_index))
+                }
+                hal::SurfaceError::Occluded => {
+                    device
+                        .last_successful_submission_index
+                        .store(present_index, Ordering::Release);
+                    self.lock_life().track_present(present_index, texture);
+                    Ok((Status::Occluded, present_index))
+                }
+                hal::SurfaceError::Lost => {
+                    device
+                        .last_successful_submission_index
+                        .store(present_index, Ordering::Release);
+                    self.lock_life().track_present(present_index, texture);
+                    Ok((Status::Lost, present_index))
+                }
+                hal::SurfaceError::Device(err) => {
+                    Err(SurfaceError::from(device.handle_hal_error(err)))
+                }
+                hal::SurfaceError::Outdated => {
+                    device
+                        .last_successful_submission_index
+                        .store(present_index, Ordering::Release);
+                    self.lock_life().track_present(present_index, texture);


If we got an error back I don't think it's correct that we are storing the present index.

teoxoy · 2026-03-26T16:06:32Z

wgpu-core/src/present.rs

+        result
+    }
+
+    /// TODO: is this needed by deno?


We should remove this function since we moved it onto the queue now.

teoxoy · 2026-03-26T16:08:32Z

wgpu-core/src/present.rs

+        let mut present_lock = surface.presentation.lock();
+
+        if let Some(ref presentation) = *present_lock {
+            let same_device = Arc::ptr_eq(&presentation.device, &queue.device);
+
+            if !same_device {
+                return Err(SurfaceError::Device(DeviceError::DeviceMismatch(Box::new(
+                    DeviceMismatch {
+                        res: queue.error_ident(),
+                        res_device: queue.device.error_ident(),
+                        target: None,
+                        target_device: presentation.device.error_ident(),
+                    },
+                ))));
+            }
+        }
+
+        let result = queue.present_impl(&surface, present_lock.as_mut());


I would move the device mismatch check in present_impl and rename present_impl to present (removing the old present).

teoxoy · 2026-03-26T16:22:36Z

wgpu-core/src/device/resource.rs


            let wait_result = unsafe {
                self.raw()
                    .wait(fence.as_ref(), target_submission_index, wait_timeout)


Won't this hang the thread if wait_submission_index is a present index?

As far as I understand present indices are not the same as submission indices since we don't use fences for presentation. We should either see how to make presentation work with fences, use a different counter for presentation or triple check that we do the right thing if a present index is passed as a submission index in all places a submission index is currently expected.

teoxoy · 2026-03-26T16:35:26Z

wgpu-core/src/device/queue.rs

+            // until the GPU finishes with it.
+            if let Some(encoder) = encoder_in_flight {
+                self.lock_life().track_submission(mini_index, vec![encoder]);
+            }


Submission of PendingWrites is more complicated than this (since it's not guaranteed it will be empty before enqueueing the clear command for the surface texture), the block in this comment is needed to do it correctly: #9307 (comment). You might have to coordinate with @andyleiserson on extracting it out to be used in places like this.

teoxoy · 2026-03-26T16:43:50Z

wgpu-core/src/device/queue.rs

+            Err(e) => {
+                panic!("Unexpected error while waiting for queue idle on drop: {e:?}");
+            }
+        }


Maybe we should still retry a few times if we get an OutOfMemory error?

teoxoy · 2026-03-26T16:59:30Z

wgpu-hal/src/lib.rs

    unsafe fn get_timestamp_period(&self) -> f32;
+    /// Block until all queue operations have completed.
+    ///
+    /// This is useful for waiting on presentations, which in vulkan don't have fences.


There is VkSwapchainPresentFenceInfoKHR from VK_KHR_swapchain_maintenance1 but only 12% of reports support it.. That's unfortunate.

I looked into it. The spec says there is an implicit semaphore operation (can't remember the exact phrasing) that vkQueueWaitIdle still waits for. But ig most drivers don't expose that :(

teoxoy · 2026-03-26T17:08:47Z

wgpu-core/src/device/queue.rs


+        // Wait for all GPU work — submissions *and* any pending presentations —
+        // to complete before releasing resources.
+        match unsafe { self.raw.wait_for_idle() } {


It could hang if the driver/platform doesn't itself have a timeout that causes device loss. I think this should be ok? cc @cwfitzgerald

teoxoy · 2026-03-26T17:10:04Z

wgpu-core/src/device/queue.rs


+        // Wait for all GPU work — submissions *and* any pending presentations —
+        // to complete before releasing resources.
+        match unsafe { self.raw.wait_for_idle() } {


It's nice that the new wait_for_idle works the same across all backends; partially resolving #6538.

This was referenced Mar 14, 2026

refactor(metal): MTLCommandQueue is thread-safe #9217

Merged

Vulkan presentation validation errors #9213

Open

inner-daemons commented Mar 15, 2026

View reviewed changes

inner-daemons marked this pull request as ready for review March 15, 2026 17:12

cwfitzgerald requested a review from Copilot March 15, 2026 17:15

Copilot started reviewing on behalf of cwfitzgerald March 15, 2026 17:15 View session

Copilot AI reviewed Mar 15, 2026

View reviewed changes

wgpu-core/src/device/resource.rs Show resolved Hide resolved

wgpu-core/src/present.rs Show resolved Hide resolved

wgpu/src/api/surface.rs Show resolved Hide resolved

wgpu-core/src/device/queue.rs Show resolved Hide resolved

wgpu-core/src/device/queue.rs Show resolved Hide resolved

cwfitzgerald assigned teoxoy Mar 18, 2026

inner-daemons mentioned this pull request Mar 18, 2026

Change get_current_texture output to a unified enum #9257

Merged

6 tasks

inner-daemons force-pushed the various-present-fixes branch from 725134e to 34745e8 Compare March 20, 2026 19:38

inner-daemons mentioned this pull request Mar 23, 2026

SYNC-HAZARD-WRITE-AFTER-PRESENT when presenting a surface just after getting it. #6748

Open

inner-daemons force-pushed the various-present-fixes branch from 3fa0d25 to 34745e8 Compare March 24, 2026 00:45

inner-daemons requested a review from teoxoy March 24, 2026 00:46

Fix present bugs & rework api

a695c5a

inner-daemons force-pushed the various-present-fixes branch from 77d04a7 to a695c5a Compare March 24, 2026 01:29

teoxoy requested changes Mar 26, 2026

View reviewed changes

Conversation

inner-daemons commented Mar 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

atlv24 commented Mar 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

inner-daemons commented Mar 15, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cwfitzgerald commented Mar 18, 2026

Uh oh!

inner-daemons commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

teoxoy commented Mar 21, 2026

Uh oh!

inner-daemons commented Mar 24, 2026

Uh oh!

teoxoy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

inner-daemons commented Mar 14, 2026 •

edited

Loading

inner-daemons commented Mar 20, 2026 •

edited

Loading