Nux

Merge lp:~paulliu/nux/nux into lp:nux

Proposed by Ying-Chun Liu
Status: Merged
Merged at revision: 727
Proposed branch: lp:~paulliu/nux/nux
Merge into: lp:nux
Diff against target: 248 lines (+155/-1)
3 files modified
Nux/TextEntry.cpp (+2/-0)
NuxGraphics/GraphicsDisplayX11.cpp (+138/-1)
NuxGraphics/GraphicsDisplayX11.h (+15/-0)
To merge this branch: bzr merge lp:~paulliu/nux/nux
Reviewer Review Type Date Requested Status
Brandon Schaefer (community) Approve
Francis Ginther Abstain
Tim Penhey (community) Abstain
Jay Taoko Pending
Review via email: mp+106875@code.launchpad.net

Description of the change

Hi Nux developers,

I'm adding XIM support for nux.
It makes nux-based apps work with other input methods (fcitx, gcin, hime..).
Please review my commit and ask me to improve it.

Many Thanks,
Paul

To post a comment you must log in.
Tim Penhey (thumper) wrote :

Thanks for this. I'm going to pass on this due to my unfamiliarity with input methods, but I'll make sure Jay looks at it :)

review: Abstain
Brandon Schaefer (brandontschaefer) wrote :

Hello Ying-Chun, I merged your branch with nux and it crashed running an example. Here are the stack traces:

http://pastebin.ubuntu.com/1078343/

full
http://pastebin.ubuntu.com/1078348/

Brandon Schaefer (brandontschaefer) wrote :

The problem was I didn't have my ibus-daemon on or any IM active...this shouldn't crash nux because of that.

Brandon Schaefer (brandontschaefer) wrote :

Also thanks for this :), this can help a lot!

Brandon Schaefer (brandontschaefer) wrote :

A few comments now:

Some small things:

Could you move that curly bracket to a new line?
98 + void GraphicsDisplay::InitXIM(struct XimClientData *data) {

There are some tabbing issues here.
7 ime_->Focus();
8 + nux::GetWindowThread()->GetGraphicsDisplay().XICFocus();

15 ime_->Blur();
16 + nux::GetWindowThread()->GetGraphicsDisplay().XICUnFocus();

To fix that crash....
Change to:

102 + xmodifier = getenv("XMODIFIERS");
103 + if (xmodifier && strstr(xmodifier,"ibus") != NULL)

As not all machines are guaranteed to have that env var.

Ill need to look more at this, as I am not very familiar with those other IM. We are going to also need to add some testing.

Also thanks again :), this also fixes the Greek and Arabic problem.

review: Needs Fixing
lp:~paulliu/nux/nux updated
616. By Ying-Chun Liu on 2012-07-09

XIM support for libnux.

This patch adds the XIM support for libnux.
It makes other input methods to work with nux based applications.
This fixes LP:973808 LP:983254 and some other similar bugs.

Ying-Chun Liu (paulliu) wrote :

Hi Brandon,

Thanks for the review.
I've fix the issues you mentioned and updated my branch.

Please take a look again.

Many Thanks,
Paul

Brandon Schaefer (brandontschaefer) wrote :

Hello Ying-Chun,

Ive been able to confirm that fcitx (note there is no preedit for fcitx) and hime working in Nux only. (It will not work in unity because unity does not Create an OpenGL window trough Nux). So this fixes bug 973808 for nux only. Though it should be easy to make it work for unity. (The OpenGL window is made in Compiz, so it uses this function in Nux: bool GraphicsDisplay::CreateFromOpenGLWindow)

I wasn't able to get gcin working, which it could be I couldn't understand the tools or any of the setup, as I sadly only know English. I am guessing it will work, but Ill keep trying.

One problem I am unsure about is. Would it be normal for people to use say ibus and another IM? Or is it always the case that only one is used? I am wondering this because if you used hime, fcitx etc then try to use IBus it will not work. A solution is to treat IBus as the default go to IM which can be dealt with when the IBus daemon starts up, which we detect.

Anyway Ill need to dig around some more and also think about a way to test this to make sure it will not cause any regression or other problems with nux or unity. Though it looks good as of now! Thank you!

Brandon Schaefer

Ying-Chun Liu (paulliu) wrote :

Hi Brandon,

Normally the user only use one IM. The user can install many IMs, and maybe running them together, but the apps will choose only one IM by XMODIFIERS environment variable. XSetLocaleModifiers() does this. And then we use XRegisterIMInstantiateCallback() which will be based on the value of XMODIFIERS.

BTW, ibus also supports XIM. So that means if without this block: "if (xmodifier && strstr(xmodifier,"ibus") != NULL) {...}", the ibus will use XIM protocol for nux and thus the ibus still works.
We adds the block here just to make sure that there's no regressions when using ibus.
ibus will use the original code, not the XIM.

A normal user would only use one IM because XMODIFIERS is assigned by im-switch.
But advanced user can use "env XMODIFIERS=otherim foo" to launch an app. And that app will use the specified IM. We can detect ibus daemon is running or not. But I think using XMODIFIERS to choose between im should be better and standard.

I can get gcin running. But gcin is tight binding on a lot of stuff. And yes, the setup is not very simple. I remember I installed a lot of gcin related stuff to make it work. Please make sure that gcin-* is all installed. And click the 4-th button of gcin-tools, make sure that some *Chinese* input methods are enabled.

Yours Sincerely,
Paul

Ying-Chun Liu (paulliu) wrote :

To make the preedit working, we have to extend this patch and use another "style".
Currently we are using "XIMStyle root_style = (XIMPreeditNothing|XIMStatusNothing);" so called "root style". Root style is simple and easy to implement.

In order to implement preedit, we have to implement "on the spot style". Needs to add mode codes in.

To implement it, we have to query xim_styles->supported_styles if "on the spot style" is provided by the XIM, and then we provide the info for "on the spot style".
The xim_styes->supported_styles is sorted by users preference. So normally we should implement a lot of different styles and match the first one in supported_styles. I can implement the rest styles but I don't have many time on this currently.

Brandon Schaefer (brandontschaefer) wrote :

Awesome, Ill start writing some test up. Those things will come in later, it would be more important to support this right now. Thank you again! Hopefully I can get some test out in the next couple days (Ill need to add a test for each IM).

I can mess with the idea of using XIM for ibus and with those styling later on :)

As soon as I finish up tests then this will be approve and merged!

Francis Ginther (fginther) wrote :

Review was claimed by accident, please ignore.

review: Abstain
Brandon Schaefer (brandontschaefer) wrote :

I've been running this branch for a while now, and when my tests get approved this should get merged as well.

review: Approve
Yu Ning (yuningdodo) wrote :

> I've been running this branch for a while now, and when my tests get approved
> this should get merged as well.

Hi Brandon Schaefer,

Could you also have a look on https://code.launchpad.net/~wengxt/nux/fcitx-support/+merge/114085 ?

Personally I had test this branch and that one for some while, and both branch works for me. However the branch from wengxt had implemented the XIM support in a not so hacky way, and the structure adjustment in that branch makes the IM not bound to IBUS, which is quite a nice improvement to nux in my personal opinion.

Thanks.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Nux/TextEntry.cpp'
2--- Nux/TextEntry.cpp 2012-07-03 23:24:53 +0000
3+++ Nux/TextEntry.cpp 2012-07-09 14:05:24 +0000
4@@ -856,6 +856,7 @@
5 need_im_reset_ = true;
6 #if defined(NUX_OS_LINUX)
7 ime_->Focus();
8+ nux::GetWindowThread()->GetGraphicsDisplay().XICFocus();
9 #endif
10 //gtk_im_context_focus_in(im_context_);
11 //UpdateIMCursorLocation();
12@@ -878,6 +879,7 @@
13 need_im_reset_ = true;
14 #if defined(NUX_OS_LINUX)
15 ime_->Blur();
16+ nux::GetWindowThread()->GetGraphicsDisplay().XICUnFocus();
17 #endif
18 //gtk_im_context_focus_out(im_context_);
19 }
20
21=== modified file 'NuxGraphics/GraphicsDisplayX11.cpp'
22--- NuxGraphics/GraphicsDisplayX11.cpp 2012-04-16 18:37:21 +0000
23+++ NuxGraphics/GraphicsDisplayX11.cpp 2012-07-09 14:05:24 +0000
24@@ -184,6 +184,111 @@
25 return(event->type == MapNotify) && (event->xmap.window == (Window) arg);
26 }
27
28+ void GraphicsDisplay::XICFocus()
29+ {
30+ if (m_ximData.m_xic)
31+ {
32+ XSetICFocus(m_ximData.m_xic);
33+ m_ximData.focus_stat=1;
34+ }
35+ }
36+
37+ void GraphicsDisplay::XICUnFocus()
38+ {
39+ if (m_ximData.m_xic)
40+ {
41+ XUnsetICFocus(m_ximData.m_xic);
42+ m_ximData.focus_stat=0;
43+ }
44+ }
45+
46+ void GraphicsDisplay::XIMEndCallback(Display *dpy, XPointer client_data, XPointer call_data)
47+ {
48+ struct XimClientData *data;
49+
50+ data = (struct XimClientData*)client_data;
51+ data->m_xim = NULL;
52+ data->m_xic = NULL;
53+ }
54+
55+ void GraphicsDisplay::XIMStartCallback(Display *dpy, XPointer client_data, XPointer call_data)
56+ {
57+ int i;
58+ XIMCallback destroy;
59+ XIMStyles *xim_styles = NULL;
60+ XIMStyle root_style = (XIMPreeditNothing|XIMStatusNothing);
61+ Window win;
62+ struct XimClientData *data;
63+
64+ data = (struct XimClientData*)client_data;
65+ win = * (data->window);
66+
67+ data->m_xim = XOpenIM(dpy, NULL, NULL, NULL);
68+ if (! (data->m_xim))
69+ {
70+ nuxDebugMsg("[GraphicsDisplay::XIMStartCallback] Failed to open IM.");
71+ return;
72+ }
73+ memset(&destroy, 0, sizeof(destroy));
74+ destroy.callback = (XIMProc)((GraphicsDisplay::XIMEndCallback));
75+ destroy.client_data = (XPointer)data;
76+ XSetIMValues((data->m_xim), XNDestroyCallback, &destroy, NULL);
77+ XGetIMValues((data->m_xim), XNQueryInputStyle, &xim_styles, NULL);
78+ for (i=0; i<xim_styles->count_styles; i++)
79+ {
80+ if (xim_styles->supported_styles[i] == root_style)
81+ {
82+ break;
83+ }
84+ }
85+ if (i>=xim_styles->count_styles)
86+ {
87+ nuxDebugMsg("[GraphicsDisplay::XIMStartCallback] root styles not supported.");
88+ return;
89+ }
90+ data->m_xic = XCreateIC(data->m_xim, XNInputStyle, root_style, XNClientWindow, win, XNFocusWindow, win, NULL);
91+ if (!(data->m_xic))
92+ {
93+ nuxDebugMsg("[GraphicsDisplay::XIMStartCallback] failed to register xic");
94+ }
95+ return;
96+ }
97+
98+ void GraphicsDisplay::InitXIM(struct XimClientData *data)
99+ {
100+ const char *xmodifier;
101+
102+ /* don't do anything if we are using ibus */
103+ xmodifier = getenv("XMODIFIERS");
104+ if (xmodifier && strstr(xmodifier,"ibus") != NULL)
105+ {
106+ nuxDebugMsg("[GraphicsDisplay::InitXIM] ibus natively supported");
107+ return;
108+ }
109+
110+ if (setlocale(LC_ALL, "") == NULL)
111+ {
112+ nuxDebugMsg("[GraphicsDisplay::InitXIM] cannot setlocale");
113+ }
114+
115+ if (!XSupportsLocale())
116+ {
117+ nuxDebugMsg("[GraphicsDisplay::InitXIM] no supported locale");
118+ }
119+
120+ if (XSetLocaleModifiers("") == NULL)
121+ {
122+ nuxDebugMsg("[GraphicsDisplay::InitXIM] XSetLocaleModifiers failed.");
123+ }
124+
125+ if (!XRegisterIMInstantiateCallback(*(data->display), NULL, NULL, NULL, GraphicsDisplay::XIMStartCallback, (XPointer)(data)))
126+ {
127+ nuxDebugMsg("[GraphicsDisplay::InitXIM] Cannot Register IM Init callback");
128+ }
129+ }
130+
131+
132+
133 // TODO: change windowWidth, windowHeight, to window_size;
134 static NCriticalSection CreateOpenGLWindow_CriticalSection;
135 bool GraphicsDisplay::CreateOpenGLWindow(const char *WindowTitle,
136@@ -568,6 +673,19 @@
137 //XMapRaised(m_X11Display, m_X11Window);
138 }
139
140+ // XIM support
141+ memset(&m_ximData,0,sizeof(m_ximData));
142+ m_ximData.window = &m_X11Window;
143+ m_ximData.display = &m_X11Display;
144+ InitXIM(&m_ximData);
145+
146+ long im_event_mask=0;
147+ if (m_ximData.m_xic)
148+ {
149+ XGetICValues(m_ximData.m_xic, XNFilterEvents, &im_event_mask, NULL);
150+ m_X11Attr.event_mask |= im_event_mask;
151+ }
152+
153 #ifndef NUX_OPENGLES_20
154 if (_has_glx_13)
155 {
156@@ -1229,6 +1347,8 @@
157 if (XPending(m_X11Display))
158 {
159 XNextEvent(m_X11Display, &xevent);
160+ if (XFilterEvent(&xevent, None) == True)
161+ return;
162
163 if (!_event_filters.empty())
164 {
165@@ -1535,6 +1655,10 @@
166 m_pEvent->dy = 0;
167 m_pEvent->virtual_code = 0;
168 //nuxDebugMsg("[GraphicsDisplay::ProcessXEvents]: FocusIn event.");
169+ if (m_ximData.m_xic && m_ximData.focus_stat==1)
170+ {
171+ XSetICFocus(m_ximData.m_xic);
172+ }
173 break;
174 }
175
176@@ -1550,6 +1674,11 @@
177 m_pEvent->dy = 0;
178 m_pEvent->virtual_code = 0;
179 //nuxDebugMsg("[GraphicsDisplay::ProcessXEvents]: FocusOut event.");
180+
181+ if (m_ximData.m_xic)
182+ {
183+ XUnsetICFocus(m_ximData.m_xic);
184+ }
185 break;
186 }
187
188@@ -1580,7 +1709,15 @@
189 skip = true;
190 }
191
192- int num_char_stored = XLookupString(&xevent.xkey, buffer, NUX_EVENT_TEXT_BUFFER_SIZE, (KeySym*) &m_pEvent->x11_keysym, NULL);
193+ int num_char_stored = 0;
194+ if (m_ximData.m_xic)
195+ {
196+ num_char_stored = XmbLookupString(m_ximData.m_xic, &xevent.xkey, buffer, NUX_EVENT_TEXT_BUFFER_SIZE, (KeySym*) &m_pEvent->x11_keysym, NULL);
197+ }
198+ else
199+ {
200+ num_char_stored = XLookupString(&xevent.xkey, buffer, NUX_EVENT_TEXT_BUFFER_SIZE, (KeySym*) &m_pEvent->x11_keysym, NULL);
201+ }
202 if (num_char_stored && (!skip))
203 {
204 Memcpy(m_pEvent->text, buffer, num_char_stored);
205
206=== modified file 'NuxGraphics/GraphicsDisplayX11.h'
207--- NuxGraphics/GraphicsDisplayX11.h 2012-01-26 23:01:57 +0000
208+++ NuxGraphics/GraphicsDisplayX11.h 2012-07-09 14:05:24 +0000
209@@ -67,6 +67,15 @@
210
211 DNDACTION_NONE,
212 };
213+
214+ struct XimClientData
215+ {
216+ XIM m_xim;
217+ XIC m_xic;
218+ Window *window;
219+ Display **display;
220+ int focus_stat;
221+ };
222
223 #define NUX_THREADMSG (WM_APP+0)
224 #define NUX_THREADMSG_START_RENDERING (WM_APP+1) // Connection established // start at WM_APP
225@@ -84,6 +93,7 @@
226 int m_X11Screen;
227 Window m_X11Window;
228 XVisualInfo *m_X11VisualInfo;
229+ struct XimClientData m_ximData;
230
231 int m_ParentWindow;
232 #ifndef NUX_OPENGLES_20
233@@ -342,10 +352,15 @@
234
235 void * KeyboardGrabData() { return _global_keyboard_grab_data; }
236 void * PointerGrabData() { return _global_pointer_grab_data; }
237+ void XICFocus();
238+ void XICUnFocus();
239
240 private:
241 void InitGlobalGrabWindow();
242
243+ void InitXIM(struct XimClientData *client_data);
244+ static void XIMEndCallback(Display *dpy, XPointer client_data, XPointer call_data);
245+ static void XIMStartCallback(Display *dpy, XPointer client_data, XPointer call_data);
246 void HandleXDndPosition(XEvent event, Event* nux_event);
247 void HandleXDndEnter (XEvent event);
248 void HandleXDndStatus (XEvent event);

Subscribers

People subscribed via source and target branches