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

Win32 trackpad gestures #31594

Merged
fluttergithubbot merged 5 commits into from
May 20, 2022
Merged

Win32 trackpad gestures #31594

fluttergithubbot merged 5 commits into from
May 20, 2022

Conversation

@moffatman
Copy link
Contributor

@moffatman moffatman commented Feb 21, 2022

Start sending PointerPanZoom events from Windows (win32) when trackpad is used

Fixes flutter/flutter#23604

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.

@cbracken cbracken self-requested a review March 4, 2022 22:20
@dillibabu1802 dillibabu1802 mentioned this pull request Mar 11, 2022
Closed
@moffatman moffatman force-pushed the trackpad_gestures_win32 branch from 216376c to 1075d27 Compare April 6, 2022 00:58
@cbracken cbracken requested a review from justinmc April 26, 2022 18:29
justinmc
justinmc reviewed Apr 27, 2022
View reviewed changes
Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, but @cbracken could you take a quick skim through this PR just for C++ style?

Also, is there anyone with a Windows machine with a trackpad that can confirm that this works and feels natural? Maybe @gspencergoog.

shell/platform/windows/direct_manipulation.cc Outdated
Comment on lines 99 to 102
// DirectManipulation provides updates with very high precision. If the user
// holds their fingers steady on a trackpad, DirectManipulation sends
// jittery updates. This calculation will reduce the precision of the scale
// value of the event to avoid jitter
Copy link
Contributor

@justinmc justinmc Apr 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@moffatman What kind of machine have you tested this on? I'm wondering the feel will vary by device.

Copy link
Contributor Author

@moffatman moffatman Apr 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@justinmc Tested on desktop with apple trackpad + precision drivers. I believe it will only work on relatively recent devices. I could not find any way to get usable gesture data from a laptop with synaptics trackpad (since the driver injects fake events to simulate scroll momentum). The best bet for it to work would probably be MS Surface line.

Copy link
Contributor

@justinmc justinmc Apr 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CC @gspencergoog, who I believe has a Surface set up?

dkwingsmt
dkwingsmt suggested changes Apr 29, 2022
View reviewed changes
Copy link
Contributor

@dkwingsmt dkwingsmt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly minor stuff.

dkwingsmt
dkwingsmt reviewed Apr 29, 2022
View reviewed changes
shell/platform/windows/direct_manipulation.cc
return S_OK;
}
if (!resetting_) {
// DirectManipulation provides updates with very high precision. If the user
Copy link
Contributor

@dkwingsmt dkwingsmt Apr 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we tested this part yet?

Copy link
Contributor Author

@moffatman moffatman May 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a test for it

cbracken
cbracken reviewed Apr 29, 2022
View reviewed changes
cbracken
cbracken suggested changes Apr 29, 2022
View reviewed changes
@moffatman moffatman force-pushed the trackpad_gestures_win32 branch 4 times, most recently from 21e93c2 to b47f951 Compare May 1, 2022 20:35
@dkwingsmt dkwingsmt requested review from cbracken and justinmc May 6, 2022 21:06
@moffatman moffatman force-pushed the trackpad_gestures_win32 branch from 023cb48 to b8af034 Compare May 11, 2022 03:39
@dkwingsmt
Copy link
Contributor

dkwingsmt commented May 11, 2022

@cbracken Can you take another look?

@moffatman moffatman force-pushed the trackpad_gestures_win32 branch from b8af034 to 071c5ba Compare May 11, 2022 04:55
@moffatman moffatman force-pushed the trackpad_gestures_win32 branch from 071c5ba to fbfe4d9 Compare May 11, 2022 04:56
cbracken
cbracken approved these changes May 19, 2022
View reviewed changes
Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agh sorry about the delay here; this slipped off my radar. Thanks for the explanation on the timer. lgtm!

@dkwingsmt dkwingsmt added waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. affects: desktop labels May 19, 2022
@fluttergithubbot
Copy link
Contributor

fluttergithubbot commented May 19, 2022

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Windows Host Engine has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Windows Unopt has failed. Please fix the issues identified (or deflake) before re-applying this label.

@fluttergithubbot fluttergithubbot removed the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label May 19, 2022
@dkwingsmt
Copy link
Contributor

dkwingsmt commented May 19, 2022 •
edited
Loading

Strangely there are some compiling issues. Can you take a look? @moffatman

@dkwingsmt dkwingsmt added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label May 20, 2022
@fluttergithubbot fluttergithubbot merged commit 4b577eb into flutter:main May 20, 2022
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 20, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 20, 2022
@engine-flutter-autoroll
4b577eb Win32 trackpad gestures (flutter/engine#31594)
b27af24
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request May 20, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 20, 2022
@engine-flutter-autoroll
4b577eb Win32 trackpad gestures (flutter/engine#31594)
db2ff7c
houhuayong pushed a commit to houhuayong/engine that referenced this pull request Jun 21, 2022
@moffatman @houhuayong
Win32 trackpad gestures (flutter#31594)
8c4ec35
This was referenced Aug 11, 2022
Closed
Merged
@loic-sharma loic-sharma mentioned this pull request Aug 18, 2022
Merged
8 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@justinmc justinmc Awaiting requested review from justinmc

2 more reviewers

@cbracken cbracken cbracken approved these changes

@dkwingsmt dkwingsmt dkwingsmt approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

affects: desktop platform-windows waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Should support trackpad gestures from host

5 participants

@moffatman @dkwingsmt @fluttergithubbot @cbracken @justinmc