Skip to content

[sdl3] Adding port for SDL3 Pre-release#40867

Merged
JavierMatosD merged 28 commits intomicrosoft:masterfrom
Honeybunch:sdl3
Nov 7, 2024
Merged

[sdl3] Adding port for SDL3 Pre-release#40867
JavierMatosD merged 28 commits intomicrosoft:masterfrom
Honeybunch:sdl3

Conversation

@Honeybunch
Copy link
Contributor

@Honeybunch Honeybunch commented Sep 8, 2024

Citing #32062 and the fact that the new SDL3 MkII GPU API got some buzz I thought people might find an SDL3 port useful. I've been using this port myself for a little while so figured I'd try up-streaming it. Should be easy to update this port for the full SDL 3.0.0 release

Note: SDL3 has dropped support for UWP so it has been removed from supports 😞

  • Changes comply with the maintainer guide.
  • The name of the port matches an existing name for this component on https://repology.org/ if possible, and/or is strongly associated with that component on search engines.
  • Optional dependencies are resolved in exactly one way. For example, if the component is built with CMake, all find_package calls are REQUIRED, are satisfied by vcpkg.json's declared dependencies, or disabled with CMAKE_DISABLE_FIND_PACKAGE_Xxx.
  • The versioning scheme in vcpkg.json matches what upstream says.
  • The license declaration in vcpkg.json matches what upstream says.
  • The installed as the "copyright" file matches what upstream says.
  • The source code of the component installed comes from an authoritative source.
  • The generated "usage text" is accurate. See adding-usage for context.
  • The version database is fixed by rerunning ./vcpkg x-add-version --all and committing the result.
  • Only one version is in the new port's versions file.
  • Only one version is added to each modified port's versions file.

@Honeybunch Honeybunch marked this pull request as draft September 8, 2024 21:40
@Honeybunch Honeybunch marked this pull request as ready for review September 8, 2024 22:16
@LilyWangLL LilyWangLL added the category:new-port The issue is requesting a new library to be added; consider making a PR! label Sep 9, 2024
@LilyWangLL
Copy link
Contributor

All features passed with x64-linux, usage test passed on x64-windows.

LilyWangLL
LilyWangLL previously approved these changes Sep 10, 2024
@LilyWangLL LilyWangLL added the info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. label Sep 10, 2024
@BillyONeal
Copy link
Member

I confirmed that SDL2 and SDL3 can be concurrently installed.

BillyONeal
BillyONeal previously approved these changes Sep 11, 2024
Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the port is good, thanks for the new port!

I would like to double check with other maintainers that we are OK with merging a port with no upstream releases yet; they might not be wanting people to treat it as generally available, and publishing it in vcpkg might do that. To that end it might be good to use version-string and the git SHA instead, as ensuring reasonable sort order with other prerelease commits sounds questionable.

If there's a statement from upstream maintainers that they're happy with this that avoids all concerns.

Thanks again!

@BillyONeal BillyONeal added requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. and removed info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. labels Sep 11, 2024
@Honeybunch
Copy link
Contributor Author

Totally! Ill file an issue on their repo and see what they say 😁

The git SHA is a good idea I can do that.

@BillyONeal BillyONeal added depends:upstream-changes Waiting on a change to the upstream project and removed requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look. labels Sep 11, 2024
@Honeybunch
Copy link
Contributor Author

Does this require any other changes?

BillyONeal
BillyONeal previously approved these changes Oct 18, 2024
Copy link
Member

@BillyONeal BillyONeal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JavierMatosD Do you know why you asked for team review?

@JavierMatosD
Copy link
Contributor

JavierMatosD commented Oct 22, 2024

@JavierMatosD Do you know why you asked for team review?

I'm asking for team review since these default features seem to enable APIs which is against our current policy. We accepted the sdl2 port's default-features but I think this was done erroneously or the policy was different when it was added.

@maia-s
Copy link

maia-s commented Oct 23, 2024

As of libsdl-org/SDL#11310, SDL fails to build on Linux if neither X11 or Wayland is enabled

