Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 331993 - Display#setCursorLocation(Point) does not cause MouseMove event on MacOSX Cocoa
Summary: Display#setCursorLocation(Point) does not cause MouseMove event on MacOSX Cocoa
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: SWT (show other bugs)
Version: 3.6.2   Edit
Hardware: PC Mac OS X
: P3 major (vote)
Target Milestone: 3.7 M5   Edit
Assignee: Scott Kovatch CLA
QA Contact: Silenio Quarti CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 242481
  Show dependency tree
 
Reported: 2010-12-07 04:50 EST by Alexander Nyßen CLA
Modified: 2011-01-25 16:35 EST (History)
3 users (show)

See Also:


Attachments
Potential fix (1.62 KB, patch)
2010-12-07 12:39 EST, Scott Kovatch CLA
no flags Details | Diff
Alternate fix (10.59 KB, patch)
2010-12-08 13:27 EST, Scott Kovatch CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Nyßen CLA 2010-12-07 04:50:22 EST
While on win32.win32.x86 a call to Display.setCursorLocation(Point) causes a SWT.MouseMove event to occur, this does not happen on mac.cocoa.x86_64. 

The following snippet may be used to reproduce the behavior:

public static void main(String[] args) {
		final Display display = new Display();
		Shell shell = new Shell();
		shell.setLayout(new FillLayout());
		final FigureCanvas canvas = new FigureCanvas(shell);
		canvas.addListener(SWT.MouseMove, new Listener() {

			public void handleEvent(Event event) {
				System.out.println("Event occured: " + event);
			}
		});
		shell.setSize(300, 300);
		shell.open();

		display.asyncExec(new Runnable() {
			public void run() {
				Point location = canvas.toDisplay(50, 50);
				canvas.getDisplay().setCursorLocation(location);
			}
		});

		while (!shell.isDisposed()) {
			while (!display.readAndDispatch()) {
				display.sleep();
			}
		}
	}

While there is no output on mac.cocoa.x86_64, running the code on win32.win32.x86 gives you the following output:
Event occured: Event {type=5 FigureCanvas {} time=6678468 data=null x=50 y=50 width=0 height=0 detail=0}

I am not sure, whether it is considered part of the API-contract of Display#setCursorLocation(Point) that a SWT.MouseMove will occur as a result. Even if not, I think the behavior should be consistent across platforms. Certain GEF mechanisms dependent on the event occurence and are currently broken (e.g. consider bug #242481), thus rating severity as major.
Comment 1 Scott Kovatch CLA 2010-12-07 12:13:21 EST
I can get the desired behavior by using Display#post() to send a SWT.MouseMove event, which should generate the same behavior on all platforms.

At the very least this is a workaround you could use. Is it enough to solve the problem for you?

