echawk / kiss-xorg

A KISS Linux Repository for Xorg
MIT License
34 stars 9 forks source link

melonDS unnecessarily depends on wayland #120

Closed davidgarland closed 1 year ago

davidgarland commented 1 year ago

The full terminal output, since it's relatively short:

bash-5.2$ kiss b melonDS
-> Building: explicit: melonDS
-> Checking for pre-built dependencies
-> melonDS Reading sources
found git+https://github.com/Arisotura/melonDS
-> melonDS Checking out FETCH_HEAD
remote: Total 0 (delta 0), reused 0 (delta 0), pack-reused 0
From https://github.com/Arisotura/melonDS
 * branch            HEAD       -> FETCH_HEAD
HEAD is now at 00edeb3 Set LSApplicationCategoryType to games
-> melonDS Verifying sources
-> melonDS Building package (1/1)
-> melonDS Extracting sources
-> melonDS Starting build
-- The C compiler identification is GNU 12.2.0
-- The CXX compiler identification is GNU 12.2.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- The ASM compiler identification is GNU
-- Found assembler: /usr/bin/cc
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE
-- Found PkgConfig: /usr/bin/pkg-config (found version "1.9.4")
-- Checking for module 'sdl2'
--   Found sdl2, version 2.26.2
-- Checking for module 'slirp'
--   Found slirp, version 4.7.0
-- Checking for module 'libarchive'
--   Found libarchive, version 3.6.2
-- Found X11: /usr/include
-- Looking for XOpenDisplay in /usr/lib/libX11.so;/usr/lib/libXext.so
-- Looking for XOpenDisplay in /usr/lib/libX11.so;/usr/lib/libXext.so - found
-- Looking for gethostbyname
-- Looking for gethostbyname - found
-- Looking for connect
-- Looking for connect - found
-- Looking for remove
-- Looking for remove - found
-- Looking for shmat
-- Looking for shmat - found
-- Looking for IceConnectionNumber in ICE
-- Looking for IceConnectionNumber in ICE - found
-- Performing Test HAVE_EGL
-- Performing Test HAVE_EGL - Success
-- Found EGL: /usr/include (found version "1.5")
CMake Warning (dev) at /usr/share/ECM/modules/ECMFindModuleHelpers.cmake:113 (message):
  Your project should require at least CMake 3.16.0 to use FindWayland.cmake
Call Stack (most recent call first):
  /usr/share/ECM/find-modules/FindWayland.cmake:61 (ecm_find_package_version_check)
  src/frontend/qt_sdl/CMakeLists.txt:116 (find_package)
This warning is for project developers.  Use -Wno-dev to suppress it.

-- Could NOT find Wayland_Client (missing: Wayland_Client_LIBRARY Wayland_Client_INCLUDE_DIR) (found version "")
CMake Error at /usr/share/cmake-3.25/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
  Could NOT find Wayland (missing: Wayland_LIBRARIES Client)
Call Stack (most recent call first):
  /usr/share/cmake-3.25/Modules/FindPackageHandleStandardArgs.cmake:600 (_FPHSA_FAILURE_MESSAGE)
  /usr/share/ECM/find-modules/FindWayland.cmake:110 (find_package_handle_standard_args)
  src/frontend/qt_sdl/CMakeLists.txt:116 (find_package)

-- Configuring incomplete, errors occurred!
See also "/home/david/.cache/kiss/proc/8844/build/melonDS/build/CMakeFiles/CMakeOutput.log".
-> melonDS Build failed
-> melonDS Log stored to /home/david/.cache/kiss/logs/2023-02-14/melonDS-2023-02-14-00:47-8844
Terminated
Terminated

It seems this owes to this line, which appears to have been done about 4 months ago according to the git blame: https://github.com/melonDS-emu/melonDS/blob/726fde4e8de3ae50df0373476ba1d952c04b7b67/src/frontend/qt_sdl/CMakeLists.txt#L116

The easiest possible "fix" would just be adding wayland make to depends, making the intended way to install melonDS for non-wayland users:

kiss b wayland
kiss b melonDS
kiss r wayland

Another option would be to patch their code to not use wayland, which seems like a reasonable solution since they use it so sparingly, though I'm not sure how stable it'll prove to be; to save you the work, this patch seems to compile fine without wayland on my end and builds a working binary:

diff --git a/src/frontend/duckstation/gl/context.cpp b/src/frontend/duckstation/gl/context.cpp
index 9f21cd6..fa92f44 100644
--- a/src/frontend/duckstation/gl/context.cpp
+++ b/src/frontend/duckstation/gl/context.cpp
@@ -15,7 +15,6 @@ Log_SetChannel(GL::Context);
 #elif defined(__APPLE__)
 #include "context_agl.h"
 #else
-#include "context_egl_wayland.h"
 #include "context_egl_x11.h"
 #include "context_glx.h"
 #endif
@@ -80,8 +79,6 @@ std::unique_ptr<GL::Context> Context::Create(const WindowInfo& wi, const Version
       context = ContextGLX::Create(wi, versions_to_try, num_versions_to_try);
   }

-  if (wi.type == WindowInfo::Type::Wayland)
-    context = ContextEGLWayland::Create(wi, versions_to_try, num_versions_to_try);
 #endif

   if (!context)
diff --git a/src/frontend/duckstation/gl/context_egl_wayland.cpp b/src/frontend/duckstation/gl/context_egl_wayland.cpp
index 16532e8..e69de29 100644
--- a/src/frontend/duckstation/gl/context_egl_wayland.cpp
+++ b/src/frontend/duckstation/gl/context_egl_wayland.cpp
@@ -1,86 +0,0 @@
-#include "context_egl_wayland.h"
-#include "../log.h"
-#include <dlfcn.h>
-Log_SetChannel(ContextEGLWayland);
-
-namespace GL {
-static const char* WAYLAND_EGL_MODNAME = "libwayland-egl.so.1";
-
-ContextEGLWayland::ContextEGLWayland(const WindowInfo& wi) : ContextEGL(wi) {}
-ContextEGLWayland::~ContextEGLWayland()
-{
-  if (m_wl_window)
-    m_wl_egl_window_destroy(m_wl_window);
-  if (m_wl_module)
-    dlclose(m_wl_module);
-}
-
-std::unique_ptr<Context> ContextEGLWayland::Create(const WindowInfo& wi, const Version* versions_to_try,
-                                                   size_t num_versions_to_try)
-{
-  std::unique_ptr<ContextEGLWayland> context = std::make_unique<ContextEGLWayland>(wi);
-  if (!context->LoadModule() || !context->Initialize(versions_to_try, num_versions_to_try))
-    return nullptr;
-
-  return context;
-}
-
-std::unique_ptr<Context> ContextEGLWayland::CreateSharedContext(const WindowInfo& wi)
-{
-  std::unique_ptr<ContextEGLWayland> context = std::make_unique<ContextEGLWayland>(wi);
-  context->m_display = m_display;
-
-  if (!context->LoadModule() || !context->CreateContextAndSurface(m_version, m_context, false))
-    return nullptr;
-
-  return context;
-}
-
-void ContextEGLWayland::ResizeSurface(u32 new_surface_width, u32 new_surface_height)
-{
-  if (m_wl_window)
-    m_wl_egl_window_resize(m_wl_window, new_surface_width, new_surface_height, 0, 0);
-
-  ContextEGL::ResizeSurface(new_surface_width, new_surface_height);
-}
-
-EGLNativeWindowType ContextEGLWayland::GetNativeWindow(EGLConfig config)
-{
-  if (m_wl_window)
-  {
-    m_wl_egl_window_destroy(m_wl_window);
-    m_wl_window = nullptr;
-  }
-
-  m_wl_window =
-    m_wl_egl_window_create(static_cast<wl_surface*>(m_wi.window_handle), m_wi.surface_width, m_wi.surface_height);
-  if (!m_wl_window)
-    return {};
-
-  return reinterpret_cast<EGLNativeWindowType>(m_wl_window);
-}
-
-bool ContextEGLWayland::LoadModule()
-{
-  m_wl_module = dlopen(WAYLAND_EGL_MODNAME, RTLD_NOW | RTLD_GLOBAL);
-  if (!m_wl_module)
-  {
-    Log_ErrorPrintf("Failed to load %s.", WAYLAND_EGL_MODNAME);
-    return false;
-  }
-
-  m_wl_egl_window_create =
-    reinterpret_cast<decltype(m_wl_egl_window_create)>(dlsym(m_wl_module, "wl_egl_window_create"));
-  m_wl_egl_window_destroy =
-    reinterpret_cast<decltype(m_wl_egl_window_destroy)>(dlsym(m_wl_module, "wl_egl_window_destroy"));
-  m_wl_egl_window_resize =
-    reinterpret_cast<decltype(m_wl_egl_window_resize)>(dlsym(m_wl_module, "wl_egl_window_resize"));
-  if (!m_wl_egl_window_create || !m_wl_egl_window_destroy || !m_wl_egl_window_resize)
-  {
-    Log_ErrorPrintf("Failed to load one or more functions from %s.", WAYLAND_EGL_MODNAME);
-    return false;
-  }
-
-  return true;
-}
-} // namespace GL
diff --git a/src/frontend/qt_sdl/CMakeLists.txt b/src/frontend/qt_sdl/CMakeLists.txt
index c74ec47..ad8597a 100644
--- a/src/frontend/qt_sdl/CMakeLists.txt
+++ b/src/frontend/qt_sdl/CMakeLists.txt
@@ -113,7 +113,6 @@ else()

     find_package(X11 REQUIRED)
     find_package(EGL REQUIRED)
-    find_package(Wayland REQUIRED Client)

     target_sources(melonDS PRIVATE
         ../duckstation/gl/context_egl.cpp

(By the way, is it considered better style to comment out large chunks of code to make the patch file smaller, rather than deleting code? I don't know which would be better behaved under them adding or deleting code on their end.. unsure where to look for best practices with this kinda stuff.)

davidgarland commented 1 year ago

Just in case, here's the version of the diff that just uses a comment (thinking about it more, this one is probably better?):

diff --git a/src/frontend/duckstation/gl/context.cpp b/src/frontend/duckstation/gl/context.cpp
index 9f21cd6..fa92f44 100644
--- a/src/frontend/duckstation/gl/context.cpp
+++ b/src/frontend/duckstation/gl/context.cpp
@@ -15,7 +15,6 @@ Log_SetChannel(GL::Context);
 #elif defined(__APPLE__)
 #include "context_agl.h"
 #else
-#include "context_egl_wayland.h"
 #include "context_egl_x11.h"
 #include "context_glx.h"
 #endif
@@ -80,8 +79,6 @@ std::unique_ptr<GL::Context> Context::Create(const WindowInfo& wi, const Version
       context = ContextGLX::Create(wi, versions_to_try, num_versions_to_try);
   }

-  if (wi.type == WindowInfo::Type::Wayland)
-    context = ContextEGLWayland::Create(wi, versions_to_try, num_versions_to_try);
 #endif

   if (!context)
diff --git a/src/frontend/duckstation/gl/context_egl_wayland.cpp b/src/frontend/duckstation/gl/context_egl_wayland.cpp
index 16532e8..9387186 100644
--- a/src/frontend/duckstation/gl/context_egl_wayland.cpp
+++ b/src/frontend/duckstation/gl/context_egl_wayland.cpp
@@ -1,3 +1,4 @@
+/*
 #include "context_egl_wayland.h"
 #include "../log.h"
 #include <dlfcn.h>
@@ -84,3 +85,4 @@ bool ContextEGLWayland::LoadModule()
   return true;
 }
 } // namespace GL
+*/
diff --git a/src/frontend/qt_sdl/CMakeLists.txt b/src/frontend/qt_sdl/CMakeLists.txt
index c74ec47..ad8597a 100644
--- a/src/frontend/qt_sdl/CMakeLists.txt
+++ b/src/frontend/qt_sdl/CMakeLists.txt
@@ -113,7 +113,6 @@ else()

     find_package(X11 REQUIRED)
     find_package(EGL REQUIRED)
-    find_package(Wayland REQUIRED Client)

     target_sources(melonDS PRIVATE
         ../duckstation/gl/context_egl.cpp
davidgarland commented 1 year ago

And of course I forgot to say: I'd be happy to make a PR for either of these solutions, depending which you'd prefer.

echawk commented 1 year ago

That second patch looks rather nice, I'd gladly take a PR!