feat: add custom cursor capabilities to web platform. Addresses #31952, #89351 and #99484

[timmaffett]

This change allows the web platform to implement custom mouse cursors.
It simply allows image-set, -webkit-image-set and url css commands to be set from the cursor's kind attribute.
This allows the kind to hold device pixel ratio supporting custom cursor images.

There is a web live example using this engine change here.

I have tried to create a package with a clean unified interface to custom cursors for windows, macOS, linux and web.
https://pub.dev/packages/custom_mouse_cursor

The package implements complete device pixel ratio responsive cursors that change automatically to support higher DPR screens and allows specifying DPR assets or multiple images for a cursor at different DPRs, etc.
It also allows creating cursors from icons.

It allows us to get closer to what's need for flutter/flutter#31952 and flutter/flutter#89351 and flutter/flutter#99484.
I have tried to implement this in a way that could possibly be considered for core flutter engine support of custom mouse cursors.

For windows this uses @Kingtous 's PR #36143 additions.
At the macos and linux advantage of @imiskolee and @Kingtous 's work in their lower level flutter_custom_cursor plugin.
(These directly mirror the platform channel methods in #36143 ).

Pre-launch Checklist

• I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
• I read the [Tree Hygiene] wiki page, which explains my responsibilities.
• I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides].
• I listed at least one issue that this PR fixes in the description above.
• 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.
• I updated/added relevant documentation (doc comments with ///).
• I signed the [CLA].
• All existing and new tests are passing.

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[kevmoo]

LOVE the example app running! Cool!

[kevmoo]

is there anyway to have a test here?

[timmaffett]

i had just started reviewing the existing tests for cursors in the engine and will be adding tests for these changes. I am away for the weekend, so it may be monday.

[dkwingsmt]

This is cool! Thank you for the beautiful (and stylish) example! Let me know once you've added some tests. :)

[kevmoo]

@timmaffett – no need to keep merging in main, as long as there are no conflicts. It just wastes CPU time.

[timmaffett]

@kevmoo - ok - I will avoid unnecessary syncs.

@kevmoo, @dkwingsmt - the tests have finally been added. The test first tests all of the existing functionality (all built in system cursors flutter uses) and then tests the new functionality allowing url/-webkit-image-set css styles.

[kevmoo]

@timmaffett – done!

[timmaffett]

@dkwingsmt Just wanted to ping you once again and remind you that the tests have been added and see if we could move forward on this ?

[dkwingsmt]

Sorry, this PR slipped through my mind and I failed to address it. And the tests look great!

But now that I look at it, I think it might not be a good idea to use the "system cursor" method to activate the url cursors, since they're simply not "system".

I think it'd be better if we create a new method to handle this. We can either make it platform-independent, such as

method: "activateUrlCursor"
payload: { "url": "data:image/png;base64[...]==", "pixelRatio": 2.0 }

Or a platform-dependent one,

method: "web/activateUrlCursor"
payload: { "command": "url(data:image/png;base64[...]==)" }

I'm currently leaning toward the platform-specific API because we haven't made up our mind how a unified custom cursor API should look like. What do you think? Also cc @mdebbar

[timmaffett]

I think if we did make a different entry point the system specific one is the way to do it - BUT on all other platforms activateSystemCursor is the way you set the cursor - the reason for this is that your define a new system cursor and give it a name (on linux,macos and windows) - and that becomes a new 'system cursor' - you then use activateSystemCursor() to activate it.
(See https://pub.dev/packages/custom_mouse_cursor for complete DPI aware example of how this works if you are interested)

So.. with that in mind activateSystemCursor really is the platform independent way that cursors are set. This is also the same as CSS (it treats url/image-set defined cursors as just another way to set a 'system cursor' - in this case not a name but a actual definition).

We could add a new entry point - but that would be equivalent to if had css decided that you set system cursors with

{
    cursor: pointer;
}

and then set url cursors with an entirely different

{
  custom-cursor: url()|imageset()
}

INSTEAD of just accepting that a url/imageset defined cursor IS just another system cursor. You are activating the system cursor (with a name and or definition) (The words 'SystemCursor' in the api entry point don't define the argument, they define what the api accomplished (activating a new sysytem cursor).

I am sorry as I am in a hurry to get out the door for a trip so I am rushing this reply... - I see where you are coming from, but I think adding a new entry point for the web is problem that does not need a solution, and there is no parity on other platforms as they all use activateSystemCursor() to activate systems cursors. The web only differs in that you instead of sending a newly mapped name in the call, you send the cursor definition.

In practice users would interact with custom cursors how the https://pub.dev/packages/custom_mouse_cursor package defines.
This api entry point is not really targeted at end users..

@dkwingsmt Does that make any sense ?

[dkwingsmt]

I've read the custom_mouse_cursor plugin, and I greatly appreciate the effort that you and others put on this! I understand it's a feature of high demand. But I still have quite some questions with your reply.

BUT on all other platforms activateSystemCursor is the way you set the cursor

The plugin's implementation actually kind of contradicts with what you said. In the plugin, desktop platforms already use setCustomCursor to set custom cursors, and it's a special case for the Web to use activateSystemCursor. Doesn't it make your code a bit cleaner if all you have to call is activeCustomCursor?

the reason for this is that your define a new system cursor and give it a name (on linux,macos and windows) - and that becomes a new 'system cursor' - you then use activateSystemCursor() to activate it.

When I came up with the method name, the name system cursors comes from the fact that these cursors come with the operating system and are available without extra assets or plugins to be shipped with the app. The term "cursor" alone means the mouse cursor. Since the cursors you're implementing in your plugin are bitmaps loaded from assets, I think they definitely are not system cursors. But it's fine. As I'll say later, what matters the most is not how we define this term, but the readability of APIs.

Also, all other desktop platforms are using the pattern of "first register with a key, then activate with the key". Yet Web is trying to send the entire data URL on every activation, which goes against your statement.

In practice users would interact with custom cursors how the https://pub.dev/packages/custom_mouse_cursor package defines. This api entry point is not really targeted at end users..

This is the toughest part: Everything added to Flutter is always targeted at end users, and that's I can't just say "sure let's land it since it works" despite I really wish I could. Once we add a new way to call this method channel, Flutter will document it and assume everything might call it. In fact, Flutter even allows people to not use our bindings, but call ui.PlatformDispatcher and method channels directly, and we make great effort to ensure those users get reasonably designed APIs and are not broken. Imagine you're calling the method channel directly and you're reading the doc for activateSystemCursor and found out that, "Oh the key argument can be one of the predefined values but also on Web it can be a CSS expression" - I wouldn't call it ideal.

So.. with that in mind activateSystemCursor really is the platform independent way that cursors are set.

With your design proposal, activateSystemCursor is not platform independent, because the CSS expression format is only available to Web.

We could add a new entry point - but that would be equivalent to if had css decided that you set system cursors with cursor: pointer;

I actually agree to this part to some extent. At the end of the day, in terms of functionality, it doesn't matter if we use the same method name of different names. If it's somehow parsable, it will work. Why make it complicated?

But that's why designing APIs is not a matter of functionality, but a matter of readability. Just like how program is for human to read, not for computers. Splitting features of different kinds into different names helps understanding how each is used, in which conditions each API is supported, etc.

To summarize, ideally, all APIs should be platform independent. Users write Flutter apps to expect they don't have to care about the resulting platform, and users will directly use the API this PR is adding. In reality, platform varies. Flutter's style guide requires avoiding the lowest common denominator, which also applies to this method channel API.

I understand the frustration of having to go back and forth with adding this API for your plugin. As I said, I really wish I could just give it a green light. In fact, the reason I didn't work on this feature was how complicated and how platform dependent the feature is to design a good enough API. That's why I'd really like to discuss more on this PR since you're making great contribution. I welcome more discussion on how users use this API, since you definitely have more experience than me for actually using it in your apps. We can also chat over Discord which can be more instant.

[Hixie]

@timmaffett Thanks for your contributions so far! Is this still something you are interesting in working on and landing? (If not that's totally fine, we can park the PR and let someone else take over when they have the time!)

[timmaffett]

@Hixie Yes I would be willing to make any required changes to land this. I created this PR to provided this missing capability for flutter web and would love to see the work included.

[Hixie]

@timmaffett awesome. I recommend discussing with @dkwingsmt either here or on our Discord to figure out next steps then!

[goderbauer]

@timmaffett Just checking in to see if you were able to connect with the person suggested above and if you're still planning on continue working in this.

[timmaffett]

@goderbauer I have not had a chance to connect with @dkwingsmt but I will put this on my whiteboard to get to it.
It has crossed my mind several times but I have been slammed with work.
@dkwingsmt I will make time to go over your review and come up with a proposal to address your comments and concerns.

[dkwingsmt]

I have to sincerely apologize for leaving this PR stale but I've just been afraid of reviewing this. I don't have the bandwidth to implement a full API for cross platform custom cursors, but it's also really hard to agree with a platform specific API at the risk of API breakage if we're able to transit to a cross platform one some day. I know you've worked hard on this and honestly, if we agree that Flutter can accept this platform specific API, I don't remember having any problems with the implementation. But approving it is a different responsibility I'm reluctant to take, especially since I can't see myself permitted to prioritize the cursors as my main project in the foreseeable future. I encourage you should seek some other reviewers with the Team Member role, since I don't see myself capable of reviewing it due to review fatigue. Again, thanks for your contribution so far, and I understand whether you continue to push it or not.

[Hixie]

Looking at the code, it's hard for me to tell if this is the right approach. The documentation for the message channel doesn't say what kind means, e.g. whether it's platform-specific or whether there's a standard API we're following on all platforms.

I think before we can land something like this we should document what the message channel API is.

[christopherfujino]

from critical issue triage: @timmaffett are you still working on this one?

[Piinks]

Thank you for your contribution. I'm going to close this PR for now since there are outstanding comments, just to get this off our PR review queue. Please don't hesitate to re-open or submit a new PR if you have the time to address the review comments. Thanks!