Merge pull request #4114 from vitorpamplona/claude/image-blurhash-load-delay-y0o5f3

fix(media): stop a no-imeta GIF rendering as an invisible note while it loads

amethyst/src/main/java/com/vitorpamplona/amethyst/AppModules.kt

@@ -184,6 +184,7 @@ import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableSharedFlow
 import kotlinx.coroutines.flow.asSharedFlow
 import kotlinx.coroutines.flow.collectLatest
+import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.conflate
 import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.drop
@@ -647,6 +648,41 @@ class AppModules(
             onionCache = onionLocationCache,
         )
 
+    // Drops pooled connections once per real Tor route change. When the user switches
+    // Tor on, the direct clients' idle sockets to real hosts would otherwise sit in the
+    // pool for its 5-minute keepalive after the user has asked for everything to go
+    // through Tor. No request could use them either way -- OkHttp keys the pool by
+    // `Address`, which includes the proxy, so a connection on a dead route is already
+    // unreachable -- which is why this is hygiene and not correctness, and why it is
+    // fine for it to be a little late.
+    //
+    // Every source here is a plain StateFlow, so subscribing costs nothing. Deliberately
+    // NOT torManager.activePortOrNull: that chains to TorManager.status, whose upstream
+    // is WhileSubscribed and calls service.start() when collected, so a process-lifetime
+    // subscription there would hold Arti's control flow open forever -- the same hazard
+    // the battery ledger above documents and sidesteps the same way.
+    //
+    // Also deliberately not the per-feature Tor switches (imagesViaTor, videosViaTor, ...):
+    // those change which of the two existing clients a request picks, not the route either
+    // one uses, so no pooled connection goes stale.
+    init {
+        applicationIOScope.launch {
+            combine(
+                torPrefs.torType,
+                torPrefs.externalSocksPort,
+                torService.status.map { it.socksPort },
+            ) { torType, externalPort, artiPort -> Triple(torType, externalPort, artiPort) }
+                .distinctUntilChanged()
+                // Only later moves count; the route in force at process construction is the
+                // status quo, and nothing is pooled yet to evict.
+                .drop(1)
+                .collect {
+                    okHttpClients.factory.evictPooledConnections()
+                    okHttpClientForRelays.factory.evictPooledConnections()
+                }
+        }
+    }
+
     // Connects the INostrClient class with okHttp
     val websocketBuilder =
         OkHttpWebSocket.Builder(

amethyst/src/main/java/com/vitorpamplona/amethyst/ui/components/GifVideoView.kt

@@ -47,10 +47,13 @@ import coil3.compose.SubcomposeAsyncImage
 import coil3.compose.SubcomposeAsyncImageContent
 import com.vitorpamplona.amethyst.commons.resources.Res
 import com.vitorpamplona.amethyst.commons.resources.gif
+import com.vitorpamplona.amethyst.commons.ui.components.LoadingAnimation
 import com.vitorpamplona.amethyst.model.MediaAspectRatioCache
 import com.vitorpamplona.amethyst.ui.screen.loggedIn.AccountViewModel
 import com.vitorpamplona.amethyst.ui.stringRes
 import com.vitorpamplona.amethyst.ui.theme.Font10SP
+import com.vitorpamplona.amethyst.ui.theme.Size40dp
+import com.vitorpamplona.amethyst.ui.theme.Size6dp
 import com.vitorpamplona.amethyst.ui.theme.SmallBorder
 import com.vitorpamplona.amethyst.ui.theme.imageModifier
 import com.vitorpamplona.quartz.nip94FileMetadata.tags.DimensionTag
@@ -72,6 +75,15 @@ fun GifVideoView(
     // remember() to avoid the recompute would cost more (slot read + N equality checks)
     // than the work it saves; that's why this stays as a plain expression.
     val ratio = dimensions?.aspectRatioOrNull() ?: MediaAspectRatioCache.get(videoUri)
+
+    // Mirrors [mediaSizingModifier]: Crop gets fillMaxSize() and a known ratio gets
+    // aspectRatio(), both of which bound the height. Everything else is a bare
+    // fillMaxWidth() that wraps its content, and only THAT case needs a loading state
+    // with intrinsic height to keep the note from collapsing to nothing. Keying the
+    // fallback on `ratio` alone would put raw URL text inside every Crop card cell --
+    // MyAsyncImage passes dimensions/blurhash/thumbhash all null, so a gif in a card
+    // slot hits this on first load, before MediaAspectRatioCache knows its size.
+    val heightIsBounded = contentScale == ContentScale.Crop || ratio != null
     val autoPlay = accountViewModel.settings.autoPlayVideos()
     val borderModifier = if (roundedCorner) MaterialTheme.colorScheme.imageModifier else Modifier
     val context = LocalContext.current
@@ -108,13 +120,29 @@ fun GifVideoView(
 
             when (state) {
                 is AsyncImagePainter.State.Loading -> {
-                    DisplayBlurHash(
-                        blurhash,
-                        contentDescription,
-                        contentScale,
-                        Modifier.fillMaxSize(),
-                        thumbhash = thumbhash,
-                    )
+                    // When the height is unbounded (see [heightIsBounded]) this branch MUST
+                    // emit something with an intrinsic height, or the box wraps nothing and the
+                    // whole note collapses to zero -- no picture, no URL, no spinner, just a gap
+                    // in the feed until the load finishes. DisplayBlurHash renders NOTHING when
+                    // both hashes are absent (placeholderModel returns null), which is exactly
+                    // what a no-imeta post hits. Mirrors UrlImageView's ladder.
+                    if (blurhash != null || thumbhash != null) {
+                        DisplayBlurHash(
+                            blurhash,
+                            contentDescription,
+                            contentScale,
+                            Modifier.fillMaxSize(),
+                            thumbhash = thumbhash,
+                        )
+                    } else if (heightIsBounded) {
+                        Box(Modifier.fillMaxSize(), contentAlignment = Alignment.Center) {
+                            LoadingAnimation(Size40dp, Size6dp)
+                        }
+                    } else {
+                        WaitAndDisplay {
+                            DisplayUrlWithLoadingSymbol(videoUri)
+                        }
+                    }
                 }
 
                 is AsyncImagePainter.State.Success -> {

commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/service/http/OkHttpClientFactory.kt

@@ -86,6 +86,16 @@ class OkHttpClientFactory(
     // Blossom/imgproxy server). OkHttp's default dispatcher caps inflight requests
     // per host at 5, which serializes feed loading. Raise the limits so the feed
     // can parallelize downloads the way a browser does.
+    //
+    // Resist trimming these on intuition. `maxRequests` is effectively the thread
+    // ceiling (Dispatcher's executor is corePoolSize=0 / maxPoolSize=MAX_VALUE over
+    // a SynchronousQueue), which makes a lower number look free -- but blocked
+    // threads commit little, these hosts are HTTP/2 so concurrent calls to one host
+    // multiplex over a single connection rather than a handshake each, and
+    // `readyAsyncCalls` is strict FIFO with no priority. PrefetchFeedMedia enqueues
+    // notes BEFORE the user reaches them, so a tighter cap makes the image actually
+    // on screen queue behind those prefetches instead of starting straight away.
+    // Change these with a benchmark/ run, not a hunch.
     private val dispatcher =
         Dispatcher().apply {
             if (!HttpClientEnvironment.isEmulator) {
@@ -138,16 +148,22 @@ class OkHttpClientFactory(
             .addInterceptor(OnionLocationInterceptor(onionCache))
             .build()
 
-    private var lastProxy: Proxy? = null
-
+    // No connection-pool eviction when the proxy changes. OkHttp's `Address` -- the
+    // pool's lookup key -- includes the proxy (`Address.equalsNonHost`), so a call
+    // is only ever handed a connection opened through the very same route. A
+    // connection left over from an old proxy is already unreachable and simply ages
+    // out of the pool; evicting was defensive, not load-bearing.
+    //
+    // It also cost more than it looked. `evictAll()` empties the ENTIRE shared pool,
+    // and this one factory mints both the proxied and the direct client (see
+    // [DualHttpClientManager]) -- `buildLocalSocksProxy` never returns null, so those
+    // two alternated a single "last proxy" field forever. Every rebuild read as a
+    // route change and dropped every warm connection the other client was using, on
+    // each network-state emission and each resubscribe.
     fun buildHttpClient(
         proxy: Proxy?,
         timeoutSeconds: Int,
     ): OkHttpClient {
-        if (proxy != lastProxy) {
-            rootClient.connectionPool.evictAll()
-            lastProxy = proxy
-        }
         val seconds = if (proxy != null) timeoutSeconds * 3 else timeoutSeconds
         return rootClient
             .newBuilder()
@@ -162,6 +178,13 @@ class OkHttpClientFactory(
             .build()
     }
 
+    /**
+     * Closes every idle pooled connection. Call only on a real proxy-route change (see
+     * [evictOnProxyRouteChange]) — connections on a dead route are already unreachable, so this
+     * is hygiene, not correctness, and it empties the pool BOTH clients share.
+     */
+    fun evictPooledConnections() = rootClient.connectionPool.evictAll()
+
     fun buildHttpClient(
         localSocksProxyPort: Int?,
         isMobile: Boolean?,

commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/service/http/OkHttpClientFactoryForRelays.kt

@@ -86,16 +86,22 @@ class OkHttpClientFactoryForRelays(
             .addInterceptor(OnionLocationInterceptor(onionCache))
             .build()
 
-    private var lastProxy: Proxy? = null
-
+    // No connection-pool eviction when the proxy changes. OkHttp's `Address` -- the
+    // pool's lookup key -- includes the proxy (`Address.equalsNonHost`), so a call
+    // is only ever handed a connection opened through the very same route. A
+    // connection left over from an old proxy is already unreachable and simply ages
+    // out of the pool; evicting was defensive, not load-bearing.
+    //
+    // It also cost more than it looked. `evictAll()` empties the ENTIRE shared pool,
+    // and this one factory mints both the proxied and the direct client (see
+    // [DualHttpClientManagerForRelays]) -- `buildLocalSocksProxy` never returns null, so those
+    // two alternated a single "last proxy" field forever. Every rebuild read as a
+    // route change and dropped every warm connection the other client was using, on
+    // each network-state emission and each resubscribe.
     fun buildHttpClient(
         proxy: Proxy?,
         timeoutSeconds: Int,
     ): OkHttpClient {
-        if (proxy != lastProxy) {
-            rootClient.connectionPool.evictAll()
-            lastProxy = proxy
-        }
         val seconds = if (proxy != null) timeoutSeconds * 3 else timeoutSeconds
         return rootClient
             .newBuilder()
@@ -108,6 +114,13 @@ class OkHttpClientFactoryForRelays(
             .build()
     }
 
+    /**
+     * Closes every idle pooled connection. Call only on a real proxy-route change (see
+     * [evictOnProxyRouteChange]) — connections on a dead route are already unreachable, so this
+     * is hygiene, not correctness, and it empties the pool BOTH clients share.
+     */
+    fun evictPooledConnections() = rootClient.connectionPool.evictAll()
+
     fun buildHttpClient(
         localSocksProxyPort: Int?,
         isMobile: Boolean?,