aura: Turn XInput2 back on.

aura: Turn XInput2 back on.

The previous attempt read values from the valuator without properly checking if
the valuator is set or not. This resulted in incorrect location at times (when X
or Y didn't change). So this change makes sure the valuator is set correctly
before reading from it.

BUG=103981
TEST=manually (on desktop and device)

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=112499

[sky, 2011-11-22 17:42:09]

LGTM

[Rick Byers, 2011-11-25 18:17:05]

Hey Sadrul,
This CL isn't working for the desktop scenario.  I haven't debugged it yet -
just saw the below CHECK being hit on Biao's build with this CL applied.

Thanks,
   Rick

http://codereview.chromium.org/8634009/diff/3001/ui/aura/desktop_host_linux.cc
File ui/aura/desktop_host_linux.cc (right):

http://codereview.chromium.org/8634009/diff/3001/ui/aura/desktop_host_linux.c...
ui/aura/desktop_host_linux.cc:58: CHECK(!expect_master_event);
This CHECK fails when using the Acer touch screen on Linux (after making the
touch screen a floating slave).

[sadrul, 2011-11-25 18:20:26]

On 2011/11/25 18:17:05, Rick Byers wrote:
> Hey Sadrul,
> This CL isn't working for the desktop scenario.  I haven't debugged it yet -
> just saw the below CHECK being hit on Biao's build with this CL applied.
> 
> Thanks,
>    Rick
> 
> http://codereview.chromium.org/8634009/diff/3001/ui/aura/desktop_host_linux.cc
> File ui/aura/desktop_host_linux.cc (right):
> 
>
http://codereview.chromium.org/8634009/diff/3001/ui/aura/desktop_host_linux.c...
> ui/aura/desktop_host_linux.cc:58: CHECK(!expect_master_event);
> This CHECK fails when using the Acer touch screen on Linux (after making the
> touch screen a floating slave).

Yes. This is not ready yet. I am still working on it. In the meantime, if you
are using this, you can return early from the function to avoid the checks
(should mostly be fine for dev purpose).

[Rick Byers, 2011-11-25 18:32:56]

On 2011/11/25 18:20:26, sadrul wrote:
> Yes. This is not ready yet. I am still working on it. In the meantime, if you
> are using this, you can return early from the function to avoid the checks
> (should mostly be fine for dev purpose).

Ah, I'm sorry - I misunderstood.  We'll just re-enable XInput2 directly without
any of this other stuff until you tell us this is ready.

Thanks!
   Rick

[sadrul, 2011-11-30 16:13:31]

I have made some more changes: the valuators are used for events coming from
both master and slave devices. Also added a check to make sure the events coming
from master and slave devices are always in sync (will cause fatal crash when
they aren't).

I tested on the desktop and on the device, with multiple connected slave pointer
devices (two USB mice). I will also try using a floating pointer device.

PTAL.

[sky, 2011-11-30 16:38:04]

LGTM

http://codereview.chromium.org/8634009/diff/21004/ui/aura/desktop_host_linux.cc
File ui/aura/desktop_host_linux.cc (right):

http://codereview.chromium.org/8634009/diff/21004/ui/aura/desktop_host_linux....
ui/aura/desktop_host_linux.cc:41: static gfx::Point slave_location;
Statics should only be primitive types.

[DaveMoore, 2011-11-30 16:51:50]

http://codereview.chromium.org/8634009/diff/21004/ui/aura/desktop_host_linux.cc
File ui/aura/desktop_host_linux.cc (right):

http://codereview.chromium.org/8634009/diff/21004/ui/aura/desktop_host_linux....
ui/aura/desktop_host_linux.cc:338: if (base::MessagePumpForUI::HasXInput2())
Do we ever expect to run in an environment w/out XI2? I expect over time the
ability to run w/out it will rot. Should this be a fatal error, if not here then
somewhere higher up?

[sadrul, 2011-11-30 16:53:50]

http://codereview.chromium.org/8634009/diff/21004/ui/aura/desktop_host_linux.cc
File ui/aura/desktop_host_linux.cc (right):

http://codereview.chromium.org/8634009/diff/21004/ui/aura/desktop_host_linux....
ui/aura/desktop_host_linux.cc:338: if (base::MessagePumpForUI::HasXInput2())
On 2011/11/30 16:51:51, DaveMoore wrote:
> Do we ever expect to run in an environment w/out XI2? I expect over time the
> ability to run w/out it will rot. Should this be a fatal error, if not here
then
> somewhere higher up?

XInput2 is usually not available when running over NX, for example. So this
should not be a fatal error.

[DaveMoore, 2011-11-30 22:59:51]

LGTM then.
On 2011/11/30 16:53:50, sadrul wrote:
>
http://codereview.chromium.org/8634009/diff/21004/ui/aura/desktop_host_linux.cc
> File ui/aura/desktop_host_linux.cc (right):
> 
>
http://codereview.chromium.org/8634009/diff/21004/ui/aura/desktop_host_linux....
> ui/aura/desktop_host_linux.cc:338: if (base::MessagePumpForUI::HasXInput2())
> On 2011/11/30 16:51:51, DaveMoore wrote:
> > Do we ever expect to run in an environment w/out XI2? I expect over time the
> > ability to run w/out it will rot. Should this be a fatal error, if not here
> then
> > somewhere higher up?
> 
> XInput2 is usually not available when running over NX, for example. So this
> should not be a fatal error.

[Daniel Kurtz, 2011-12-01 14:51:53]

Can we be more selective about the events that we are requesting from the
server?

I understand wanting both Master pointer events (all mice / touchpads / tablet)
& Floating pointer events (touchscreen).  But why do we want the events for all
of the Slave Pointers attached to a Master?

It seems like we would want to walk the device hierarchy, and only select Master
Pointers, Keyboards and Floating Slave Pointers for input.

This would eliminate the fundamental "problem", which is described in who-t's
blog (http://who-t.blogspot.com/2010/07/input-event-processing-in-x.html):

"During event delivery, the events are taken out of the event queue and
processed one-by-one in the order they were received. The event delivery itself
is staged into slave delivery and master delivery. As said above, an event may
only be generated by a slave device. Once that happens, the event is processed
for this slave device and delivered to any listening clients - if there are
any... Regardless of the event delivery, the same event is then processed again
but this time for the respective master device...  When the event passes through
the master device, it updates the master's internal state and is then delivered
to the client."

I think we can select just masters by specifying XIAllMasterDevices when calling
XISelectEvents() (in the TouchFactory constructor?).  Then, walk the device
hierarchy, and select events for any Floating touchscreen devices.


On 2011/11/30 22:59:51, DaveMoore wrote:
> LGTM then.
> On 2011/11/30 16:53:50, sadrul wrote:
> >
>
http://codereview.chromium.org/8634009/diff/21004/ui/aura/desktop_host_linux.cc
> > File ui/aura/desktop_host_linux.cc (right):
> > 
> >
>
http://codereview.chromium.org/8634009/diff/21004/ui/aura/desktop_host_linux....
> > ui/aura/desktop_host_linux.cc:338: if (base::MessagePumpForUI::HasXInput2())
> > On 2011/11/30 16:51:51, DaveMoore wrote:
> > > Do we ever expect to run in an environment w/out XI2? I expect over time
the
> > > ability to run w/out it will rot. Should this be a fatal error, if not
here
> > then
> > > somewhere higher up?
> > 
> > XInput2 is usually not available when running over NX, for example. So this
> > should not be a fatal error.