Merge lp://qastaging/~brandontschaefer/nux/xim-tests into lp://qastaging/nux

Resubmitted as a lot has changed now....

76 + if (m_xim_controller)
77 + {
78 + delete m_xim_controller;
79 + }

What about using a smart ptr?

95 + void GraphicsDisplay::XICFocus()
96 + {
97 + m_xim_controller->FocusInXIC();
98 + }
99 +
100 + void GraphicsDisplay::XICUnFocus()
101 + {
102 + m_xim_controller->FocusOutXIC();
103 + }

I see that m_xim_controller is not initialized in the ctor. So I think that we should add a null check. Or am I missing something?

+* Copyright 2010-2012 Inalogic® Inc.

I think you don't need 2010 :)

I'll continue the review tomorrow (it's late now...).

Well we want to avoid using the new C++ standard for now in Nux. Pointers aren't that bad ;).

The reason I didn't do a NULL check is the m_xim_controller gets initialized when the OpenGL window is created.

In GLWindowManager.cpp

112 GraphicsDisplay *glwindow = new GraphicsDisplay();
113 glwindow->CreateOpenGLWindow(WindowTitle, WindowWidth, WindowHeight, Style, GLWindow, FullscreenFlag, create_rendering_data);

or this function:

128 GraphicsDisplay *glwindow = new GraphicsDisplay();
129 glwindow->CreateFromOpenGLWindow(WindowHandle, WindowDCHandle, OpenGLRenderingContext);

Right after the GrapichsDisplay is created the window is made which allocates the controller.

For sanity reason I can check for NULL...but it doesn't seem necessary, because if the m_xim_controller is NULL then so is the m_X11Display and m_X11Window which I don't see those checked for NULL else where in there...let me know what you think.

And yes...it is no longer 2010 haha :)

Or are you talking about nux::ObjectPtr?

> Well we want to avoid using the new C++ standard for now in Nux. Pointers
> aren't that bad ;).

I think that the 'old" C++ standard has smart ptrs too :) Boost can help here too.

>
> The reason I didn't do a NULL check is the m_xim_controller gets initialized
> when the OpenGL window is created.
>
> In GLWindowManager.cpp
>
> 112 GraphicsDisplay *glwindow = new GraphicsDisplay();
> 113 glwindow->CreateOpenGLWindow(WindowTitle, WindowWidth, WindowHeight,
> Style, GLWindow, FullscreenFlag, create_rendering_data);
>
> or this function:
>
> 128 GraphicsDisplay *glwindow = new GraphicsDisplay();
> 129 glwindow->CreateFromOpenGLWindow(WindowHandle, WindowDCHandle,
> OpenGLRenderingContext);
>
> Right after the GrapichsDisplay is created the window is made which allocates
> the controller.
>
> For sanity reason I can check for NULL...but it doesn't seem necessary,
> because if the m_xim_controller is NULL then so is the m_X11Display and
> m_X11Window which I don't see those checked for NULL else where in there...let
> me know what you think.

Perfect. Yeah it's not necessary. Never mind my comment ;)

>
> And yes...it is no longer 2010 haha :)

> Or are you talking about nux::ObjectPtr?

No, don't use nux::ObjectPtr. boost::shared_ptr should be good.

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-nux/184/console reported an error when processing this lp:~brandontschaefer/nux/xim-tests branch.
Not merging it.

Ok, first of all - I approve of this code and think it's very useful to have, but.

After consulting with the release team, we have decided to postpone this feature to 13.04. So sadly, here I need to disapprove of it so that it does not accidentally land in 12.10 lp:nux trunk.

But what I would like to do is (what Iain proposed) creating a PPA with unity and nux versions including this feature, making it public for people to test and comment (through the 2 bugs that are mentioned). So that we have it fully tested for 13.04.