detectTapGestures flakes and hangs in the down state on the emulator with mouse input [179239764]

Fixed

Bug

Status Update

No update yet.

Description

ma...@google.com

created issue #1

Feb 3, 2021 11:55AM

What title says, we had to revert clickable to tapGestureFilters and other filters instead of using detectTapGestures because click wasn't fired sometimes under certain circumstances. More context:

After we migrated clickable to detectTapGestures, we were missing clicks on the buttons.

Symptoms: tap with regular speed, after click is up (lifted) sometimes ripple animates to pressed state and stays there and onClick is not fired. Reproducible not on every click and not for everyone.

Notable discoveries throughout exploration/revert:

It was reproducible only on emulators. All attempts to repro this on the
It was flaky, meaning that not everyone was able to reproduce it. 2.1 We were able to reproduce it on some linux machines that operates with mouse 2.2 We were able to reproduce it on the macs when you TAP on the trackpad. Not reproducible when you click.
Revert to tapGestureFilter fully solves the issue.

Seems like a timing/race issue when delay between down and up is very low, or smth along this lines.

It's an important issue to allow us to continue to migrate production code to suspend pointer to solve, hence p1.

Comments

ap...@google.com <ap...@google.com> #2Mar 24, 2021 11:13AM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

https://android-review.googlesource.com/1360099

ap...@google.com <ap...@google.com> #3Aug 19, 2021 02:40AM

Project: platform/frameworks/support
Branch: androidx-main

commit 0261205e8e5e63ca0a84fd2e9be43fe79e85591e
Author: George Mount <mount@google.com>
Date: Wed Aug 11 17:00:49 2021

Add withTimeout() and withTimeoutOrNull() to AwaitPointerEventScope

Bug: 179239764
Bug: 182397793

withTimeout() and withTimeoutOrNull() are useful functions
when working with gesture detectors that have time involved
like long press and double click. Since those two methods
are not available within AwaitPointerEventScope, the
gesture detectors exit the pointer event loop after the
withTimeout() block. This means that many gesture detectors
were having trouble with losing pointer events.

A previous change fixed this by using two independent
jobs, one that manages the pointer event loop and sends
the events through a channel, and the other reads the
channel and can use withTimeout().

This change adds withTimeout() and withTimeoutOrNull()
to AwaitPointerEventScope so that gesture detectors
can manage pointer events with timed events without
leaving the event loop.

detectTapGestures() was modified to use the new
mechanism.

Relnote: "AwaitPointerEventScope now has withTimeout()
and withTimeoutOrNull()"

Test: new tests for withTimeout() and withTimeoutOrNull()
Test: ran existing tests for detectTapGestures()

Change-Id: I507f0e696311ac0504126681c376f73beaa021fb

M compose/foundation/foundation/src/commonMain/kotlin/androidx/compose/foundation/gestures/TapGestureDetector.kt
M compose/foundation/foundation/src/test/kotlin/androidx/compose/foundation/gestures/TapGestureDetectorTest.kt
M compose/ui/ui/api/current.txt
M compose/ui/ui/api/public_plus_experimental_current.txt
M compose/ui/ui/api/restricted_current.txt
M compose/ui/ui/src/androidAndroidTest/kotlin/androidx/compose/ui/input/pointer/SuspendingPointerInputFilterTest.kt
M compose/ui/ui/src/commonMain/kotlin/androidx/compose/ui/input/pointer/SuspendingPointerInputFilter.kt

https://android-review.googlesource.com/1794927

Issue 179239764

Description

Issue summary

Comments

ap...@google.com <ap...@google.com> #2Mar 24, 2021 11:13AM

ap...@google.com <ap...@google.com> #3Aug 19, 2021 02:40AM

Add comment

Issue metadata