Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

XIM Preedit Area #1162

Open
xorgy opened this issue Oct 25, 2024 · 18 comments
Open

XIM Preedit Area #1162

xorgy opened this issue Oct 25, 2024 · 18 comments

Comments

@xorgy
Copy link
Contributor

xorgy commented Oct 25, 2024

Summary

It is unclear to me whether the vanilla XIM protocol has any way for a client to report the preedit area to an IM server, but either way it would be nice if Fcitx5 had some way of accepting a preedit region (in addition to the caret position) via XIM.
I am interested in adding the required code to XIM clients (particularly winit and SDL3).

The documentation of X IM is very poor so I'm not entirely sure whether this already exists, or would need to be an extension.

@xorgy
Copy link
Contributor Author

xorgy commented Oct 25, 2024

I'd also be willing to write the patch for Fcitx5, but I want to make sure there's interest in accepting it before I put in the work.

@wengxt
Copy link
Member

wengxt commented Oct 25, 2024

Can you explain what's the meaning of it and how can it be used?

One thing that you may want to know is that, winit use libX11. You'll end up push changes to libx11.
I'm not sure about SDL, since they already dropped xim support in sdl 2.

Based on the protocol itself, if would be ok to add an new attribute. Attribute name is passed by string and im server will announce the supported attribute. See XIM_OPEN_REPLY in https://www.x.org/releases/X11R7.6/doc/libX11/specs/XIM/xim.html

@xorgy
Copy link
Contributor Author

xorgy commented Oct 25, 2024

其他的輸入法協議(iBus, Fcitx Dbus)有一個編輯區屬性有報告那個屬性的方法
Other IME protocols (iBus, Fcitx Dbus) have a way to report an area as the preedit area rectangle.

現在只有插入點的座標
Right now It is only the caret position:

setCursorRect(
Rect().setPosition(reply->dst_x, reply->dst_y).setSize(0, 0));

@xorgy
Copy link
Contributor Author

xorgy commented Oct 25, 2024

Based on the protocol itself, if would be ok to add an new attribute. Attribute name is passed by string and im server will announce the supported attribute. See XIM_OPEN_REPLY in https://www.x.org/releases/X11R7.6/doc/libX11/specs/XIM/xim.html

Yeah, that may be the way to go, though there may be an issue with deciding when to report one or the other, and we would need to add state in the XIM frontend for Fcitx to ignore carets after receiving a rectangle.

@wengxt
Copy link
Member

wengxt commented Oct 25, 2024

I'm asking is because there's a XNArea in the protocol, that does something different (It's like negotiate with input method for a window size to render.)

https://docs.oracle.com/cd/E19620-01/805-3916/xtxlib-7/index.html

Yes, if you want cursor "point" to be cursor "rectangle", then it's indeed a new concept.

You can try to look for XNSpotLocation in the code, so you can know where to add a new one. Then XSetICValues would be able to handle it.

@wengxt
Copy link
Member

wengxt commented Oct 25, 2024

Based on the protocol itself, if would be ok to add an new attribute. Attribute name is passed by string and im server will announce the supported attribute. See XIM_OPEN_REPLY in https://www.x.org/releases/X11R7.6/doc/libX11/specs/XIM/xim.html

Yeah, that may be the way to go, though there may be an issue with deciding when to report one or the other, and we would need to add state in the XIM frontend for Fcitx to ignore carets after receiving a rectangle.

You can simply write the code by checking rectangle first (Assume it's called XNSpotRectangle), if not, fallback to the XNSpotLocation, that's not a big deal.

@wengxt
Copy link
Member

wengxt commented Oct 25, 2024

But just to let you know, that fcitx will always assume a fixed height (scale with dpi) when height is not available.

It has always been like since fcitx3 which means it's there for over 20 years that and it works in most case for xim.

it's never a big issue for fcitx.

@xorgy
Copy link
Contributor Author

xorgy commented Oct 26, 2024

I appreciate that in general, though in my case I am seeing it overlap, and I'd like to, at least in the handful of places I can implement this change, have the real rectangle. I have this working with ibus's XIM adapter too, and I'm looking at what might be the best way to get it accepted in libx11.
Supporting the real height is the right way to go ultimately, for edge cases (line near bottom of screen, etc.)

@xorgy
Copy link
Contributor Author

xorgy commented Oct 26, 2024

Assuming it is acceptable to Alan Coopersmith, do you think that there is anything wrong with reusing XNArea as a client-set preedit area in on-the-spot mode? It is barely documented anyhow, and I have found exactly zero implementations of geometry negotiation (which is itself almost entirely undocumented). I prototyped it this way with iBus and Fcitx5 and the libx11 change was very simple (adding a couple of bits to a mask in imRm.c). xcb-imdkit already interprets and parses XNArea as a preedit attr.

Adding a name is not a huge issue, but involves a more significant ABI change.

@wengxt
Copy link
Member

wengxt commented Oct 26, 2024

@xorgy If libx11 accepts your patch, I'm fine with it.

However if I'm maintainer of libx11, I won't take risk to make an incompatible change.

@xorgy
Copy link
Contributor Author

xorgy commented Oct 26, 2024

How do you feel about merging the “Show preedit in application” setting with the “Use On The Spot Style (Needs restarting)” one? As far as I'm aware they're the same setting expressed two ways. Not sure if global configs are accessible to frontends though..

@wengxt
Copy link
Member

wengxt commented Oct 26, 2024

No they are two separate settings.

@xorgy
Copy link
Contributor Author

xorgy commented Oct 28, 2024

libx11 tracking issue for doing this with XNArea in OnTheSpot mode at https://gitlab.freedesktop.org/xorg/lib/libx11/-/issues/222
with a patch to do this in libx11 at https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/270.
I've also put up an enabling patch for ibus in draft PR ibus/ibus#2693 and a tracking issue similar to this one at ibus/ibus#2691. I have draft PR #1163 for implementing this in Fcitx5.

@wengxt
Copy link
Member

wengxt commented Oct 30, 2024

How do you feel about merging the “Show preedit in application” setting with the “Use On The Spot Style (Needs restarting)” one? As far as I'm aware they're the same setting expressed two ways. Not sure if global configs are accessible to frontends though..

The reason that over the spot is default mainly due to this bug:

https://bugs.freedesktop.org/show_bug.cgi?id=1580

https://gitlab.freedesktop.org/xorg/lib/libx11/-/merge_requests/127

This is a style announce on xim server and can't be changed at run time if client is already connected.

The enable preedit in fcitx's global config is simply a fcitx side option. Well, fcitx can choose to send preedit or not, even if it's on the spot style.

Maybe now it's a better time to use on-the-spot style by default. However, it's tricky because I'd prefer to check on-host libx11 to contain the commit, but fcitx doesn't use libx11 directly so there's no way to check libx11 version.

Not to mention that app installed via flatpak may ship an older version of libx11.

@xorgy
Copy link
Contributor Author

xorgy commented Oct 31, 2024

Yeah, I am familiar with Kirill's XNSpotLocation change (and the history of it being merged in 2004 and then broken later). If we're going to be checking libx11 versions then at least we can check for both of them (when XNArea/whatever they allow is merged).
Main problem right now is that it's nobody's job to maintain libx11, but not just anyone can do maintainer work on it either... so maybe it's time to buy somebody lunch if I want this fixed upstream.

I can add the libx11 version check in a separate patch at least; I recognize that Ubuntu 22.04 (libx11 1.7) will be in regular support until 2027.

Then again if somebody's on the next release of fcitx5, are they on a distro with libx11 older than 1.8.2?

@xorgy
Copy link
Contributor Author

xorgy commented Oct 31, 2024

...there's no way to check libx11 version.

Not to mention that app installed via flatpak may ship an older version of libx11.

Could use wm hints to get the pid, which can get you the real path to the executable, which can be checked with ld... but this is evil and fragile, probably breaks in portals and over network, and most importantly doesn't work for the most popular XIM-only client (winit) which links libX11 at runtime and thus doesn't have linker info for it.

@wengxt
Copy link
Member

wengxt commented Nov 6, 2024

Yeah, I am familiar with Kirill's XNSpotLocation change (and the history of it being merged in 2004 and then broken later). If we're going to be checking libx11 versions then at least we can check for both of them (when XNArea/whatever they allow is merged). Main problem right now is that it's nobody's job to maintain libx11, but not just anyone can do maintainer work on it either... so maybe it's time to buy somebody lunch if I want this fixed upstream.

I can add the libx11 version check in a separate patch at least; I recognize that Ubuntu 22.04 (libx11 1.7) will be in regular support until 2027.

Then again if somebody's on the next release of fcitx5, are they on a distro with libx11 older than 1.8.2?

Well, since now fcitx can run as an flatpak app itself, such trick won't really work across the sandbox boundary.

Not to mention that if the client is running via ssh X forwarding.

I care position more than embedded preedit, so most likely I will keep it as is, or maybe add a compile time option to set the default value.

@xorgy
Copy link
Contributor Author

xorgy commented Nov 7, 2024

I care position more than embedded preedit, so most likely I will keep it as is, or maybe add a compile time option to set the default value.

Yeah, that would be useful. Some rolling distros would probably enable that by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants