meson: Bump SDL3 to 3.4.2#2751
Conversation
Summary of ChangesHello @mborgerson, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request updates the project's dependency on the SDL3 library to version 3.4.2. The change primarily involves updating the Meson wrap file to point to the new SDL3 release and incorporating its comprehensive Meson build configurations. This ensures the project leverages the most current version of SDL3, benefiting from its latest advancements and maintaining a robust build setup. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request successfully bumps SDL3 to version 3.4.2 and transitions from using a WrapDB patch to local package files. The build system logic has been updated to reflect the new version and structure. I have identified a few minor issues related to typo in variable names, misplaced function checks in header lists, and potential inconsistencies in platform detection logic.
| ['ddraw.h', 'HAVE_DDRAW_H'], | ||
| ['dsound.h', 'HAVE_DSOUND_H'], | ||
| ['dinput.h', 'HAVE_DINPUT_H'], | ||
| ['signal.h', 'HAVE_SIGTIMEDWAIT'], |
There was a problem hiding this comment.
The entry ['signal.h', 'HAVE_SIGTIMEDWAIT'] is misplaced in the basic_headers list. sigtimedwait is a function, not a header file. It should be moved to the check_functions list, and signal.h is already checked on line 57.
| sdl_deps += dependency('libudev') | ||
| sdl_deps += dependency('dbus-1') | ||
| sdl_deps += dependency('libpipewire-0.3') | ||
| sdl_deps += dependency('wayland-client') | ||
| sdl_deps += dependency('wayland-egl') | ||
| sdl_deps += dependency('wayland-cursor') |
There was a problem hiding this comment.
These dependencies (udev, dbus, pipewire, wayland-*) are added to sdl_deps without required: false. This will cause the build to fail on Linux systems where these libraries are missing. In SDL, these are typically optional features.
sdl_deps += dependency('libudev', required: false)
sdl_deps += dependency('dbus-1', required: false)
sdl_deps += dependency('libpipewire-0.3', required: false)
sdl_deps += dependency('wayland-client', required: false)
sdl_deps += dependency('wayland-egl', required: false)
sdl_deps += dependency('wayland-cursor', required: false)
| sdl_deps += cc.find_library('setupapi') | ||
| sdl_deps += cc.find_library('winmm') | ||
| sdl_deps += cc.find_library('opengl32') | ||
| elif host_machine.subsystem() == 'macos' |
There was a problem hiding this comment.
Using host_machine.subsystem() == 'macos' is inconsistent with the check on line 235 which uses host_machine.system() == 'darwin'. It is generally safer and more standard in Meson to use host_machine.system() == 'darwin' for macOS detection.
elif host_machine.system() == 'darwin'
| ) | ||
|
|
||
| # parse XKB version to avoid issues within SDL3 config | ||
| xkdbcommon_version = xkbcommon_dep.version().split('.') |
There was a problem hiding this comment.
There is a typo in the variable name xkdbcommon_version. It should likely be xkbcommon_version to match the dependency name xkbcommon_dep.
xkbcommon_version = xkbcommon_dep.version().split('.')
| xkdbcommon_major = xkdbcommon_version[0].to_int() | ||
| xkdbcommon_minor = xkdbcommon_version[1].to_int() | ||
| xkdbcommon_patch = xkdbcommon_version[2].to_int() |
There was a problem hiding this comment.
These variable names also contain the typo 'xkdb'.
xkbcommon_major = xkbcommon_version[0].to_int()
xkbcommon_minor = xkbcommon_version[1].to_int()
xkbcommon_patch = xkbcommon_version[2].to_int()
| cdata.set('SDL_XKBCOMMON_VERSION_MAJOR', xkdbcommon_major) | ||
| cdata.set('SDL_XKBCOMMON_VERSION_MINOR', xkdbcommon_minor) | ||
| cdata.set('SDL_XKBCOMMON_VERSION_PATCH', xkdbcommon_patch) |
There was a problem hiding this comment.
Updating the cdata.set calls to use the corrected variable names.
cdata.set('SDL_XKBCOMMON_VERSION_MAJOR', xkbcommon_major)
cdata.set('SDL_XKBCOMMON_VERSION_MINOR', xkbcommon_minor)
cdata.set('SDL_XKBCOMMON_VERSION_PATCH', xkbcommon_patch)
mborgerson
force-pushed
the
sync/sdl-3.4.2
branch
from
5fe8033 to
249bde2
Compare
|
Just did a quick test and the over brightened lighting issue in Xemu seems to be fixed in that new SDL update. |
mborgerson
force-pushed
the
sync/sdl-3.4.2
branch
from
249bde2 to
bf4b87d
Compare
mborgerson
force-pushed
the
sync/sdl-3.4.2
branch
from
bf4b87d to
490145f
Compare
|
Not gonna wait for wrapdb. We'll handle it independently for now. |
2 participants
Uh oh!
There was an error while loading. Please reload this page.