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

VUID 04451 Introduces new validation errors for all apps running on MoltenVK (and any other portability platform) #14

Closed
ncesario-lunarg opened this issue Dec 11, 2020 · 15 comments

Comments

@ncesario-lunarg
Copy link

Due to the nature of VUID 04451, any existing app running on a portability platform (e.g., MoltenVK, gfx-rs) running validation will encounter new validation errors. This issue is to confirm that this is indeed the intended behavior, which will require all of the aforementioned apps to update.

While this makes sense functionally (as the portability validation will not occur if the extension is not enabled), we were surprised by the new error, which means that other folks will almost certainly be surprised as well. Perhaps some additional messaging or communication would be helpful to ensure developers targeting a portability platform are better prepared for this change?

For reference, VUID 04451 states:

If the VK_KHR_portability_subset extension is included in pProperties of
vkEnumerateDeviceExtensionProperties, ppEnabledExtensions must include
"VK_KHR_portability_subset".

FYI @mark-lunarg @karl-lunarg

@krOoze

This comment has been minimized.

@ncesario-lunarg
Copy link
Author

From my understanding this is expected behavior. If a driver exposes VK_KHR_portability_subset, it is telling the app (again, from my understanding), it does not adhere to the vulkan spec proper, and the app must enable the extension to effectively say, "yes, I'm aware this is not a proper Vulkan implementation I'm running on, and I will make the appropriate queries to make sure I don't crash."

In the case of vkcube, it was only impacted on platforms that expose VK_KHR_portability_subset (e.g., where we encountered it, MoltenVK), and I believe that is the expected behavior.

There are others (@billhollings?) who can explain this far better than I can, though.

@richard-lunarg
Copy link

Just an FYI, vkCube was updated recently to show the proper way to query for these extensions and no longer throws this validation error itself.

@krOoze

This comment has been minimized.

@richard-lunarg
Copy link

I do not want it to break all my games and stuff, right?

You only fail validation. The app does not crash or terminate, and typically you would not want the validation layer loaded during actual game play anyway.

@krOoze

This comment has been minimized.

@richard-lunarg
Copy link

If I understand correctly, you are asking about an existing working app on a portability platform, such as macOS. The addition of the portability extension will not break those apps in and of itself. The extension is meant to help you by advertising that your app targets, or is expected to run well on a non-conformant platform. The validation layers can now be better used to inform you of any mistakes you have made.

If you are bringing a new app to macOS, it may well indeed not work as expected, and the new validation tools should help you zero in on what those issues are. You can use this in conjunction with the device simulation layer (even on Windows) to find out where you have tried to make use of a feature that is not available on the target platform or have exceeded limits that are inherent for that environment.

Again, an already working app (such as vkcube or a shipping game), will not be broken by this. But it may help you find bugs you did not realize you had previously (and this is only when the validation layers are loaded, which is not typical for an end user scenario). It is just your way of letting the loader know that you are paying attention to these issues. If you do not use the extension it is a flag that your Vulkan code "may" not work as expected on this platform, and it's purpose is to alert the developer that they need to do so if they haven't already.

@krOoze

This comment has been minimized.

@billhollings
Copy link
Contributor

The purpose of VK_KHR_portability_subset is to highlight an implementation that is not fully conformant, and to identify in what way that is so.

Based on the design of that extension, if a (possibly) non-conformant implementation is selected, and the app ignores the VK_KHR_portability_subset by not enabling it, validation will flag it (and technically the implementation itself should return an initialization error, although currently MoltenVK does not).

@krOoze Presumably you are looking for a mechanism within the loader to ensure that if VK_KHR_portability_subset is not enabled, and a fully-conformant option exists at the physical device level, that only it will be used?

@richard-lunarg
Copy link

richard-lunarg commented Mar 1, 2021

@krOoze Okay, you are right, and I think I see where you are coming from now.

#1 You can enumerate the physical devices, but you might or might not get a conformant device.

#2 Now, we have a device extension. You can look at the devices and see if it exposes the portability extension, and you'll know the physical device has limits and can insist in code that it only run on a truly conformant physical device. This is meant to solve the problem of #1.

#3 You think this was not the best approach. Better to simply not list any non-conformant physical devices at all unless explicitly asked for. Because... an existing game on Windows may break, if for example a new non-conformant implementation becomes available on Windows, and the game's physical device selection logic selects that new device without knowing that it is not conformant.

This is a legitimate objection I think. I'm not qualified to argue either approach or why the current one was made. But at least "I" understand your issue now.

@krOoze

This comment has been minimized.

@billhollings
Copy link
Contributor

billhollings commented Mar 1, 2021

@krOoze I interpret the issue you are raising (selecting between avaialble conformant and non-conformant options) as different from the original issue (legacy apps being confronted by a new and unexpected validation error).

If you want to summarize the discussion into a separate issue, that would be helpful. Otherwise, I can do that.

And thanks for raising the issue you have!

I also agree that VK_KHR_portability_subset being a device extension needs to be addressed vis-a-vis determining how to select an implementation at the instance level based on expectations of conformance.

@krOoze

This comment has been minimized.

@billhollings
Copy link
Contributor

Alright, it is now Issue #18.

Thanks!

@KarenGhavam-lunarG
Copy link

This issue can now be closed. With the implementation of the VK_KHR_portability_enumeration extension in the Vulkan loader, Vulkan applications will no longer get unexpected validation errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants