Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Implement cursor support for Windows #19459

Merged
stuartmorgan-g merged 3 commits into from
Jul 9, 2020
Merged

Implement cursor support for Windows #19459

stuartmorgan-g merged 3 commits into from
Jul 9, 2020

Conversation

@stuartmorgan-g
Copy link
Contributor

@stuartmorgan-g stuartmorgan-g commented Jul 1, 2020

Description

Adds the basic support for the cursor channel in the Windows embedding, supporting the subset of the current Dart cursors that are standard Win32 cursors.

Related Issues

flutter/flutter#31952

Tests

I added the following tests: None

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the contributor guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the C++, Objective-C, Java style guides for the engine.
  • I read the tree hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation.
  • All existing and new tests are passing.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read handling breaking changes.

  • No, no existing tests failed, so this is not a breaking change.
  • Yes, this is a breaking change.

@stuartmorgan-g
Implement cursor support for Windows
f8b009e 
Implements the basic engine support for Windows for
flutter/flutter#31952
@stuartmorgan-g stuartmorgan-g requested a review from clarkezone July 1, 2020 21:15
@stuartmorgan-g
Copy link
Contributor Author

stuartmorgan-g commented Jul 1, 2020

@dkwingsmt FYI

@gspencergoog gspencergoog mentioned this pull request Jul 1, 2020
Open
17 tasks
@auto-assign auto-assign bot requested a review from GaryQian July 1, 2020 21:16
@dkwingsmt
Copy link
Contributor

dkwingsmt commented Jul 7, 2020

@stuartmorgan We'll probably want to use a hashmap for GetCursorForKind. flutter/flutter#60931 is adding the complete set of cursors (> 60). You don't have to support the new ones in this PR, but the linear search is probably unwanted.

@stuartmorgan-g
Copy link
Contributor Author

stuartmorgan-g commented Jul 8, 2020

@stuartmorgan We'll probably want to use a hashmap for GetCursorForKind. flutter/flutter#60931 is adding the complete set of cursors (> 60).

Thanks for the heads up, done.

@stuartmorgan-g
Copy link
Contributor Author

stuartmorgan-g commented Jul 9, 2020

Mac Android Debug Engine was infra flake, and green on re-run

@stuartmorgan-g stuartmorgan-g merged commit 6726a46 into flutter:master Jul 9, 2020
@stuartmorgan-g stuartmorgan-g deleted the windows-cursor branch July 9, 2020 18:18
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 9, 2020
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 9, 2020
@engine-flutter-autoroll
6726a46 Implement cursor support for Windows (flutter/engine#19459)
a58b0da
@SahajRana
Copy link

SahajRana commented Jul 12, 2020

Hi @stuartmorgan, I tried to work with new cursor implementation. It looks like it currently only supports: SIMPLE, HAND, TEXT & UNAVAILABLE cursor works for Windows. Should we expect the addition of Resize arrow cursors, CROSS hair, BUSY system cursors soon, or later?

@dkwingsmt
Copy link
Contributor

dkwingsmt commented Jul 12, 2020 •
edited
Loading

Yes, they eill be available after #19550 is merged.

Edit: Oops... the said PR doesn't add to Windows yet. zbut maybe I can squish in an update soon.

@SahajRana
Copy link

SahajRana commented Jul 12, 2020

@dkwingsmt, Sure, it would be a great help.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@clarkezone clarkezone Awaiting requested review from clarkezone

@GaryQian GaryQian Awaiting requested review from GaryQian

Assignees

No one assigned

Labels

affects: desktop cla: yes needs tests platform-windows

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@stuartmorgan-g @dkwingsmt @SahajRana @gw280 @googlebot