button->down used inconsistently

[Peter Hutterer]

On Tue, Jun 17, 2008 at 03:19:53PM -0700, Keith Packard wrote:
> > This changes the server behaviour if 2 or more SDs are attached to the same
> > MD. In the current server, the button count is increased and the release is
> > sent when the _last_ SD releases the respective button.
> > With this patch, the release is sent for each SD that releases a button.
> 
> Given that this is new behaviour, and that the server didn't do this
> before, I'd say it's fixing a new bug. Whether the  button is released
> on the first or final release is a matter of opinion, and having it
> released on the first is more reliable, in case the button down count is
> broken, hitting and releasing a button will fix it in the boolean case,
> but not in the counting case.

By current server I meant "master without the patch", so yes, this bug only
affects master, not 1.5 or before. 

> > Reproducable by simply using mouse + touchpad, press button 1 on both, release
> > button 1 on both and the event sequence is 
> > 
> > press 1 - state 0x0 
> >   (second press is swallowed)
> > release 1 - state 0x100 
> > release 1 - state 0x0
> 
> If you look at the patch, the second release is swallowed as releases
> when the button is up aren't reported.

actually, I ran the server with the patch applied and the above was xev's
output. The problem is the fact that we swap the device classes between the
devices around, and copy the device state as well, resulting in the second
release not being swallowed.

If button 1 on device 1 is pressed, it's respective master device also gets a
button 1 down. When device 2 (on the same master) then emits any event, we
copy the second device's classes over into the master, including the button
down state.

This allows us to do a button 1 down on one device, move a second device and
still get a dragging operation. Assume the following input by the user:

mouse 1 down
mouse 2 move
mouse 1 move

The event sequence as seen by the client is (note that the devices aren't
actually visible):

button 1 down (device 1)
drag          (device 2)
drag          (device 1)

We can opt to use bitmasks but we would then have to force a button release
when the device classes change, so that the state is guaranteed.
With the above scenario of one mouse pressing the button, the other mouse
moving we would get the following event sequence.

button 1 down    (device 1)
button 1 up      (device 1, emulated)
move w/o buttons (device 2)
button 1 down    (device 1, emulated)
drag             (device 1)

This would be similar to what happens for keyboards, pressing ctrl on one and
C on the other does not result in a ctrl + c. For mice, we merge the state.

> >  So the
> > current code, albeit messy, is preferable since it provides the correct
> > behaviour.
> 
> The current code has a horrible bug -- it misreports button state as it
> copies the 'down' array to the wire.

If I understand this correctly, this is fixed by 
d21155a3e9b51df946766926bc6155c8972c4439. If I have missed anything, please
let me know.

> Given the increased reliability of using simple booleans, and the fact
> that the protocol requires the booleans, it seems like my patch fixed
> the protocol incorrectness, increased overall reliability in the face of
> stuck buttons and still preserved the up/down serialization requirement,
> although with slightly different 'first occurance' semantics.

If we are willing to lose the two-mouse drag ability, then bitmasks are a
solution. Otherwise, the counter is required.

Cheers,
  Peter
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.x.org/archives/xorg/attachments/20080618/fa822ac8/attachment.pgp>