As you said, there's no requirement that setCursorLocation() generate a mouse-move event, so I'm hesitant to change working code. Currently we just call CGWarpMouseCursorPosition, which is documented to not generate mouse move events.
Comment 2 Alexander Nyßen CLA 2010-12-07 12:24:03 EST
(In reply to comment #1)
> I can get the desired behavior by using Display#post() to send a SWT.MouseMove
> event, which should generate the same behavior on all platforms.
> 
> At the very least this is a workaround you could use. Is it enough to solve the
> problem for you?
> 
> As you said, there's no requirement that setCursorLocation() generate a
> mouse-move event, so I'm hesitant to change working code. Currently we just
> call CGWarpMouseCursorPosition, which is documented to not generate mouse move
> events.

Scott, do I get it right that your intention is to do the post() within Display#setCursorLocation() of the mac fragment? This would seem to be a valid solution to me (otherwise the GEF client code, which is up to now not platform specific, would have to be made "platform-specific"). If this is what you have in mind it would be nice if you could provide a patched version of the mac fragment for me that I could test against the issue in 242481.
Comment 3 Scott Kovatch CLA 2010-12-07 12:39:16 EST
(In reply to comment #2)
> (In reply to comment #1)
> > At the very least this is a workaround you could use. Is it enough to solve the
> Scott, do I get it right that your intention is to do the post() within
> Display#setCursorLocation() of the mac fragment? This would seem to be a valid
> solution to me (otherwise the GEF client code, which is up to now not platform
> specific, would have to be made "platform-specific"). If this is what you have
> in mind it would be nice if you could provide a patched version of the mac
> fragment for me that I could test against the issue in 242481.

Is it GEF that calls setCursorLocation() or your code? It sounds like you're saying it's GEF. Instead of calling setCursorLocation, just call Display.post() with the MouseMove. In the example below, it would be:

Event e = new Event();
e.type = SWT.MouseMove;
e.x = location.x;
e.y = location.y;
canvas.getDisplay().post(e);
//canvas.getDisplay().setCursorLocation(location);

No need for platform-specific checks -- it will work everywhere.

I can give you a patch against SWT 3.7, but I would want to talk to Silenio about it first before checking it in.
Comment 4 Scott Kovatch CLA 2010-12-07 12:39:54 EST
Created attachment 184736 [details]
Potential fix
Comment 5 Alexander Nyßen CLA 2010-12-07 12:45:01 EST
(In reply to comment #3)
> (In reply to comment #2)
> > (In reply to comment #1)
> > > At the very least this is a workaround you could use. Is it enough to solve the
> > Scott, do I get it right that your intention is to do the post() within
> > Display#setCursorLocation() of the mac fragment? This would seem to be a valid
> > solution to me (otherwise the GEF client code, which is up to now not platform
> > specific, would have to be made "platform-specific"). If this is what you have
> > in mind it would be nice if you could provide a patched version of the mac
> > fragment for me that I could test against the issue in 242481.
> 
> Is it GEF that calls setCursorLocation() or your code? It sounds like you're
> saying it's GEF. Instead of calling setCursorLocation, just call Display.post()
> with the MouseMove. In the example below, it would be:
> 
> Event e = new Event();
> e.type = SWT.MouseMove;
> e.x = location.x;
> e.y = location.y;
> canvas.getDisplay().post(e);
> //canvas.getDisplay().setCursorLocation(location);
> 
> No need for platform-specific checks -- it will work everywhere.
> 
> I can give you a patch against SWT 3.7, but I would want to talk to Silenio
> about it first before checking it in.

Well, GEF is my code :-). We are using this in AbstractTool#placeMouseInViewer(), which is currently implemented as follows:

protected void placeMouseInViewer(Point p) {
		if (getCurrentViewer() == null)
			return;
		Control c = getCurrentViewer().getControl();
		Rectangle rect;
		if (c instanceof Scrollable)
			rect = ((Scrollable) c).getClientArea();
		else
			rect = c.getBounds();
		if (p.x > rect.x + rect.width - 1)
			p.x = rect.x + rect.width - 1;
		else if (p.x < rect.x)
			p.x = rect.x;
		if (p.y > rect.y + rect.height - 1)
			p.y = rect.y + rect.height - 1;
		else if (p.y < rect.y)
			p.y = rect.y;
		org.eclipse.swt.graphics.Point swt = new org.eclipse.swt.graphics.Point(
				p.x, p.y);
		swt = c.toDisplay(swt);
		c.getDisplay().setCursorLocation(swt);
}
Comment 6 Alexander Nyßen CLA 2010-12-07 14:48:59 EST
Scott, I verified that changing the implementation of AbstractTool#placeMouseInViewer() to

if (getCurrentViewer() == null)
			return;
		Control c = getCurrentViewer().getControl();
		Rectangle rect;
		if (c instanceof Scrollable)
			rect = ((Scrollable) c).getClientArea();
		else
			rect = c.getBounds();
		if (p.x > rect.x + rect.width - 1)
			p.x = rect.x + rect.width - 1;
		else if (p.x < rect.x)
			p.x = rect.x;
		if (p.y > rect.y + rect.height - 1)
			p.y = rect.y + rect.height - 1;
		else if (p.y < rect.y)
			p.y = rect.y;
		org.eclipse.swt.graphics.Point swt = new org.eclipse.swt.graphics.Point(
				p.x, p.y);
		swt = c.toDisplay(swt);

		// fix for bug #242481 -> instead of calling
		// c.getDisplay().setCursorLocation(swt), which does
		// not generate a mouse move event on all platforms, use
		// c.getDisplay().post(event) with a dummy event.
		Event event = new Event();
		event.type = SWT.MouseMove;
		event.x = swt.x;
		event.y = swt.y;
		c.getDisplay().post(event);
}

would work on win32 as well as macosx cocoa. However, it seems that on mac, as a result of posting the event (although I could not yet reproduce this with a snippet outside of GEF), I always also obtain a MouseHover event (directly afterwards), which always has coordinates of x=-247 and y=-84 (independent of the MouseMove coordinates). Do you have any idea, where this could result from?
Comment 7 Scott Kovatch CLA 2010-12-07 15:14:41 EST
(In reply to comment #6)
> would work on win32 as well as macosx cocoa. However, it seems that on mac, as
> a result of posting the event (although I could not yet reproduce this with a
> snippet outside of GEF), I always also obtain a MouseHover event (directly
> afterwards), which always has coordinates of x=-247 and y=-84 (independent of
> the MouseMove coordinates). Do you have any idea, where this could result from?

The MouseHover is going to the last Control that the mouse entered before you moved the cursor. MouseEnter/MouseExit has to be synthesized on Cocoa, so we keep track of the last control that had the cursor.

I added a MouseHover listener to your original example that was just calling setCursorLocation, and it happens there as well, so it's not related to the use of Display#post(). It is worth filing a new bug, though; we need to update the current control since the cursor is now potentially over a new control after changing the cursor location.
Comment 8 Alexander Nyßen CLA 2010-12-07 15:25:02 EST
Created new bug #332083 to keep track of the invalid hover event. Could you attach the modified snippet there, so it can be reproduced?

I think we can close this one afterwards, as the Diplay#post(Event) based solution seems to successfully deal with the problem in GEF's AbstractTool and can be fixed as part of bug #242481
Comment 9 Scott Kovatch CLA 2010-12-07 18:02:07 EST
Okay, resolving as 'invalid'.
Comment 10 Randy Hudson CLA 2010-12-08 11:07:32 EST
(In reply to comment #1)
> I can get the desired behavior by using Display#post() to send a SWT.MouseMove
> event, which should generate the same behavior on all platforms.

I think this is the right approach.

> As you said, there's no requirement that setCursorLocation() generate a
> mouse-move event, so I'm hesitant to change working code.

Are you saying the javadoc for Display#setCursorLocation() doesn't mention the mouse-move event?  Clients should be able to assume that SWT has cross-platform behavior unless otherwise and explicitly stated.  For example, from SWT#SHEET:

	 * A sheet window is a window intended to be used as a temporary modal
	 * dialog that is attached to a parent window. It is typically used to
	 * prompt the user before proceeding. The window trim, positioning and
	 * general look of a sheet window is platform specific. For example,
	 * on the Macintosh, at the time this documentation was written, the
	 * window title is not visible.
	 * <br>Note that this is a <em>HINT</em>.

The workaround seems problematic. Does simply posting a mouse event also cause the cursor to move?
Comment 11 Alexander Nyßen CLA 2010-12-08 13:12:17 EST
(In reply to comment #10)
> (In reply to comment #1)
> > I can get the desired behavior by using Display#post() to send a SWT.MouseMove
> > event, which should generate the same behavior on all platforms.
> 
> I think this is the right approach.
> 
> > As you said, there's no requirement that setCursorLocation() generate a
> > mouse-move event, so I'm hesitant to change working code.
> 
> Are you saying the javadoc for Display#setCursorLocation() doesn't mention the
> mouse-move event?  Clients should be able to assume that SWT has cross-platform
> behavior unless otherwise and explicitly stated.  For example, from SWT#SHEET:
> 
>      * A sheet window is a window intended to be used as a temporary modal
>      * dialog that is attached to a parent window. It is typically used to
>      * prompt the user before proceeding. The window trim, positioning and
>      * general look of a sheet window is platform specific. For example,
>      * on the Macintosh, at the time this documentation was written, the
>      * window title is not visible.
>      * <br>Note that this is a <em>HINT</em>.
> 
> The workaround seems problematic. Does simply posting a mouse event also cause
> the cursor to move?

Having reconsidered this I agree to Randy in that it would probably be best to have the Display#post() directly within Display#setCursorLocation(), as this would also resolve the inconsistency of having this method dispatching an event on some platforms, while not doing so on others.
Comment 12 Scott Kovatch CLA 2010-12-08 13:24:55 EST
(In reply to comment #10)
> The workaround seems problematic. Does simply posting a mouse event also cause
> the cursor to move?

It does -- I wouldn't have suggested it if it didn't. :-) 

I originally was hesitant about using post() because I wasn't sure if the internal state of the mouse tracking code was being updated properly. Also, the CGEvent methods used in post() are not that reliable, but the code for SWT.MouseMove does work well.

I have an alternate fix that posts a fake NSMouseMoved event to the window under the cursor after the cursor is moved. Now that I've convinced I'm not going to break anything or reduce the reliability I'm comfortable reopening this.
Comment 13 Scott Kovatch CLA 2010-12-08 13:25:10 EST
.
Comment 14 Scott Kovatch CLA 2010-12-08 13:27:27 EST
Created attachment 184804 [details]
Alternate fix

Alternate fix. I'll pick one of these for post-M4.
Comment 15 Scott Kovatch CLA 2010-12-08 13:29:33 EST
Comment on attachment 184736 [details]
Potential fix

Ignore the diffs that touch the timer code. That's part of a different bug I'm working on.
Comment 16 Alexander Nyßen CLA 2010-12-08 16:11:22 EST
Scott, while investigating bug #332091 under windows xp, I also found an inconsistency of Display#setCursorLocation(Point) resp. Display#post(Event) w.r.t. actually moving the visible mouse cursor. I opened bug #332170 to track it. Maybe this is of interest in this context as well...
Comment 17 Randy Hudson CLA 2010-12-08 17:24:16 EST
If the cursor is already at the request location, I'm not sure if mouse move events are generated on other platforms.
Comment 18 Scott Kovatch CLA 2010-12-13 20:05:41 EST
Fixed using the above patch (without the timer changes.)

Fixed > 20101213.