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.

Sign up for GitHub

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

Jump to bottom

Remove Gesture Handler limits on Android #2672

Merged
merged 6 commits into from
Nov 20, 2023
Merged

Remove Gesture Handler limits on Android #2672

Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7e3cc86
Change implementation to ArrayList
m-bert Nov 9, 2023
6957bb1
Some changes in orchestrator
m-bert Nov 14, 2023
93c8fb7
Remove compactHandlersIf
m-bert Nov 14, 2023
892c095
Change how handlers are removed
m-bert Nov 15, 2023
da2c421
Remove handlersToCancel
m-bert Nov 16, 2023
3d3d241
reversed to asReversed
m-bert Nov 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 41 additions & 70 deletions android/src/main/java/com/swmansion/gesturehandler/core/GestureHandlerOrchestrator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,10 @@ class GestureHandlerOrchestrator(
* traversing view hierarchy and looking for gesture handlers.
*/
var minimumAlphaForTraversal = DEFAULT_MIN_ALPHA_FOR_TRAVERSAL

private val gestureHandlers = arrayOfNulls<GestureHandler<*>?>(SIMULTANEOUS_GESTURE_HANDLER_LIMIT)
private val awaitingHandlers = arrayOfNulls<GestureHandler<*>?>(SIMULTANEOUS_GESTURE_HANDLER_LIMIT)
private val preparedHandlers = arrayOfNulls<GestureHandler<*>?>(SIMULTANEOUS_GESTURE_HANDLER_LIMIT)
private val handlersToCancel = arrayOfNulls<GestureHandler<*>?>(SIMULTANEOUS_GESTURE_HANDLER_LIMIT)
private var gestureHandlersCount = 0
private var awaitingHandlersCount = 0
private val gestureHandlers = arrayListOf<GestureHandler<*>>()
private val awaitingHandlers = arrayListOf<GestureHandler<*>>()
private val preparedHandlers = arrayListOf<GestureHandler<*>>()
private val handlersToCancel = arrayListOf<GestureHandler<*>>()
private var isHandlingTouch = false
private var handlingChangeSemaphore = 0
private var finishedHandlersCleanupScheduled = false
Expand Down Expand Up @@ -60,23 +57,10 @@ class GestureHandlerOrchestrator(
}
}

private inline fun compactHandlersIf(handlers: Array<GestureHandler<*>?>, count: Int, predicate: (handler: GestureHandler<*>?) -> Boolean): Int {
var out = 0
for (i in 0 until count) {
if (predicate(handlers[i])) {
handlers[out++] = handlers[i]
}
}
return out
}

private fun cleanupFinishedHandlers() {
var shouldCleanEmptyCells = false
for (i in gestureHandlersCount - 1 downTo 0) {
val handler = gestureHandlers[i]!!
for (handler in gestureHandlers.reversed()) {
if (isFinished(handler.state) && !handler.isAwaiting) {
gestureHandlers[i] = null
shouldCleanEmptyCells = true
gestureHandlers.remove(handler)
Copy link
Member

Choose a reason for hiding this comment

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

Can we modify the collection while iterating over it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm pretty sure that calling Collection.reversed() returns new collection - in that case it should be fine (see docs for context)

handler.reset()
handler.apply {
isActive = false
Expand All @@ -85,17 +69,12 @@ class GestureHandlerOrchestrator(
}
}
}
if (shouldCleanEmptyCells) {
gestureHandlersCount = compactHandlersIf(gestureHandlers, gestureHandlersCount) { handler ->
handler != null
}
}

finishedHandlersCleanupScheduled = false
}

private fun hasOtherHandlerToWaitFor(handler: GestureHandler<*>): Boolean {
for (i in 0 until gestureHandlersCount) {
val otherHandler = gestureHandlers[i]!!
for (otherHandler in gestureHandlers) {
if (!isFinished(otherHandler.state) && shouldHandlerWaitForOther(handler, otherHandler)) {
return true
}
Expand All @@ -115,19 +94,16 @@ class GestureHandlerOrchestrator(
}

private fun cleanupAwaitingHandlers() {
awaitingHandlersCount = compactHandlersIf(awaitingHandlers, awaitingHandlersCount) { handler ->
handler!!.isAwaiting
}
awaitingHandlers.removeAll { !it.isAwaiting }
}

/*package*/
fun onHandlerStateChange(handler: GestureHandler<*>, newState: Int, prevState: Int) {
handlingChangeSemaphore += 1
if (isFinished(newState)) {
// if there were handlers awaiting completion of this handler, we can trigger active state
for (i in 0 until awaitingHandlersCount) {
val otherHandler = awaitingHandlers[i]
if (shouldHandlerWaitForOther(otherHandler!!, handler)) {
for (otherHandler in awaitingHandlers) {
if (shouldHandlerWaitForOther(otherHandler, handler)) {
if (newState == GestureHandler.STATE_END) {
// gesture has ended, we need to kill the awaiting handler
otherHandler.cancel()
Expand Down Expand Up @@ -182,21 +158,22 @@ class GestureHandlerOrchestrator(
shouldResetProgress = true
activationIndex = this@GestureHandlerOrchestrator.activationIndex++
}
var toCancelCount = 0

// Cancel all handlers that are required to be cancel upon current handler's activation
for (i in 0 until gestureHandlersCount) {
val otherHandler = gestureHandlers[i]!!
for (otherHandler in gestureHandlers) {
if (shouldHandlerBeCancelledBy(otherHandler, handler)) {
handlersToCancel[toCancelCount++] = otherHandler
handlersToCancel.add(otherHandler)
}
}
for (i in toCancelCount - 1 downTo 0) {
handlersToCancel[i]!!.cancel()

for (otherHandler in handlersToCancel.reversed()) {
otherHandler.cancel()
}

handlersToCancel.clear()
Copy link
Member

Choose a reason for hiding this comment

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

Do we need handlersToCancel at all here? I think you can iterate over gestureHandlers.asReversed() and just cancel relevant handlers inside the loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree - done in da2c421


// Clear all awaiting handlers waiting for the current handler to fail
for (i in awaitingHandlersCount - 1 downTo 0) {
val otherHandler = awaitingHandlers[i]!!
for (otherHandler in awaitingHandlers.reversed()) {
if (shouldHandlerBeCancelledBy(otherHandler, handler)) {
otherHandler.cancel()
otherHandler.isAwaiting = false
Expand All @@ -218,32 +195,31 @@ class GestureHandlerOrchestrator(
private fun deliverEventToGestureHandlers(event: MotionEvent) {
// Copy handlers to "prepared handlers" array, because the list of active handlers can change
// as a result of state updates
val handlersCount = gestureHandlersCount
preparedHandlers.clear()
preparedHandlers.addAll(gestureHandlers)

gestureHandlers.copyInto(preparedHandlers, 0, 0, handlersCount)
// We want to deliver events to active handlers first in order of their activation (handlers
// that activated first will first get event delivered). Otherwise we deliver events in the
// order in which handlers has been added ("most direct" children goes first). Therefore we rely
// on Arrays.sort providing a stable sort (as children are registered in order in which they
// should be tested)
preparedHandlers.sortWith(handlersComparator, 0, handlersCount)
for (i in 0 until handlersCount) {
deliverEventToGestureHandler(preparedHandlers[i]!!, event)
preparedHandlers.sortWith(handlersComparator)
for (handler in preparedHandlers) {
deliverEventToGestureHandler(handler, event)
}
}

private fun cancelAll() {
for (i in awaitingHandlersCount - 1 downTo 0) {
awaitingHandlers[i]!!.cancel()
for (handler in awaitingHandlers.reversed()) {
handler.cancel()
}
// Copy handlers to "prepared handlers" array, because the list of active handlers can change
// as a result of state updates
val handlersCount = gestureHandlersCount
for (i in 0 until handlersCount) {
preparedHandlers[i] = gestureHandlers[i]
}
for (i in handlersCount - 1 downTo 0) {
preparedHandlers[i]!!.cancel()
preparedHandlers.clear()
preparedHandlers.addAll(gestureHandlers)

for (handler in gestureHandlers.reversed()) {
handler.cancel()
}
}

Expand Down Expand Up @@ -325,7 +301,7 @@ class GestureHandlerOrchestrator(
return parent === wrapperView
}

fun isAnyHandlerActive() = gestureHandlers.any { it?.state == GestureHandler.STATE_ACTIVE }
fun isAnyHandlerActive() = gestureHandlers.any { it.state == GestureHandler.STATE_ACTIVE }

/**
* Transforms an event in the coordinates of wrapperView into the coordinate space of the received view.
Expand Down Expand Up @@ -399,27 +375,23 @@ class GestureHandlerOrchestrator(
}

private fun addAwaitingHandler(handler: GestureHandler<*>) {
for (i in 0 until awaitingHandlersCount) {
if (awaitingHandlers[i] === handler) {
return
}
if (awaitingHandlers.contains(handler)) {
return
}
check(awaitingHandlersCount < awaitingHandlers.size) { "Too many recognizers" }
awaitingHandlers[awaitingHandlersCount++] = handler

awaitingHandlers.add(handler)
with(handler) {
isAwaiting = true
activationIndex = this@GestureHandlerOrchestrator.activationIndex++
}
}

private fun recordHandlerIfNotPresent(handler: GestureHandler<*>, view: View) {
for (i in 0 until gestureHandlersCount) {
if (gestureHandlers[i] === handler) {
return
}
if (gestureHandlers.contains(handler)) {
return
}
check(gestureHandlersCount < gestureHandlers.size) { "Too many recognizers" }
gestureHandlers[gestureHandlersCount++] = handler

gestureHandlers.add(handler)
handler.isActive = false
handler.isAwaiting = false
handler.activationIndex = Int.MAX_VALUE
Expand Down Expand Up @@ -608,7 +580,6 @@ class GestureHandlerOrchestrator(
companion object {
// The limit doesn't necessarily need to exists, it was just simpler to implement it that way
// it is also more allocation-wise efficient to have a fixed limit
private const val SIMULTANEOUS_GESTURE_HANDLER_LIMIT = 20

// Be default fully transparent views can receive touch
private const val DEFAULT_MIN_ALPHA_FOR_TRAVERSAL = 0f
Expand Down
Loading