Bug 16700 - getGraphicsEvent does not pass mouse motion or button events correctly for X11
Summary: getGraphicsEvent does not pass mouse motion or button events correctly for X11
Status: CLOSED FIXED
Alias: None
Product: R
Classification: Unclassified
Component: Graphics (show other bugs)
Version: R 3.2.3
Hardware: All Linux
: P5 minor
Assignee: R-core
URL:
Depends on:
Blocks:
 
Reported: 2016-02-05 10:15 UTC by Mark O'Connell
Modified: 2016-02-06 21:59 UTC (History)
2 users (show)

See Also:


Attachments
simple script to test mouse interaction. trails red points with mouse motion, blue points when left button held, and green points when right button held (1.16 KB, text/plain)
2016-02-05 10:15 UTC, Mark O'Connell
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mark O'Connell 2016-02-05 10:15:50 UTC
Created attachment 2016 [details]
simple script to test mouse interaction. trails red points with mouse motion, blue points when left button held, and green points when right button held

getGraphicsEvent(), when called on an X11 device, does not perform in line with the Windows version. The two problems I've observed are:

1. onMouseMove only gets mouse motion events when a button is also pressed
2. The button values passed to onMouseDown, onMouseUp and onMouseMove are incorrect. (e.g. c(0, 1) for right-click, which should be 2)

For 1, I suggest changing PointerMotionHintMask to PointerMotionMask at lines 1485 and 1699 of devX11.c.

For 2, I suggest passing mask>>8 as the third argument to doMouseEvent (line 2634 of devX11.c) instead of event.xbutton.button.

The reason for the >>8 is as follows...

In X.h, the button masks are declared as...

#define Button1Mask		(1<<8)
#define Button2Mask		(1<<9)
#define Button3Mask		(1<<10)

Whereas in GraphicsDevice.h, the values for doMouseEvent are declared as...

#define leftButton   1   (which is (1<<0))
#define middleButton 2   (which is (1<<1))
#define rightButton  4   (which is (1<<2))
Comment 1 Duncan Murdoch 2016-02-06 13:53:57 UTC
Thanks for the research into this.  I'll put in a patch, but not what you suggested.  For now I'll only put it into R-devel.

The suggestion for 1 isn't right:  it should be ButtonMotionMask that is changed to PointerMotionMask.  ButtonMotionMask suppresses motion events when a button is not pressed.  I believe PointerMotionHintMask allows the server to drop motion events when they arrive too quickly; we need that.  (I might be wrong about this, I'm no X11 expert.)  Since this is going to lead to R handling a lot more events, I'm only making this change in R-devel for now.  We might need something more sophisticated, i.e. only make this change if the user has set a handler on motion events.

The suggestion for 2 is partly okay.  The trouble is that buttons are encoded differently during motion events and during button press events, and both need to be fixed.  Your fix is fine for motion events.

After a bit more testing I'll commit this.
Comment 2 Mark O'Connell 2016-02-06 21:42:48 UTC
Well, I'm afraid I have no expertise in X11 myself, so all I can give are suggestions.

I see what you mean about 1. The doc at https://tronche.com/gui/x/xlib/events/keyboard-pointer/keyboard-pointer.html says that PointerMotionHintMask limits the server to the sending of one MotionNotify event to the client. I was assuming that events were somewhat throttled by going through doMouseEvent on their way back to R anyway.

For 2, can you just call XQueryPointer for both motion and button events, and use the mask variable in both cases? Avoid the use of event.xbutton.button altogether?
Comment 3 Duncan Murdoch 2016-02-06 21:59:25 UTC
I think it's fixed, but it is worth testing before porting it to R-patched.  You need to use an R-devel build with svn revision 70110 or larger.