Why does querying for required Vulkan extensions require an SDL_Window?

ninepoints · August 20, 2018, 11:13am

I may have missed something but this seems like a misstep in the API design.

A Vulkan instance must be created only after the required extensions/layers are supplied. Given that the relevant swapchain/presentation extensions are statically defined per-platform, SDL_VulkanGetInstnaceExtensions should not require that an SDL_Window already exists. This is important, for example, in multi-window applications that don’t necessarily need to spawn a window at the start, but need a working Vulkan instance for compute or some other task (printing diagnostic information for example). AFAIK, the GLFW API does not have this issue.

msb · August 20, 2018, 2:44pm

I may have missed something but this seems like a misstep in the API
design.

A Vulkan instance must be created only after the required
extensions/layers are supplied. Given that the relevant
swapchain/presentation extensions are statically defined
per-platform, SDL_VulkanGetInstnaceExtensions should not require that
an SDL_Window already exists.

I had a cursory glance at the SDL sources and it seems you are right.
None of the actual platform functions seem to actually use the window
parameter. Unfortunately SDL_Vulkan_GetInstanceExtensions() checks the
validity of the window parameter before passing the call on to the
platform function, so you can’t pass NULL.

This is important, for example, in
multi-window applications that don’t necessarily need to spawn a
window at the start, but need a working Vulkan instance for compute
or some other task (printing diagnostic information for example).
AFAIK, the GLFW API does not have this issue.

Is there a reason why you can’t just create a window with
SDL_WINDOW_HIDDEN flag?

MSB

ninepoints · August 21, 2018, 12:33am

HG changeset patch

User jeremyong

Date 1534766019 25200

Mon Aug 20 04:53:39 2018 -0700

Node ID 61a0d2c2ea549fbc1d07f62e2f454078ec847fef

Parent 4358e537000a3d15db80a276afd9cfd464d7619f

Modify all instances of Vulkan_GetInstanceExtensions to not require an initialized SDL_Window

Retrieving instance extensions prior to first window creation is valuable for certain types of applications.
This commit relaxes the API by removing SDL_Window from the signature altogether. It also removes checks that
determined if the window was initialized with the SDL_WINDOW_VULKAN_FLAG.

diff -r 4358e537000a -r 61a0d2c2ea54 include/SDL_vulkan.h
— a/include/SDL_vulkan.h Sat Aug 18 12:30:04 2018 +0100
+++ b/include/SDL_vulkan.h Mon Aug 20 04:53:39 2018 -0700
@@ -135,8 +135,6 @@

\brief Get the names of the Vulkan instance extensions needed to create

    a surface with \c SDL_Vulkan_CreateSurface().

- \param [in] window Window for which the required Vulkan instance
- ```
             extensions should be retrieved
```
- \param [in,out] count pointer to an \c unsigned related to the number of
- ```
             required Vulkan instance extensions
```
- \param [out] names \c NULL or a pointer to an array to be filled with the
  @@ -160,12 +158,10 @@
- \note The extension names queried here must be enabled when calling
- ```
   VkCreateInstance, otherwise surface creation will fail.
```
- \note \c window should have been created with the \c SDL_WINDOW_VULKAN flag.
- \code
- unsigned int count;
- // get count of required extensions
- if(!SDL_Vulkan_GetInstanceExtensions(window, &count, NULL))

- if(!SDL_Vulkan_GetInstanceExtensions(&count, NULL))
- ```
 handle_error();
```
- static const char *const additionalExtensions[] =
  @@ -179,7 +175,7 @@
- ```
 handle_error();
```
- // get names of required extensions

- if(!SDL_Vulkan_GetInstanceExtensions(window, &count, names))

- if(!SDL_Vulkan_GetInstanceExtensions(&count, names))
- ```
 handle_error();
```
- // copy additional extensions after required extensions
  @@ -200,7 +196,6 @@
- \sa SDL_Vulkan_CreateSurface()
  */
  extern DECLSPEC SDL_bool SDLCALL SDL_Vulkan_GetInstanceExtensions(

  												SDL_Window *window,
  												unsigned int *pCount,
  												const char **pNames);

