diff --git a/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/components/HdrWindowModeTest.kt b/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/components/HdrWindowModeTest.kt index 2f4df413b4..4c94193825 100644 --- a/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/components/HdrWindowModeTest.kt +++ b/amethyst/src/androidTest/java/com/vitorpamplona/amethyst/ui/components/HdrWindowModeTest.kt @@ -65,8 +65,8 @@ class HdrWindowModeTest { rule.setContent { window = getActivityWindow() - if (showFirst) RequestHdrFor(image, HdrRequests.FEED_HEADROOM) - if (showSecond) RequestHdrFor(image, HdrRequests.FEED_HEADROOM) + if (showFirst) RequestHdrFor(image, fullscreen = false) + if (showSecond) RequestHdrFor(image, fullscreen = false) } rule.waitForIdle() assertEquals(ActivityInfo.COLOR_MODE_HDR, window!!.colorMode) @@ -85,7 +85,7 @@ class HdrWindowModeTest { var window: Window? = null rule.setContent { window = getActivityWindow() - RequestHdrFor(sdrImage(), HdrRequests.FEED_HEADROOM) + RequestHdrFor(sdrImage(), fullscreen = false) } rule.waitForIdle() assertEquals(ActivityInfo.COLOR_MODE_DEFAULT, window!!.colorMode) @@ -101,7 +101,7 @@ class HdrWindowModeTest { activityWindow = getActivityWindow() Dialog(onDismissRequest = {}) { dialogWindow = getDialogWindow() - RequestHdrFor(image, HdrRequests.UNCAPPED) + RequestHdrFor(image, fullscreen = true) } } rule.waitForIdle() @@ -118,7 +118,7 @@ class HdrWindowModeTest { rule.setContent { window = getActivityWindow() - repeat(count) { RequestHdrFor(image, HdrRequests.FEED_HEADROOM) } + repeat(count) { RequestHdrFor(image, fullscreen = false) } } rule.waitForIdle() rule.runOnUiThread { diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/HdrWindowMode.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/HdrWindowMode.kt index 3457150f38..bc44178bbf 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/HdrWindowMode.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/HdrWindowMode.kt @@ -38,25 +38,25 @@ import java.util.WeakHashMap * An Ultra HDR photo (a JPEG with a gain map) only renders brighter than SDR white while its * window is in [ActivityInfo.COLOR_MODE_HDR]; otherwise the platform silently draws the SDR base * image. The window is shared by every card in a feed, so HDR stays on until the last image - * asking for it leaves composition. + * asking for it leaves composition, and then the window gets its [originalColorMode] back. */ -class HdrRequests { - private val tokens = mutableListOf() +class HdrRequests( + val originalColorMode: Int, +) { + private val headrooms = mutableListOf() - private class Request( - val headroom: Float, - ) + val wantsHdr: Boolean get() = headrooms.isNotEmpty() - val wantsHdr: Boolean get() = tokens.isNotEmpty() + /** [UNCAPPED] if any request is uncapped or there are none, else the largest cap asked for. */ + val headroom: Float get() = if (UNCAPPED in headrooms) UNCAPPED else headrooms.maxOrNull() ?: UNCAPPED - /** [UNCAPPED] if any request is uncapped, else the largest cap asked for. */ - val headroom: Float - get() = if (tokens.any { it.headroom == UNCAPPED }) UNCAPPED else tokens.maxOfOrNull { it.headroom } ?: UNCAPPED + fun add(headroom: Float) { + headrooms.add(headroom) + } - fun add(headroom: Float): Any = Request(headroom).also { tokens.add(it) } - - fun remove(token: Any) { - tokens.removeAll { it === token } + /** Drops one request for [headroom]; equal requests are interchangeable. */ + fun remove(headroom: Float) { + headrooms.remove(headroom) } companion object { @@ -71,23 +71,16 @@ class HdrRequests { } } -private class HdrWindowState( - val originalColorMode: Int, -) { - val requests = HdrRequests() -} - // Main-thread only: composition and disposal both run there. -private val windowStates = WeakHashMap() +private val windowRequests = WeakHashMap() // Each setter dispatches the window attributes to the window manager even when the value is // unchanged, and cards scroll in and out of a feed constantly: only write what actually changed. -private fun Window.applyHdr(state: HdrWindowState) { - val mode = if (state.requests.wantsHdr) ActivityInfo.COLOR_MODE_HDR else state.originalColorMode +private fun Window.applyHdr(requests: HdrRequests) { + val mode = if (requests.wantsHdr) ActivityInfo.COLOR_MODE_HDR else requests.originalColorMode if (colorMode != mode) colorMode = mode if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.VANILLA_ICE_CREAM) { - val headroom = if (state.requests.wantsHdr) state.requests.headroom else HdrRequests.UNCAPPED - if (desiredHdrHeadroom != headroom) desiredHdrHeadroom = headroom + if (desiredHdrHeadroom != requests.headroom) desiredHdrHeadroom = requests.headroom } } @@ -96,24 +89,26 @@ fun Image.hasGainmap(): Boolean = Build.VERSION.SDK_INT >= Build.VERSION_CODES.U /** * Puts the hosting window (the dialog's own window inside a `Dialog`) into HDR mode while this * is in composition and [image] carries a gain map, so an Ultra HDR photo shows its highlights. + * A [fullscreen] image gets the display's full HDR range; a feed card is capped. */ @Composable fun RequestHdrFor( image: Image, - headroom: Float, + fullscreen: Boolean, ) { if (!remember(image) { image.hasGainmap() }) return val window = getDialogWindow() ?: getActivityWindow() ?: return + val headroom = if (fullscreen) HdrRequests.UNCAPPED else HdrRequests.FEED_HEADROOM DisposableEffect(window, headroom) { - val state = windowStates.getOrPut(window) { HdrWindowState(window.colorMode) } - val token = state.requests.add(headroom) - window.applyHdr(state) + val requests = windowRequests.getOrPut(window) { HdrRequests(window.colorMode) } + requests.add(headroom) + window.applyHdr(requests) onDispose { - state.requests.remove(token) - window.applyHdr(state) - if (!state.requests.wantsHdr) windowStates.remove(window) + requests.remove(headroom) + window.applyHdr(requests) + if (!requests.wantsHdr) windowRequests.remove(window) } } } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentDialog.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentDialog.kt index 7a010cfe21..8e982bba44 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentDialog.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentDialog.kt @@ -184,8 +184,8 @@ fun ZoomableImageDialog( val activityWindow = getActivityWindow() val dialogWindow = getDialogWindow() - // Keyed, not inline: this content recomposes on every frame of a pager swipe (the image - // bounds it reads move), and each assignment below is a window-manager round trip. + // Keyed, not inline: each assignment below is a window-manager round trip, so it must run + // on orientation changes only, not on every recomposition of this content. DisposableEffect(orientation, activityWindow, dialogWindow) { if (activityWindow != null && dialogWindow != null) { // Preserve what the dialog window owns: the brightness override applied by the @@ -226,9 +226,9 @@ fun ZoomableImageDialog( allImages = allImages, imageUrl = imageUrl, sourceBounds = sourceBounds, - imageBounds = imageBounds, + imageBounds = { imageBounds }, onImageBoundsChanged = updateImageBounds, - currentZoomState = currentZoomState, + currentZoomState = { currentZoomState }, onZoomStateChanged = { currentZoomState = it }, progress = progressProvider, onDismiss = dismissWithAnimation, @@ -243,9 +243,9 @@ private fun DialogContent( allImages: ImmutableList, imageUrl: BaseMediaContent, sourceBounds: Rect?, - imageBounds: Rect?, + imageBounds: () -> Rect?, onImageBoundsChanged: (Rect) -> Unit, - currentZoomState: ZoomState?, + currentZoomState: () -> ZoomState?, onZoomStateChanged: (ZoomState) -> Unit, progress: () -> Float, onDismiss: () -> Unit, @@ -282,12 +282,12 @@ private fun DialogContent( .fillMaxSize() .graphicsLayer { val src = sourceBounds - val img = imageBounds + val img = imageBounds() if (src != null && img != null && src.hasArea() && img.hasArea()) { // Account for user-applied zoom: the exit animation must start from // the visible bounds, not the unzoomed layout bounds — otherwise // dismissing a zoomed-in image jumps. - val zoomed = img.zoomedBy(currentZoomState) + val zoomed = img.zoomedBy(currentZoomState()) // Uniform scale so non-square images keep their aspect ratio during // the grow animation. The image covers the source rect; the overflow // is clipped below. diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentView.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentView.kt index 4bcce185aa..40c99907e2 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentView.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/ZoomableContentView.kt @@ -458,7 +458,7 @@ fun LocalImageView( SubcomposeAsyncImageContent(loadedImageModifier) val image = (state as AsyncImagePainter.State.Success).result.image - RequestHdrFor(image, if (fullResolution) HdrRequests.UNCAPPED else HdrRequests.FEED_HEADROOM) + RequestHdrFor(image, fullscreen = fullResolution) SideEffect { MediaAspectRatioCache.add(content.localJavaFile.toString(), image.width, image.height) @@ -602,7 +602,7 @@ fun UrlImageView( ShowHashAnimated(content, controllerVisible, Modifier.align(Alignment.TopEnd)) val image = (state as AsyncImagePainter.State.Success).result.image - RequestHdrFor(image, if (fullResolution) HdrRequests.UNCAPPED else HdrRequests.FEED_HEADROOM) + RequestHdrFor(image, fullscreen = fullResolution) SideEffect { MediaAspectRatioCache.add(content.url, image.width, image.height) diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/components/HdrRequestsTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/components/HdrRequestsTest.kt index c014b9787d..bc594b173d 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/components/HdrRequestsTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/components/HdrRequestsTest.kt @@ -26,28 +26,31 @@ import org.junit.Assert.assertTrue import org.junit.Test class HdrRequestsTest { + private fun requests() = HdrRequests(originalColorMode = 0) + @Test fun noRequestsMeansSdr() { - val requests = HdrRequests() + val requests = requests() assertFalse(requests.wantsHdr) + assertEquals(HdrRequests.UNCAPPED, requests.headroom) } @Test fun hdrStaysOnUntilTheLastRequestLeaves() { - val requests = HdrRequests() - val first = requests.add(2f) - val second = requests.add(2f) + val requests = requests() + requests.add(2f) + requests.add(2f) - requests.remove(first) + requests.remove(2f) assertTrue(requests.wantsHdr) - requests.remove(second) + requests.remove(2f) assertFalse(requests.wantsHdr) } @Test fun theLargestCappedHeadroomWins() { - val requests = HdrRequests() + val requests = requests() requests.add(2f) requests.add(3f) assertEquals(3f, requests.headroom) @@ -55,23 +58,12 @@ class HdrRequestsTest { @Test fun anUncappedRequestLiftsTheCap() { - val requests = HdrRequests() + val requests = requests() requests.add(2f) - val fullscreen = requests.add(HdrRequests.UNCAPPED) + requests.add(HdrRequests.UNCAPPED) assertEquals(HdrRequests.UNCAPPED, requests.headroom) - requests.remove(fullscreen) + requests.remove(HdrRequests.UNCAPPED) assertEquals(2f, requests.headroom) } - - @Test - fun equalHeadroomsAreStillSeparateRequests() { - val requests = HdrRequests() - val first = requests.add(2f) - requests.add(2f) - - requests.remove(first) - requests.remove(first) - assertTrue("removing one token twice must not release the other", requests.wantsHdr) - } }