i82973 scroll mouse wheel support #44724

reidbaker · 2023-08-15T14:49:38Z

Mouse scroll wheel support for android.

I chose to not cache the vertical and horizontal scale factors that come from view configuration. The primary reason is that in the current code path context is only used when the user scrolls which was the unimplemented feature. This smaller blast radius I decided was worth the additional calls. A secondary reason is that when the scale factors are changed is not a well documented path nor is there a lifecycle callback to listen to. Scroll factor is cached on api 25 and below because that more closely mirrors the behavior I see in pre 25 versions of android scroll view.

Note flutter takes longer to "see" a mouse that then scrolls than android

Fixes #flutter/flutter/82973

Todo list prior to merge

Verify on Pixel fold
Verify in multi window
Test on a api 25 or lower device
File bug for flutter taking a while to "see" mouse scroll. Android Mouse takes several seconds before being recognized by flutter. flutter#132866
File bug for AndroidTouchProcessor cleanup. AndroidTouchProcessor code structure is fragile flutter#132853
Figure out how to test Horizontal scroll wheel against real device.

Links

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.

…e that then scrolls than android

shell/platform/android/io/flutter/embedding/android/AndroidTouchProcessor.java

reidbaker · 2023-08-16T19:48:49Z

@LoveJello When merging in #43949 my code causes your tests to fail. Any ideas what broke? One of the changes I made was to the mock where I made isFromSource more closely match the actual implementation,

LoveJello · 2023-08-18T11:17:01Z

@LoveJello When merging in #43949 my code causes your tests to fail. Any ideas what broke? One of the changes I made was to the mock where I made isFromSource more closely match the actual implementation,

Sorry for late response.I added some comment in the code.
And you'd better to ask @moffatman to do the code review.
Thanks.

reidbaker · 2023-08-18T15:50:27Z

shell/platform/android/io/flutter/embedding/android/AndroidTouchProcessor.java

@@ -192,7 +195,9 @@ public boolean onGenericMotionEvent(@NonNull MotionEvent event) {
    boolean isMovementEvent =
        (event.getActionMasked() == MotionEvent.ACTION_HOVER_MOVE
            || event.getActionMasked() == MotionEvent.ACTION_SCROLL);


The change below was made for readability since @camsim99 and I both thought the implementation was confusing. It should be functionally identical and easier to follow. If a reviewer disagree please comment here and let me know.

reidbaker · 2023-08-18T16:21:20Z

John offered to test horizontal scroll with a mouse with a second scroll wheel after this lands.

flutter-dashboard · 2023-08-18T18:21:09Z

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

Piinks

Piinks · 2023-08-21T18:55:51Z

shell/platform/android/test/io/flutter/embedding/android/AndroidTouchProcessorTest.java

@@ -8,11 +8,16 @@
 import static org.mockito.Mockito.when;


These tests! 🤌 Thanks for shaving this down and adding all this coverage.

shell/platform/android/io/flutter/embedding/android/AndroidTouchProcessor.java

…chProcessor.java

moffatman

Looks good, thanks 😄

reidbaker · 2023-08-22T18:23:54Z

Pre api 26, horizontal scroll wheel testing will happen post merge given my difficulty finding hardware and not being able to test against an emulator.

…133075) flutter/engine@21437d3...28b8bd5 2023-08-22 reidbaker@google.com i82973 scroll mouse wheel support (flutter/engine#44724) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC jimgraham@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

Fixes flutter/flutter#82973 Mouse scroll wheel support for android. I chose to not cache the vertical and horizontal scale factors that come from view configuration. The primary reason is that in the current code path context is only used when the user scrolls which was the unimplemented feature. This smaller blast radius I decided was worth the additional calls. A secondary reason is that when the scale factors are changed is not a well documented path nor is there a lifecycle callback to listen to. Scroll factor is cached on api 25 and below because that more closely mirrors the behavior I see in pre 25 versions of android scroll view. Note flutter takes longer to "see" a mouse that then scrolls than android Fixes #flutter/flutter/82973 Todo list prior to merge ## Links * Some pre api 26 scroll factor code - https://cs.android.com/android/platform/superproject/main/+/main:frameworks/base/core/java/android/view/View.java?q=function:getVerticalScrollFactor%20filepath:android%2Fview%2FView.java&ss=android%2Fplatform%2Fsuperproject%2Fmain - https://cs.android.com/android/_/android/platform/frameworks/base/+/main:core/java/android/widget/ScrollView.java;l=930;drc=2fe301db7555bccf53e465436d4cb7442c803df3;bpv=0;bpt=0 * Post api 26 scroll factor code - https://cs.android.com/android/_/android/platform/frameworks/base/+/main:core/java/android/widget/ScrollView.java;l=361;drc=406e0f655387f27dda874c2b975fb0ddbd61aa13;bpv=0;bpt=0 - https://developer.android.com/reference/android/view/ViewConfiguration#getScaledVerticalScrollFactor()

Piinks and others added 2 commits August 9, 2023 14:58

Pair progress

4fc9130

Working mouse scroll wheel, note flutter takes longer to "see" a mous…

17faa0d

…e that then scrolls than android

github-actions bot added the platform-android label Aug 15, 2023

Piinks mentioned this pull request Aug 15, 2023

[WIP] Fix Android mouse scroll wheel input #44558

Closed

8 tasks

johnmccutchan reviewed Aug 15, 2023

View reviewed changes

reidbaker added 6 commits August 15, 2023 18:15

refactor packet method to have an override when context is null

95f5aa0

Add unit test for scroll wheel

2b9686c

Add unit test for scroll wheel

43619cc

Add unit test for scroll wheel

aa01e98

make pointer logic easier to read

05420ee

revert unrelated file

e14b961

reidbaker requested review from Piinks and a team August 16, 2023 15:08

Merge branch 'main' into i82973-scroll-mouse-wheel-support

9efded6

johnmccutchan approved these changes Aug 16, 2023

View reviewed changes

Fix merge, add timestamp test, code does not compile

2ea445d

Piinks reviewed Aug 16, 2023

View reviewed changes

shell/platform/android/io/flutter/embedding/android/AndroidTouchProcessor.java Outdated Show resolved Hide resolved

reidbaker added 2 commits August 16, 2023 14:29

Merge branch 'main' into i82973-scroll-mouse-wheel-support

5140201

Make tests compile

e76fec0

reidbaker added 9 commits August 17, 2023 10:41

Make tests pass

22e8a77

add device test

e7dfa9d

Add tests for pressure, obscured and synth

53ac277

Add tests for pressure min and max

02e1176

Add test for stylus distance

2c823c7

Add test for stylus distance

c2f3c56

Add tests for size and radius

168164f

Add tests for pan deltax and rotation and scale

06cda9a

formatting

bb157fb

reidbaker added 3 commits August 18, 2023 08:48

Merge branch 'main' into i82973-scroll-mouse-wheel-support

4df1fee

extract mock event values into function

eb4dbb0

formatting

33be4af

reidbaker commented Aug 18, 2023

View reviewed changes

reidbaker marked this pull request as ready for review August 18, 2023 16:20

reidbaker mentioned this pull request Aug 18, 2023

AndroidTouchProcessor code structure is fragile flutter/flutter#132853

Open

reidbaker requested a review from moffatman August 18, 2023 16:30

reidbaker marked this pull request as draft August 18, 2023 18:15

Tests for pre 26 scroll behavior

2cd1f46

reidbaker mentioned this pull request Aug 18, 2023

Android Mouse takes several seconds before being recognized by flutter. flutter/flutter#132866

Open

reidbaker marked this pull request as ready for review August 18, 2023 20:16

reidbaker mentioned this pull request Aug 18, 2023

Gradle OOM error across different platforms flutter/flutter#131006

Closed

Merge branch 'main' into i82973-scroll-mouse-wheel-support

3848a28

Piinks approved these changes Aug 21, 2023

View reviewed changes

Update shell/platform/android/io/flutter/embedding/android/AndroidTou…

ae7439f

…chProcessor.java

moffatman approved these changes Aug 21, 2023

View reviewed changes

reidbaker added autosubmit Merge PR when tree becomes green via auto submit App and removed autosubmit Merge PR when tree becomes green via auto submit App labels Aug 21, 2023

auto-submit bot merged commit 28b8bd5 into flutter:main Aug 22, 2023
25 checks passed

reidbaker deleted the i82973-scroll-mouse-wheel-support branch August 22, 2023 18:23

engine-flutter-autoroll mentioned this pull request Aug 22, 2023

Roll Flutter Engine from 21437d384b56 to 28b8bd5d5d91 (1 revision) flutter/flutter#133075

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 22, 2023

28b8bd5d5 i82973 scroll mouse wheel support (flutter/engine#44724)

dd688cd

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

i82973 scroll mouse wheel support #44724

i82973 scroll mouse wheel support #44724

reidbaker commented Aug 15, 2023 •

edited

Loading

reidbaker commented Aug 16, 2023

LoveJello commented Aug 18, 2023

reidbaker Aug 18, 2023

reidbaker commented Aug 18, 2023

flutter-dashboard bot commented Aug 18, 2023

Piinks left a comment

Piinks Aug 21, 2023 •

edited

Loading

moffatman left a comment

reidbaker commented Aug 22, 2023

i82973 scroll mouse wheel support #44724

i82973 scroll mouse wheel support #44724

Conversation

reidbaker commented Aug 15, 2023 • edited Loading

Links

Pre-launch Checklist

reidbaker commented Aug 16, 2023

LoveJello commented Aug 18, 2023

reidbaker Aug 18, 2023

Choose a reason for hiding this comment

reidbaker commented Aug 18, 2023

flutter-dashboard bot commented Aug 18, 2023

Piinks left a comment

Choose a reason for hiding this comment

Piinks Aug 21, 2023 • edited Loading

Choose a reason for hiding this comment

moffatman left a comment

Choose a reason for hiding this comment

reidbaker commented Aug 22, 2023

reidbaker commented Aug 15, 2023 •

edited

Loading

Piinks Aug 21, 2023 •

edited

Loading