diff -r 4358e537000a -r 61a0d2c2ea54 src/dynapi/SDL_dynapi_procs.h
— a/src/dynapi/SDL_dynapi_procs.h Sat Aug 18 12:30:04 2018 +0100
+++ b/src/dynapi/SDL_dynapi_procs.h Mon Aug 20 04:53:39 2018 -0700
@@ -666,7 +666,7 @@
SDL_DYNAPI_PROC(int,SDL_Vulkan_LoadLibrary,(const char a),(a),return)
SDL_DYNAPI_PROC(void,SDL_Vulkan_GetVkGetInstanceProcAddr,(void),(),return)
SDL_DYNAPI_PROC(void,SDL_Vulkan_UnloadLibrary,(void),(),)
-SDL_DYNAPI_PROC(SDL_bool,SDL_Vulkan_GetInstanceExtensions,(SDL_Window *a, unsigned int *b, const char **c),(a,b,c),return)
+SDL_DYNAPI_PROC(SDL_bool,SDL_Vulkan_GetInstanceExtensions,(unsigned int *a, const char **b),(a,b),return)
SDL_DYNAPI_PROC(SDL_bool,SDL_Vulkan_CreateSurface,(SDL_Window *a, VkInstance b, VkSurfaceKHR *c),(a,b,c),return)
SDL_DYNAPI_PROC(void,SDL_Vulkan_GetDrawableSize,(SDL_Window *a, int *b, int *c),(a,b,c),)
SDL_DYNAPI_PROC(void,SDL_LockJoysticks,(void),(),)
diff -r 4358e537000a -r 61a0d2c2ea54 src/video/SDL_sysvideo.h
— a/src/video/SDL_sysvideo.h Sat Aug 18 12:30:04 2018 +0100
+++ b/src/video/SDL_sysvideo.h Mon Aug 20 04:53:39 2018 -0700
@@ -269,7 +269,7 @@
*/
int (*Vulkan_LoadLibrary) (_THIS, const char *path);
void (*Vulkan_UnloadLibrary) (_THIS);

SDL_bool (*Vulkan_GetInstanceExtensions) (_THIS, SDL_Window *window, unsigned *count, const char **names);

SDL_bool (*Vulkan_GetInstanceExtensions) (_THIS, unsigned *count, const char **names);
SDL_bool (*Vulkan_CreateSurface) (_THIS, SDL_Window *window, VkInstance instance, VkSurfaceKHR *surface);
void (*Vulkan_GetDrawableSize) (_THIS, SDL_Window * window, int *w, int *h);

diff -r 4358e537000a -r 61a0d2c2ea54 src/video/SDL_video.c
— a/src/video/SDL_video.c Sat Aug 18 12:30:04 2018 +0100
+++ b/src/video/SDL_video.c Mon Aug 20 04:53:39 2018 -0700
@@ -4082,21 +4082,14 @@
}
}

-SDL_bool SDL_Vulkan_GetInstanceExtensions(SDL_Window *window, unsigned *count, const char **names)
+SDL_bool SDL_Vulkan_GetInstanceExtensions(unsigned *count, const char **names)
{

CHECK_WINDOW_MAGIC(window, SDL_FALSE);
if (!(window->flags & SDL_WINDOW_VULKAN)) {
```
   SDL_SetError(NOT_A_VULKAN_WINDOW);
```
```
   return SDL_FALSE;
```
}
if (!count) {
SDL_InvalidParamError(“count”);
return SDL_FALSE;
}
return _this->Vulkan_GetInstanceExtensions(_this, window, count, names);

return _this->Vulkan_GetInstanceExtensions(_this, count, names);
}

SDL_bool SDL_Vulkan_CreateSurface(SDL_Window *window,
diff -r 4358e537000a -r 61a0d2c2ea54 src/video/android/SDL_androidvulkan.c
— a/src/video/android/SDL_androidvulkan.c Sat Aug 18 12:30:04 2018 +0100
+++ b/src/video/android/SDL_androidvulkan.c Mon Aug 20 04:53:39 2018 -0700
@@ -110,18 +110,12 @@
}

SDL_bool Android_Vulkan_GetInstanceExtensions(_THIS,

                                     SDL_Window *window,
                                     unsigned *count,
                                     const char **names)

