[SRU] SDL2 2.0.6 isn't compiled with Vulkan support

[Axel Gneiting]

[Impact]
* Lots of Vulkan games can't run because of libsdl2 missing support

[Test Case]
* Install e.g. vkQuake

[Regression Potential]
* Low, it is a new feature but it is dynamically used only when useful.

[Other Info]
The SDL2 shipped with Ubuntu 17.10 does not have Vulkan support compiled in, causing e.g. vkQuake not to work.

SDL2 needs libvulkan-dev when configure/make for this to work. Maybe there is just a missing dependency in the build process?

[Launchpad Janitor (janitor)]

Status changed to 'Confirmed' because the bug affects multiple users.

[Wladimir J. van der Laan (laanwj)]

Just ran into the same issue, it would be very nice if this was supported.

[Andreas Schultes (andreasschultes)]

I create a ppa with vulkan enabled.

https://launchpad.net/~andreasschultes/+archive/ubuntu/libsdl2-vulkan

The only change is that loadso configuration options is enabled, and the disable vulkan line is removed.

[Ryan C. Gordon (icculus)]

Just adding a note that this is still missing Vulkan support in 18.04.

Not to be overdramatic, but this bug means that basically no Vulkan-based game going forward is likely to work with Ubuntu out of the box until this bug is resolved.

This is in debian/rules for the package:

    # the SDL module for Vulkan not compiling even in Linux at the moment
    confflags += --disable-video-vulkan

It would be useful to understand why that is necessary (_is_ it still necessary?) and resolve the issue. It is building just fine on Linux for me. :)

--ryan.

[Ross Campbell (ross-campbell)]

To be slightly more dramatic than Ryan... Vulkan is a major graphics performance improvement for Linux, and gamers on Linux -- especially gamers using Steam -- want Vulkan.

https://www.khronos.org/vulkan/

Ubuntu is the #1 Linux distribution for Steam . Please help us get vulkan working out of the box.

https://store.steampowered.com/hwsurvey/Steam-Hardware-Software-Survey-Welcome-to-Steam?platform=linux

[Gianfranco Costamagna (costamagnagianfranco)]

where is the patch?
where is the fix for cosmic?
I tried "removing only the line, and put the stuff in ppa", and the result is a build failure.
It has been disabled for this reason
https://launchpad.net/~costamagnagianfranco/+archive/ubuntu/locutusofborg-ppa/+sourcepub/9428378/+listing-archive-extra

/<<BUILDDIR>>/libsdl2-2.0.8+dfsg1/src/video/x11/SDL_x11vulkan.c: In function ‘X11_Vulkan_LoadLibrary’:
/<<BUILDDIR>>/libsdl2-2.0.8+dfsg1/src/video/x11/SDL_x11vulkan.c:41:5: error: unknown type name ‘VkExtensionProperties’; did you mean ‘XExtensionHooks’?
     VkExtensionProperties *extensions = NULL;
     ^~~~~~~~~~~~~~~~~~~~~
     XExtensionHooks
/<<BUILDDIR>>/libsdl2-2.0.8+dfsg1/src/video/x11/SDL_x11vulkan.c:67:13: error: ‘VK_NULL_HANDLE’ undeclared (first use in this function); did you mean ‘VK_DEFINE_HANDLE’?
             VK_NULL_HANDLE, "vkEnumerateInstanceExtensionProperties");
             ^~~~~~~~~~~~~~
             VK_DEFINE_HANDLE
/<<BUILDDIR>>/libsdl2-2.0.8+dfsg1/src/video/x11/SDL_x11vulkan.c:67:13: note: each undeclared identifier is reported only once for each function it appears in
/<<BUILDDIR>>/libsdl2-2.0.8+dfsg1/src/video/x11/SDL_x11vulkan.c:66:18: error: too many arguments to function ‘(void (*)(void))_this->vulkan_config.vkGetInstanceProcAddr’
         (void *)((PFN_vkGetInstanceProcAddr)_this->vulkan_config.vkGetInstanceProcAddr)(
                 ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/<<BUILDDIR>>/libsdl2-2.0.8+dfsg1/src/video/x11/SDL_x11vulkan.c:66:9: error: invalid use of void expression
         (void *)((PFN_vkGetInstanceProcAddr)_this->vulkan_config.vkGetInstanceProcAddr)(
         ^
/<<BUILDDIR>>/libsdl2-2.0.8+dfsg1/src/video/x11/SDL_x11vulkan.c:70:18: warning: implicit declaration of function ‘SDL_Vulkan_CreateInstanceExtensionsList’; did you mean ‘SDL_Vulkan_GetInstanceExtensions’? [-Wimplicit-function-declaration]
     extensions = SDL_Vulkan_CreateInstanceExtensionsList(
                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                  SDL_Vulkan_GetInstanceExtensions
/<<BUILDDIR>>/libsdl2-2.0.8+dfsg1/src/video/x11/SDL_x11vulkan.c:70:16: warning: assignment to ‘int *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
     extensions = SDL_Vulkan_CreateInstanceExtensionsList(
                ^
/<<BUILDDIR>>/libsdl2-2.0.8+dfsg1/src/video/x11/SDL_x11vulkan.c:78:23: error: ‘VK_KHR_SURFACE_EXTENSION_NAME’ undeclared (first use in this function); did you mean ‘GLX_EXTENSION_NAME’?
         if(SDL_strcmp(VK_KHR_SURFACE_EXTENSION_NAME, extensions[i].extensionName) == 0)
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                       GLX_EXTENSION_NAME
/<<BUILDDIR>>/libsdl2-2.0.8+dfsg1/src/video/x11/SDL_x11vulkan.c:78:67: error: request for member ‘extensionName’ in something not a structure or union
         if(SDL_strcmp(VK_KHR_SURFACE_EXTENSION_NAME, extensions[i].extensionName) == 0)
                                                                   ^
/<<BUILDDIR>>/libsdl2-2.0.8+dfsg1/src/video/x11/SDL_x11vulkan.c:80:28: error: ‘VK_KHR_XCB_SURFACE_EXTENSION_NAME’ undeclared (first use in this function)
         else if(SDL_strcmp(VK_KHR_XCB_SU...

[Gianfranco Costamagna (costamagnagianfranco)]

I see other changes in the mentioned ppa, but I'm not going to test and understand what has been disabled and what changed, without a clear rationale and explanation.

[Ryan C. Gordon (icculus)]

Ok, so following up: Vulkan appears to be disabled because "loadso" (which is short for "load shared objects" in this case) is disabled too, and this is not a configuration that we generally test for, because that turns off SDL ever dynamically loading a library, either internally for its own use (to answer runtime questions like "is Wayland available? No? Ok, is Xlib available, then?"), or for users of the API to dlopen() libraries.

(As an unrelated bug, "configure --disable-loadso" will still build SDL with dlopen() support, but the macros get set that make the build believe it isn't supported.)

So this line in src/video/SDL_vulkan_internal.h causes our specific problem when Vulkan wasn't explicitly disabled by the configure script:

    #if defined(SDL_LOADSO_DISABLED)
    #undef SDL_VIDEO_VULKAN
    #define SDL_VIDEO_VULKAN 0
    #endif

...this makes it, shortly thereafter, not include the actual Vulkan headers, causing all those errors Gianfranco listed.

So to answer the question: Vulkan was disabled because it broke the build, and it breaks the build because --disable-loadso also got added to debian/rules. So the next question is why did _that_ get added?

Because honestly? I'm amazed --disable-loadso didn't cause problems earlier. Removing it causes the build to succeed with or without Vulkan.

(as this is actually an issue in Debian's package, which Ubuntu inherits, should we take this conversation to Debian's bug tracker?)

--ryan.

[Gianfranco Costamagna (costamagnagianfranco)]

git log shows that that stuff has been there since the begin, since
commit 01084253a42b1c20956277869a8c09e7de501844 (tag: debian/1.3.0_20111204-1)
Author: Sam Hocevar <email address hidden>
Date: Sun Dec 4 14:35:05 2011 +0100

    Imported Debian patch 1.3.0~20111204-1

diff --git a/debian/rules b/debian/rules

(I'm committing its disable shortly in debian too)

[Gianfranco Costamagna (costamagnagianfranco)]

Added in libsdl1.2
commit 44520b122631527aa8491ed9339eb018a7440f0b (tag: debian/1.2.14-6.4)
Author: Josselin Mouette <email address hidden>
Date: Tue May 24 22:51:57 2011 +0200

    Imported Debian patch 1.2.14-6.4

+libsdl1.2 (1.2.14-6.4) unstable; urgency=low
+
+ * Non-maintainer upload.
+ * Build improvements from Guillem Jover that should really have gone
+ with the previous upload. Closes: #614332.
+ + Line-wrap build-depends.
+ + Use -any syntax for architecture-specific stuff.
+ + Fix cross-compilation by using DEB_HOST_* variables.
+ + Update or remove non-existing configure flags.
+ + Forbid dlopening for X11.
+ * Use --enable-debug with the debug keyword, not the noopt one.
+ * Refactor configure options.
+ * Disable NAS. We have PulseAudio now.
+ * Pass --disable-loadso, just to be sure.
+
+ -- Josselin Mouette <email address hidden> Tue, 24 May 2011 22:51:57 +0200

[Gianfranco Costamagna (costamagnagianfranco)]

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=614332
it has been disabled to fix this bug "just to be sure"
I don't know what does it mean

[Ryan C. Gordon (icculus)]

So what they did was try to force SDL to not dynamically load Xlib, because they were concerned SDL wouldn't have X11 as a package dependency if so (although that seems like something that should be listed as a dependency at the package level, and in a land where Wayland exists, is now incorrect behavior anyhow), and the "just to be sure" was to attempt to disable _all_ dynamic loading, which is also incorrect.

Also, it was for SDL 1.2, which was a totally different beast. :)

One could argue that this makes sense for a distro package, because they are guaranteeing that the dependencies will exist...a lot of SDL's dynamic loading mojo is to let it make decisions about what targets to choose on an arbitrary system, but in a day when both Wayland and X11 (and OpenGL and Vulkan...) may or may not exist on a given system, this functionality is needed now.

Also, bugs in SDL's configure script aside, --disable-loadso will break SDL_LoadObject()'s functionality, so it's not a good idea in any case.

I'm not well-versed in how Debian packaging works, but it seems to me that it should list X11 as an explicit dependency of the package, and then let SDL dlopen it at runtime (and recover gracefully if it can't). I wonder if Debian can say "this needs the X11 headers when building, but doesn't need the X11 package at runtime"...

[Gianfranco Costamagna (costamagnagianfranco)]

ok I like it, uploaded, thanks!
(I uploaded libsdl1.2, libsdl2 in both debian and ubuntu!)

[Gianfranco Costamagna (costamagnagianfranco)]

Ryan, can you please check if the bug description is accurate? I'm willing to sponsor for bionic once cosmic is fixed

[Gianfranco Costamagna (costamagnagianfranco)]

please test the build in
https://launchpad.net/~costamagnagianfranco/+archive/ubuntu/locutusofborg-ppa/+packages

[Launchpad Janitor (janitor)]

This bug was fixed in the package libsdl2 - 2.0.8+dfsg1-2ubuntu1

---------------
libsdl2 (2.0.8+dfsg1-2ubuntu1) cosmic; urgency=medium

  * Merge from Debian unstable. Remaining changes:
    - Build-depend on libmirclient-dev
    - Explicitly enable --enable-video-mir in rules

libsdl2 (2.0.8+dfsg1-2) unstable; urgency=medium

  [ Felix Geyer ]
  * Remove Multi-Arch: same from libsdl2-dev as SDL_config.h is architecture
    dependent.

  [ Gianfranco Costamagna ]
  * Team upload
  * Enable vulkan everywhere
  * debian/patches/SDL2-dynapi-symbol-resolution-fix.patch:
    cherry-pick upstream fix for Unity-based games shipping bundled
    libsdl2 version (LP: #1772471)
    - thanks Ryan Gordon for the patch and help

  [ Ryan C. Gordon (icculus) ]
  * Disable --disable-loadso switch, it was useless and it is wrong
    in many cases (see LP: #1740517 for discussion)

 -- Gianfranco Costamagna <email address hidden> Fri, 14 Sep 2018 20:40:19 +0200

[Ryan C. Gordon (icculus)]

Wow, you're fast! :)

Is this going in Cosmic and/or Bionic?

Thanks,
--ryan.

[Gianfranco Costamagna (costamagnagianfranco)]

Cosmic is already ok, for both sdl1.2 and sdl2, also debian unstable.

this sru is now for bionic.

Do you see any possible ABI issue with the change or any potential risk to fix an LTS?

Should I also fix sdl1.2 in bionic too? (There is no vulkan so I presume disableso won't hurt here?)

[Ryan C. Gordon (icculus)]

No need to update 1.2, at least for Vulkan support. If no one is complaining about loadso for 1.2, I think it's safe to leave that alone in Bionic and just be correct going forward into Cosmic. We can always revisit in a separate bug report if something serious arises, but I don't predict that will happen.

There shouldn't be any ABI issues; SDL still exports the same symbols, just some of those functions will return an error code immediately if support has been disabled. Since we need SDL to work with binaries that are shipped for generic "Linux" platforms that can't promise a specific distro or configuration (both for Steam and just delivering games outside of package managers in general), we are _religious_ about keeping a stable ABI in all circumstances for SDL...so it's safe to change those configure options even for LTS.

--ryan.

[Andreas Schultes (andreasschultes)]

Are there still reasons which prevent to fix this for bionic?

[Gianfranco Costamagna (costamagnagianfranco)]

uploaded to bionic

[CatKiller (catkiller)]

When will this fix make it into bionic?

[Gianfranco Costamagna (costamagnagianfranco)]

waiting for SRU team to pick it up.

[Brian Murray (brian-murray)]

@Gianfranco Costamagna - What changed your mind between September and February about this being appropriate as an SRU for Ubuntu 18.04? I don't see any reasoning for it.

Additionally, the test case in the bug description is insufficient for someone unfamiliar with the issue to be able to test the bug and the fix for it. Please improve the test case.

[Adam Conrad (adconrad)]

Additionally, while the proposed SRU changes --disable-loadso, it doesn't appear to enable Vulkan. Why would you do this in two steps? Or did you just miss half of the SRU? Based on this, and by Brian's request (he hit the wrong button), I'm removing this from proposed for now.

[Adam Conrad (adconrad)]

Most notably, it seems to be missing this part from the cosmic/unstable uploads:

-# the SDL module for Vulkan not compiling even in Linux at the moment
-confflags += --disable-video-vulkan
-

[Gianfranco Costamagna (costamagnagianfranco)]

Hello, I have reuploaded with a proper fix also for Vulkan.
I have been tricked by the fact that we are fixing actually two bugs in this SRU, so I forgot about the original issue.

>@Gianfranco Costamagna - What changed your mind between September and February about this being >appropriate as an SRU for Ubuntu 18.04? I don't see any reasoning for it.

I became more confident with the sdl2 code, and their dynamic "plugin" system.

For the test case, I can write some simple vulkan application to test this...

G.

[Gianfranco Costamagna (costamagnagianfranco)]

@Axel Gneiting can you please post a step-by-step guide to reproduce the issue and test the fix?

[Timo Aaltonen (tjaalton)]

Hello Axel, or anyone else affected,

Accepted libsdl2 into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/libsdl2/2.0.8+dfsg1-1ubuntu1.18.04.3 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

[Gianfranco Costamagna (costamagnagianfranco)]

I did update libsdl2
Do you want to continue? [Y/n]
Get:1 http://archive.ubuntu.com/ubuntu bionic-proposed/universe amd64 libsdl2-dev amd64 2.0.8+dfsg1-1ubuntu1.18.04.3 [684 kB]
Get:2 http://archive.ubuntu.com/ubuntu bionic-proposed/universe amd64 libsdl2-2.0-0 amd64 2.0.8+dfsg1-1ubuntu1.18.04.3 [381 kB]
Fetched 1.065 kB in 0s (3.521 kB/s)

Before vulkan support wasn't detected at all:
Command line: ./vkquake
Found SDL version 2.0.8
Detected 8 CPUs.
Quake 1.09 (c) id Software
GLQuake 1.00 (c) id Software
FitzQuake 0.85 (c) John Fitzgibbons
FitzQuake SDL port (c) SleepwalkR, Baker
QuakeSpasm 0.93.0 (c) Ozkan Sezer, Eric Wasylishen & others
vkQuake 1.01.0 (c) Axel Gneiting & others
Host_Init
Playing registered version.
Console initialized.
UDP Initialized
Server using protocol 666 (FitzQuake)
Exe: 11:54:22 Mar 11 2019
256.0 megabyte heap

ERROR-OUT BEGIN

QUAKE ERROR: Couldn't create window: Vulkan support is either not configured in SDL or not available in video driver

now, it is complaining about an outdated vulkan driver, so the sdl2 part is fully working!
Command line: ./vkquake
Found SDL version 2.0.8
Detected 8 CPUs.
Quake 1.09 (c) id Software
GLQuake 1.00 (c) id Software
FitzQuake 0.85 (c) John Fitzgibbons
FitzQuake SDL port (c) SleepwalkR, Baker
QuakeSpasm 0.93.0 (c) Ozkan Sezer, Eric Wasylishen & others
vkQuake 1.01.0 (c) Axel Gneiting & others
Host_Init
Playing registered version.
Console initialized.
UDP Initialized
Server using protocol 666 (FitzQuake)
Exe: 11:54:22 Mar 11 2019
256.0 megabyte heap

ERROR-OUT BEGIN

QUAKE ERROR: Couldn't create window: Installed Vulkan doesn't implement the VK_KHR_surface extension

if you want this particular game I suggest you to update the vulkan driver, the libsdl2 part is correctly working now :)

[Gianfranco Costamagna (costamagnagianfranco)]

(probably I should update mesa on my laptop, not sure)

[Brian Murray (brian-murray)]

Thank you for taking the time to verify this stable release fix. We have noticed that you have used the verification-done tag for marking the bug as verified and would like to point out that due to a recent change in SRU bug verification policy fixes now have to be marked with per-release tags (i.e. verification-done-$RELEASE). Please remove the verification-done tag and add one for the release you have tested the package in. Thank you!

https://wiki.ubuntu.com/StableReleaseUpdates#Verification

[Gianfranco Costamagna (costamagnagianfranco)]

sorry, I fixed the bug tags :)

[Łukasz Zemczak (sil2100)]

The verification of the Stable Release Update for libsdl2 has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

[Launchpad Janitor (janitor)]

This bug was fixed in the package libsdl2 - 2.0.8+dfsg1-1ubuntu1.18.04.3

---------------
libsdl2 (2.0.8+dfsg1-1ubuntu1.18.04.3) bionic; urgency=medium

  [ Gianfranco Costamagna ]
  * Enable vulkan everywhere

  [ Ryan C. Gordon (icculus) ]
  * Disable --disable-loadso switch, it was useless and it is wrong
    in many cases (see LP: #1740517 for discussion)

 -- Gianfranco Costamagna <email address hidden> Thu, 28 Feb 2019 10:38:26 +0100