engine

engine copied to clipboard

Fixes https://github.com/flutter/flutter/issues/129085

Changes to FlutterMouseCursorPlugin

• none cursor is not handled by hiding the cursor anymore. That was too big of a hammer as it also affects other views (and platform views). Instead empty cursor is created from empty NSImage.

• Cursor plugin now notifies the engine when cursor has changed. The engine forwards the change to last FlutterView that handled mouse event. This is necessary because on occasion FlutterView needs to be able to restore cursor without framework being involved.

Preventing PlatformView from changing cursor when it is obscured by Flutter Content.

Generally in Cocoa cursor changes are done as response to mouseMoved event, which is driven by a NSTrackingArea. The issue here is that this is not affected by hit testing and tracking areas form a hierarchy parallel to view hierarchy and are not affected by being obscured by another view (or tracking area). This means that platform view will receive mouseMoved event even when is obscured by Flutter content. To work around this, the mutator view puts a tracking area above platform view, which means it gets the mouseMove event first, and when it decides that mouse is over Flutter content, it will prevent platform view from changing the cursor for the rest of RunLoop turn (see NSCursor+IgnoreChange).

Actual hit testing

This part is rather straightforward, the area where FlutterContent obscures mutator view is provided to the mutator view, which will return nil from hitTest: when the point is in the obscured area.

Example of hit testing

https://github.com/flutter/engine/assets/96958/bbac0cfd-8c44-44d3-addd-921c91a8a539

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 Hixie said 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.

Ping @cbracken. Perhaps @cyanglaz also has some context.

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

@knopp Are you still planning on working on this? If not, could you either close it or mark it as draft so we can get it off the PR backlog? (Desktop Triage)

This is pending another round of review from me, but been stuck working on two internal customer issues, I'll try to get back to this soon. Apologies for the delay.

Converting this to Draft, as @cbracken is still looking at alternatives.

Spoke to @cbracken and he says this is still on his radar.

I have removed the method swizzling part. The downside is slightly reduced fidelity in a corner case where platform view has a NSTrackingArea that it uses to change mouse cursor and is overlaid by Flutter content. Given that method swizzling seems a bit contentious I think this is a reasonable tradeoff.