{
static const char *const extensionsForAndroid[] = {
VK_KHR_SURFACE_EXTENSION_NAME, VK_KHR_ANDROID_SURFACE_EXTENSION_NAME
};

if(!_this->vulkan_config.loader_handle)
{

   SDL_SetError("Vulkan is not loaded");

```
   return SDL_FALSE;
```
}
return SDL_Vulkan_GetInstanceExtensions_Helper(
count, names, SDL_arraysize(extensionsForAndroid),
extensionsForAndroid);
diff -r 4358e537000a -r 61a0d2c2ea54 src/video/android/SDL_androidvulkan.h
— a/src/video/android/SDL_androidvulkan.h Sat Aug 18 12:30:04 2018 +0100
+++ b/src/video/android/SDL_androidvulkan.h Mon Aug 20 04:53:39 2018 -0700
@@ -37,7 +37,6 @@
int Android_Vulkan_LoadLibrary(_THIS, const char *path);
void Android_Vulkan_UnloadLibrary(_THIS);
SDL_bool Android_Vulkan_GetInstanceExtensions(_THIS,

                                     SDL_Window *window,
                                     unsigned *count,
                                     const char **names);

SDL_bool Android_Vulkan_CreateSurface(_THIS,
diff -r 4358e537000a -r 61a0d2c2ea54 src/video/cocoa/SDL_cocoavulkan.h
— a/src/video/cocoa/SDL_cocoavulkan.h Sat Aug 18 12:30:04 2018 +0100
+++ b/src/video/cocoa/SDL_cocoavulkan.h Mon Aug 20 04:53:39 2018 -0700
@@ -38,7 +38,6 @@
int Cocoa_Vulkan_LoadLibrary(_THIS, const char *path);
void Cocoa_Vulkan_UnloadLibrary(_THIS);
SDL_bool Cocoa_Vulkan_GetInstanceExtensions(_THIS,

                                     SDL_Window *window,
                                     unsigned *count,
                                     const char **names);

SDL_bool Cocoa_Vulkan_CreateSurface(_THIS,
diff -r 4358e537000a -r 61a0d2c2ea54 src/video/cocoa/SDL_cocoavulkan.m
— a/src/video/cocoa/SDL_cocoavulkan.m Sat Aug 18 12:30:04 2018 +0100
+++ b/src/video/cocoa/SDL_cocoavulkan.m Mon Aug 20 04:53:39 2018 -0700
@@ -167,17 +167,12 @@
}

SDL_bool Cocoa_Vulkan_GetInstanceExtensions(_THIS,

                                     SDL_Window *window,
                                     unsigned *count,
                                     const char **names)

{
static const char *const extensionsForCocoa[] = {
VK_KHR_SURFACE_EXTENSION_NAME, VK_MVK_MACOS_SURFACE_EXTENSION_NAME
};

if (!_this->vulkan_config.loader_handle) {

   SDL_SetError("Vulkan is not loaded");

```
   return SDL_FALSE;
```
}
return SDL_Vulkan_GetInstanceExtensions_Helper(
count, names, SDL_arraysize(extensionsForCocoa),
extensionsForCocoa);
diff -r 4358e537000a -r 61a0d2c2ea54 src/video/mir/SDL_mirvulkan.c
— a/src/video/mir/SDL_mirvulkan.c Sat Aug 18 12:30:04 2018 +0100
+++ b/src/video/mir/SDL_mirvulkan.c Mon Aug 20 04:53:39 2018 -0700
@@ -110,18 +110,12 @@
}

SDL_bool MIR_Vulkan_GetInstanceExtensions(_THIS,

                                     SDL_Window *window,
                                     unsigned *count,
                                     const char **names)

{
static const char *const extensionsForMir[] = {
VK_KHR_SURFACE_EXTENSION_NAME, VK_KHR_MIR_SURFACE_EXTENSION_NAME
};

if(!_this->vulkan_config.loader_handle)
{

   SDL_SetError("Vulkan is not loaded");

```
   return SDL_FALSE;
```
}
return SDL_Vulkan_GetInstanceExtensions_Helper(
count, names, SDL_arraysize(extensionsForMir),
extensionsForMir);
diff -r 4358e537000a -r 61a0d2c2ea54 src/video/mir/SDL_mirvulkan.h
— a/src/video/mir/SDL_mirvulkan.h Sat Aug 18 12:30:04 2018 +0100
+++ b/src/video/mir/SDL_mirvulkan.h Mon Aug 20 04:53:39 2018 -0700
@@ -37,7 +37,6 @@
int MIR_Vulkan_LoadLibrary(_THIS, const char *path);
void MIR_Vulkan_UnloadLibrary(_THIS);
SDL_bool MIR_Vulkan_GetInstanceExtensions(_THIS,

                                     SDL_Window *window,
                                     unsigned *count,
                                     const char **names);

SDL_bool MIR_Vulkan_CreateSurface(_THIS,
diff -r 4358e537000a -r 61a0d2c2ea54 src/video/uikit/SDL_uikitvulkan.h
— a/src/video/uikit/SDL_uikitvulkan.h Sat Aug 18 12:30:04 2018 +0100
+++ b/src/video/uikit/SDL_uikitvulkan.h Mon Aug 20 04:53:39 2018 -0700
@@ -37,7 +37,6 @@
int UIKit_Vulkan_LoadLibrary(_THIS, const char *path);
void UIKit_Vulkan_UnloadLibrary(_THIS);
SDL_bool UIKit_Vulkan_GetInstanceExtensions(_THIS,

                                     SDL_Window *window,
                                     unsigned *count,
                                     const char **names);

SDL_bool UIKit_Vulkan_CreateSurface(_THIS,
diff -r 4358e537000a -r 61a0d2c2ea54 src/video/uikit/SDL_uikitvulkan.m
— a/src/video/uikit/SDL_uikitvulkan.m Sat Aug 18 12:30:04 2018 +0100
+++ b/src/video/uikit/SDL_uikitvulkan.m Mon Aug 20 04:53:39 2018 -0700
@@ -155,17 +155,12 @@
}

SDL_bool UIKit_Vulkan_GetInstanceExtensions(_THIS,

                                     SDL_Window *window,
                                     unsigned *count,
                                     const char **names)

{
static const char *const extensionsForUIKit[] = {
VK_KHR_SURFACE_EXTENSION_NAME, VK_MVK_IOS_SURFACE_EXTENSION_NAME
};

if (!_this->vulkan_config.loader_handle) {

   SDL_SetError("Vulkan is not loaded");

```
   return SDL_FALSE;
```
}

return SDL_Vulkan_GetInstanceExtensions_Helper(
count, names, SDL_arraysize(extensionsForUIKit),
diff -r 4358e537000a -r 61a0d2c2ea54 src/video/wayland/SDL_waylandvulkan.c
— a/src/video/wayland/SDL_waylandvulkan.c Sat Aug 18 12:30:04 2018 +0100
+++ b/src/video/wayland/SDL_waylandvulkan.c Mon Aug 20 04:53:39 2018 -0700
@@ -110,18 +110,13 @@
}

SDL_bool Wayland_Vulkan_GetInstanceExtensions(_THIS,

                                     SDL_Window *window,
                                     unsigned *count,
                                     const char **names)

{
static const char *const extensionsForWayland[] = {
VK_KHR_SURFACE_EXTENSION_NAME, VK_KHR_WAYLAND_SURFACE_EXTENSION_NAME
};

if(!_this->vulkan_config.loader_handle)
{

   SDL_SetError("Vulkan is not loaded");

```
   return SDL_FALSE;
```
}

return SDL_Vulkan_GetInstanceExtensions_Helper(
count, names, SDL_arraysize(extensionsForWayland),
extensionsForWayland);
diff -r 4358e537000a -r 61a0d2c2ea54 src/video/wayland/SDL_waylandvulkan.h
— a/src/video/wayland/SDL_waylandvulkan.h Sat Aug 18 12:30:04 2018 +0100
+++ b/src/video/wayland/SDL_waylandvulkan.h Mon Aug 20 04:53:39 2018 -0700
@@ -37,7 +37,6 @@
int Wayland_Vulkan_LoadLibrary(_THIS, const char *path);
void Wayland_Vulkan_UnloadLibrary(_THIS);
SDL_bool Wayland_Vulkan_GetInstanceExtensions(_THIS,

                                     SDL_Window *window,
                                     unsigned *count,
                                     const char **names);

SDL_bool Wayland_Vulkan_CreateSurface(_THIS,
diff -r 4358e537000a -r 61a0d2c2ea54 src/video/windows/SDL_windowsvulkan.c
— a/src/video/windows/SDL_windowsvulkan.c Sat Aug 18 12:30:04 2018 +0100
+++ b/src/video/windows/SDL_windowsvulkan.c Mon Aug 20 04:53:39 2018 -0700
@@ -111,18 +111,13 @@
}

SDL_bool WIN_Vulkan_GetInstanceExtensions(_THIS,

                                     SDL_Window *window,
                                     unsigned *count,
                                     const char **names)

{
static const char *const extensionsForWin32[] = {
VK_KHR_SURFACE_EXTENSION_NAME, VK_KHR_WIN32_SURFACE_EXTENSION_NAME
};

if(!_this->vulkan_config.loader_handle)
{

   SDL_SetError("Vulkan is not loaded");

```
   return SDL_FALSE;
```
}

return SDL_Vulkan_GetInstanceExtensions_Helper(
count, names, SDL_arraysize(extensionsForWin32),
extensionsForWin32);
diff -r 4358e537000a -r 61a0d2c2ea54 src/video/windows/SDL_windowsvulkan.h
— a/src/video/windows/SDL_windowsvulkan.h Sat Aug 18 12:30:04 2018 +0100
+++ b/src/video/windows/SDL_windowsvulkan.h Mon Aug 20 04:53:39 2018 -0700
@@ -37,7 +37,6 @@
int WIN_Vulkan_LoadLibrary(_THIS, const char *path);
void WIN_Vulkan_UnloadLibrary(_THIS);
SDL_bool WIN_Vulkan_GetInstanceExtensions(_THIS,

                                     SDL_Window *window,
                                     unsigned *count,
                                     const char **names);

SDL_bool WIN_Vulkan_CreateSurface(_THIS,
diff -r 4358e537000a -r 61a0d2c2ea54 src/video/x11/SDL_x11vulkan.c
— a/src/video/x11/SDL_x11vulkan.c Sat Aug 18 12:30:04 2018 +0100
+++ b/src/video/x11/SDL_x11vulkan.c Mon Aug 20 04:53:39 2018 -0700
@@ -137,16 +137,10 @@
}

SDL_bool X11_Vulkan_GetInstanceExtensions(_THIS,

                                     SDL_Window *window,
                                     unsigned *count,
                                     const char **names)

{
SDL_VideoData *videoData = (SDL_VideoData *)_this->driverdata;

if(!_this->vulkan_config.loader_handle)
{

   SDL_SetError("Vulkan is not loaded");

```
   return SDL_FALSE;
```
}
if(videoData->vulkan_xlib_xcb_library)
{
static const char *const extensionsForXCB[] = {
diff -r 4358e537000a -r 61a0d2c2ea54 src/video/x11/SDL_x11vulkan.h
— a/src/video/x11/SDL_x11vulkan.h Sat Aug 18 12:30:04 2018 +0100
+++ b/src/video/x11/SDL_x11vulkan.h Mon Aug 20 04:53:39 2018 -0700
@@ -33,7 +33,6 @@
int X11_Vulkan_LoadLibrary(_THIS, const char *path);
void X11_Vulkan_UnloadLibrary(_THIS);
SDL_bool X11_Vulkan_GetInstanceExtensions(_THIS,

                                     SDL_Window *window,
                                     unsigned *count,
                                     const char **names);

SDL_bool X11_Vulkan_CreateSurface(_THIS,
diff -r 4358e537000a -r 61a0d2c2ea54 test/testvulkan.c
— a/test/testvulkan.c Sat Aug 18 12:30:04 2018 +0100
+++ b/test/testvulkan.c Mon Aug 20 04:53:39 2018 -0700
@@ -255,7 +255,7 @@
appInfo.apiVersion = VK_API_VERSION_1_0;
instanceCreateInfo.sType = VK_STRUCTURE_TYPE_INSTANCE_CREATE_INFO;
instanceCreateInfo.pApplicationInfo = &appInfo;

if(!SDL_Vulkan_GetInstanceExtensions(state->windows[0], &extensionCount, NULL))

if(!SDL_Vulkan_GetInstanceExtensions(&extensionCount, NULL))
{
SDL_LogError(SDL_LOG_CATEGORY_APPLICATION,
“SDL_Vulkan_GetInstanceExtensions(): %s\n”,
@@ -268,7 +268,7 @@
SDL_OutOfMemory();
quit(2);
}

if(!SDL_Vulkan_GetInstanceExtensions(state->windows[0], &extensionCount, extensions))

if(!SDL_Vulkan_GetInstanceExtensions(&extensionCount, extensions))
{
SDL_free((void*)extensions);
SDL_LogError(SDL_LOG_CATEGORY_APPLICATION,

Daniel_Gibson · August 21, 2018, 12:52am

Pro-Tip: You can attach files to posts here (via that “Upload” button, the one with the arrow pointing up)

The next best thing: adding ``` before and after the code (or patch in this case) will make it a code-block that is not markdown-formatted

ninepoints · August 21, 2018, 2:09am

Yea my bad, I realized my mistake but then was already locked out of the system. I filed the patch to the issue tracker already. Thanks