Skip to content

vk: move swapchain acquire() to as late as possible#9541

Merged
poweifeng merged 3 commits intomainfrom
pf/vk-move-acquire-later
Jan 7, 2026
Merged

vk: move swapchain acquire() to as late as possible#9541
poweifeng merged 3 commits intomainfrom
pf/vk-move-acquire-later

Conversation

@poweifeng
Copy link
Contributor

makeCurrent is not meant to acquire the swapchain; we should do so at the momemnt we need it, to shorten the amount of time a swapchain image/buffer needs to be held.

@poweifeng poweifeng added the internal Issue/PR does not affect clients label Dec 19, 2025
@pixelflinger
Copy link
Collaborator

@poweifeng change looks good, but it fails the presubmit, triggering an assert.

@pixelflinger pixelflinger requested a review from z3moon December 22, 2025 20:27
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this trying to fix?

I think its better to do the acquire as early as possible, why wait until the middle of the frame to acquire the new image?

if all the images are currently in use, why start working on the next frame?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because we've set vkAcquireNextImageKHR to be a blocking call, and that's currently associated with makeCurrent, which happens at the beginning of the frame.

Assume all of the swapchain images are currently being queued or presented, then we'll be blocking at the beginning of the frame. But if we are rendering into a shadow map, let's say, then we shouldn't be blocking on trying to acquire the swapchain image at that point. So this change just moves it to the place where we're actually rendering into the swapchain.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if you already into the point of needing to wait for an image to be released, then the backend should just pace and not submit anymore work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pixelflinger can explain the finer points of this.

But in my mind, the swapchain images being queued does not equal to GPU being busy. The compositor has its own logic/reasoning for holding or releasing buffers. This is a bit like holding a lock; we want our critical section to be as little as possible.

@poweifeng poweifeng force-pushed the pf/vk-move-acquire-later branch 2 times, most recently from ee6b1f4 to 2323515 Compare January 6, 2026 22:15
makeCurrent is not meant to acquire the swapchain; we should
do so at the momemnt we need it, to shorten the amount of time
a swapchain image/buffer needs to be held.
@poweifeng poweifeng force-pushed the pf/vk-move-acquire-later branch from 2323515 to 1e96737 Compare January 7, 2026 01:30
@poweifeng poweifeng enabled auto-merge (squash) January 7, 2026 18:20
@poweifeng poweifeng merged commit f33a779 into main Jan 7, 2026
17 checks passed
@poweifeng poweifeng deleted the pf/vk-move-acquire-later branch January 7, 2026 18:36
poweifeng added a commit that referenced this pull request Jan 15, 2026
Removing this behavior in #9541 caused a breakage in one of our
clients. We use a feature flag to enable the old behavior.
poweifeng added a commit that referenced this pull request Jan 15, 2026
Removing this behavior in #9541 caused a breakage in one of our
clients. We use a feature flag to enable the old behavior.

BUGS=476144715
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal Issue/PR does not affect clients

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants