KhronosGroup / Vulkan-Docs

The Vulkan API Specification and related tools
Other
2.72k stars 453 forks source link

swapchain size different from wsi surface size could be not suboptimal? #1555

Closed sylware closed 2 years ago

sylware commented 3 years ago

Hi, the specs suggests that if the wsi window is resized and now is different from the swapchain size, an implementation could return VK_SUCCESS via its WSI API calls, that due to the use of the "may" word. It means a vulkan app cannot rely solely on vulkan API to know if the swapchain is still a perfect fit for the wsi window. This seems weird. Isn't the "right"(TM) thing to do as an implementation, is it "must" return VK_SUBOPTIMAL to the vulkan app as soon as the swapchain is not a "perfect fit" for the wsi window?

krOoze commented 3 years ago

I think #396 is relevant here.

critsec commented 3 years ago

The window system you're using should provide notifications when the window size changes. The VK_SUBOPTIMAL success code isn't meant to be a reliable indicatator that the swapchain and window size don't match -- it's meant to be a hint that there is some kind of mismatch (size, rotation, etc.) is likely causing something about system behavior (e.g. performance) to be worse than if they matched.

stonesthrow commented 3 years ago

We'll discuss this next week in WG agenda. @krOoze , I'll revisit - #396 and see if we can be sure the text on SUBOPTIMAL is clear.

sylware commented 3 years ago

Allright, I'll wait for your feedback then.

stonesthrow commented 3 years ago

So I will try and update spec wording ASAP. We see that the "May" in the note can be confusing. I will try to add some clarification.

sylware commented 3 years ago

I hear you. But if we have to rely solely on the WSI in order to know that the swapchain is not a "perfect fit" (for instance a size mismatch) anymore for the "window", then what's the point of the VK_SUBOPTIMAL non-error return code? Debugging? Something else? Because then, vk applications would not handle it as it brings no significant "value". Do I miss something here?

stonesthrow commented 3 years ago

I didn't have time last weekend. I will try to get to get something this week. The WG discussed and gave some guidance.

SUBOPTIMAL, the most general case being the window was resized, and the swapchain images need to be reallocated to the new size (which takes destroy old swapchain, create new). These can still be presented, allowing for a transition image or two until the new swapchain is ready and new images rendered and ready for present, thus no blank window, Or the images are scaled for the new size, and that may be acceptable to the app, even if it may look stretched. Scaling may be wanted under some scenarios.

sylware commented 3 years ago

Ok. But does it mean a vk application can rely solely on SUBOPTIMAL(and OUT_OF_DATE) to recreate a swapchain for a WSI window? Namely, a vk application, in order to keep its swapchain a "perfect fit" for WSI window would not need to listen to WSI events. Because the usage of "may" implies SUBOPTIMAL "may" not be enough to keep the swapchain as a "perfect fit" for a WSI window. In other words, this is very important for vk applications, because if it was "must", those very applications would not try to "guess" when the swapchain needs recreation based on WSI events and only rely on the vk API, that would be way more consistent.

stonesthrow commented 3 years ago

That note is not spec normative text. The "may" is only for SUBOPTIMAL case to tell you that AcquireNextImage can return SUBOPTIMAL and explain why. I will try and clear this up.

So, I think we recommend that your app listen for events from the window system telling your app that the window has changed. That way you can resize your Swapchain then; If you wait for a OUT_OF_DATE/SUBOPTIMAL, then you have an Image you need to return.

krOoze commented 3 years ago

I am getting lost in the answer.

So, which one is true:

a) it is conformant that swapchain becomes suboptimal due to surface dimensions, and the driver does not return VK_SUBOPTIMAL_KHR and pretend everything is peachy with VK_SUCCESS b) if swapchain becomes suboptimal due to surface dimensions, driver rfc-must return VK_SUBOPTIMAL_KHR


Additionally it would help to unambiguously clarify how it differs across the Vulkan WSI surface platforms. (e.g. the Google platform says VK_SUBOPTIMAL_KHR will be returned, which is inconsistent with the "may return". And additionally I think win32 does not ever have reasons to return it and always give OUT_OF_DATE instead? ).

stonesthrow commented 3 years ago

b. This is what it should be. The Notes are there just to provide some information. The spec like language that says May do this, or Must do this, needs rewrite and not be in notes. The real spec language must say what the API will do/what will be returned. It needs to be definitive.

I'll get some of the platform people to revisit those sections. It is possible that platform specific extensions will have platform specific differences - that's some of the complexity of WSI, can't be helped.

sylware commented 3 years ago

I guess the specs should say something like: vulkan "must" return SUBOPTIMAL if the swapchain is not a perfect fit for the WSI window. Because to let vk implementations return SUCCESS while the swapchain is not anymore a "perfect fit" for the WSI window seems somewhat broken and weird. If vk apps must rely solely on WSI events to "guess" that a swapchain is not anymore a "perfect fit" for a WSI window, then SUBOPTIMAL is useless, or at best can help detect bugs in vk/WSI implementations. :'(

