[CameraPipe] Sync fix for correct display selection based on state
DisplayInfoManager.getMaxDisplay incorrectly choses the display when multiple displays are present. It can end up choosing a display in off state in current implementation.
To fix it, logic from aosp/1974040 and aosp/2267063 are sync'ed.
The method will return the max size display that is not in off state. It will return max size off state display as a fallback if all displays are in off state.
Bug: b/255754025
Test: ./gradlew bOS
Change-Id: I13013637def795d7f5c5356b5b4a8ea8ba0b871b
Signed-off-by: Tahsin Masrur <tahsinm@google.com>
diff --git a/camera/camera-camera2-pipe-integration/src/androidTest/java/androidx/camera/camera2/pipe/integration/impl/DisplayInfoManagerTest.kt b/camera/camera-camera2-pipe-integration/src/androidTest/java/androidx/camera/camera2/pipe/integration/impl/DisplayInfoManagerTest.kt
new file mode 100644
index 0000000..7bbc024
--- /dev/null
+++ b/camera/camera-camera2-pipe-integration/src/androidTest/java/androidx/camera/camera2/pipe/integration/impl/DisplayInfoManagerTest.kt
@@ -0,0 +1,65 @@
+/*
+ * Copyright 2022 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package androidx.camera.camera2.pipe.integration.impl
+
+import android.content.Context
+import android.graphics.Point
+import android.hardware.display.DisplayManager
+import androidx.test.core.app.ApplicationProvider
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import androidx.test.filters.SdkSuppress
+import androidx.test.filters.SmallTest
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
+import org.junit.Assume
+import org.junit.Test
+import org.junit.runner.RunWith
+
+@SmallTest
+@Suppress("DEPRECATION") // getRealSize
+@RunWith(AndroidJUnit4::class)
+@SdkSuppress(minSdkVersion = 21)
+class DisplayInfoManagerTest {
+ private val displayInfoManager = DisplayInfoManager(ApplicationProvider.getApplicationContext())
+
+ @Test
+ fun defaultDisplayIsDeviceDisplay_whenOneDisplay() {
+ // Arrange
+ val displayManager = (ApplicationProvider.getApplicationContext() as Context)
+ .getSystemService(Context.DISPLAY_SERVICE) as DisplayManager
+
+ Assume.assumeTrue(displayManager.displays.size == 1)
+
+ val currentDisplaySize = Point()
+ displayManager.displays[0].getRealSize(currentDisplaySize)
+
+ // Act
+ val size = Point()
+ displayInfoManager.defaultDisplay.getRealSize(size)
+
+ // Assert
+ assertEquals(currentDisplaySize, size)
+ }
+
+ @Test
+ fun previewSizeAreaIsWithinMaxPreviewArea() {
+ // Act & Assert
+ val previewSize = displayInfoManager.previewSize
+ assertTrue("$previewSize has larger area than 1920 * 1080",
+ previewSize.width * previewSize.height <= 1920 * 1080)
+ }
+}
diff --git a/camera/camera-camera2-pipe-integration/src/main/java/androidx/camera/camera2/pipe/integration/impl/DisplayInfoManager.kt b/camera/camera-camera2-pipe-integration/src/main/java/androidx/camera/camera2/pipe/integration/impl/DisplayInfoManager.kt
index 1babffd..c7745f6 100644
--- a/camera/camera-camera2-pipe-integration/src/main/java/androidx/camera/camera2/pipe/integration/impl/DisplayInfoManager.kt
+++ b/camera/camera-camera2-pipe-integration/src/main/java/androidx/camera/camera2/pipe/integration/impl/DisplayInfoManager.kt
@@ -19,6 +19,9 @@
import android.content.Context
import android.graphics.Point
import android.hardware.display.DisplayManager
+import android.hardware.display.DisplayManager.DisplayListener
+import android.os.Handler
+import android.os.Looper
import android.util.Size
import android.view.Display
import androidx.annotation.RequiresApi
@@ -31,45 +34,86 @@
class DisplayInfoManager @Inject constructor(context: Context) {
private val MAX_PREVIEW_SIZE = Size(1920, 1080)
+ companion object {
+ private var lazyMaxDisplay: Display? = null
+ private var lazyPreviewSize: Size? = null
+
+ internal fun invalidateLazyFields() {
+ lazyMaxDisplay = null
+ lazyPreviewSize = null
+ }
+
+ internal val displayListener by lazy {
+ object : DisplayListener {
+ override fun onDisplayAdded(displayId: Int) {
+ invalidateLazyFields()
+ }
+
+ override fun onDisplayRemoved(displayId: Int) {
+ invalidateLazyFields()
+ }
+
+ override fun onDisplayChanged(displayId: Int) {
+ invalidateLazyFields()
+ }
+ }
+ }
+ }
+
private val displayManager: DisplayManager by lazy {
- context.getSystemService(Context.DISPLAY_SERVICE) as DisplayManager
+ (context.getSystemService(Context.DISPLAY_SERVICE) as DisplayManager).also {
+ it.registerDisplayListener(displayListener, Handler(Looper.getMainLooper()))
+ }
}
- // TODO(b/198257203): Fetch latest display information for devices where display size is not
- // guaranteed to be fixed. (e.g. foldable devices or devices with multiple displays)
+ val defaultDisplay: Display
+ get() = getMaxSizeDisplay()
- val defaultDisplay: Display by lazy {
- getMaxSizeDisplay()
- }
-
- val previewSize: Size by lazy {
- calculatePreviewSize()
- }
+ val previewSize: Size
+ get() = calculatePreviewSize()
private fun getMaxSizeDisplay(): Display {
+ lazyMaxDisplay?.let { return it }
+
val displays = displayManager.displays
+
+ var maxDisplayWhenStateNotOff: Display? = null
+ var maxDisplaySizeWhenStateNotOff = -1
+
var maxDisplay: Display? = null
var maxDisplaySize = -1
- // TODO(b/211945950, b/255170076): Handle STATE_OFF displays.
-
for (display: Display in displays) {
val displaySize = Point()
// TODO(b/230400472): Use WindowManager#getCurrentWindowMetrics(). Display#getRealSize()
// is deprecated since API level 31.
display.getRealSize(displaySize)
+
if (displaySize.x * displaySize.y > maxDisplaySize) {
maxDisplaySize = displaySize.x * displaySize.y
maxDisplay = display
}
+ if (display.state != Display.STATE_OFF) {
+ if (displaySize.x * displaySize.y > maxDisplaySizeWhenStateNotOff) {
+ maxDisplaySizeWhenStateNotOff = displaySize.x * displaySize.y
+ maxDisplayWhenStateNotOff = display
+ }
+ }
}
- return checkNotNull(maxDisplay) { "No displays found from ${displayManager.displays}!" }
+
+ lazyMaxDisplay = maxDisplayWhenStateNotOff ?: maxDisplay
+
+ return checkNotNull(lazyMaxDisplay) {
+ "No displays found from ${displayManager.displays}!"
+ }
}
/**
* Calculates the device's screen resolution, or MAX_PREVIEW_SIZE, whichever is smaller.
*/
private fun calculatePreviewSize(): Size {
+ lazyPreviewSize?.let { return it }
+
val displaySize = Point()
val display: Display = defaultDisplay
// TODO(b/230400472): Use WindowManager#getCurrentWindowMetrics(). Display#getRealSize()
@@ -87,6 +131,7 @@
displayViewSize = MAX_PREVIEW_SIZE
}
// TODO(b/230402463): Migrate extra cropping quirk from CameraX.
- return displayViewSize
+
+ return displayViewSize.also { lazyPreviewSize = displayViewSize }
}
}
diff --git a/camera/camera-camera2-pipe-integration/src/test/java/androidx/camera/camera2/pipe/integration/impl/DisplayInfoManagerTest.kt b/camera/camera-camera2-pipe-integration/src/test/java/androidx/camera/camera2/pipe/integration/impl/DisplayInfoManagerTest.kt
index f550d39..1d12d9b 100644
--- a/camera/camera-camera2-pipe-integration/src/test/java/androidx/camera/camera2/pipe/integration/impl/DisplayInfoManagerTest.kt
+++ b/camera/camera-camera2-pipe-integration/src/test/java/androidx/camera/camera2/pipe/integration/impl/DisplayInfoManagerTest.kt
@@ -20,22 +20,57 @@
import android.graphics.Point
import android.hardware.display.DisplayManager
import android.util.Size
+import android.view.Display
+import androidx.camera.camera2.pipe.integration.adapter.RobolectricCameraPipeTestRunner
import androidx.test.core.app.ApplicationProvider
+import org.junit.After
import org.junit.Assert.assertEquals
+import org.junit.BeforeClass
import org.junit.Test
import org.junit.runner.RunWith
-import org.robolectric.RobolectricTestRunner
import org.robolectric.annotation.internal.DoNotInstrument
+import org.robolectric.shadow.api.Shadow
+import org.robolectric.shadows.ShadowDisplay
import org.robolectric.shadows.ShadowDisplayManager
+import org.robolectric.shadows.ShadowDisplayManager.removeDisplay
@Suppress("DEPRECATION") // getRealSize
-@RunWith(RobolectricTestRunner::class)
+@RunWith(RobolectricCameraPipeTestRunner::class)
@DoNotInstrument
class DisplayInfoManagerTest {
private val displayInfoManager = DisplayInfoManager(ApplicationProvider.getApplicationContext())
- private fun addDisplay(width: Int, height: Int) {
- ShadowDisplayManager.addDisplay(String.format("w%ddp-h%ddp", width, height))
+ private fun addDisplay(width: Int, height: Int, state: Int = Display.STATE_ON): Int {
+ val displayStr = String.format("w%ddp-h%ddp", width, height)
+ val displayId = ShadowDisplayManager.addDisplay(displayStr)
+
+ if (state != Display.STATE_ON) {
+ val displayManager = (ApplicationProvider.getApplicationContext() as Context)
+ .getSystemService(Context.DISPLAY_SERVICE) as DisplayManager
+ (Shadow.extract(displayManager.getDisplay(displayId)) as ShadowDisplay).setState(state)
+ }
+
+ return displayId
+ }
+
+ companion object {
+ @JvmStatic
+ @BeforeClass
+ fun classSetUp() {
+ DisplayInfoManager.invalidateLazyFields()
+ }
+ }
+
+ @After
+ fun tearDown() {
+ val displayManager = (ApplicationProvider.getApplicationContext() as Context)
+ .getSystemService(Context.DISPLAY_SERVICE) as DisplayManager?
+
+ displayManager?.let {
+ for (display in it.displays) {
+ removeDisplay(display.displayId)
+ }
+ }
}
@Test
@@ -68,10 +103,89 @@
assertEquals(Point(2000, 3000), size)
}
+ @Test
+ fun defaultDisplayIsMaxSizeDisplay_whenPreviousMaxDisplayRemoved() {
+ // Arrange
+ val id = addDisplay(2000, 3000)
+ addDisplay(480, 640)
+ removeDisplay(id)
+
+ // Act
+ val size = Point()
+ displayInfoManager.defaultDisplay.getRealSize(size)
+
+ // Assert
+ assertEquals(Point(480, 640), size)
+ }
+
+ @Test
+ fun defaultDisplayIsMaxSizeDisplay_whenNewMaxDisplayAddedAfterGettingPrevious() {
+ // Arrange
+ addDisplay(480, 640)
+
+ // Act
+ displayInfoManager.defaultDisplay
+ addDisplay(2000, 3000)
+
+ val size = Point()
+ displayInfoManager.defaultDisplay.getRealSize(size)
+
+ // Assert
+ assertEquals(Point(2000, 3000), size)
+ }
+
+ @Test
+ fun defaultDisplayIsMaxSizeInNotOffState_whenMultipleDisplayWithSomeOffState() {
+ // Arrange
+ addDisplay(2000, 3000, Display.STATE_OFF)
+ addDisplay(480, 640)
+ addDisplay(240, 320)
+ addDisplay(200, 300, Display.STATE_OFF)
+
+ // Act
+ val size = Point()
+ displayInfoManager.defaultDisplay.getRealSize(size)
+
+ // Assert
+ assertEquals(Point(480, 640), size)
+ }
+
+ @Test
+ fun defaultDisplayIsMaxSizeInNotOffState_whenMultipleDisplayWithNoOnState() {
+ // Arrange
+ addDisplay(2000, 3000, Display.STATE_OFF)
+ addDisplay(480, 640, Display.STATE_UNKNOWN)
+ addDisplay(240, 320, Display.STATE_UNKNOWN)
+ addDisplay(200, 300, Display.STATE_OFF)
+
+ // Act
+ val size = Point()
+ displayInfoManager.defaultDisplay.getRealSize(size)
+
+ // Assert
+ assertEquals(Point(480, 640), size)
+ }
+
+ @Test
+ fun defaultDisplayIsMaxSizeInOffState_whenMultipleDisplayWithAllOffState() {
+ // Arrange
+ addDisplay(2000, 3000, Display.STATE_OFF)
+ addDisplay(480, 640, Display.STATE_OFF)
+ addDisplay(200, 300, Display.STATE_OFF)
+ removeDisplay(0)
+
+ // Act
+ val size = Point()
+ displayInfoManager.defaultDisplay.getRealSize(size)
+
+ // Assert
+ assertEquals(Point(2000, 3000), size)
+ }
+
@Test(expected = IllegalStateException::class)
fun throwsCorrectExceptionForDefaultDisplay_whenNoDisplay() {
// Arrange
- ShadowDisplayManager.removeDisplay(0)
+ removeDisplay(0)
// Act
val size = Point()
@@ -95,4 +209,17 @@
// Act & Assert
assertEquals(Size(1920, 1080), displayInfoManager.previewSize)
}
+
+ @Test
+ fun previewSizeIsUpdated_whenNewDisplayAddedAfterPreviousUse() {
+ // Arrange
+ addDisplay(480, 640)
+
+ // Act
+ displayInfoManager.previewSize
+ addDisplay(2000, 3000)
+
+ // Assert
+ assertEquals(Size(1920, 1080), displayInfoManager.previewSize)
+ }
}
diff --git a/camera/camera-camera2-pipe-integration/src/test/java/androidx/camera/camera2/pipe/integration/impl/MeteringRepeatingTest.kt b/camera/camera-camera2-pipe-integration/src/test/java/androidx/camera/camera2/pipe/integration/impl/MeteringRepeatingTest.kt
index f40a4cf..cc035a9 100644
--- a/camera/camera-camera2-pipe-integration/src/test/java/androidx/camera/camera2/pipe/integration/impl/MeteringRepeatingTest.kt
+++ b/camera/camera-camera2-pipe-integration/src/test/java/androidx/camera/camera2/pipe/integration/impl/MeteringRepeatingTest.kt
@@ -16,19 +16,24 @@
package androidx.camera.camera2.pipe.integration.impl
+import android.content.Context
import android.hardware.camera2.CameraCharacteristics
+import android.hardware.display.DisplayManager
import android.os.Build
import android.util.Size
import androidx.camera.camera2.pipe.integration.adapter.RobolectricCameraPipeTestRunner
import androidx.camera.camera2.pipe.integration.testing.FakeCameraProperties
import androidx.camera.camera2.pipe.testing.FakeCameraMetadata
import androidx.test.core.app.ApplicationProvider
+import org.junit.After
import org.junit.Assert.assertEquals
+import org.junit.BeforeClass
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.annotation.Config
import org.robolectric.annotation.internal.DoNotInstrument
import org.robolectric.shadows.ShadowDisplayManager
+import org.robolectric.shadows.ShadowDisplayManager.removeDisplay
import org.robolectric.shadows.StreamConfigurationMapBuilder
@RunWith(RobolectricCameraPipeTestRunner::class)
@@ -80,6 +85,12 @@
)
)
}
+
+ @JvmStatic
+ @BeforeClass
+ fun classSetUp() {
+ DisplayInfoManager.invalidateLazyFields()
+ }
}
private lateinit var meteringRepeating: MeteringRepeating
@@ -103,6 +114,18 @@
).build()
}
+ @After
+ fun tearDown() {
+ val displayManager = (ApplicationProvider.getApplicationContext() as Context)
+ .getSystemService(Context.DISPLAY_SERVICE) as DisplayManager?
+
+ displayManager?.let {
+ for (display in it.displays) {
+ removeDisplay(display.displayId)
+ }
+ }
+ }
+
@Test
fun attachedSurfaceResolutionIsLargestLessThan640x480_when640x480NotPresentInOutputSizes() {
meteringRepeating = getMeteringRepeatingAndInitDisplay(dummySizeListWithout640x480)
