Status Update
No update yet.
Comments
ig...@jetbrains.com
The inversion should probably be done inside AndroidInputDispatcher.enqueueScroll
je...@google.com
scroll(2f) should generate a MotionEvent who's y scroll axis is 2f, not -2f. Whether that should be called "upward" or "downward" depends on the sign of the y scroll axis for MotionEvents generated by an actual mouse.
If this means that performMouseInput { scroll(2f) } scrolls in opposite directions on different platforms, that would be very unfortunate indeed.
Is the sign of raw scroll events of the same mouse the same on Android and on Desktop, or do the platforms produce different signs for the same scroll rotation of the same mouse?
ig...@jetbrains.com
Is the sign of raw scroll events of the same mouse the same on Android and on Desktop, or do the platforms produce different signs for the same scroll rotation of the same mouse
Platforms produce different signs. Desktop and new Api for web produce the same sign, Android and old api for web - opposite sign.
If we scroll mouse wheel downward on desktop, we will have positive delta, and scroll down the scrollable list.
If we scroll downward on Android, we will have negative delta in raw native android event, but positive delta in Compose PointerEvent (we inverted in
scroll(2f) should generate a MotionEvent who's y scroll axis is 2f, not -2f
I think, that delta in a raw native event shouldn't matter, because our test API abstract away from platform details. What is matter, is that sign in scroll(2f) and sign in PointerEvent.scrollDelta should be the same.
ig...@jetbrains.com
Platforms produce different signs
Because of that, we have to invert signs on some platform, so behaviour is the same on all platforms:
call test scroll -> [start of platform code] -> invert delta -> nativeScroll -> invert delta -> [end of platform code] -> scroll the list
We chose to invert the delta on Android, because two other platforms have opposite sign (desktop/web), and because the usual coordinate system starts from top to bottom.
je...@google.com
On second thought, I think you're right. It does make more sense if scroll(2f) yields a scrollDelta of 2f. Will update my review accordingly
ap...@google.com
Branch: androidx-main
commit f7459926705b1c79810fa321844f060d2d4276f1
Author: Igor Demin <igor.demin@jetbrains.com>
Date: Thu Apr 21 17:35:32 2022
Invert delta in MouseInjectionScope.scroll for vertical scroll
Similar to
Bug: 224992993
RelNote: MouseInjectionScope.scroll(delta = someDelta) is now inverted on Android if we scroll vertically (if someDelta is positive, it will scroll downward)
Test: ./gradlew buildOnServer
Change-Id: Ifb697a9ae8bc05a2d54d0a6cb4018713e156baf8
M compose/ui/ui-test/src/commonMain/kotlin/androidx/compose/ui/test/InputDispatcher.kt
M compose/ui/ui-test/src/test/kotlin/androidx/compose/ui/test/inputdispatcher/MouseEventsTest.kt
M compose/ui/ui-test/src/androidMain/kotlin/androidx/compose/ui/test/AndroidInputDispatcher.android.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/ScrollableTest.kt
M compose/ui/ui-test/src/commonMain/kotlin/androidx/compose/ui/test/MouseInjectionScope.kt
Description
Jetpack Compose version: 3621f0a7 Jetpack Compose component used: performMouseInput, scroll
Inhttps://android-review.googlesource.com/c/platform/frameworks/support/+/1913489 we inverted vertical mouse scroll, because on Android when we scroll down (the top touch point of the wheel moves to the user), scrollX is positive, but it should be negative.
In tests when we make artificial scroll event, we should invert x axis too (to avoid confusion).
See for example the test:https://android-review.googlesource.com/c/platform/frameworks/support/+/1994356/3/compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/input/pointer/OnPointerEventTest.kt#155
Now it is:
But it should be: