Fix WebGL render target resizing

compose/ui/ui/src/webMain/kotlin/androidx/compose/ui/platform/webgl/WebGLRenderTarget.web.kt

@@ -34,7 +34,6 @@ import androidx.compose.ui.unit.IntSize
 import androidx.compose.ui.unit.toIntSize
 import androidx.compose.ui.window.LocalComposeWindow
 import org.jetbrains.skia.DirectContext
-import org.jetbrains.skia.Image
 import org.khronos.webgl.WebGLFramebuffer
 import org.khronos.webgl.WebGLRenderbuffer
 import org.khronos.webgl.WebGLRenderingContext
@@ -137,18 +136,26 @@ class WebGLRenderTarget internal constructor(
         webGLContext.createFramebuffer() ?: error("gl.createFramebuffer() returned null")
     }
 
+    private var currentTexture: WebGLTexture? = null
+
     /**
      * The WebGL texture backing this render target.
      *
-     * Its storage is allocated lazily and resized when necessary in [render].
-     *
-     * Callers may register or attach this texture to another framebuffer, but must not delete it,
-     * reallocate its storage, or change its texture parameters. Callers must stop using it when
-     * [onTextureWillBeInvalidated] is invoked.
+     * The current texture is allocated lazily and replaced when the target changes size. Callers
+     * may register or attach it to another framebuffer, but must not delete it, reallocate its
+     * storage, or change its texture parameters. Callers must stop using it when
+     * [onTextureWillBeInvalidated] is invoked. Access this property after the callback returns to
+     * obtain the replacement.
      */
-    val webGlTexture: WebGLTexture by lazy {
-        webGLContext.createTexture() ?: error("gl.createTexture() returned null")
-    }
+    val webGlTexture: WebGLTexture
+        get() {
+            val current = currentTexture
+            if (current != null) return current
+
+            val created = webGLContext.createTexture() ?: error("gl.createTexture() returned null")
+            currentTexture = created
+            return created
+        }
 
     /** The depth/stencil attachment of [framebuffer]; like it, created once and only resized. */
     private val depthStencil: WebGLRenderbuffer by lazy {
@@ -181,7 +188,7 @@ class WebGLRenderTarget internal constructor(
 
     /**
      * Called before the current texture-backed render resource becomes unavailable.
-     * It happens when the texture is about to be reconfigured for a new size or the
+     * It happens when the texture is about to be replaced for a new size or the
      * [WebGLRenderTarget] is being disposed.
      */
     var onTextureWillBeInvalidated: (() -> Unit)? = null
@@ -265,14 +272,17 @@ class WebGLRenderTarget internal constructor(
         if (current != null && current.size == size) return
 
         if (current != null) {
-            onTextureWillBeInvalidated?.invoke()
-            current.dispose()
+            releaseTexture()
         }
 
-        adoptedTexture = null
-
-        webGLContext.configureWebGLTexture(webGlTexture, size)
-        val adopted = webGLContext.adoptNewTexture(context, size, webGlTexture)
+        val newTexture = webGlTexture
+        webGLContext.configureWebGLTexture(newTexture, size)
+        val adopted = try {
+            webGLContext.adoptNewTexture(context, size, newTexture)
+        } catch (error: Throwable) {
+            currentTexture = null
+            throw error
+        }
         this.adoptedTexture = adopted
 
         webGLContext.bindRenderbuffer(RENDERBUFFER, depthStencil)
@@ -284,7 +294,7 @@ class WebGLRenderTarget internal constructor(
             FRAMEBUFFER,
             COLOR_ATTACHMENT0,
             TEXTURE_2D,
-            adopted.texture,
+            newTexture,
             0,
         )
         webGLContext.framebufferRenderbuffer(
@@ -303,6 +313,22 @@ class WebGLRenderTarget internal constructor(
         generation++
     }
 
+    private fun releaseTexture() {
+        val current = currentTexture ?: return
+        try {
+            onTextureWillBeInvalidated?.invoke()
+        } finally {
+            val adopted = adoptedTexture
+            adoptedTexture = null
+            currentTexture = null
+            if (adopted == null) {
+                webGLContext.deleteTexture(current)
+            } else {
+                adopted.dispose()
+            }
+        }
+    }
+
     /**
      * Disposes the target's GPU resources.
      *
@@ -312,16 +338,15 @@ class WebGLRenderTarget internal constructor(
     internal fun dispose() {
         if (isDisposed) return
         isDisposed = true
-        if (adoptedTexture != null) {
-            onTextureWillBeInvalidated?.invoke()
-            adoptedTexture?.dispose()
-            adoptedTexture = null
+        try {
+            releaseTexture()
+        } finally {
+            size = IntSize.Zero
+            webGLContext.deleteFramebuffer(framebuffer)
+            webGLContext.deleteRenderbuffer(depthStencil)
+            webGLContext.bindFramebuffer(FRAMEBUFFER, null)
+            directContext()?.resetAll()
         }
-        size = IntSize.Zero
-        webGLContext.deleteFramebuffer(framebuffer)
-        webGLContext.deleteRenderbuffer(depthStencil)
-        webGLContext.bindFramebuffer(FRAMEBUFFER, null)
-        directContext()?.resetAll()
     }
 }
 

compose/ui/ui/src/webTest/kotlin/androidx/compose/ui/platform/webgl/WebGLRenderTargetTests.kt

@@ -38,9 +38,11 @@ import androidx.compose.ui.unit.IntSize
 import androidx.compose.ui.unit.dp
 import kotlin.test.Test
 import kotlin.test.assertEquals
+import kotlin.test.assertFailsWith
 import kotlin.test.assertFalse
 import kotlin.test.assertNotEquals
 import kotlin.test.assertNotNull
+import kotlin.test.assertNotSame
 import kotlin.test.assertSame
 import kotlin.test.assertTrue
 import org.khronos.webgl.WebGLRenderingContext
@@ -52,13 +54,18 @@ import org.khronos.webgl.WebGLRenderingContext.Companion.NO_ERROR
 
 /** Opaque red as `0xRRGGBBAA`, chosen because every channel is exact in RGBA8. */
 private const val OPAQUE_RED = (255 shl 24) or 255
+private const val OPAQUE_GREEN = (255 shl 16) or 255
 
-/** The whole "renderer": clear the target to opaque red, like the ColorPulse demo. */
-private fun clearToRed(target: WebGLRenderTarget): Boolean = target.render {
-    target.webGLContext.viewport(0, 0, target.size.width, target.size.height)
-    target.webGLContext.clearColor(1f, 0f, 0f, 1f)
-    target.webGLContext.clear(COLOR_BUFFER_BIT)
-}
+private fun clearTo(target: WebGLRenderTarget, red: Float, green: Float, blue: Float): Boolean =
+    target.render {
+        target.webGLContext.viewport(0, 0, target.size.width, target.size.height)
+        target.webGLContext.clearColor(red, green, blue, 1f)
+        target.webGLContext.clear(COLOR_BUFFER_BIT)
+    }
+
+private fun clearToRed(target: WebGLRenderTarget): Boolean = clearTo(target, 1f, 0f, 0f)
+
+private fun clearToGreen(target: WebGLRenderTarget): Boolean = clearTo(target, 0f, 1f, 0f)
 
 class WebGLRenderTargetTests : OnCanvasTests {
 
@@ -173,17 +180,23 @@ class WebGLRenderTargetTests : OnCanvasTests {
         }
 
         val target = renderTarget ?: return@runApplicationTest skipWithoutWebGL2()
-        var invalidationCount = 0
-        target.onTextureWillBeInvalidated = { invalidationCount++ }
-
         awaitAnimationFrame()
         awaitIdle()
 
         assertTrue(clearToRed(target), "the first render() did not run")
-        assertEquals(0, invalidationCount, "the first render unexpectedly invalidated the texture")
         assertEquals(IntSize(32, 32), target.size, "unexpected initial size")
         val generationBefore = target.generation
         val framebufferBefore = target.framebuffer
+        val textureBefore = target.webGlTexture
+        var invalidationCount = 0
+        target.onTextureWillBeInvalidated = {
+            invalidationCount++
+            assertSame(textureBefore, target.webGlTexture, "the old texture was replaced too early")
+            assertTrue(
+                target.webGLContext.isTexture(textureBefore),
+                "the old texture was deleted before the invalidation callback",
+            )
+        }
 
         requestedSize.value = IntSize(48, 24)
         awaitAnimationFrame()
@@ -202,7 +215,28 @@ class WebGLRenderTargetTests : OnCanvasTests {
             target.framebuffer,
             "the framebuffer itself was replaced by the size change"
         )
+        assertNotSame(
+            textureBefore,
+            target.webGlTexture,
+            "the texture adopted and deleted by Skia was reused after the size change",
+        )
         assertEquals(NO_ERROR, target.webGLContext.getError(), "reallocation reported a GL error")
+
+        val textureAfterResize = target.webGlTexture
+        target.onTextureWillBeInvalidated = { error("expected invalidation failure") }
+        requestedSize.value = IntSize(24, 48)
+        awaitAnimationFrame()
+        awaitIdle()
+        assertFailsWith<IllegalStateException> { clearToRed(target) }
+        assertEquals(null, target.adoptedTexture, "a failing callback kept the adopted image")
+        assertNotSame(
+            textureAfterResize,
+            target.webGlTexture,
+            "a failing callback kept the old texture as the current texture",
+        )
+        target.onTextureWillBeInvalidated = null
+        assertTrue(clearToGreen(target), "render() did not recover after the callback failed")
+        assertEquals(IntSize(24, 48), target.size, "the size was not applied after recovery")
     }
 
     /**
@@ -213,13 +247,15 @@ class WebGLRenderTargetTests : OnCanvasTests {
      * once the browser has composited it.
      */
     @Test
-    fun theRenderedFrameReachesTheComposeCanvas() = runApplicationTest {
+    fun theRenderedFrameReachesTheComposeCanvasBeforeAndAfterResize() = runApplicationTest {
         assertTrue(forcePreserveDrawingBuffer(), "could not force preserveDrawingBuffer")
         try {
             var renderTarget: WebGLRenderTarget? = null
+            val requestedSize = mutableStateOf(IntSize(64, 64))
 
             createComposeWindow {
-                val target = rememberWebGLRenderTarget(IntSize(64, 64))
+                val size by requestedSize
+                val target = rememberWebGLRenderTarget(size)
                 renderTarget = target
                 if (target != null) {
                     LaunchedEffect(target) {
@@ -261,6 +297,17 @@ class WebGLRenderTargetTests : OnCanvasTests {
                 readCanvasPixelRgba8(gl, 600, outsideY).toHexString(),
                 "the texture was drawn outside the composable"
             )
+
+            requestedSize.value = IntSize(48, 24)
+            awaitAnimationFrame()
+            awaitIdle()
+            assertTrue(clearToGreen(target), "render() did not run after the size change")
+            val resized = awaitCanvasPixel(gl, x = 100, y = insideY, expected = OPAQUE_GREEN)
+            assertEquals(
+                OPAQUE_GREEN.toHexString(),
+                resized.toHexString(),
+                "the resized texture did not reach the canvas inside the composable",
+            )
         } finally {
             restorePreserveDrawingBuffer()
         }