Fix T82870 Continuous grab, jumps when cursor crosses a window border (GHOST/macOS)
AbandonedPublic
Actions

Authored by Yevgeny Makarov (jenkm) on Nov 28 2020, 5:19 PM.

Details

Reviewers

Brecht Van Lommel (brecht)

Maniphest Tasks

T82870: Continuous grab is broken 2.92.0 Alpha macOS

Summary

Fix T82870: Continuous grab, jumps when cursor crosses a window border.

We can have a queue of multiple events with the same cursor position.
After wrapping and accumulation, these events with old cursor position
lead to extra accumulation.

This issue was present earlier but became particularly noticeable
after rB96200110eb0f and rB9e85812acc88.

Enable event coalescing for wrap/grab modes
Use mouseLocationOutsideOfEventStream instead of locationInWindow
Use time of last wrap event, similar to how it is already done for Linux

Diff Detail

Repository: rB Blender

Event Timeline

Yevgeny Makarov (jenkm) requested review of this revision.Nov 28 2020, 5:19 PM

Yevgeny Makarov (jenkm) created this revision.

Nicholas Rishel (nicholas_rishel) added a subscriber: Nicholas Rishel (nicholas_rishel).Nov 28 2020, 10:06 PM

Nicholas Rishel (nicholas_rishel) added inline comments.

intern/ghost/intern/GHOST_SystemCocoa.mm
852–853	Could you document why this change is now valid in the commit message?

In fact, I can't add anything. The mentioned https://stackoverflow.com/a/17559012 says that it doesn't work with CGWarpMouseCursorPosition, but we use CGDisplayMoveCursorToPoint. Moreover a newer comment states that this works with CGWarpMouseCursorPosition as well.

Thanks for looking into this.

I can see how using mouseLocationOutsideOfEventStream, enabling coalescing during warp, and a .025 time offset help. But they seem to be about reducing the probability of the problem occurring, rather than eliminating it entirely?

To me the most straightforward solution seems to be ignoring mouse moved events before the warp. Any reason this is not enough?

P1827 (An Untitled Masterwork)

1	diff --git a/intern/ghost/intern/GHOST_SystemCocoa.h b/intern/ghost/intern/GHOST_SystemCocoa.h
2	index 5637dfb0565..cee6398b5a6 100644
3	--- a/intern/ghost/intern/GHOST_SystemCocoa.h
4	+++ b/intern/ghost/intern/GHOST_SystemCocoa.h
5	@@ -311,4 +311,7 @@ class GHOST_SystemCocoa : public GHOST_System {
6	bool m_ignoreMomentumScroll;
7	/** Is the scroll wheel event generated by a multitouch trackpad or mouse? */
8	bool m_multiTouchScroll;
9	+ /** To prevent multiple warp, we store the time of the last warp event
10	+ * and ignore mouse moved events generated before that. */
11	+ double m_last_warp_timestamp;
12	};
13	diff --git a/intern/ghost/intern/GHOST_SystemCocoa.mm b/intern/ghost/intern/GHOST_SystemCocoa.mm
14	index 2a3f6e0b0b9..d5b8311349b 100644
15	--- a/intern/ghost/intern/GHOST_SystemCocoa.mm
16	+++ b/intern/ghost/intern/GHOST_SystemCocoa.mm
17	@@ -53,6 +53,8 @@
18	#include <sys/time.h>
19	#include <sys/types.h>
20
21	+#include <mach/mach_time.h>
22	+
23	#pragma mark KeyMap, mouse converters
24
25	static GHOST_TButtonMask convertButton(int button)
26	@@ -537,6 +539,7 @@ GHOST_SystemCocoa::GHOST_SystemCocoa()
27	m_ignoreWindowSizedMessages = false;
28	m_ignoreMomentumScroll = false;
29	m_multiTouchScroll = false;
30	+ m_last_warp_timestamp = 0;
31	}
32
33	GHOST_SystemCocoa::~GHOST_SystemCocoa()
34	@@ -1590,6 +1593,13 @@ GHOST_TSuccess GHOST_SystemCocoa::handleMouseEvent(void *eventPtr)
35	}
36	case GHOST_kGrabWrap: // Wrap cursor at area/window boundaries
37	{
38	+ NSTimeInterval timestamp = [event timestamp];
39	+ if (timestamp < m_last_warp_timestamp) {
40	+ /* After warping we can still receive older unwarped mouse events,
41	+ * ignore those. */
42	+ break;
43	+ }
44	+
45	NSPoint mousePos = [event locationInWindow];
46	GHOST_TInt32 x_mouse = mousePos.x;
47	GHOST_TInt32 y_mouse = mousePos.y;
48	@@ -1625,6 +1635,9 @@ GHOST_TSuccess GHOST_SystemCocoa::handleMouseEvent(void *eventPtr)
49	setMouseCursorPosition(warped_x, warped_y); /* wrap */
50	window->setCursorGrabAccum(x_accum + (x_mouse - warped_x_mouse),
51	y_accum + (y_mouse - warped_y_mouse));
52	+
53	+ /* This is the current time that matches NSEvent timestamp. */
54	+ m_last_warp_timestamp = mach_absolute_time() * 1e-9;
55	}
56
57	// Generate event

... Any reason this is not enough?

No, it looks even better.

In D9662#240169, @Yevgeny Makarov (jenkm) wrote:

The mentioned https://stackoverflow.com/a/17559012 says that it doesn't work with CGWarpMouseCursorPosition, but we use CGDisplayMoveCursorToPoint. Moreover a newer comment states that this works with CGWarpMouseCursorPosition as well.

What I meant was adding what you just wrote to the commit message, so that someone browsing through git's history will understand this part of the change.

Sean Robertson (seanr) mentioned this in T82870: Continuous grab is broken 2.92.0 Alpha macOS.Nov 29 2020, 10:25 PM

I'll commit my solution then.

It's not clear if there is anything in this patch you want to keep, will leave it up to you to close it if not.

Brecht Van Lommel (brecht) mentioned this in rB9306e01b10cd: Fix T82870: on macOS, continuous grab jumps when crossing editor border.Nov 30 2020, 1:44 PM

Yevgeny Makarov (jenkm) abandoned this revision.Dec 1 2020, 12:34 PM