[PATCH 1/2] winevulkan: Allow some experimental VK extensions to be translated.
Signed-off-by: Liam Middlebrook <lmiddlebrook(a)nvidia.com> Signed-off-by: Daniel Koch <dkoch(a)nvidia.com> --- dlls/winevulkan/make_vulkan | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/dlls/winevulkan/make_vulkan b/dlls/winevulkan/make_vulkan index 76d5fa94155..201fe46d935 100755 --- a/dlls/winevulkan/make_vulkan +++ b/dlls/winevulkan/make_vulkan @@ -131,6 +131,13 @@ CORE_EXTENSIONS = [ "VK_KHR_win32_surface", ] +# Some experimental extensions are used by shipping applications so their API is extremely unlikely +# to change in a backwards-incompatible way. Allow translation of those extensions with WineVulkan. +ALLOWED_X_EXTENSIONS = [ + "VK_NVX_binary_import", + "VK_NVX_image_view_handle", +] + # Functions part of our winevulkan graphics driver interface. # DRIVER_VERSION should be bumped on any change to driver interface # in FUNCTION_OVERRIDES @@ -3095,7 +3102,7 @@ class VkRegistry(object): # Disable highly experimental extensions as the APIs are unstable and can # change between minor Vulkan revisions until API is final and becomes KHR # or NV. - if "KHX" in ext_name or "NVX" in ext_name: + if ("KHX" in ext_name or "NVX" in ext_name) and ext_name not in ALLOWED_X_EXTENSIONS: LOGGER.debug("Skipping experimental extension: {0}".format(ext_name)) skipped_exts.append(ext_name) return -- 2.31.1
Signed-off-by: Liam Middlebrook <lmiddlebrook(a)nvidia.com> Signed-off-by: Daniel Koch <dkoch(a)nvidia.com> --- dlls/winevulkan/make_vulkan | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/winevulkan/make_vulkan b/dlls/winevulkan/make_vulkan index 201fe46d935..d5cc62d2097 100755 --- a/dlls/winevulkan/make_vulkan +++ b/dlls/winevulkan/make_vulkan @@ -64,7 +64,7 @@ from enum import Enum LOGGER = logging.Logger("vulkan") LOGGER.addHandler(logging.StreamHandler()) -VK_XML_VERSION = "1.2.176" +VK_XML_VERSION = "1.2.178" WINE_VK_VERSION = (1, 2) # Filenames to create. -- 2.31.1
On Mon, 10 May 2021 at 12:29, Liam Middlebrook <lmiddlebrook(a)nvidia.com> wrote:
+# Some experimental extensions are used by shipping applications so their API is extremely unlikely +# to change in a backwards-incompatible way. Allow translation of those extensions with WineVulkan. +ALLOWED_X_EXTENSIONS = [ + "VK_NVX_binary_import", + "VK_NVX_image_view_handle", +] + I would be inclined to say those applications are abusing the system, and we shouldn't be enabling that. On the other hand, if NVIDIA is generally fine with applications/vendors doing this kind of thing, is there any point in distinguishing between NV and NVX extensions at all? I.e., in that case, we might as well just get rid of the code skipping NVX extensions, because their API is effectively going to be frozen the moment a non-zero number of applications use them.
On 5/10/21 6:10 AM, Henri Verbeet wrote:
On Mon, 10 May 2021 at 12:29, Liam Middlebrook <lmiddlebrook(a)nvidia.com> wrote:
+# Some experimental extensions are used by shipping applications so their API is extremely unlikely +# to change in a backwards-incompatible way. Allow translation of those extensions with WineVulkan. +ALLOWED_X_EXTENSIONS = [ + "VK_NVX_binary_import", + "VK_NVX_image_view_handle", +] + I would be inclined to say those applications are abusing the system, and we shouldn't be enabling that. On the other hand, if NVIDIA is generally fine with applications/vendors doing this kind of thing, is there any point in distinguishing between NV and NVX extensions at all? I.e., in that case, we might as well just get rid of the code skipping NVX extensions, because their API is effectively going to be frozen the moment a non-zero number of applications use them.
I wouldn't classify our stance as "generally fine" here. The position here certainly isn't a good one for anyone to be in. Production applications shouldn't be making use of experimental extensions (as the API normally could change with later spec versions). But given that applications have started to use these extensions the API for them can't change (at least not in a backwards-incompatible way), since we can't break those use cases. I think given the above, it makes sense to allow WineVulkan to add support for these two extensions. We shouldn't encourage applications to make use of experimental extensions in production environments in the future, but I don't see a strong benefit to holding that stance for these extensions which are de facto API stable. Thanks, Liam Middlebrook
On Mon, 10 May 2021 at 19:36, Liam Middlebrook <lmiddlebrook(a)nvidia.com> wrote:
On 5/10/21 6:10 AM, Henri Verbeet wrote:
On Mon, 10 May 2021 at 12:29, Liam Middlebrook <lmiddlebrook(a)nvidia.com> wrote:
+# Some experimental extensions are used by shipping applications so their API is extremely unlikely +# to change in a backwards-incompatible way. Allow translation of those extensions with WineVulkan. +ALLOWED_X_EXTENSIONS = [ + "VK_NVX_binary_import", + "VK_NVX_image_view_handle", +] + I would be inclined to say those applications are abusing the system, and we shouldn't be enabling that. On the other hand, if NVIDIA is generally fine with applications/vendors doing this kind of thing, is there any point in distinguishing between NV and NVX extensions at all? I.e., in that case, we might as well just get rid of the code skipping NVX extensions, because their API is effectively going to be frozen the moment a non-zero number of applications use them.
I wouldn't classify our stance as "generally fine" here. The position here certainly isn't a good one for anyone to be in. Production applications shouldn't be making use of experimental extensions (as the API normally could change with later spec versions). But given that applications have started to use these extensions the API for them can't change (at least not in a backwards-incompatible way), since we can't break those use cases.
Would it make sense to make that explicit in the extension spec? E.g. by adding a note along the lines of "Despite starting out as an experimental extension, the API has been frozen since revision <revision>."
I think given the above, it makes sense to allow WineVulkan to add support for these two extensions. We shouldn't encourage applications to make use of experimental extensions in production environments in the future, but I don't see a strong benefit to holding that stance for these extensions which are de facto API stable.
I think there are (at least) two parts to this: - On the winevulkan side, sure, but then that same reasoning works if a sufficient important (for some definition of "important") application starts using e.g. VK_NVX_multiview_per_view_attributes. At the same time, if no application we particularly care about uses a specific NVX extension, there's no particular harm in exposing it, and we might as well save ourselves the trouble of skipping NVX extensions. - On the broader Vulkan ecosystem side, the current approach has the effect that some applications/vendors can force NVIDIA to make an experimental extension de facto stable, while some other applications/vendors can't. Ultimately that seems more harmful than simply asking the offending people to please fix their application, and breaking it if they don't.
participants (2)
-
Henri Verbeet -
Liam Middlebrook