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

Scroll inertia cancel for Win32 #34452

Merged
merged 4 commits into from
Oct 26, 2022
Merged

Conversation

moffatman
Copy link
Contributor

Generate PointerScrollInertiaCancel event on Win32 when DirectManipulation inertia is prematurely ended. This should correspond to a trackpad touch.

Part of flutter/flutter#106979

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 flutter-dashboard bot added embedder Related to the embedder API platform-windows labels Jul 4, 2022
@moffatman moffatman force-pushed the inertia_win32 branch 2 times, most recently from d985f0e to 5c95e94 Compare July 8, 2022 04:22
@moffatman moffatman requested a review from dkwingsmt September 16, 2022 13:59
@@ -53,16 +56,31 @@ HRESULT DirectManipulationEventHandler::OnViewportStatusChanged(
(int32_t) reinterpret_cast<int64_t>(this));
}
}
} else if (previous == DIRECTMANIPULATION_RUNNING) {
}
if (previous == DIRECTMANIPULATION_RUNNING) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we combine the previous if clause and this one into

Suggested change
if (previous == DIRECTMANIPULATION_RUNNING) {
if (previous == DIRECTMANIPULATION_RUNNING) {
if (during_synthesized_reset_) {
during_synthesized_reset_ = false;
} else if (current == DIRECTMANIPULATION_RUNNING) {
...
}
// Reset deltas to ensure only inertia values will be compared later.
...

(Did I suggest the current way when I first reviewed this file? Sorry if I did.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't do that since the else if would no longer make any sense (previous and current state will not ever be the same).

if (owner_->binding_handler_delegate) {
owner_->binding_handler_delegate->OnPointerPanZoomEnd(
(int32_t) reinterpret_cast<int64_t>(this));
}
} else if (previous == DIRECTMANIPULATION_INERTIA) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We only compare the previous status but not the current, does it mean if the inertia movement dies out naturally it will also dispatch a cancel event?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Current should always be DIRECTMANIPULATION_READY whether it dies out naturally or is cancelled. We can only tell if it's cancelled prematurely by seeing if the last event still had meaningful velocity left (hadn't already decelerated to near-zero).

@moffatman moffatman requested a review from dkwingsmt October 20, 2022 14:33
@alexmercerind
Copy link

alexmercerind commented Oct 26, 2022

May we show some love to this one?
Thanks a lot for the great work.

I don't know how to explain it, but when I scroll on Windows with my trackpad (post Flutter 3.3.x), it feels sort of "un-natural".

Scrolling abruptly stops or starts scrolling in completely opposite direction when making a swipe on my trackpad while a ListView is "already in scroll inertia motion". I have a precision touch-pad. I'm still on Flutter 3.0.5 because of this. I'm not sure if this will fix it, but once again, thanks!

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.

LGTM

@moffatman moffatman added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 26, 2022
@auto-submit auto-submit bot merged commit 7ded18a into flutter:main Oct 26, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 26, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 26, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 26, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 26, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App embedder Related to the embedder API platform-windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants