Bug 16700

Summary: getGraphicsEvent does not pass mouse motion or button events correctly for X11
Product: R Reporter: Mark O'Connell <mark.oconnell>
Component: GraphicsAssignee: R-core <R-core>
Status: CLOSED FIXED    
Severity: minor CC: mark.oconnell, murdoch
Priority: P5    
Version: R 3.2.3   
Hardware: All   
OS: Linux   
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

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.