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.

Sign up for GitHub

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

Some more suspicious swapchain rfc keyword usage #1680

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Some more suspicious swapchain rfc keyword usage #1680

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
0329fb3
Update wsi.txt
stonesthrow Nov 8, 2021
5c24ec7
Update swapchain_counters.txt
stonesthrow Nov 8, 2021
41150d5
Update wsi.txt
stonesthrow Nov 8, 2021
e36c231
vkGetSwapchainStatusKHR codes
stonesthrow Dec 6, 2021
f12c413
Removed as-usual text
stonesthrow Dec 6, 2021
32416d0
Merge branch 'KhronosGroup:main' into Some-more-suspicious-swapchain-…
stonesthrow Dec 6, 2021
6cdf707
Change Status code line
stonesthrow Feb 4, 2022
b64bd1f
Reword return from retired oldSwapchain on vkQueuePresent
stonesthrow Feb 4, 2022
af04900
Merge branch 'KhronosGroup:main' into Some-more-suspicious-swapchain-…
stonesthrow Jun 17, 2022
ceb4ba8
Merge branch 'KhronosGroup:main' into Some-more-suspicious-swapchain-…
stonesthrow Jul 11, 2022
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
2 changes: 1 addition & 1 deletion chapters/VK_EXT_display_control/swapchain_counters.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ include::{generated}/api/protos/vkGetSwapchainCounterEXT.txt[]
* pname:pCounterValue will return the current value of the counter.

If a counter is not available because the swapchain is out of date, the
implementation may: return ename:VK_ERROR_OUT_OF_DATE_KHR.
implementation must: return ename:VK_ERROR_OUT_OF_DATE_KHR.
Copy link

Choose a reason for hiding this comment

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

When we drafted the OUT_OF_DATE language, some IHVs were uncomfortable with requiring errors in states that are defined as OUT_OF_DATE, and carry on as if things are fine instead. This language is fine with me, but other driver vendors may still object to such requirements.

Copy link
Contributor Author

@stonesthrow stonesthrow Jan 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

OK. Good history. The issue by is KrO0ze identifying non-exact language. Do we know who objected? see if they reaffirm stance or not.

Copy link
Contributor

@krOoze krOoze Jan 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Lot of code relies on mustiness of this. So if you preserve "may", it needs some serious acompanying note explaining what exactly that implies as well as what is the correct way to deal with swapchain state.

I can imagine though the failure could be allowed to be deferred. E.g. acquire pretends success, and then it fails at present. Still needs to be copiously noted in spec if that can happen and explicitly how far the driver can push that leeway.

Copy link
Contributor Author

@stonesthrow stonesthrow Feb 4, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@cubanismo - lets see if we can craft some text that will work here.

Copy link
Contributor

@krOoze krOoze Jul 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure if resolved per @cubanismo comment, but I certainly am happy with "must"...


.Valid Usage
****
Expand Down
3 changes: 1 addition & 2 deletions chapters/VK_KHR_shared_presentable_image/wsi.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ include::{generated}/api/protos/vkGetSwapchainStatusKHR.txt[]
include::{generated}/validity/protos/vkGetSwapchainStatusKHR.txt[]
--

The possible return values for fname:vkGetSwapchainStatusKHR should: be
interpreted as follows:
Status codes returned by flink:vkGetSwapchainStatusKHR have the following meaning:

* ename:VK_SUCCESS specifies the presentation engine is presenting the
contents of the shared presentable image, as per the swapchain's
Expand Down
6 changes: 1 addition & 5 deletions chapters/VK_KHR_swapchain/wsi.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,11 +332,7 @@ outnumber calls to flink:vkDestroySwapchainKHR.

After pname:oldSwapchain is retired, the application can: pass to
flink:vkQueuePresentKHR any images it had already acquired from
pname:oldSwapchain.
E.g., an application may present an image from the old swapchain before an
image from the new swapchain is ready to be presented.
As usual, flink:vkQueuePresentKHR may: fail if pname:oldSwapchain has
entered a state that causes ename:VK_ERROR_OUT_OF_DATE_KHR to be returned.
pname:oldSwapchain. flink:vkQueuePresentKHR must: return ename:VK_ERROR_OUT_OF_DATE_KHR.
Copy link
Contributor

@krOoze krOoze Jul 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This seems wrong, and contrary to the original text.


ifdef::VK_KHR_shared_presentable_image[]
The application can: continue to use a shared presentable image obtained
Expand Down