stonesthrow commented 2 years ago

I'll check that. Most implementation are going to give you a OUT_OF_DATE. The SUBOPTIMAL will be just a few implementation that know that the suboptimal is still presentable.

sylware commented 2 years ago

Huh? That would mean we must rely only on WSI events to detect a swapchain is not anymore a "perfect fit" for a WSI window.

baw... it seems detection using only SUBOPTIMAL would be much more cleaner, but the specs must be updated and clarified for this.

Keep us in the loop on what was decided by the WG. Then we can fix mesa the right way...

sylware commented 2 years ago

ping, any news on the matter?

stonesthrow commented 2 years ago

Sorry, I've been out suffering from dental issues. The plan is:

The note is going to be changed so that "May" is not highlighted as normative language. The the note will just give example/why you might get SUBOPTIMAL - just informative.

The paragraph above it need s some wordsmithing to read better. That SUBOPTIMAL is a possible return result. And I may further explain that condition, that it is presentable, but suboptimal, as in possibly scaled to fit the new size.

sylware commented 2 years ago

Then if I understand well, a vulkan application must rely on WSI events and SUBOPTIMAL and OUT_OF_DATE to detected a swapchain is not a "perfect fit" anymore for the WSI window. Or there is still a plan to 100% tie this detection to SUBOPTIMAL and OUT_OF_DATE without requiring to listen to WSI events?

krOoze commented 2 years ago

You can rely on OUT_OF_DATE. That was hopefully never in question. Pretty much what should always happen on Windows and X wrt surface size.

Above I was answered you can rely on SUBOPTIMAL, so hopefully that is still true and will end up in strong spec wording.

On my driver it seems though the error is always deferred to vkQueuePresentKHR (and never returned from vkAcquireNextImageKHR). That behavior should also be noted in spec.

That being said, there might be benefits to using platform events.

sylware commented 2 years ago

well, I dare to think that the consistent and clean way would be to rely only on SUBOPTIMAL and OUT_OF_DATE for a vulkan application to detect a swapchain is not anymore a perfect fit for a WSI window. But as you said, that should become strong spec wording. That said, already there vulkan applications which use the WSI to try to detect that their swapchain is not anymore a perfect fit for a WSI window, won't see any difference. So it seems to be a benign and good fix for the specs and vulkan. But vulkan WG guys of the various WSIs must have a look at it though, coze some the nasty devil could be hidding in those details.

cubanismo commented 2 years ago

On my driver it seems though the error is always deferred to vkQueuePresentKHR (and never returned from vkAcquireNextImageKHR). That behavior should also be noted in spec.

While allowed, this is not currently required behavior, and is not universally the case in deployed implementations, so we can't currently strengthen the spec here. It is worth considering for an extension though.

More generally, since there is an inherent race between window system events occurring and any client-side application code, Vulkan applications included, noticing & handling them, there is no way to guarantee OUT_OF_DATE or SUBOPTIMAL will be returned within finite time after a window's geometry or other properties have changed. Keep in mind the presentation operation itself is not required to occur during the vkQueuePresentKHR() call (as a queue operation, it is inherently enqueued rather than executed immediately), nor is it even required to occur on the Vulkan queue. It occurs asynchronously in the "presentation engine," a rather loosely defined term referring to whatever mechanism puts images some place the user can see them given the window system binding in use. While some implementations on some windowing systems can pre-validate that a given present operation will or will not succeed during the vkQueuePresentKHR() call, this is not true in general.

However, something to the effect that OUT_OF_DATE or SUBOPTIMAL must be returned if any prior presents up to and including the current vkQueuePresentKHR() are known to have failed to produce "reasonable" results (where the definition of reasonable would have to be further refined by window-system specific language, and might be very broad for some windowing systems), which could probably be tied in with wording refining when an implementation is required to know about failures. Something like if you ever present swapchain image (i) and then get it back in vkAcquireNextImageKHR() (which itself is never guaranteed to occur in general, but happens in finite time for at least some subset of the swapchain images on reasonable implementations), you must: know by that point whether presentation of (i) succeeded.

krOoze commented 2 years ago

On my driver it seems though the error is always deferred to vkQueuePresentKHR (and never returned from vkAcquireNextImageKHR). That behavior should also be noted in spec.

While allowed, this is not currently required behavior, and is not universally the case in deployed implementations, so we can't currently strengthen the spec here. It is worth considering for an extension though.

The surprising part here though was the vkAcquireNextImageKHR never checks for it. I was thinking more along "weakening" the spec. It was not immediately clear it is allowed for the implementation to just skip those checks. In fact, the spec says:

If the swapchain images no longer match native surface properties, either VK_SUBOPTIMAL_KHR or VK_ERROR_OUT_OF_DATE_KHR must be returned.

Which is pretty close to disallowing it.

sylware commented 2 years ago

If I understand well, some WSI with some vulkan implementations out there do not check a swapchain is a perfect fit for a WSI window on "acquiring" and "queueing for presentation" operations, well it is not mandated by the specs. It means those vulkan implementations would be sort of passive, for instance only letting the WSI know of a new pointer of the swapchain pixel buffer (out of bounds buffer access would not be a pb for those implementations), which, in the best scenario might give a chance to the WSI to tell the vulkan implementation that some major event did happen, hence OUT_OF_DATE.

Ok, that is the devil I was talking about.

It means vulkan applications must rely on the WSI to detect a swapchain is not anymore a "perfect fit" for a WSI window. Ofc, some vulkan implementations "could" be tied more closely to a WSI and return OUT_OF_DATE for major issues, or SUBOPTIMAL if rendering was weird but kind of done. Basically speaking a vulkan implementation could be really, really loosely coupled to a WSI (I bet some android super far away, electronically speaking, framebuffer). That should be indeed clarified in the specs: due to the above reasons, vulkan applications must rely on WSI "events" and OUT_OF_DATE and SUBOPTIMAL in order to guess their swapchain is not "good" anymore for a WSI window.

stonesthrow commented 2 years ago

Version 1.2.185 has new text about VK_SUBOPTIMAL_EXT

stonesthrow commented 2 years ago

I will close this unless someone feels the latest spec does not resolve the spec text sufficiently.

krOoze commented 2 years ago

It is inconsistent with what I have been told above by you. You told me the return code is mandatory for the driver (rfc-must), but the spec is saying it is optional (rfc-may).

stonesthrow commented 2 years ago

OK, I'll take another pass.

sylware commented 2 years ago

specs 1.2.189

I read: "If the swapchain images no longer match native surface properties, either VK_SUBOPTIMAL_KHR or VK_ERROR_OUT_OF_DATE_KHR must be returned." in vkAcquireNextImageKHR section.

Then now it is clear: a vulkan application can rely solely on SUBOPTIMAL and OUT_OF_DATE for to keep its swapchain a "perfect fit" for a WSI window.

Now I need to tell the mesa guys.

A big thx to all.

krOoze commented 2 years ago

@sylware That was there before actually since 1.0.69

In 1.2.185 only this was added:

The implementation may return VK_SUBOPTIMAL_KHR if the swapchain no longer matches the surface properties exactly, but can still be used for presentation.

stonesthrow commented 2 years ago

I don't know where the recent changes came from.

I think "may" in this case is the window system/compositor/ PE - MAY have the functionality to deal with swapchain not matching surface property changes in the sense only a few PE will have the capability to still present, most PE will not have this capability and return OUT_OF_DATE. So the ambiguity is in ability, I think an app needs to expect OUT_OF_DATE, but there may be a platform, Android is the only one I think does this, that can return SUBOPTIMAL. If that SUBOPTIMAL is usable by your app, then include that. If not useful to your app then consider OUT_OF_DATE and SUBOPTIMAL the same - recreate the swapchain.

I'd like to rework the spec text that would be clear for you and others. Feedback appreciated

sylware commented 2 years ago

Why 1.2.185???

In 1.2.189, "If the swapchain images no longer match native surface properties, either VK_SUBOPTIMAL_KHR or VK_ERROR_OUT_OF_DATE_KHR must be returned."

stonesthrow commented 2 years ago

It was a change by someone else and I was not able to find the source.

sylware commented 2 years ago

I don't understand? Which one is correct: 1.2.185 or 1.2.189?

stonesthrow commented 2 years ago

In 1.2.189, "If the swapchain images no longer match native surface properties, either VK_SUBOPTIMAL_KHR or VK_ERROR_OUT_OF_DATE_KHR must be returned."

krOoze commented 2 years ago

@sylware I don't understand the question. AFAIK, there was no relevant change since 185.

sylware commented 2 years ago

Version 1.2.189, I read ""If the swapchain images no longer match native surface properties, either VK_SUBOPTIMAL_KHR or VK_ERROR_OUT_OF_DATE_KHR must be returned." which is right. Then the specs is fixed now I guess.

stonesthrow commented 2 years ago

Is it OK to close this? if no other clarification needed.

sylware commented 2 years ago

On my side, I think we can close the issue. Everything is sound and clear for the vulkan API implementers and vulkan application developers.

stonesthrow commented 2 years ago

Closing with latest spec language as answer.

krOoze commented 2 years ago

I mean, I think nothing has changed since the Issue was opened. The only change that did happen only made it more ambiguous.