From 0dc6ed3af112ea6464a579cf1211beb55cb940dc Mon Sep 17 00:00:00 2001 From: Matej Knopp Date: Wed, 17 Apr 2024 15:17:53 +0200 Subject: [PATCH] [macOS] FlutterView should not override platform view cursor (flutter/engine#52159) This fixes an edge case that I've overlooked in https://github.com/flutter/engine/pull/43101. It is possible to configure `FlutterViewController` tracking area with `FlutterMouseTrackingModeAlways` option, in which case the tracking area will call `[FlutterView cursorUpdate:]` unconditionally even if the mouse cursor is over platform view and the platform view has already handled the mouse update. The solution is to prevent the `FlutterView` from updating the cursor when the cursor is over a platform view. This can be accomplished by doing a hitTest inside `[FlutterView cursorUpdate:]`. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat --- .../macos/framework/Source/FlutterView.mm | 5 + .../macos/framework/Source/FlutterViewTest.mm | 121 ++++++++++++++++++ 2 files changed, 126 insertions(+) diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterView.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterView.mm index 29eec7364d4..bafe6984a9e 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterView.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterView.mm @@ -106,6 +106,11 @@ // - When context menu above FlutterView is closed. Context menu will change current cursor to arrow // and will not restore it back. - (void)cursorUpdate:(NSEvent*)event { + // Make sure to not override cursor when over a platform view. + NSView* hitTestView = [self hitTest:[self convertPoint:event.locationInWindow fromView:nil]]; + if (hitTestView != self) { + return; + } [_lastCursor set]; // It is possible that there is a platform view with NSTrackingArea below flutter content. // This could override the mouse cursor as a result of mouse move event. There is no good way diff --git a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewTest.mm b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewTest.mm index 65620a53824..8e80077906c 100644 --- a/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewTest.mm +++ b/engine/src/flutter/shell/platform/darwin/macos/framework/Source/FlutterViewTest.mm @@ -37,3 +37,124 @@ TEST(FlutterView, ShouldInheritContentsScaleReturnsYes) { viewId:kImplicitViewId]; EXPECT_EQ([view layer:view.layer shouldInheritContentsScale:3.0 fromWindow:view.window], YES); } + +@interface TestFlutterView : FlutterView + +@property(readwrite, nonatomic) NSView* (^onHitTest)(NSPoint point); + +@end + +@implementation TestFlutterView + +@synthesize onHitTest; + +- (NSView*)hitTest:(NSPoint)point { + return self.onHitTest(point); +} + +- (void)reshaped { + // Disable resize synchronization for testing. +} + +@end + +@interface TestCursor : NSCursor +@property(readwrite, nonatomic) BOOL setCalled; +@end + +@implementation TestCursor + +- (void)set { + self.setCalled = YES; +} + +@end + +TEST(FlutterView, CursorUpdateDoesHitTest) { + id device = MTLCreateSystemDefaultDevice(); + id queue = [device newCommandQueue]; + TestFlutterViewDelegate* delegate = [[TestFlutterViewDelegate alloc] init]; + FlutterThreadSynchronizer* threadSynchronizer = [[FlutterThreadSynchronizer alloc] init]; + TestFlutterView* view = [[TestFlutterView alloc] initWithMTLDevice:device + commandQueue:queue + delegate:delegate + threadSynchronizer:threadSynchronizer + viewId:kImplicitViewId]; + NSWindow* window = [[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 800, 600) + styleMask:NSBorderlessWindowMask + backing:NSBackingStoreBuffered + defer:NO]; + + TestCursor* cursor = [[TestCursor alloc] init]; + + window.contentView = view; + __weak NSView* weakView = view; + __block BOOL hitTestCalled = NO; + __block NSPoint hitTestCoordinate = NSZeroPoint; + view.onHitTest = ^NSView*(NSPoint point) { + hitTestCalled = YES; + hitTestCoordinate = point; + return weakView; + }; + NSEvent* mouseEvent = [NSEvent mouseEventWithType:NSEventTypeMouseMoved + location:NSMakePoint(100, 100) + modifierFlags:0 + timestamp:0 + windowNumber:0 + context:nil + eventNumber:0 + clickCount:0 + pressure:0]; + [view didUpdateMouseCursor:cursor]; + [view cursorUpdate:mouseEvent]; + + EXPECT_TRUE(hitTestCalled); + // The hit test coordinate should be in the window coordinate system. + EXPECT_TRUE(CGPointEqualToPoint(hitTestCoordinate, CGPointMake(100, 500))); + EXPECT_TRUE(cursor.setCalled); +} + +TEST(FlutterView, CursorUpdateDoesNotOverridePlatformView) { + id device = MTLCreateSystemDefaultDevice(); + id queue = [device newCommandQueue]; + TestFlutterViewDelegate* delegate = [[TestFlutterViewDelegate alloc] init]; + FlutterThreadSynchronizer* threadSynchronizer = [[FlutterThreadSynchronizer alloc] init]; + TestFlutterView* view = [[TestFlutterView alloc] initWithMTLDevice:device + commandQueue:queue + delegate:delegate + threadSynchronizer:threadSynchronizer + viewId:kImplicitViewId]; + NSWindow* window = [[NSWindow alloc] initWithContentRect:NSMakeRect(0, 0, 800, 600) + styleMask:NSBorderlessWindowMask + backing:NSBackingStoreBuffered + defer:NO]; + + TestCursor* cursor = [[TestCursor alloc] init]; + + NSView* platformView = [[NSView alloc] initWithFrame:NSMakeRect(0, 0, 100, 100)]; + + window.contentView = view; + __block BOOL hitTestCalled = NO; + __block NSPoint hitTestCoordinate = NSZeroPoint; + view.onHitTest = ^NSView*(NSPoint point) { + hitTestCalled = YES; + hitTestCoordinate = point; + return platformView; + }; + NSEvent* mouseEvent = [NSEvent mouseEventWithType:NSEventTypeMouseMoved + location:NSMakePoint(100, 100) + modifierFlags:0 + timestamp:0 + windowNumber:0 + context:nil + eventNumber:0 + clickCount:0 + pressure:0]; + [view didUpdateMouseCursor:cursor]; + [view cursorUpdate:mouseEvent]; + + EXPECT_TRUE(hitTestCalled); + // The hit test coordinate should be in the window coordinate system. + EXPECT_TRUE(CGPointEqualToPoint(hitTestCoordinate, CGPointMake(100, 500))); + EXPECT_FALSE(cursor.setCalled); +}