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

WSI Swapchain interframe dependency tracking #12

Closed
karl-lunarg opened this issue May 14, 2018 · 12 comments · Fixed by #7145
Closed

WSI Swapchain interframe dependency tracking #12

karl-lunarg opened this issue May 14, 2018 · 12 comments · Fixed by #7145
Assignees
Labels
Bug Something isn't working WSI Window System Integration related issues

Comments

@karl-lunarg
Copy link
Contributor

Issue by szdarkhack (MIGRATED)
Friday Dec 02, 2016 at 13:29 GMT
Originally opened as KhronosGroup/Vulkan-LoaderAndValidationLayers#1225


In the current implementation of core validation, when a WSI fence is retired it is not associated with any queue and thus does not retire any work, leading to incorrect errors about resources being in use when in fact they are not. It should be the case that when an acquire fence is waited upon for a particular image index, all previous work for which the respective presentation submission waited should in turn be retired. For instance:

render loop
acquire image and wait on fence
submit work that signals a semaphore
queue present that waits upon that semaphore
end render loop

It should be the case that when the acquire fence is signaled the presentation must have finished, and as such the work submitted prior to the presentation (to the same image index) must also have finished. Therefore, resources used by that submission should be marked as not being in flight (command buffers etc).

@karl-lunarg karl-lunarg added this to the P1 milestone May 14, 2018
@karl-lunarg
Copy link
Contributor Author

Comment by mark-lunarg (MIGRATED)
Friday Dec 02, 2016 at 14:41 GMT


Maybe @chrisforbes might have a go at this one?

@karl-lunarg karl-lunarg added the Bug Something isn't working label May 14, 2018
@karl-lunarg
Copy link
Contributor Author

Comment by chrisforbes (MIGRATED)
Wednesday Dec 07, 2016 at 18:41 GMT


@mark-lunarg Yes, I'll take care of this.

@mark-lunarg
Copy link
Contributor

Waiting on the acquire fence is not a guarantee that the submitted work has been completed, even on the same image index. The only way to guarantee this is to wait on the signaled fence from the QueueSubmit().

@kvark
Copy link

kvark commented Mar 25, 2020

@mark-lunarg so this resolution comes in conflict with the original assumption by @karl-lunarg

It should be the case that when the acquire fence is signaled the presentation must have finished, and as such the work submitted prior to the presentation (to the same image index) must also have finished. Therefore, resources used by that submission should be marked as not being in flight (command buffers etc).

Can you share a bit more background on why this decision was made? Perhaps, there are examples of work by the driver that could assist in a more intuitive understanding of the issue. What does it mean now that the fence for acquire_image is ready?

@mark-lunarg
Copy link
Contributor

The original assumption wasn't by Mr. Schultz, but by a user of the validation layers who had a question soon after Vulkan was initially released. @jzulauf-lunarg is the Vulkan synchronization expert, whom I relied upon for the short update above. John, might you be able to add a bit more detail to my thin comment above?

@kvark
Copy link

kvark commented Aug 21, 2020

I'd like to bump this again, since it appears to be lost. Question is:

What does it mean now that the fence for acquire_image is ready?

@mark-lunarg
Copy link
Contributor

mark-lunarg commented Aug 21, 2020

@jzulauf-lunarg, @Tony-LunarG, can you add additional information you may have?

@mark-lunarg
Copy link
Contributor

After talking to the relevant folks, it seems the assumptions made for closing this initially were not entirely valid.

Given that present could present multiple images, we don't know that the Present is done [by waiting on the fence],
until we've waited on all of the image indices, but we would know that anything Present waited on could
be retired.

So anyway, reopening this issue.

@kvark
Copy link

kvark commented Jan 7, 2021

Reading KhronosGroup/Vulkan-Docs#117 now, there is a lot of interesting detail on how the fence is different from the semaphore in the wait. It also mentions that the spec has this text fixed in 1.0.9.
Going back to the original post by @karl-lunarg it would be great to get a definite resolution on whether this is a valid render loop pseudo-code or not.

@nickkuk
Copy link

nickkuk commented Apr 13, 2021

Somewhat related question: when one could reuse binary Semaphore that signaled in vkQueueSubmit and waited in vkQueuePresentKHR?

One could

  1. wait for Fence or timeline Semaphore that signaled in vkQueueSubmit;
  2. wait for vkAcquireNextImageKHR to return the same image index, which was used in vkQueuePresentKHR before;
  3. even more: wait for Fence or timeline Semaphore that signaled in vkQueueSubmit for next iteration of the same image index;
  4. etc.

@kvark, have you encountered such an issue in gfx or wgpu?

@nickkuk
Copy link

nickkuk commented Apr 14, 2021

I've always used timeline Semaphores instead of Fences, so I've missed another option:

3-ε. wait for Fence from vkAcquireNextImageKHR that return the same image index, which was used in vkQueuePresentKHR before.

In the world without Fences ε approaches to zero and 3-ε becomes 3.

Note that in 2 I meant just vkAcquireNextImageKHR function call without Fence waiting.

@artem-lunarg
Copy link
Contributor

artem-lunarg commented Nov 17, 2023

Added test that reproduces the issue: artem-lunarg@808eba0

The test's comments explain synchronization chain that allows to use acquire fence to synchronize with one of the previous submissions.

So far I couldn't find details that suggest it's invalid to organize frame synchronization this way. Until additional evidence I assume it's correct and we need to retire corresponding submission.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working WSI Window System Integration related issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants