| Summary: | SDL_Vulkan_GetInstanceExtensions does not need an SDL_Window | ||
|---|---|---|---|
| Product: | SDL | Reporter: | Jeremy Ong <jeremycong> |
| Component: | video | Assignee: | Ryan C. Gordon <icculus> |
| Status: | RESOLVED FIXED | QA Contact: | Sam Lantinga <slouken> |
| Severity: | API change | ||
| Priority: | P2 | CC: | icculus, jeremycong, libsdl.org |
| Version: | 2.0.8 | ||
| Hardware: | All | ||
| OS: | All | ||
| Attachments: |
Patch to relax SDL_Vulkan_GetInstanceExtensions parameters
Revised patch accounting for feedback |
||
Retagged version found in as this limitation exists on 2.0.8 (the first version with Vulkan support I believe) To be clear: we can't change the existing API, because there are binaries in the wild that use these functions. It's not unreasonable to leave the SDL_Window argument and accept a NULL there, though, as a workaround. I think it's worth continuing to check that the window is valid (when not NULL), as it can be an early catch for when the window wasn't created with SDL_WINDOW_VULKAN, etc. CC'ing Mark Callow for his opinion on the matter. Is there ever a time we could conceivably _need_ a window here, maybe for some yet-unknown future platform? --ryan. On point 1, note that SDL_Vulkan_GetInstanceExtensions purpose is to return the platform-specific surface extensions so there is no need for programs wishing to run headless to call it. They have no use for the extensions. (Why is there any need to use SDL in this case?) On point 2, the *only* Vulkan-related thing that SDL_Window creation does is call SDL_Vulkan_LoadLibrary. If that fails you already get suitable error messages and the names of the platform-specific surface extensions are irrelevant. On point 3, see point 2. There is therefore nothing about the API that forces device selection to happen before instance creation. In fact that is not possible in Vulkan. An app. that wants to run headless can call SDL_Vulkan_LoadLibrary, ignore the surface creation functions, which is really the only help that SDL offers for Vulkan, and proceed on its merry way. I need to delve a bit more deeply into whether we could ever conceivably need a window (or a loaded Vulkan library) here. Assuming not, we can remove the SDL_Window parameter in the next major release. @mark You may want to write a program that operates equally well if there is no display-enabled gfx card attached or not. Say, a machine learning program that optionally provides UI if it can. Either way, the API is more constrained than it needs to be. In this example use case, the developer would want to create a Vulkan instance whether a window is available or not. This decision would occur AFTER the instance is created with the required WSI extensions (specifically, at the point when the program queries for existing queues and discovers that no presentation queue exists). At this point, the instance is still usable for compute work, but the step to create an SDL_Window would be skipped. There are workarounds this of course, like catching a failure to create a window, but I thought not requiring an unneeded parameter was the "right" solution. @mark @ryan If you guys want, I can remove the SDL_Window validity checks in a separate patch to keep the API the same (In reply to Jeremy Ong from comment #4) > @mark > > Say, a machine learning program > that optionally provides UI if it can. Wouldn't SDL_CreateWindow fail in this case giving enough of a clue not to bother calling SDL_Vulkan_GetInstanceExtensions? Or maybe the app would like to provide the UI via SDL rendering or OpenGL. I'll report back once I've checked whether there could ever be a need for a window parameter. I've consulted with the Vulkan WSI TSG in Khronos. Vulkan supports multiple "video devices", as SDL calls them, on a system. E.g., some Linux distributions and versions support an application using both Wayland and x11 windows. This falls out from Wayland having an x11 server for compatibility with old apps. Vulkan app's can create a surface using the matching extension. However SDL does not support this, as best as I can tell. SDL_VideoInit() sets up a global static variable that points to the SDL_VideoDevice. So a given application can use only one or the other. Unless I'm mistaken, or there are plans to allow an app to use multiple video devices, then the SDL_Window* parameter can be removed in the next major release. For now we can allow NULL as Ryan suggested. I think this is an aspect of the API we inherited when we decided to be compatible with what Tizen had done. When updating the patch, please be sure to update the SDL_Vulkan_GetInstanceExtensions doxygen comment appropriately. Thanks for the feedback @Mark. I created a second (much smaller) patch that updates the comment and only checks for a valid window if the window provided is not null. Created attachment 3288 [details]
Revised patch accounting for feedback
Sorry for the spam but I forgot to mention for the reviewer's benefit, I changed the Vulkan test case to pass NULL as the window and verified that it compiled and ran properly on Windows with no errors or warnings. I did not test other platforms but in this patch, no platform specific code was touched. Cheers (In reply to Jeremy Ong from comment #8) > Created attachment 3288 [details] > Revised patch accounting for feedback This patch is now https://hg.libsdl.org/SDL/rev/5b9895d82ebe, thanks Jeremy and Mark! --ryan. Thanks Ryan and Mark for the quick feedback and turnaround! |
Created attachment 3286 [details] Patch to relax SDL_Vulkan_GetInstanceExtensions parameters Vulkan instance extensions for WSI are statically defined per-platform, and the API should permit user querying of such extensions without needing to first construct an SDL_Window. The attached patch modifies the API accordingly which simply removes a check that the SDL_Window was initialized with the Vulkan flag everywhere. Use cases for such a change: 1. During device selection, we may not have a valid display at all. This is useful for a program that can do useful things with or without a gfx capable device (for example, a game that can operate in headless mode as a server). 2. On client devices, it may be useful to report this information in the event that something went wrong and a window cannot be initialized for whatever reason (driver bug, etc). 3. A useful abstraction is for instance creation to precede device selection, however, the API as is enforces that the window be created before the instance (instance creation requires specification of needed extensions), inverting the expected call-flow.