Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

iOS scroll bugs #3335

Closed
elijah-semyonov opened this issue Jul 10, 2023 · 1 comment · Fixed by JetBrains/compose-multiplatform-core#776
Closed

iOS scroll bugs #3335

elijah-semyonov opened this issue Jul 10, 2023 · 1 comment · Fixed by JetBrains/compose-multiplatform-core#776
Assignees
Labels
bug Something isn't working ios scroll

Comments

@elijah-semyonov
Copy link
Contributor

elijah-semyonov commented Jul 10, 2023

Describe the bug
Look video below.

Affected platforms
Select one of the platforms below:

  • iOS

Versions
aba6dd347f81c86596b91b749a80e22acb0c7d6f

To Reproduce

val colors = listOf(
            Color.Black,
            Color.LightGray,
            Color.DarkGray,
            Color.Gray
        )

        val rows = 20
        val columns = 20

        val rowHeight = 200.dp

        LazyColumn(Modifier.fillMaxSize()) {
            items(rows) {row ->
                LazyRow(Modifier.height(rowHeight)) {
                    items(columns) {col ->
                        val color = colors[(row + col) % colors.size]

                        Box(
                            Modifier
                                .size(200.dp, rowHeight)
                                .background(color)
                        )
                    }
                }
            }
        }

Do short drags with moving finger up approx 20-30 degrees from vertical axis in.

Expected behavior
On Android scrolling axis is selected based on which axis (vertical or horizontal) is closer to current drag direction.
On iOS, the selection is sometimes incorrect (horizontal scroll starts instead)

IMG_0015.MOV
@elijah-semyonov elijah-semyonov added bug Something isn't working submitted labels Jul 10, 2023
@elijah-semyonov elijah-semyonov self-assigned this Jul 10, 2023
@elijah-semyonov elijah-semyonov changed the title iOS bidirectional scroll inadequate dispatch iOS scroll bugs Jul 17, 2023
@elijah-semyonov
Copy link
Contributor Author

elijah-semyonov commented Jul 17, 2023

Do drag, hold finger still, raise it, ends up with very high velocity fling. (merge move + end touches events to avoid invalid velocity calculations?)

IMG_0017.mov

elijah-semyonov added a commit to JetBrains/compose-multiplatform-core that referenced this issue Sep 27, 2023
## PR scope changed
The velocity calculation formula is a separate issue now, it will take
all iOS platform specific datapoints into account, so no input data
correction is needed.
Duplicating data points will be fixed in
#752.
 
## Proposed Changes
1. Refactor touch sending on iOS, throw SkikoPointerEvent out as
redundant
2. Send coalesced touches(higher than display refresh rate touches) as
historical data (don't send them for synthetics). Users can retrieve it
now and use in case they need accurate touches data.
3. ~Adjust velocity calculator to filter duplicated data points in a
single timestamp (because synthetic touches), leading to inadequate
velocity (unexpected scrolls to top)~
4. ~Override position reported in the `ended` phase with last known
position of the touch. Solves two problems~:
4.1. ~Current velocity calculator plays bad with that value, because
it's slightly exaggerated to help UIKit with proper fling calculation
(opaque implementation detail) and since iOS can report two events in
the same frame (`moved` and `ended` with exaggerated position with small
timestamp delta, it causes awkward unexpected motions in the end of
scroll which was not supposed to lead to a fling)~
4.2. ~Delta in `ended` phase is not taken into consideration when
scrolling UIScrollView (proved by reverse engineering). Passing that
metadata in call tree of Draggable implementation will require internal
common API changes.~

## Testing

Test: N/A

## Issues Fixed

Fixes: [inadequate fling velocity during "quick-drag-and-stop" touch
events
sequence](JetBrains/compose-multiplatform#3335).
~Awkward motion caused by `ended` delta in the end of the drag without a
fling~.

## API Change

`ComposeScene.Pointer` now has a new constructor argument `val
historical: List<HistoricalChange> = emptyList()`

## Video

~https://github.com/JetBrains/compose-multiplatform-core/assets/4167681/bd5f530f-d26e-4a65-ab54-8085b22b5e1d~

---------

Co-authored-by: dima.avdeev <dima.avdeev@jetbrains.com>
igordmn pushed a commit to JetBrains/compose-multiplatform-core that referenced this issue Nov 15, 2023
## PR scope changed
The velocity calculation formula is a separate issue now, it will take
all iOS platform specific datapoints into account, so no input data
correction is needed.
Duplicating data points will be fixed in
#752.

## Proposed Changes
1. Refactor touch sending on iOS, throw SkikoPointerEvent out as
redundant
2. Send coalesced touches(higher than display refresh rate touches) as
historical data (don't send them for synthetics). Users can retrieve it
now and use in case they need accurate touches data.
3. ~Adjust velocity calculator to filter duplicated data points in a
single timestamp (because synthetic touches), leading to inadequate
velocity (unexpected scrolls to top)~
4. ~Override position reported in the `ended` phase with last known
position of the touch. Solves two problems~:
4.1. ~Current velocity calculator plays bad with that value, because
it's slightly exaggerated to help UIKit with proper fling calculation
(opaque implementation detail) and since iOS can report two events in
the same frame (`moved` and `ended` with exaggerated position with small
timestamp delta, it causes awkward unexpected motions in the end of
scroll which was not supposed to lead to a fling)~
4.2. ~Delta in `ended` phase is not taken into consideration when
scrolling UIScrollView (proved by reverse engineering). Passing that
metadata in call tree of Draggable implementation will require internal
common API changes.~

## Testing

Test: N/A

## Issues Fixed

Fixes: [inadequate fling velocity during "quick-drag-and-stop" touch
events
sequence](JetBrains/compose-multiplatform#3335).
~Awkward motion caused by `ended` delta in the end of the drag without a
fling~.

## API Change

`ComposeScene.Pointer` now has a new constructor argument `val
historical: List<HistoricalChange> = emptyList()`

## Video

~https://github.com/JetBrains/compose-multiplatform-core/assets/4167681/bd5f530f-d26e-4a65-ab54-8085b22b5e1d~

---------

Co-authored-by: dima.avdeev <dima.avdeev@jetbrains.com>
ASalavei added a commit to JetBrains/compose-multiplatform-core that referenced this issue Feb 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ios scroll
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants