Merge lp:~dandrader/nux/geis into lp:nux
| Status: | Merged |
|---|---|
| Merged at revision: | 640 |
| Proposed branch: | lp:~dandrader/nux/geis |
| Merge into: | lp:nux |
| Diff against target: |
5504 lines (+4518/-215) (has conflicts) 48 files modified
.bzrignore (+2/-0) Nux/Area.cpp (+33/-0) Nux/Area.h (+19/-0) Nux/BaseWindow.cpp (+23/-0) Nux/BaseWindow.h (+10/-0) Nux/Features.h.in (+14/-0) Nux/GeisAdapter.cpp (+528/-0) Nux/GeisAdapter.h (+122/-0) Nux/Gesture.cpp (+295/-0) Nux/Gesture.h (+170/-0) Nux/GestureBroker.cpp (+200/-0) Nux/GestureBroker.h (+63/-0) Nux/GesturesSubscription.cpp (+379/-0) Nux/GesturesSubscription.h (+181/-0) Nux/InputArea.cpp (+77/-1) Nux/InputArea.h (+79/-0) Nux/Layout.cpp (+20/-0) Nux/Layout.h (+4/-0) Nux/MainLoopGLib.cpp (+22/-2) Nux/Makefile.am (+20/-1) Nux/View.cpp (+23/-0) Nux/View.h (+7/-0) Nux/WindowCompositor.cpp (+69/-0) Nux/WindowCompositor.h (+29/-0) Nux/WindowThread.cpp (+246/-177) Nux/WindowThread.h (+40/-24) NuxCore/Rect.cpp (+6/-0) NuxCore/Rect.h (+1/-0) NuxGraphics/Events.h (+5/-1) NuxGraphics/GestureEvent.cpp (+80/-0) NuxGraphics/GestureEvent.h (+162/-0) NuxGraphics/GraphicsDisplayWin.cpp (+6/-1) NuxGraphics/GraphicsDisplayWin.h (+4/-1) NuxGraphics/GraphicsDisplayX11.cpp (+7/-2) NuxGraphics/GraphicsDisplayX11.h (+4/-1) NuxGraphics/Makefile.am (+14/-2) NuxGraphics/nux-graphics.pc.in (+1/-1) configure.ac (+55/-1) examples/Makefile.am (+8/-0) examples/gestures.cpp (+100/-0) tests/FakeGestureEvent.h (+63/-0) tests/Makefile.am (+7/-0) tests/geis_mock.cpp (+483/-0) tests/geis_mock.h (+148/-0) tests/gtest-nux-geisadapter.cpp (+230/-0) tests/gtest-nux-gesturebroker.cpp (+142/-0) tests/gtest-nux-main.cpp (+17/-0) tests/gtest-nux-windowcompositor.cpp (+300/-0) Text conflict in configure.ac |
| To merge this branch: | bzr merge lp:~dandrader/nux/geis |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Jay Taoko (community) | Approve on 2012-08-01 | ||
| Chase Douglas (community) | 2012-05-31 | Approve on 2012-06-15 | |
| Francis Ginther | continuous-integration | Pending | |
|
Review via email:
|
|||
Description of the Change
Adding gestures support to Nux.
| Daniel d'Andrada (dandrader) wrote : | # |
| Chase Douglas (chasedouglas) wrote : | # |
* Please use one of the c++ or NUX-specific casts. For example:
GeisAdapterEven
should be:
GeisAdapterEven
* Why is GeisAdapter:
* How do we make sure that a nux area that receives a gesture begin will also receive the gesture updates and end for the same gesture? What if the area moves such that the touches no longer are in it? The gesture events must still go to the area that received the begin event. I don't see code that handles this.
* GET_LATEST_EVENT should be a real function, not a macro. Use anonymous namespaces or static functions.
I have reviewed all the utouch-specific code. I think it's sound other than the above issues. I have not reviewed any code that modifies how Nux works.
| Jay Taoko (jaytaoko) wrote : | # |
I get a build error:
make[2]: Entering directory `/home/
CXX libnux_
./GeisAdapter.cpp: In member function 'void nux::GeisAdapte
./GeisAdapter.
Am I missing a library on my system? I am still on Precise. I have libutouch-geis-dev installed.
| Daniel d'Andrada (dandrader) wrote : | # |
> I get a build error:
>
> make[2]: Entering directory `/home/
> CXX libnux_
> ./GeisAdapter.cpp: In member function 'void
> nux::GeisAdapte
> nux::EventType)':
> ./GeisAdapter.
> was not declared in this scope
>
> Am I missing a library on my system? I am still on Precise. I have libutouch-
> geis-dev installed.
No. That's a recent addition to utouch-geis API. You will have to get it directly from lp:utouch-geis/trunk.
| Daniel d'Andrada (dandrader) wrote : | # |
Back to "work in progress" status as GesturesSubscri
| Daniel d'Andrada (dandrader) wrote : | # |
> * How do we make sure that a nux area that receives a gesture begin will also
> receive the gesture updates and end for the same gesture? What if the area
> moves such that the touches no longer are in it? The gesture events must still
> go to the area that received the begin event. I don't see code that handles
> this.
The class Gesture binds a gesture to a target InputArea. That is done on GESTURE_BEGIN event and doesn't change after that.
| Daniel d'Andrada (dandrader) wrote : | # |
> * Why is GeisAdapter:
> ProcessGeisEvents?
It's a small optimization to avoid unnecessary memory allocations.
- 620. By Gord Allott on 2012-06-11
-
removes some executable flags from nux cpp/h files. Fixes: . Approved by Jay Taoko.
- 621. By Steve Baker on 2012-06-12
-
Cleaning up some coverity warnings.. Fixes: https:/
/bugs.launchpad .net/bugs/ 937564, https:/ /bugs.launchpad .net/bugs/ 937576, https:/ /bugs.launchpad .net/bugs/ 937586, https:/ /bugs.launchpad .net/bugs/ 937588. Approved by Tim Penhey. - 622. By Andrea Azzarone on 2012-06-13
-
Don't call QueueDraw during the layout process.. Fixes: https:/
/bugs.launchpad .net/bugs/ 994884. Approved by Michal Hruby, Tim Penhey.
| Daniel d'Andrada (dandrader) wrote : | # |
> * GET_LATEST_EVENT should be a real function, not a macro. Use anonymous
> namespaces or static functions.
That won't work because that code is called from both a const and a regular method that return a const reference and a regular one respectively. The only other way I see would be to call the const version from withing the non-const one and doing a const_cast on the return value. But I don't see it as a better option.
| Daniel d'Andrada (dandrader) wrote : | # |
It's been improved and all comments from Chase have been worked on (either fixed in code or explained).
A patch for unity to make is work with that new Nux is also ready in lp:~dandrader/unity/nux_gestures.
Ready for a new review.
| Chase Douglas (chasedouglas) wrote : | # |
> > * Why is GeisAdapter:
> > ProcessGeisEvents?
>
> It's a small optimization to avoid unnecessary memory allocations.
Sorry for not replying to this sooner.
Variables that are only accessed in a function should be local to that function. It's a rule in place in order to keep the source code structured. If a developer needs to change the code that manipulates the object, they would have to check how the object is used elsewhere unless it is local.
The object is a class with one virtual function and a bunch of data. This shouldn't be an issue when allocated on the stack.
| Chase Douglas (chasedouglas) wrote : | # |
> > * GET_LATEST_EVENT should be a real function, not a macro. Use anonymous
> > namespaces or static functions.
>
> That won't work because that code is called from both a const and a regular
> method that return a const reference and a regular one respectively. The only
> other way I see would be to call the const version from withing the non-const
> one and doing a const_cast on the return value. But I don't see it as a better
> option.
I guess I'm confused. Why do you need a const version of the method? Why not just return a non-const version, and any callers that want to keep it as const can just assign it to add const-ness.
| Chase Douglas (chasedouglas) wrote : | # |
> > * How do we make sure that a nux area that receives a gesture begin will
> also
> > receive the gesture updates and end for the same gesture? What if the area
> > moves such that the touches no longer are in it? The gesture events must
> still
> > go to the area that received the begin event. I don't see code that handles
> > this.
>
> The class Gesture binds a gesture to a target InputArea. That is done on
> GESTURE_BEGIN event and doesn't change after that.
I think I see how it's working now. I'm happy :).
| Chase Douglas (chasedouglas) wrote : | # |
I notice that only one gesture is allowed per target area. Theoretically this should be possible. The issue is when there are gestures with overlapping touches. Why now allow for multiple non-overlapping gestures?
| Daniel d'Andrada (dandrader) wrote : | # |
> I notice that only one gesture is allowed per target area. Theoretically this
> should be possible. The issue is when there are gestures with overlapping
> touches. Why now allow for multiple non-overlapping gestures?
What's the use for that? I fear this could make the implementation overly complex for no use case in sight.
| Daniel d'Andrada (dandrader) wrote : | # |
> > > * GET_LATEST_EVENT should be a real function, not a macro. Use anonymous
> > > namespaces or static functions.
> >
> > That won't work because that code is called from both a const and a regular
> > method that return a const reference and a regular one respectively. The
> only
> > other way I see would be to call the const version from withing the non-
> const
> > one and doing a const_cast on the return value. But I don't see it as a
> better
> > option.
>
> I guess I'm confused. Why do you need a const version of the method?
Because it's called from within other const methods.
> Why not just return a non-const version, and any callers that want to keep it as const
> can just assign it to add const-ness.
I didn't understand you. Could you please write down some example code?
| Daniel d'Andrada (dandrader) wrote : | # |
> > > * Why is GeisAdapter:
> > > ProcessGeisEvents?
> >
> > It's a small optimization to avoid unnecessary memory allocations.
>
> Sorry for not replying to this sooner.
>
> Variables that are only accessed in a function should be local to that
> function. It's a rule in place in order to keep the source code structured. If
> a developer needs to change the code that manipulates the object, they would
> have to check how the object is used elsewhere unless it is local.
>
> The object is a class with one virtual function and a bunch of data. This
> shouldn't be an issue when allocated on the stack.
Ok, I will remove that optimization since you insist on it.
| Chase Douglas (chasedouglas) wrote : | # |
On 06/15/2012 11:48 AM, Daniel d'Andrada wrote:
>>>> * GET_LATEST_EVENT should be a real function, not a macro. Use anonymous
>>>> namespaces or static functions.
>>>
>>> That won't work because that code is called from both a const and a regular
>>> method that return a const reference and a regular one respectively. The
>> only
>>> other way I see would be to call the const version from withing the non-
>> const
>>> one and doing a const_cast on the return value. But I don't see it as a
>> better
>>> option.
>>
>> I guess I'm confused. Why do you need a const version of the method?
>
> Because it's called from within other const methods.
>
>> Why not just return a non-const version, and any callers that want to keep it as const
>> can just assign it to add const-ness.
>
> I didn't understand you. Could you please write down some example code?
Sorry, I realize there are a few things going on, including me
forgetting about const methods only being able to call const methods.
I think we can resolve all this by removing the non-const
GetLatestEvent() method and making GestureEvent:
GestureEvent:
methods that modify the GestureEvent object.
| Daniel d'Andrada (dandrader) wrote : | # |
> On 06/15/2012 11:48 AM, Daniel d'Andrada wrote:
> >>>> * GET_LATEST_EVENT should be a real function, not a macro. Use anonymous
> >>>> namespaces or static functions.
> >>>
> >>> That won't work because that code is called from both a const and a
> regular
> >>> method that return a const reference and a regular one respectively. The
> >> only
> >>> other way I see would be to call the const version from withing the non-
> >> const
> >>> one and doing a const_cast on the return value. But I don't see it as a
> >> better
> >>> option.
> >>
> >> I guess I'm confused. Why do you need a const version of the method?
> >
> > Because it's called from within other const methods.
> >
> >> Why not just return a non-const version, and any callers that want to keep
> it as const
> >> can just assign it to add const-ness.
> >
> > I didn't understand you. Could you please write down some example code?
>
> Sorry, I realize there are a few things going on, including me
> forgetting about const methods only being able to call const methods.
>
> I think we can resolve all this by removing the non-const
> GetLatestEvent() method and making GestureEvent:
> GestureEvent:
> methods that modify the GestureEvent object.
That works but makes for a weird API. Accept() and Reject() doesn't sound like methods that should be const (like getters) even though in their current implementation they can be so.
| Chase Douglas (chasedouglas) wrote : | # |
On 06/15/2012 01:34 PM, Daniel d'Andrada wrote:
>> On 06/15/2012 11:48 AM, Daniel d'Andrada wrote:
>>>>>> * GET_LATEST_EVENT should be a real function, not a macro. Use anonymous
>>>>>> namespaces or static functions.
>>>>>
>>>>> That won't work because that code is called from both a const and a
>> regular
>>>>> method that return a const reference and a regular one respectively. The
>>>> only
>>>>> other way I see would be to call the const version from withing the non-
>>>> const
>>>>> one and doing a const_cast on the return value. But I don't see it as a
>>>> better
>>>>> option.
>>>>
>>>> I guess I'm confused. Why do you need a const version of the method?
>>>
>>> Because it's called from within other const methods.
>>>
>>>> Why not just return a non-const version, and any callers that want to keep
>> it as const
>>>> can just assign it to add const-ness.
>>>
>>> I didn't understand you. Could you please write down some example code?
>>
>> Sorry, I realize there are a few things going on, including me
>> forgetting about const methods only being able to call const methods.
>>
>> I think we can resolve all this by removing the non-const
>> GetLatestEvent() method and making GestureEvent:
>> GestureEvent:
>> methods that modify the GestureEvent object.
>
> That works but makes for a weird API. Accept() and Reject() doesn't sound like methods that should be const (like getters) even though in their current implementation they can be so.
I understand the logic, but c++ constness is really an API construct,
not a logic construct. Usually the domains overlap, but not always.
Admittedly, I don't have any good examples, but I feel being true to the
execution when using constness is best.
Please enlighten me if you think I'm wrong. I'm not an expert on C++, I
just hope I know how to not shoot my foot with it :).
| Daniel d'Andrada (dandrader) wrote : | # |
> On 06/15/2012 01:34 PM, Daniel d'Andrada wrote:
> >> On 06/15/2012 11:48 AM, Daniel d'Andrada wrote:
> >>>>>> * GET_LATEST_EVENT should be a real function, not a macro. Use
> anonymous
> >>>>>> namespaces or static functions.
> >>>>>
> >>>>> That won't work because that code is called from both a const and a
> >> regular
> >>>>> method that return a const reference and a regular one respectively. The
> >>>> only
> >>>>> other way I see would be to call the const version from withing the non-
> >>>> const
> >>>>> one and doing a const_cast on the return value. But I don't see it as a
> >>>> better
> >>>>> option.
> >>>>
> >>>> I guess I'm confused. Why do you need a const version of the method?
> >>>
> >>> Because it's called from within other const methods.
> >>>
> >>>> Why not just return a non-const version, and any callers that want to
> keep
> >> it as const
> >>>> can just assign it to add const-ness.
> >>>
> >>> I didn't understand you. Could you please write down some example code?
> >>
> >> Sorry, I realize there are a few things going on, including me
> >> forgetting about const methods only being able to call const methods.
> >>
> >> I think we can resolve all this by removing the non-const
> >> GetLatestEvent() method and making GestureEvent:
> >> GestureEvent:
> >> methods that modify the GestureEvent object.
> >
> > That works but makes for a weird API. Accept() and Reject() doesn't sound
> like methods that should be const (like getters) even though in their current
> implementation they can be so.
>
> I understand the logic, but c++ constness is really an API construct,
> not a logic construct. Usually the domains overlap, but not always.
> Admittedly, I don't have any good examples, but I feel being true to the
> execution when using constness is best.
>
> Please enlighten me if you think I'm wrong. I'm not an expert on C++, I
> just hope I know how to not shoot my foot with it :).
Changed from the macro to the const_cast way that people say "Effective C++" book advocates.
Hope you like it better :)
| Chase Douglas (chasedouglas) wrote : | # |
> > On 06/15/2012 01:34 PM, Daniel d'Andrada wrote:
> > >> On 06/15/2012 11:48 AM, Daniel d'Andrada wrote:
> > >>>>>> * GET_LATEST_EVENT should be a real function, not a macro. Use
> > anonymous
> > >>>>>> namespaces or static functions.
> > >>>>>
> > >>>>> That won't work because that code is called from both a const and a
> > >> regular
> > >>>>> method that return a const reference and a regular one respectively.
> The
> > >>>> only
> > >>>>> other way I see would be to call the const version from withing the
> non-
> > >>>> const
> > >>>>> one and doing a const_cast on the return value. But I don't see it as
> a
> > >>>> better
> > >>>>> option.
> > >>>>
> > >>>> I guess I'm confused. Why do you need a const version of the method?
> > >>>
> > >>> Because it's called from within other const methods.
> > >>>
> > >>>> Why not just return a non-const version, and any callers that want to
> > keep
> > >> it as const
> > >>>> can just assign it to add const-ness.
> > >>>
> > >>> I didn't understand you. Could you please write down some example code?
> > >>
> > >> Sorry, I realize there are a few things going on, including me
> > >> forgetting about const methods only being able to call const methods.
> > >>
> > >> I think we can resolve all this by removing the non-const
> > >> GetLatestEvent() method and making GestureEvent:
> > >> GestureEvent:
> > >> methods that modify the GestureEvent object.
> > >
> > > That works but makes for a weird API. Accept() and Reject() doesn't sound
> > like methods that should be const (like getters) even though in their
> current
> > implementation they can be so.
> >
> > I understand the logic, but c++ constness is really an API construct,
> > not a logic construct. Usually the domains overlap, but not always.
> > Admittedly, I don't have any good examples, but I feel being true to the
> > execution when using constness is best.
> >
> > Please enlighten me if you think I'm wrong. I'm not an expert on C++, I
> > just hope I know how to not shoot my foot with it :).
>
> Changed from the macro to the const_cast way that people say "Effective C++"
> book advocates.
> Hope you like it better :)
It's better, but I'm not sure it's 100% correct. It feels to me like this shows the design isn't proper, but I admit to having written similar things in the past because I couldn't figure out how to do it "correctly" either. I certainly don't have any better suggestions, so it's good enough for me :).
Everything else I've noted has been taken care of, so I approve :). Yay!
- 623. By Daniel van Vugt on 2012-06-19
-
Don't allow a View to be queued multiple times on the draw list. It causes
unbounded list growth and the UI to freeze in some cases. (LP: #1014610)
. Fixes: https://bugs.launchpad .net/bugs/ 1014610. Approved by Tim Penhey. - 624. By Brandon Schaefer on 2012-06-22
-
Fix memory leak in InputMethdIBus:
:OnPreeditUpdat e.. Fixes: . Approved by Marco Trevisan (Treviño). - 625. By Marco Trevisan (Treviño) on 2012-06-27
-
TextEntry: Handle also Key-Up events so that ibus will work with release hotkeys
InputMethodIBus: keep a list of hotkey events and add IsHotkeyEvent and IsConnected. Fixes: https:/
/bugs.launchpad .net/bugs/ 1016665. Approved by Brandon Schaefer. - 626. By Łukasz Zemczak on 2012-07-03
-
Updated Nux API version to 3.0. Deprecated nux-image library. The feature by nux-image has been merged into nux-graphics.. Fixes: . Approved by Łukasz Zemczak, Didier Roche, Tim Penhey.
- 627. By Marco Trevisan (Treviño) on 2012-07-03
-
TextEntry: don't allow invalid character to be written and ignore keypress when holding Alt or Super. Fixes: https:/
/bugs.launchpad .net/bugs/ 1013751. Approved by Brandon Schaefer. - 628. By Marco Trevisan (Treviño) on 2012-07-03
-
TextEntry: make the clipboard functions virtual to be implemented by clients. Fixes: https:/
/bugs.launchpad .net/bugs/ 1016354. Approved by Thomi Richards, Brandon Schaefer. - 629. By Marco Trevisan (Treviño) on 2012-07-10
-
TextEntry: ignore meta keypresses when in composition mode. Fixes: https:/
/bugs.launchpad .net/bugs/ 961741. Approved by Andrea Azzarone. - 630. By Marco Trevisan (Treviño) on 2012-07-11
-
TextEntry: correctly handle a "double-deadkey" pressure
Correctly reset the dead_key mode when terminated.. Fixes: https:/
/bugs.launchpad .net/bugs/ 950740, https:/ /bugs.launchpad .net/bugs/ 961741. Approved by Brandon Schaefer. - 631. By Michal Hruby on 2012-07-11
-
Make GdkPixbuf conversions faster. Fixes: . Approved by Jay Taoko, Sam Spilsbury.
- 632. By Jay Taoko on 2012-07-11
-
* Fix in volume texture API. Fixes: . Approved by Gord Allott.
- 633. By Marco Trevisan (Treviño) on 2012-07-11
-
TextEntryCompos
eSeqs: updated with new sequencies computed using compose key list generator script List based on:
http://cgit.freedeskto p.org/xorg/ lib/libX11/ plain/nls/ en_US.UTF- 8/Compose. pre Updated also the dead_keys_map with valid values and updated to char*. Fixes: https:/
/bugs.launchpad .net/bugs/ 961741. Approved by Brandon Schaefer. - 634. By Jay Taoko on 2012-07-12
-
* Refactoring Nux timer API: Some cleanup and update were required for the animation framework.. Fixes: . Approved by Jay Taoko.
- 635. By Marco Trevisan (Treviño) on 2012-07-14
-
TextEntry: refactored the composition code to use KeySym's. Fixes: . Approved by Brandon Schaefer.
- 636. By Brandon Schaefer on 2012-07-16
-
Fixed some tests.. Fixes: . Approved by Thomi Richards.
- 637. By Jay Taoko on 2012-07-18
-
* Added support for character hiding in the text entry (password input). Fixes: . Approved by Brandon Schaefer.
- 650. By Daniel d'Andrada on 2012-06-07
-
Obey event inspectors and do discard events when told so
- 651. By Daniel d'Andrada on 2012-06-13
-
Split a Geis Tap Update into a Tap Begin and a Tap End
That's to ensure that all gestures behave consistenly. They
should be comprised by a Begin event followed by zero or more
Update events and end with a End event.Tap gestures don't follow this rule as it only sends a single Update
and nothing more. Exposing this to upper layers in the code would require
special casing of Tap gestures everywhere. - 652. By Daniel d'Andrada on 2012-06-14
-
Make GesturesSubscri
ptions accept multiple gesture classes - 653. By Daniel d'Andrada on 2012-06-14
-
Use C++ casts instead of C casts in GeisAdapter
- 654. By Daniel d'Andrada on 2012-06-14
-
Correctly process gesture events that finish their constructions only when ending.
- 655. By Daniel d'Andrada on 2012-06-15
-
Expose configuration options in GesturesSubscri
ption - 656. By Daniel d'Andrada on 2012-06-15
-
GeisAdapter: make nux_event_ a local variable
As it's only used iniside ProcessGeisEven
ts().
The code gets clearer and the performance penalty should be irrelevant. - 657. By Daniel d'Andrada on 2012-06-15
-
Gestures support needs at least version 2.2.10 or utouch-geis
- 658. By Daniel d'Andrada on 2012-06-15
-
Resolve the const&non-const getters issue the const_cast way
As "Effective C++" advocates
- 659. By Daniel d'Andrada on 2012-06-26
-
Some refactoring so that unity can use more code from nux
instead of having unity using a modified copy of GestureBroker, Gesture, etc.
- 660. By Daniel d'Andrada on 2012-06-26
-
Fix build
- 661. By Daniel d'Andrada on 2012-07-27
-
Use geis from Open Input Framework instead of utouch-geis
- 662. By Daniel d'Andrada on 2012-07-27
-
Fix build of gtest-nux
- 663. By Daniel d'Andrada on 2012-08-01
-
Bump ABI version

It's worth noting that the Unity compiz plugin will have to be changed in order to work with a nux version that has gestures support. Currently it uses utouch-geis directly. With that nux patch in, it will have to use gestures via the nux API instead.