X Tutup
Skip to content

Fix compiling SDL without DBus under Linux#111146

Merged
Repiteo merged 1 commit intogodotengine:masterfrom
Kaleb-Reid:fix-sdl-no-dbus
Oct 2, 2025
Merged

Fix compiling SDL without DBus under Linux#111146
Repiteo merged 1 commit intogodotengine:masterfrom
Kaleb-Reid:fix-sdl-no-dbus

Conversation

@Kaleb-Reid
Copy link
Contributor

@Kaleb-Reid Kaleb-Reid commented Oct 2, 2025

Some files being compiled in the sdl driver which require DBus support do not seem to actually be used.

Fixes #111049

@Kaleb-Reid Kaleb-Reid requested a review from a team as a code owner October 2, 2025 06:33
Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

Is it building and joystick input working without these files? If it is, we should remove them completely. System theme code should not be used at all, and FCITX (same for ibus) is for IME, and we only use SDL for joystick, not text input.

@Kaleb-Reid Kaleb-Reid force-pushed the fix-sdl-no-dbus branch 2 times, most recently from a11159f to 64fd116 Compare October 2, 2025 07:14
@Nintorch
Copy link
Contributor

Nintorch commented Oct 2, 2025

Can patch files be changed after they've been created? If we remove these files, I think we should also remove the related changes from the patch files, see:
https://github.com/godotengine/godot/blob/master/thirdparty/sdl/patches/0001-remove-unnecessary-subsystems.patch#L360-L822

SDL update script should also probably be changed: https://github.com/godotengine/godot/blob/master/thirdparty/sdl/update-sdl.sh#L50

@Kaleb-Reid
Copy link
Contributor Author

@bruvzg I can build and joystick works without either of these files, so I've just removed them. I also removed two other files related to IME which don't appear to do anything.

I do get a bug when compiling with threads=no which causes input to not work (can't click anything on the project selection screen) and

WARNING: Keeping screen on not supported by this display server.
     at: screen_set_keep_on (servers/display_server.cpp:579)

to be printed to the console, but my assumption is that this doesn't have anything to do with this pr.

@akien-mga
Copy link
Member

akien-mga commented Oct 2, 2025

Can patch files be changed after they've been created? If we remove these files, I think we should also remove the related changes from the patch files, see: https://github.com/godotengine/godot/blob/master/thirdparty/sdl/patches/0001-remove-unnecessary-subsystems.patch#L360-L822

Yes, the way I typically do it is to reverse the patch (e.g. with patch -p3 -R < old_patch.patch), stage changes, then reapply the patch, do the changes which are needed, and git diff > new_patch.patch. Then stage everything and commit. The new patch name can be the same as the old so there's just a diff on the diff :P

@Kaleb-Reid
Copy link
Contributor Author

@Nintorch I removed the files from the patch which are no longer being compiled.

I'm not sure what to change about the update script, should I just rm the unused files?

@akien-mga
Copy link
Member

Yeah you should rm the files (and their headers, also unused) and likewise delete them from the Godot repo.

diff --git a/thirdparty/sdl/update-sdl.sh b/thirdparty/sdl/update-sdl.sh
index 11a9d37f48..7aa8c2b915 100755
--- a/thirdparty/sdl/update-sdl.sh
+++ b/thirdparty/sdl/update-sdl.sh
@@ -49,6 +49,7 @@ cp -v io/SDL_iostream*.{c,h} $target/io
 mkdir $target/core
 cp -rv core/{linux,unix,windows} $target/core
 rm -f $target/core/windows/version.rc
+rm -f $target/core/linux/SDL_{fcitx,ibus,ime,system_theme}.*
 
 mkdir $target/haptic
 cp -rv haptic/{*.{c,h},darwin,linux,windows} $target/haptic

@akien-mga akien-mga added this to the 4.6 milestone Oct 2, 2025
@akien-mga akien-mga added the cherrypick:4.5 Considered for cherry-picking into a future 4.5.x release label Oct 2, 2025
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Looks good to me, always nice when a diff is:
image

@Repiteo Repiteo merged commit 51ba8c1 into godotengine:master Oct 2, 2025
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Oct 2, 2025

Thanks!

@Kaleb-Reid Kaleb-Reid deleted the fix-sdl-no-dbus branch October 3, 2025 04:50
@akien-mga
Copy link
Member

Cherry-picked for 4.5.1.

@akien-mga akien-mga removed the cherrypick:4.5 Considered for cherry-picking into a future 4.5.x release label Oct 8, 2025
kilian-diener added a commit to kilian-diener/godot that referenced this pull request Oct 15, 2025
Co-authored-by: Kaleb Reid <78945904+Kaleb-Reid@users.noreply.github.com>
kilian-diener added a commit to kilian-diener/godot that referenced this pull request Feb 26, 2026
Co-authored-by: Kaleb Reid <78945904+Kaleb-Reid@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can't build with threads=no on Linux following the introduction of SDL

5 participants

X Tutup