I implemented two features:
- allow us to input non-latin languages such as Japanese/Chinese
- recover XIM connection and its input contexts when XIM server restarted
Currently it supports only "root window" style input, while most people (and I) want "over the spot" or "on the spot" style one. Probably the implementation of "over the spot" or "on the spot" style becomes much complicated, because XIM server requires the coordinates of current cursor location relative to the screen in order to show the candidate window in appropriate position.
Please check the patch.
Closed, ResolvedPublicXIM improvement (non-latin support + connection recovery)
- Assigned To
- Campbell Barton (campbellbarton)
- Shinsuke Irie (irie)
- Dalai Felinto (dfelinto), mato T (mato_sus304), Sergey Sharybin (sergey) and 4 others
I implemented two features:
Tiny fix. New patch is XIM_non-latin_V2.patch.
In my setting of IBus, control+m is assigned to "commit string", but this key stroke didn't work in the text editor. For example, if I'm going to input a utf8 character "あ", typing control+m makes a pseudo key event あ, but the control key remains pressed, so this event will be recognized as control+あ unwantedly. I modified the initial patch so that the modifiers in utf8 character events are ignored.
Although it works fine now, utf8 string still cannot be displayed in the text editor :p
Hi, quick review of the patch, though I'm not exert in this area there are a few Q's I have about this patch.
* setlocale() call is suspicious. I'm not sure of why this is needed but worry it would conflict with our translation support, or different users configurations. Can't the existing locale be used?
see ./source/blender/blenfont/intern/blf_lang.c's use of setlocale()
* utf8_buf[256 * 6] --- this size seems arbitrary (and very large), is there some define that can be used instead?
Hi Campbell, sorry for late reply.
> * setlocale() call is suspicious. I'm not sure of why this is needed but worry it would conflict with our translation support, or different users configurations. Can't the existing locale be used?
setlocale() is essential here. If removing this, no utf8 character will be inserted. In my patch, setlocale() uses system locale, but the other locales should be OK if it's available on the system. So, probably BLF_lang_set() can be used instead of setlocale(), but I couldn't use it here because I don't know if GHOST is permitted to include BLF.
> * utf8_buf[256 * 6] --- this size seems arbitrary (and very large), is there some define that can be used instead?
I think (256 * 6) is sufficiently large value to input one sentence in any languages. If using small array and the string exceeds this length, Xutf8LookupString() sets the status "BufferOverflow". In such case, we still can receive the string by calling Xutf8LookupString() again after expanding the array, but the code becomes somewhat complicated.
Changed the buffer size to 16 * 6 = 96 bytes. If the length of the input string is larger than 96 bytes, allocate another memory area and call Xutf8LookupString() again.
The other apps use very different sizes. For example, Wine and Emacs use 24 bytes and 513 bytes, respectively.
If WITH_X11_XINPUT=OFF, use 1 byte array to store one null character.
Changed to set utf8_buf NULL if WITH_X11_XINPUT=OFF. NULL pointer means not an utf8 character.
BTW, the documentation of GHOST_EventKey's constructor needs updating because the parameter btf8_buf is not described.
Removed setlocale() in GHOST_SystemX11.cpp. Instead, moved BLF_lang_init() call in WM_init() so that it will be called before wm_ghost_init(), and modified BLF_lang_init() to call setlocale() even if U.transopts&USER_DOTRANSLATE is zero.
Enlarged utf8_buf by adding extra 5 bytes. The last 5 bytes of the buffer won't be used to obtain the utf8 data but should be useful to avoid the segfault, though such segfault have never happened on my system. Not using the extra memory area at the end of the buffer is potentially dangerous, because the constructor of GHOST_EventKey reads 6 bytes unconditionally even if the actual data is only 2-3 bytes.
Reverted the changes of the patch V6 because UI translation became not working.
blf_lang_set() cannot be used instead of setlocale() because GHOST_CreateSystemPaths() has not been executed yet. Don't worry, setlocale() in GHOST_SystemX11.cpp should not conflict with the other locale settings in Blender, because this setlocale() is executed earlier than all of them so its locale simply will be overridden by them, and XIM can work after locale is changed.
This patch fixes the problem of inputing ' " and accented letters in the Text Editor and in the UI elements
It still doesn't work for the 'Console' though.
(ps I had to apply the patch with `patch -p1 < XIM....patch`. next time it's better to create the patch from inside the blender folder)
Thanks for the testing.
> It still doesn't work for the 'Console' though.
This patch solves only X11 specific problems. I think the console issue should be fixed by another patch.
> (ps I had to apply the patch with `patch -p1 < XIM....patch`. next time it's better to create the patch from inside the blender folder)
Sorry, this is a Debian package style patch because I'm testing it in PPA in Launchpad. (https://launchpad.net/~irie/+archive/blender)
- it's ok to use printf for error reporting in ghost but for diagnostics messages like "XIM input context destroyed" better to use GHOST_PRINT which will print messages only if blender is run with -d flag.
- It's still looks a bit hackish things happening with setlocale. You're initializing XIM with one locale which would be changed later. Not sure if it'll always work fine. Maky XIM input might be initialized after loading startup.blend and setting locale used by the interface?
1. Don't use setlocale(), and don't open connection to XIM server on startup. The connection will be opened when the first KeyPress event is received after entering the event loop.
2. Use GHOST_PRINT() instead of printf() to indicate that the XIM connection is opened or XIC is created.
Thanks for the review.
> diagnostics messages like "XIM input context destroyed"
"XIM input context destroyed" is an error message that will be displayed only when the XIM server unexpectedly shuts down.
> GHOST_PRINT which will print messages only if blender is run with -d flag.
GHOST_PRINT won't be enabled by -d flag but GHOST_DEBUG macro should enable it. (Currently GHOST_DEBUG seems to be broken.)
Not only KeyPress event, but also FocusIn event opens the XIM connection and creates the input context. Typically FocusIn event occurs earlier than KeyPress event, so checking the KeyPress event works as a fallback.
I have tested this patch and confirm that it works as expected with IBus and Ubuntu 12.04. It does not interfere with non-XIM input method for non-latin languages such as russian, and does not change visible behavior when WITH_X11_XINPUT is disabled. The only thing XIM user have to remember is to switch off IBus with ctrl+space after finishing non-latin text input. I hope it's not a big deal for XIM users.