SDL icon indicating copy to clipboard operation
SDL copied to clipboard

Recent ibus compatibility

Open sorsarre opened this issue 3 years ago • 3 comments

Looks like recent versions of ibus (as of 1.5.25) are not compatible with sdl master — ibus has migrated to using ibus portal (org.freedesktop.portal.IBus et al) for CreateInputContext at some point, so that seems to be causing the problem.

I've tried a quick shoddy patch-up changing service/path/iface, but somehow it couldn't even create input context. Could be ibus' problem, but could be my crooked hands as well though.

The old call sequence somehow still manages to create an input context object, but all calls to it fail every time so SDL defaults to non-IME processing on X11, which results in wierd things like Alt+F generating TextInputEvent.

Setup:

  • arch linux
  • ibus 1.5.25
  • sdl @ master tip

sorsarre avatar Aug 30 '21 13:08 sorsarre

So this appears to be working (at least for me mashing keys and not understanding Japanese at all), on the latest update of Ubuntu 21.10, which uses ibus 1.5.25:

2021-11-19_09-13

It's popping up an IME UI in the correct place as I type and appears to be supplying the composed characters to SDL appropriately.

It's possible this was an ibus or SDL bug that was fixed, or an issue on Arch Linux's package. If ibus has moved to a new interface and deprecated the current one we should deal with that (in SDL 2.0.20), but unless I'm misunderstanding the problem, I think we're okay on our end for 2.0.18.

icculus avatar Nov 19 '21 14:11 icculus

Bumping this to 2.0.20 for now, in case Arch Linux is the canary in the coal mine on a deeper bug.

icculus avatar Nov 20 '21 15:11 icculus

Okay, I can't get it to make an InputContext through the portal interface, either.

static const char IBUS_SERVICE[]         = "org.freedesktop.portal.IBus";
static const char IBUS_PATH[]            = "/org/freedesktop/IBus";
static const char IBUS_INTERFACE[]       = "org.freedesktop.IBus.Portal";

I think this is right, but the call to CreateInputContext always fails. I'm not sure why.

The only thing I can imagine being wrong is we're on the wrong bus address entirely. d-feet can see the interface with a session bus address of unix:path=/run/user/1000/bus but we're connecting to something like unix:abstract=/home/icculus/.cache/ibus/dbus-6JuFxgTp,guid=b07a526c75a163d630fc00c561c09ce9 and I suspect this ibus-specific address doesn't have the portal interface. Just giving an update here; I have to dig into this code more.

icculus avatar Dec 23 '21 16:12 icculus

@smcv Do you happen to have any insight into this? We just can't coerce the "portal" version of this d-bus interface to work, and we're wondering if this is the sort of thing where you already magically know what we're doing wrong. :) No worries if not.

icculus avatar Oct 26 '22 03:10 icculus

The D-Bus session bus is how you access most D-Bus services, including things like xdg-desktop-portal, o.fd.Screensaver, o.fd.Notifications, systemd --user and similar low-frequency, not-particularly-latency-sensitive services. Its address is often unix:path=/run/user/1000/bus, but not always. It's implemented by dbus-daemon --session or sometimes dbus-broker (depending on distro and configuration). On the message bus, you never talk to the services directly; instead, you connect to the message bus socket and send messages, and the message bus passes them on to the actual services (a "star" topology). In SDL, the SDL_DBusContext->session_conn is connected to this message bus.

My understanding is that ibus is (or has historically been) an unusual user of D-Bus: it listens on its own separate AF_UNIX socket, and you connect directly to that socket and send it messages, without going via a message bus implementation like dbus-daemon or dbus-broker. This is more complicated and needs more individual connections, but can be better for higher-frequency or lower-latency messaging. This private bus is the reason why the ibus code has to call dbus->connection_open_private instead of using the session_conn.

I don't know whether ibus' new "portal" interface is on the special ibus socket, or on the D-Bus session bus. If you're able to see it in d-feet, then I suspect it's on the D-Bus session bus? If so, you need to connect to the D-Bus session bus to talk to it (like you do for xdg-desktop-portal or org.freedesktop.Screensaver or whatever), and connecting to the special ibus socket will not work.

For completeness: the other common way to use D-Bus is the system bus, implemented by dbus-daemon --system or sometimes dbus-broker, and typically listening on unix:path=/run/dbus/system_bus_socket. This is another message bus with a "star" topology, similar to the session bus, but unlike the session bus it's shared by all users of a machine and used to talk to system-level services (NetworkManager, udisks, systemd, logind, that sort of thing). In SDL, the SDL_DBusContext->system_conn is connected to this message bus.

smcv avatar Oct 26 '22 10:10 smcv

I suspect the API might go something like this:

  • if you are running unconfined on the host system, then you continue to talk to ibus the old way, via a private dbus_connection_open_private();
  • but if you are in a Flatpak/Snap/... container that doesn't have access to the appropriate path for ibus' private socket, then you need to use the portal instead, via the dbus->session_conn

smcv avatar Oct 26 '22 16:10 smcv

Aaaah, I got it to work, thanks to @smcv's help.

I'm not sure, but this patch is probably too big to put in for 2.26.0 at this point...it's not something I can really test heavily, but testime definitely pops up a little UI to help me enter Hirigana chars when this is talking to the portal interface.

If this is getting bumped, I'm move this to a Pull Request so we can apply it to SDL3 or whatnot.

diff --git a/src/core/linux/SDL_ibus.c b/src/core/linux/SDL_ibus.c
index b4fe23154..94f7aa626 100644
--- a/src/core/linux/SDL_ibus.c
+++ b/src/core/linux/SDL_ibus.c
@@ -37,17 +37,27 @@
 #include <unistd.h>
 #include <fcntl.h>
 
-static const char IBUS_SERVICE[]         = "org.freedesktop.IBus";
 static const char IBUS_PATH[]            = "/org/freedesktop/IBus";
+
+static const char IBUS_SERVICE[]         = "org.freedesktop.IBus";
 static const char IBUS_INTERFACE[]       = "org.freedesktop.IBus";
 static const char IBUS_INPUT_INTERFACE[] = "org.freedesktop.IBus.InputContext";
 
+static const char IBUS_PORTAL_SERVICE[]         = "org.freedesktop.portal.IBus";
+static const char IBUS_PORTAL_INTERFACE[]       = "org.freedesktop.IBus.Portal";
+static const char IBUS_PORTAL_INPUT_INTERFACE[] = "org.freedesktop.IBus.InputContext";
+
+static const char *ibus_service = NULL;
+static const char *ibus_interface = NULL;
+static const char *ibus_input_interface = NULL;
 static char *input_ctx_path = NULL;
 static SDL_Rect ibus_cursor_rect = { 0, 0, 0, 0 };
 static DBusConnection *ibus_conn = NULL;
+static SDL_bool ibus_is_portal_interface = SDL_FALSE;
 static char *ibus_addr_file = NULL;
 static int inotify_fd = -1, inotify_wd = -1;
 
+
 static Uint32
 IBus_ModState(void)
 {
@@ -202,7 +212,7 @@ IBus_MessageHandler(DBusConnection *conn, DBusMessage *msg, void *user_data)
 {
     SDL_DBusContext *dbus = (SDL_DBusContext *)user_data;
 
-    if (dbus->message_is_signal(msg, IBUS_INPUT_INTERFACE, "CommitText")) {
+    if (dbus->message_is_signal(msg, ibus_input_interface, "CommitText")) {
         DBusMessageIter iter;
         const char *text;
 
@@ -224,7 +234,7 @@ IBus_MessageHandler(DBusConnection *conn, DBusMessage *msg, void *user_data)
         return DBUS_HANDLER_RESULT_HANDLED;
     }
 
-    if (dbus->message_is_signal(msg, IBUS_INPUT_INTERFACE, "UpdatePreeditText")) {
+    if (dbus->message_is_signal(msg, ibus_input_interface, "UpdatePreeditText")) {
         DBusMessageIter iter;
         const char *text;
 
@@ -273,7 +283,7 @@ IBus_MessageHandler(DBusConnection *conn, DBusMessage *msg, void *user_data)
         return DBUS_HANDLER_RESULT_HANDLED;
     }
 
-    if (dbus->message_is_signal(msg, IBUS_INPUT_INTERFACE, "HidePreeditText")) {
+    if (dbus->message_is_signal(msg, ibus_input_interface, "HidePreeditText")) {
         SDL_SendEditingText("", 0, 0);
         return DBUS_HANDLER_RESULT_HANDLED;
     }
@@ -415,7 +425,7 @@ IBus_SetCapabilities(void *data, const char *name, const char *old_val,
             caps |= IBUS_CAP_PREEDIT_TEXT;
         }
 
-        SDL_DBus_CallVoidMethodOnConnection(ibus_conn, IBUS_SERVICE, input_ctx_path, IBUS_INPUT_INTERFACE, "SetCapabilities",
+        SDL_DBus_CallVoidMethodOnConnection(ibus_conn, ibus_service, input_ctx_path, ibus_input_interface, "SetCapabilities",
                                 DBUS_TYPE_UINT32, &caps, DBUS_TYPE_INVALID);
     }
 }
@@ -428,36 +438,56 @@ IBus_SetupConnection(SDL_DBusContext *dbus, const char* addr)
     const char *path = NULL;
     SDL_bool result = SDL_FALSE;
     DBusObjectPathVTable ibus_vtable;
-
+    
     SDL_zero(ibus_vtable);
     ibus_vtable.message_function = &IBus_MessageHandler;
 
-    ibus_conn = dbus->connection_open_private(addr, NULL);
-
-    if (!ibus_conn) {
-        return SDL_FALSE;
-    }
+    /* try the portal interface first. Modern systems have this in general,
+       and sandbox things like FlakPak and Snaps, etc, require it. */
+
+    ibus_is_portal_interface = SDL_TRUE;
+    ibus_service = IBUS_PORTAL_SERVICE;
+    ibus_interface = IBUS_PORTAL_INTERFACE;
+    ibus_input_interface = IBUS_PORTAL_INPUT_INTERFACE;
+    ibus_conn = dbus->session_conn;
+
+    result = SDL_DBus_CallMethodOnConnection(ibus_conn, ibus_service, IBUS_PATH, ibus_interface, "CreateInputContext",
+                                             DBUS_TYPE_STRING, &client_name, DBUS_TYPE_INVALID,
+                                             DBUS_TYPE_OBJECT_PATH, &path, DBUS_TYPE_INVALID);
+    if (!result) {
+        ibus_is_portal_interface = SDL_FALSE;
+        ibus_service = IBUS_SERVICE;
+        ibus_interface = IBUS_INTERFACE;
+        ibus_input_interface = IBUS_INPUT_INTERFACE;
+        ibus_conn = dbus->connection_open_private(addr, NULL);
+
+        if (!ibus_conn) {
+            return SDL_FALSE;  /* oh well. */
+        }
 
-    dbus->connection_flush(ibus_conn);
+        dbus->connection_flush(ibus_conn);
     
-    if (!dbus->bus_register(ibus_conn, NULL)) {
-        ibus_conn = NULL;
-        return SDL_FALSE;
-    }
+        if (!dbus->bus_register(ibus_conn, NULL)) {
+            ibus_conn = NULL;
+            return SDL_FALSE;
+        }
     
-    dbus->connection_flush(ibus_conn);
+        dbus->connection_flush(ibus_conn);
+
+        result = SDL_DBus_CallMethodOnConnection(ibus_conn, ibus_service, IBUS_PATH, ibus_interface, "CreateInputContext",
+                                                 DBUS_TYPE_STRING, &client_name, DBUS_TYPE_INVALID,
+                                                 DBUS_TYPE_OBJECT_PATH, &path, DBUS_TYPE_INVALID);
+    }
 
-    if (SDL_DBus_CallMethodOnConnection(ibus_conn, IBUS_SERVICE, IBUS_PATH, IBUS_INTERFACE, "CreateInputContext",
-            DBUS_TYPE_STRING, &client_name, DBUS_TYPE_INVALID,
-            DBUS_TYPE_OBJECT_PATH, &path, DBUS_TYPE_INVALID)) {
+    if (result) {
+        char matchstr[128];
+        SDL_snprintf(matchstr, sizeof (matchstr), "type='signal',interface='%s'", ibus_input_interface);
         SDL_free(input_ctx_path);
         input_ctx_path = SDL_strdup(path);
         SDL_AddHintCallback(SDL_HINT_IME_INTERNAL_EDITING, IBus_SetCapabilities, NULL);
-        
-        dbus->bus_add_match(ibus_conn, "type='signal',interface='org.freedesktop.IBus.InputContext'", NULL);
+        dbus->bus_add_match(ibus_conn, matchstr, NULL);
         dbus->connection_try_register_object_path(ibus_conn, input_ctx_path, &ibus_vtable, dbus, NULL);
         dbus->connection_flush(ibus_conn);
-        result = SDL_TRUE;
     }
 
     SDL_IBus_SetFocus(SDL_GetKeyboardFocus() != NULL);
@@ -551,6 +581,18 @@ SDL_IBus_Init(void)
         
         result = IBus_SetupConnection(dbus, addr);
         SDL_free(addr);
+
+        /* don't use the addr_file if using the portal interface. */
+        if (result && ibus_is_portal_interface) {
+            if (inotify_fd > 0) {
+                if (inotify_wd > 0) {
+                    inotify_rm_watch(inotify_fd, inotify_wd);
+                    inotify_wd = -1;
+                }
+                close(inotify_fd);
+                inotify_fd = -1;
+            }
+        }
     }
     
     return result;
@@ -573,16 +615,25 @@ SDL_IBus_Quit(void)
     
     dbus = SDL_DBus_GetContext();
     
-    if (dbus && ibus_conn) {
+    /* if using portal, ibus_conn == session_conn; don't release it here. */
+    if (dbus && ibus_conn && !ibus_is_portal_interface) {
         dbus->connection_close(ibus_conn);
         dbus->connection_unref(ibus_conn);
     }
-    
+
+    ibus_conn = NULL;
+    ibus_service = NULL;
+    ibus_interface = NULL;
+    ibus_input_interface = NULL;
+    ibus_is_portal_interface = SDL_FALSE;
+
     if (inotify_fd > 0 && inotify_wd > 0) {
         inotify_rm_watch(inotify_fd, inotify_wd);
         inotify_wd = -1;
     }
-    
+
+    /* !!! FIXME: should we close(inotify_fd) here? */
+
     SDL_DelHintCallback(SDL_HINT_IME_INTERNAL_EDITING, IBus_SetCapabilities, NULL);
     
     SDL_memset(&ibus_cursor_rect, 0, sizeof(ibus_cursor_rect));
@@ -594,7 +645,7 @@ IBus_SimpleMessage(const char *method)
     SDL_DBusContext *dbus = SDL_DBus_GetContext();
     
     if ((input_ctx_path != NULL) && (IBus_CheckConnection(dbus))) {
-        SDL_DBus_CallVoidMethodOnConnection(ibus_conn, IBUS_SERVICE, input_ctx_path, IBUS_INPUT_INTERFACE, method, DBUS_TYPE_INVALID);
+        SDL_DBus_CallVoidMethodOnConnection(ibus_conn, ibus_service, input_ctx_path, ibus_input_interface, method, DBUS_TYPE_INVALID);
     }
 }
 
@@ -624,7 +675,7 @@ SDL_IBus_ProcessKeyEvent(Uint32 keysym, Uint32 keycode, Uint8 state)
         if (state == SDL_RELEASED) {
             mods |= (1 << 30); // IBUS_RELEASE_MASK
         }
-        if (!SDL_DBus_CallMethodOnConnection(ibus_conn, IBUS_SERVICE, input_ctx_path, IBUS_INPUT_INTERFACE, "ProcessKeyEvent",
+        if (!SDL_DBus_CallMethodOnConnection(ibus_conn, ibus_service, input_ctx_path, ibus_input_interface, "ProcessKeyEvent",
                 DBUS_TYPE_UINT32, &keysym, DBUS_TYPE_UINT32, &ibus_keycode, DBUS_TYPE_UINT32, &mods, DBUS_TYPE_INVALID,
                 DBUS_TYPE_BOOLEAN, &result, DBUS_TYPE_INVALID)) {
             result = 0;
@@ -679,7 +730,7 @@ SDL_IBus_UpdateTextRect(const SDL_Rect *rect)
     dbus = SDL_DBus_GetContext();
     
     if (IBus_CheckConnection(dbus)) {
-        SDL_DBus_CallVoidMethodOnConnection(ibus_conn, IBUS_SERVICE, input_ctx_path, IBUS_INPUT_INTERFACE, "SetCursorLocation",
+        SDL_DBus_CallVoidMethodOnConnection(ibus_conn, ibus_service, input_ctx_path, ibus_input_interface, "SetCursorLocation",
                 DBUS_TYPE_INT32, &x, DBUS_TYPE_INT32, &y, DBUS_TYPE_INT32, &ibus_cursor_rect.w, DBUS_TYPE_INT32, &ibus_cursor_rect.h, DBUS_TYPE_INVALID);
     }
 }

icculus avatar Nov 17 '22 04:11 icculus

I'll leave the call to @slouken on whether to apply it for 2.26.0.

icculus avatar Nov 17 '22 04:11 icculus

I vote we include it.

slouken avatar Nov 17 '22 04:11 slouken

Included!

icculus avatar Nov 17 '22 04:11 icculus