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

[Ying-Chun Liu]

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

[Tim Penhey (thumper)]

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)]

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)]

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)]

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

[Brandon Schaefer (brandontschaefer)]

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]

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)]

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)]

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)]

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)]

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)]

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)]

Review was claimed by accident, please ignore.

review: Abstain

[Brandon Schaefer (brandontschaefer)]

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)]

> 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.

=== modified file 'Nux/TextEntry.cpp'
--- Nux/TextEntry.cpp	2012-07-03 23:24:53 +0000
+++ Nux/TextEntry.cpp	2012-07-09 14:05:24 +0000
@@ -856,6 +856,7 @@
         need_im_reset_ = true;
 #if defined(NUX_OS_LINUX)
         ime_->Focus();
+        nux::GetWindowThread()->GetGraphicsDisplay().XICFocus();
 #endif
         //gtk_im_context_focus_in(im_context_);
         //UpdateIMCursorLocation();
@@ -878,6 +879,7 @@
         need_im_reset_ = true;
 #if defined(NUX_OS_LINUX)
         ime_->Blur();
+        nux::GetWindowThread()->GetGraphicsDisplay().XICUnFocus();
 #endif
         //gtk_im_context_focus_out(im_context_);
       }
 
=== modified file 'NuxGraphics/GraphicsDisplayX11.cpp'
--- NuxGraphics/GraphicsDisplayX11.cpp	2012-04-16 18:37:21 +0000
+++ NuxGraphics/GraphicsDisplayX11.cpp	2012-07-09 14:05:24 +0000
@@ -184,6 +184,111 @@
     return(event->type == MapNotify) && (event->xmap.window == (Window) arg);
   }
 
+  void GraphicsDisplay::XICFocus()
+  {
+    if (m_ximData.m_xic)
+    {
+      XSetICFocus(m_ximData.m_xic);
+      m_ximData.focus_stat=1;
+    }
+  }
+
+  void GraphicsDisplay::XICUnFocus()
+  {
+    if (m_ximData.m_xic)
+    {
+      XUnsetICFocus(m_ximData.m_xic);
+      m_ximData.focus_stat=0;
+    }
+  }
+
+  void GraphicsDisplay::XIMEndCallback(Display *dpy, XPointer client_data, XPointer call_data)
+  {
+    struct XimClientData *data;
+
+    data = (struct XimClientData*)client_data;
+    data->m_xim = NULL;
+    data->m_xic = NULL;
+  }
+
+  void GraphicsDisplay::XIMStartCallback(Display *dpy, XPointer client_data, XPointer call_data)
+  {
+    int i;
+    XIMCallback destroy;
+    XIMStyles *xim_styles = NULL;
+    XIMStyle root_style = (XIMPreeditNothing|XIMStatusNothing);
+    Window win;
+    struct XimClientData *data;
+
+    data = (struct XimClientData*)client_data;
+    win = * (data->window);
+
+    data->m_xim = XOpenIM(dpy, NULL, NULL, NULL);
+    if (! (data->m_xim))
+    {
+      nuxDebugMsg("[GraphicsDisplay::XIMStartCallback] Failed to open IM.");
+      return;
+    }
+    memset(&destroy, 0, sizeof(destroy));
+    destroy.callback = (XIMProc)((GraphicsDisplay::XIMEndCallback));
+    destroy.client_data = (XPointer)data;
+    XSetIMValues((data->m_xim), XNDestroyCallback, &destroy, NULL);
+    XGetIMValues((data->m_xim), XNQueryInputStyle, &xim_styles, NULL);
+    for (i=0; i<xim_styles->count_styles; i++)
+    {
+      if (xim_styles->supported_styles[i] == root_style)
+      {
+	break;
+      }
+    }
+    if (i>=xim_styles->count_styles)
+    {
+      nuxDebugMsg("[GraphicsDisplay::XIMStartCallback] root styles not supported.");
+      return;
+    }
+    data->m_xic = XCreateIC(data->m_xim, XNInputStyle, root_style, XNClientWindow, win, XNFocusWindow, win, NULL);
+    if (!(data->m_xic))
+    {
+      nuxDebugMsg("[GraphicsDisplay::XIMStartCallback] failed to register xic");
+    }
+    return;
+  }
+
+  void GraphicsDisplay::InitXIM(struct XimClientData *data)
+  {
+    const char *xmodifier;
+
+    /* don't do anything if we are using ibus */
+    xmodifier = getenv("XMODIFIERS");
+    if (xmodifier && strstr(xmodifier,"ibus") != NULL)
+    {
+      nuxDebugMsg("[GraphicsDisplay::InitXIM] ibus natively supported");
+      return;
+    }
+
+    if (setlocale(LC_ALL, "") == NULL)
+    {
+      nuxDebugMsg("[GraphicsDisplay::InitXIM] cannot setlocale");
+    }
+
+    if (!XSupportsLocale())
+    {
+      nuxDebugMsg("[GraphicsDisplay::InitXIM] no supported locale");
+    }
+
+    if (XSetLocaleModifiers("") == NULL)
+    {
+      nuxDebugMsg("[GraphicsDisplay::InitXIM] XSetLocaleModifiers failed.");
+    }
+
+    if (!XRegisterIMInstantiateCallback(*(data->display), NULL, NULL, NULL, GraphicsDisplay::XIMStartCallback, (XPointer)(data)))
+    {
+      nuxDebugMsg("[GraphicsDisplay::InitXIM] Cannot Register IM Init callback");
+    }
+  }
+
+
+
 // TODO: change windowWidth, windowHeight, to window_size;
   static NCriticalSection CreateOpenGLWindow_CriticalSection;
   bool GraphicsDisplay::CreateOpenGLWindow(const char *WindowTitle,
@@ -568,6 +673,19 @@
       //XMapRaised(m_X11Display, m_X11Window);
     }
 
+    // XIM support
+    memset(&m_ximData,0,sizeof(m_ximData));
+    m_ximData.window = &m_X11Window;
+    m_ximData.display = &m_X11Display;
+    InitXIM(&m_ximData);
+
+    long im_event_mask=0;
+    if (m_ximData.m_xic)
+    {
+      XGetICValues(m_ximData.m_xic, XNFilterEvents, &im_event_mask, NULL);
+      m_X11Attr.event_mask |= im_event_mask;
+    }
+
 #ifndef NUX_OPENGLES_20
     if (_has_glx_13)
     {
@@ -1229,6 +1347,8 @@
     if (XPending(m_X11Display))
     {
       XNextEvent(m_X11Display, &xevent);
+      if (XFilterEvent(&xevent, None) == True)
+	return;
 
       if (!_event_filters.empty())
       {
@@ -1535,6 +1655,10 @@
         m_pEvent->dy = 0;
         m_pEvent->virtual_code = 0;
         //nuxDebugMsg("[GraphicsDisplay::ProcessXEvents]: FocusIn event.");
+        if (m_ximData.m_xic && m_ximData.focus_stat==1)
+        {
+          XSetICFocus(m_ximData.m_xic);
+        }
         break;
       }
 
@@ -1550,6 +1674,11 @@
         m_pEvent->dy = 0;
         m_pEvent->virtual_code = 0;
         //nuxDebugMsg("[GraphicsDisplay::ProcessXEvents]: FocusOut event.");
+
+        if (m_ximData.m_xic)
+        {
+          XUnsetICFocus(m_ximData.m_xic);
+        }
         break;
       }
 
@@ -1580,7 +1709,15 @@
          skip = true; 
         }
         
-        int num_char_stored = XLookupString(&xevent.xkey, buffer, NUX_EVENT_TEXT_BUFFER_SIZE, (KeySym*) &m_pEvent->x11_keysym, NULL);
+        int num_char_stored = 0;
+        if (m_ximData.m_xic)
+        {
+          num_char_stored = XmbLookupString(m_ximData.m_xic, &xevent.xkey, buffer, NUX_EVENT_TEXT_BUFFER_SIZE, (KeySym*) &m_pEvent->x11_keysym, NULL);
+        }
+        else
+        {
+          num_char_stored = XLookupString(&xevent.xkey, buffer, NUX_EVENT_TEXT_BUFFER_SIZE, (KeySym*) &m_pEvent->x11_keysym, NULL);
+        }
         if (num_char_stored && (!skip))
         {
           Memcpy(m_pEvent->text, buffer, num_char_stored);
 
=== modified file 'NuxGraphics/GraphicsDisplayX11.h'
--- NuxGraphics/GraphicsDisplayX11.h	2012-01-26 23:01:57 +0000
+++ NuxGraphics/GraphicsDisplayX11.h	2012-07-09 14:05:24 +0000
@@ -67,6 +67,15 @@
     
     DNDACTION_NONE,
   };
+
+  struct XimClientData
+  {
+    XIM m_xim;
+    XIC m_xic;
+    Window *window;
+    Display **display;
+    int focus_stat;
+  };
   
 #define NUX_THREADMSG                           (WM_APP+0)
 #define NUX_THREADMSG_START_RENDERING           (WM_APP+1)  // Connection established // start at WM_APP
@@ -84,6 +93,7 @@
     int         m_X11Screen;
     Window      m_X11Window;
     XVisualInfo *m_X11VisualInfo;
+    struct XimClientData m_ximData;
 
     int         m_ParentWindow;
 #ifndef NUX_OPENGLES_20
@@ -342,10 +352,15 @@
     
     void * KeyboardGrabData() { return _global_keyboard_grab_data; }
     void * PointerGrabData() { return _global_pointer_grab_data; }
+    void XICFocus();
+    void XICUnFocus();
 
   private:
     void InitGlobalGrabWindow();
   
+    void InitXIM(struct XimClientData *client_data);
+    static void XIMEndCallback(Display *dpy, XPointer client_data, XPointer call_data);
+    static void XIMStartCallback(Display *dpy, XPointer client_data, XPointer call_data);
     void HandleXDndPosition(XEvent event, Event* nux_event);
     void HandleXDndEnter    (XEvent event);
     void HandleXDndStatus   (XEvent event);