@Honeybunch
Copy link
Contributor Author

Any ETA on this?

Copy link
Contributor

@madebr madebr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zby-c
Copy link

zby-c commented Nov 2, 2024

MERGE plz 🔥☺️

@Honeybunch Honeybunch dismissed stale reviews from BillyONeal and LilyWangLL via 882a0f3 November 7, 2024 01:47
@JavierMatosD JavierMatosD merged commit 6046e48 into microsoft:master Nov 7, 2024
@Honeybunch Honeybunch deleted the sdl3 branch November 7, 2024 16:20
@Pentarctagon
Copy link

To go along with this - are there any plans to add the SDL3 version of SDL_mixer and SDL_image?

@Honeybunch
Copy link
Contributor Author

I do have an sdl3 mixer port I can try to upstream. Not sure if that or SDL3_Image have had an ABI lock and a prerelease tag cut though

@Honeybunch
Copy link
Contributor Author

I do not have an SDL3_image port since i havent needed it

@madebr
Copy link
Contributor

madebr commented Nov 8, 2024

Only SDL3 is in API/ABI lock. None of SDL's satellite libraries are considered stable at this moment.

@Honeybunch
Copy link
Contributor Author

Thanks for the clarification 😁

@dg0yt
Copy link
Contributor

dg0yt commented Nov 9, 2024

This new port has uncontrolled dependencies with installation order problems. For example, it fails with hidapi and static libusb installed on android. This affects vcpkg CI (e.g. #42045).

There may be more issues, but due to pkg_check_modules it is difficult to track and control.

I also see copies of khronos headers which have ports in vcpkg.

@pbdot
Copy link

pbdot commented Nov 17, 2024

The import prefix fails on ubuntu, because the config cmake is in vcpkg\packages\sdl3_x64-linux\share\sdl3\SDL3 vs on Windows vcpkg\packages\sdl3_x64-windows-static\share\sdl3 meaning that it is looking for the required include and lib files in share and not the actual package folder.

get_filename_component(_IMPORT_PREFIX "${CMAKE_CURRENT_LIST_FILE}" PATH)
get_filename_component(_IMPORT_PREFIX "${_IMPORT_PREFIX}" PATH)
get_filename_component(_IMPORT_PREFIX "${_IMPORT_PREFIX}" PATH)

Edit: I was able to fix this using a patched overlay port, with this patch on root SDL3 CMakeLists.txt

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 217d9f5..c94f0d3 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -3455,7 +3455,7 @@ elseif(SDL_FRAMEWORK)
   set(SDL_INSTALL_LICENSEDIR "Resources")
   set(SDL_INSTALL_HEADERSDIR "Headers")
 else()
-  set(SDL_INSTALL_CMAKEDIR "${SDL_INSTALL_CMAKEDIR_ROOT}/SDL3")
+  set(SDL_INSTALL_CMAKEDIR "${SDL_INSTALL_CMAKEDIR_ROOT}")
   set(SDL_INSTALL_LICENSEDIR "${CMAKE_INSTALL_DATAROOTDIR}/licenses/${PROJECT_NAME}")
   set(SDL_INSTALL_HEADERSDIR "${CMAKE_INSTALL_INCLUDEDIR}/SDL3")
 endif()

@dg0yt dg0yt mentioned this pull request Nov 18, 2024
@dg0yt
Copy link
Contributor

dg0yt commented Nov 18, 2024

The import prefix fails on ubuntu, because the config cmake is in vcpkg\packages\sdl3_x64-linux\share\sdl3\SDL3

That's the wrong location, indeed. But my diagnose is a little bit different. #42206.

@constcuriosity constcuriosity mentioned this pull request Dec 23, 2024
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category:new-port The issue is requesting a new library to be added; consider making a PR! info:needs-maintainer-attention Lets the current 'on rotation' vcpkg maintainer know they need to look at this. requires:vcpkg-team-review This PR or issue requires someone on the vcpkg team to take a further look.

Projects

None yet

Development

Successfully merging this pull request may close these issues.