From 7b0a7f15b08bf59437627e33946f2c301948317a Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sat, 20 Jul 2024 21:06:07 +0200 Subject: [PATCH 1/3] Android: Use structural equality for sensorDetails To fix the crash in input device sensor handling, we should look up Sensors using structural equality. Unfortunately, Sensor.equals implements referential equality, and HashMap doesn't let us provide a custom comparator. Because the number of sensors is relatively small, and because we have a reason to keep a sorted list of sensors around anyway, let's switch from HashMap to ArrayList. --- .../input/model/DolphinSensorEventListener.kt | 61 ++++++++++--------- 1 file changed, 33 insertions(+), 28 deletions(-) diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/input/model/DolphinSensorEventListener.kt b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/input/model/DolphinSensorEventListener.kt index e89143aa31..ee9885ce7a 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/input/model/DolphinSensorEventListener.kt +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/input/model/DolphinSensorEventListener.kt @@ -19,6 +19,7 @@ class DolphinSensorEventListener : SensorEventListener { private class AxisSetDetails(val firstAxisOfSet: Int, val axisSetType: Int) private class SensorDetails( + val sensor: Sensor, val sensorType: Int, val axisNames: Array, val axisSetDetails: Array @@ -28,7 +29,7 @@ class DolphinSensorEventListener : SensorEventListener { private val sensorManager: SensorManager? - private val sensorDetails = HashMap() + private val sensorDetails = ArrayList() private val rotateCoordinatesForScreenOrientation: Boolean @@ -40,6 +41,7 @@ class DolphinSensorEventListener : SensorEventListener { .getSystemService(Context.SENSOR_SERVICE) as SensorManager? rotateCoordinatesForScreenOrientation = true addSensors() + sortSensorDetails() } @Keep @@ -58,6 +60,7 @@ class DolphinSensorEventListener : SensorEventListener { } else { null } + sortSensorDetails() } private fun addSensors() { @@ -254,15 +257,22 @@ class DolphinSensorEventListener : SensorEventListener { ) { val sensor = sensorManager!!.getDefaultSensor(sensorType) if (sensor != null) { - sensorDetails[sensor] = SensorDetails(sensorType, axisNames, axisSetDetails) + sensorDetails.add(SensorDetails(sensor, sensorType, axisNames, axisSetDetails)) } } + private fun sortSensorDetails() { + Collections.sort( + sensorDetails, + Comparator.comparingInt { s: SensorDetails -> s.sensorType } + ) + } + override fun onSensorChanged(sensorEvent: SensorEvent) { - val sensorDetails = sensorDetails[sensorEvent.sensor] + val sensorDetails = sensorDetails.first{s -> sensorsAreEqual(s.sensor, sensorEvent.sensor)} val values = sensorEvent.values - val axisNames = sensorDetails!!.axisNames + val axisNames = sensorDetails.axisNames val axisSetDetails = sensorDetails.axisSetDetails var eventAxisIndex = 0 @@ -356,7 +366,7 @@ class DolphinSensorEventListener : SensorEventListener { } } if (!keepSensorAlive) { - setSensorSuspended(sensorEvent.sensor, sensorDetails, true) + setSensorSuspended(sensorDetails, true) } } @@ -381,18 +391,14 @@ class DolphinSensorEventListener : SensorEventListener { */ @Keep fun requestUnsuspendSensor(axisName: String) { - for ((key, value) in sensorDetails) { - if (listOf(*value.axisNames).contains(axisName)) { - setSensorSuspended(key, value, false) + for (sd in sensorDetails) { + if (listOf(*sd.axisNames).contains(axisName)) { + setSensorSuspended(sd, false) } } } - private fun setSensorSuspended( - sensor: Sensor, - sensorDetails: SensorDetails, - suspend: Boolean - ) { + private fun setSensorSuspended(sensorDetails: SensorDetails, suspend: Boolean) { var changeOccurred = false synchronized(sensorDetails) { @@ -404,9 +410,9 @@ class DolphinSensorEventListener : SensorEventListener { ) if (suspend) - sensorManager!!.unregisterListener(this, sensor) + sensorManager!!.unregisterListener(this, sensorDetails.sensor) else - sensorManager!!.registerListener(this, sensor, SAMPLING_PERIOD_US) + sensorManager!!.registerListener(this, sensorDetails.sensor, SAMPLING_PERIOD_US) sensorDetails.isSuspended = suspend @@ -415,14 +421,14 @@ class DolphinSensorEventListener : SensorEventListener { } if (changeOccurred) { - Log.info((if (suspend) "Suspended sensor " else "Unsuspended sensor ") + sensor.name) + Log.info((if (suspend) "Suspended sensor " else "Unsuspended sensor ") + sensorDetails.sensor.name) } } @Keep fun getAxisNames(): Array { val axisNames = ArrayList() - for (sensorDetails in sensorDetailsSorted) { + for (sensorDetails in sensorDetails) { sensorDetails.axisNames.forEach { axisNames.add(it) } } return axisNames.toArray(arrayOf()) @@ -432,7 +438,7 @@ class DolphinSensorEventListener : SensorEventListener { fun getNegativeAxes(): BooleanArray { val negativeAxes = ArrayList() - for (sensorDetails in sensorDetailsSorted) { + for (sensorDetails in sensorDetails) { var eventAxisIndex = 0 var detailsAxisIndex = 0 var detailsAxisSetIndex = 0 @@ -467,22 +473,13 @@ class DolphinSensorEventListener : SensorEventListener { return result } - private val sensorDetailsSorted: List - get() { - val sensorDetails = ArrayList(sensorDetails.values) - Collections.sort( - sensorDetails, - Comparator.comparingInt { s: SensorDetails -> s.sensorType } - ) - return sensorDetails - } - companion object { // Set of three axes. Creates a negative companion to each axis, and corrects for device rotation. private const val AXIS_SET_TYPE_DEVICE_COORDINATES = 0 // Set of three axes. Creates a negative companion to each axis. private const val AXIS_SET_TYPE_OTHER_COORDINATES = 1 + private var deviceRotation = Surface.ROTATION_0 // The fastest sampling rate Android lets us use without declaring the HIGH_SAMPLING_RATE_SENSORS @@ -500,5 +497,13 @@ class DolphinSensorEventListener : SensorEventListener { fun setDeviceRotation(deviceRotation: Int) { this.deviceRotation = deviceRotation } + + private fun sensorsAreEqual(s1: Sensor, s2: Sensor): Boolean { + return if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N && s1.id > 0 && s2.id > 0) { + s1.type == s2.type && s1.id == s2.id + } else { + s1.type == s2.type && s1.name == s2.name + } + } } } From 59cbe5a84324e079535babce673c9116d2cfe56f Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sat, 20 Jul 2024 21:41:38 +0200 Subject: [PATCH 2/3] Android: Add workaround for AOSP input device sensor suspend bug --- .../input/model/DolphinSensorEventListener.kt | 57 +++++++++++++++++-- 1 file changed, 53 insertions(+), 4 deletions(-) diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/input/model/DolphinSensorEventListener.kt b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/input/model/DolphinSensorEventListener.kt index ee9885ce7a..6076b67384 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/input/model/DolphinSensorEventListener.kt +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/input/model/DolphinSensorEventListener.kt @@ -25,6 +25,7 @@ class DolphinSensorEventListener : SensorEventListener { val axisSetDetails: Array ) { var isSuspended = true + var hasRegisteredListener = false } private val sensorManager: SensorManager? @@ -33,6 +34,16 @@ class DolphinSensorEventListener : SensorEventListener { private val rotateCoordinatesForScreenOrientation: Boolean + /** + * AOSP has a bug in InputDeviceSensorManager where + * InputSensorEventListenerDelegate.removeSensor attempts to modify an ArrayList it's iterating + * through in a way that throws a ConcurrentModificationException. Because of this, we can't + * suspend individual sensors for InputDevices, but we can suspend all sensors at once. + */ + private val canSuspendSensorsIndividually: Boolean + + private var unsuspendedSensors = 0 + private var deviceQualifier = "" @Keep @@ -40,6 +51,7 @@ class DolphinSensorEventListener : SensorEventListener { sensorManager = DolphinApplication.getAppContext() .getSystemService(Context.SENSOR_SERVICE) as SensorManager? rotateCoordinatesForScreenOrientation = true + canSuspendSensorsIndividually = true addSensors() sortSensorDetails() } @@ -47,6 +59,7 @@ class DolphinSensorEventListener : SensorEventListener { @Keep constructor(inputDevice: InputDevice) { rotateCoordinatesForScreenOrientation = false + canSuspendSensorsIndividually = false sensorManager = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { inputDevice.sensorManager @@ -409,10 +422,46 @@ class DolphinSensorEventListener : SensorEventListener { suspend ) - if (suspend) - sensorManager!!.unregisterListener(this, sensorDetails.sensor) - else - sensorManager!!.registerListener(this, sensorDetails.sensor, SAMPLING_PERIOD_US) + if (suspend) { + unsuspendedSensors -= 1 + } else { + unsuspendedSensors += 1 + } + + if (canSuspendSensorsIndividually) { + if (suspend) { + sensorManager!!.unregisterListener(this, sensorDetails.sensor) + } else { + sensorManager!!.registerListener( + this, + sensorDetails.sensor, + SAMPLING_PERIOD_US + ) + } + sensorDetails.hasRegisteredListener = !suspend + } else { + if (suspend) { + // If there are no unsuspended sensors left, unregister them all. + // Otherwise, leave unregistering for later. A possible alternative could be + // to unregister everything and then re-register the sensors we still want, + // but I fear this could lead to dropped inputs. + if (unsuspendedSensors == 0) { + sensorManager!!.unregisterListener(this) + for (sd in this.sensorDetails) { + sd.hasRegisteredListener = false + } + } + } else { + if (!sensorDetails.hasRegisteredListener) { + sensorManager!!.registerListener( + this, + sensorDetails.sensor, + SAMPLING_PERIOD_US + ) + sensorDetails.hasRegisteredListener = true + } + } + } sensorDetails.isSuspended = suspend From 91930459895f179f360414ae9a0d7a9253a4fedd Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sat, 15 Jun 2024 14:09:22 +0200 Subject: [PATCH 3/3] Revert "Android: Disable input device sensor input due to crash" This reverts commit 75fb1a7edfb12f490b08ab0b76a8e2f1536d2d9b. --- .../input/model/DolphinSensorEventListener.kt | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/input/model/DolphinSensorEventListener.kt b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/input/model/DolphinSensorEventListener.kt index 6076b67384..6aa5ce2fc0 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/input/model/DolphinSensorEventListener.kt +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/input/model/DolphinSensorEventListener.kt @@ -60,18 +60,11 @@ class DolphinSensorEventListener : SensorEventListener { constructor(inputDevice: InputDevice) { rotateCoordinatesForScreenOrientation = false canSuspendSensorsIndividually = false - sensorManager = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { - inputDevice.sensorManager - - // TODO: There is a bug where after suspending sensors, onSensorChanged can get called for - // a sensor that we never registered as a listener for. The way our code is currently written, - // this causes a NullPointerException, but if we checked for null we would instead have the - // problem of being spammed with onSensorChanged calls even though the sensor shouldn't be - // enabled. For now, let's comment out the ability to use InputDevice sensors. - - //addSensors(); + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { + sensorManager = inputDevice.sensorManager + addSensors() } else { - null + sensorManager = null } sortSensorDetails() }