libsdl-org / SDL

Simple Directmedia Layer
https://libsdl.org
zlib License
8.73k stars 1.64k forks source link

SDL build with configure (autoconf) incorrectly tries to build with Vulkan support on Solaris 10 (probably other UNIXes) #10100

Open joncox123 opened 2 weeks ago

joncox123 commented 2 weeks ago

When trying to compile SDL2 on Solaris 10 1/13, the "configure" script incorrectly tries to build SDL2 with support for Vulkan. Obviously, this fails because the likelihood of most UNIXes, especially Solaris 10, ever supporting Vulkan is slim to none. As a result, I bet this issue impacts other UNIXes, as well.

In order to build SDL2 on Solaris 10 (UltraSPARC IIIi processor), I have to use: './configure --enable-video-vulkan=no '

I suggest this is resolved in one of two ways. One hack would be to specifically disable Vulkan support in configure if compiling on Solaris. A better way to do it would be to improve the mechanism in configure for detecting Vulkan support, such as by trying to compile a test.

On the other hand, if the autoconf/configure build system is obsolete and historical at this point, maybe Option 1 is the best option.

I wish I could help more, but as I do not have a PhD in autoconf, it is above my paygrade. A professional build system engineer would be better suited to implement a fix.

slouken commented 2 weeks ago

For SDL2, the configure workaround is probably fine, but we should make sure CMake is doing the right thing in SDL3.

icculus commented 1 week ago

The correct fix, fwiw, is to disable Vulkan if libxcb isn't available, but no one should spend time writing autoconf tests at this point, so I'm just disabling Vulkan for Solaris in SDL2.

(I also don't understand why our Xlib-based code requires libxcb for Vulkan, when Vulkan offers an Xlib equivalent. Didn't we fix this in SDL3?)

icculus commented 1 week ago

I also don't understand why our Xlib-based code requires libxcb for Vulkan

Turns out (in SDL3 at least, if not SDL2) we favor Xlib, but if the Vulkan implementation doesn't have Xlib support but only xcb, we attempt to get the proper lowlevel handles from Xlib to use xcb instead.

We could check for xcb headers in CMake, sprinkle around some ifdefs, and fail in X11_Vulkan_LoadLibrary if we're forced into needing xcb, but I'm not confident this is worth it instead of just turning off Vulkan for Solaris.

Unless we want to do something more for SDL3, this can be closed now.

madebr commented 1 week ago

The xcb.h include is commented out in both SDL2 and SDL3.

If libxcb is not available on Solaris, then won't the xcb workaround in SDL_x11vulkan.c simply fail always? Do we really need to conditionally #ifdef-out the workaround? Also, because no xcb.h header is included, I think current SDL will build successfully (with and without vulkan enabled) on Solaris.

icculus commented 1 week ago

The xcb.h include is commented out in both SDL2 and SDL3.

That one is commented out, but vulkan.h includes it, if VK_USE_PLATFORM_XCB_KHR is defined, which SDL_vulkan_internal.h does.

icculus commented 1 week ago

(But again, there isn't even a hypothetical world where Solaris 10--last official update was in 2013--is getting a Vulkan implementation, so it doesn't really matter if it compiles there. :) )

madebr commented 1 week ago

I have the patch below queued for solaris. I've got some questions unanswered in other issues (CMAKE_C_COMPILER_ID of the compiler + what predefined macros the toolchain defines)

--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -232,6 +232,8 @@ if(SDL_SHARED_DEFAULT AND SDL_STATIC_DEFAULT AND SDL_SHARED_AVAILABLE)
   endif()
 endif()

+set(SDL_VULKAN_DEFAULT ON)
+
 set(SDL_X11_OPTIONS Xcursor Xdbe XInput Xfixes Xrandr Xscrnsaver XShape)
 foreach(_SUB ${SDL_X11_OPTIONS})
   string(TOUPPER "SDL_X11_${_SUB}_DEFAULT" _OPT)
@@ -239,6 +241,7 @@ foreach(_SUB ${SDL_X11_OPTIONS})
 endforeach()

 if(SOLARIS)
+  set(SDL_VULKAN_DEFAULT OFF)
   set(SDL_X11_XRANDR_DEFAULT OFF)
 endif()

@@ -351,7 +354,7 @@ dep_option(SDL_WASAPI              "Use the Windows WASAPI audio driver" ON "WIN
 dep_option(SDL_RENDER_D3D          "Enable the Direct3D render driver" ON "SDL_RENDER" OFF)
 dep_option(SDL_RENDER_METAL        "Enable the Metal render driver" ON "SDL_RENDER;${APPLE}" OFF)
 dep_option(SDL_VIVANTE             "Use Vivante EGL video driver" ON "${UNIX_SYS};SDL_CPU_ARM32" OFF)
-dep_option(SDL_VULKAN              "Enable Vulkan support" ON "SDL_VIDEO;ANDROID OR APPLE OR LINUX OR WINDOWS" OFF)
+dep_option(SDL_VULKAN              "Enable Vulkan support" "${SDL_VULKAN_DEFAULT}" "SDL_VIDEO;ANDROID OR APPLE OR LINUX OR WINDOWS" OFF)
 dep_option(SDL_RENDER_VULKAN       "Enable the Vulkan render driver" ON "SDL_RENDER;SDL_VULKAN;ANDROID OR APPLE OR LINUX OR WINDOWS" OFF)
 dep_option(SDL_METAL               "Enable Metal support" ON "APPLE" OFF)
 dep_option(SDL_KMSDRM              "Use KMS DRM video driver" ${UNIX_SYS} "SDL_VIDEO" OFF)
icculus commented 1 week ago

I couldn't coerce CMake itself to build from source, and the prebuilt package of it I dug up fails to load at all...but this isn't my area of expertise, there's probably a super-easy way to install this on Solaris that I missed.

(EDIT: I got it working, see the other bug report.)

icculus commented 1 week ago

Yeah, @madebr, go ahead and push the above patch and close this one.