Skip to content

Commit dca97b2

Browse files
authored
Fix WebGL render target resizing (#3344)
Fix `WebGLRenderTarget` rendering after its size changes. `Image.adoptTextureFrom` transfers ownership of the WebGL texture to Skia, so closing the old image deletes that texture. The previous resize path then tried to allocate storage on the deleted texture, which left the framebuffer incomplete. This change creates a texture for each size generation and releases the previous texture through one path after notifying borrowers. PR #3323 and its Skiko update are now on `jb-main`. This PR is the focused resize follow-up. ## Testing In this repo: - `./gradlew :compose:ui:ui:jsBrowserTest --no-daemon --no-configuration-cache` - `./gradlew :mpp:publishComposeJbToMavenLocal -Pcompose.platforms=web -Pjetbrains.publication.libraries=COMPOSE --no-daemon --no-configuration-cache` In maplibre/maplibre-compose#1114: - MapLibre Compose demo startup and resize in Chromium, Safari, and Firefox. - MapLibre Compose JS browser suite: 204 tests, no failures or skips. ## Release Notes N/A
1 parent f4ceedb commit dca97b2

2 files changed

Lines changed: 110 additions & 38 deletions

File tree

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

Lines changed: 51 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ import androidx.compose.ui.unit.IntSize
3434
import androidx.compose.ui.unit.toIntSize
3535
import androidx.compose.ui.window.LocalComposeWindow
3636
import org.jetbrains.skia.DirectContext
37-
import org.jetbrains.skia.Image
3837
import org.khronos.webgl.WebGLFramebuffer
3938
import org.khronos.webgl.WebGLRenderbuffer
4039
import org.khronos.webgl.WebGLRenderingContext
@@ -137,18 +136,26 @@ class WebGLRenderTarget internal constructor(
137136
webGLContext.createFramebuffer() ?: error("gl.createFramebuffer() returned null")
138137
}
139138

139+
private var currentTexture: WebGLTexture? = null
140+
140141
/**
141142
* The WebGL texture backing this render target.
142143
*
143-
* Its storage is allocated lazily and resized when necessary in [render].
144-
*
145-
* Callers may register or attach this texture to another framebuffer, but must not delete it,
146-
* reallocate its storage, or change its texture parameters. Callers must stop using it when
147-
* [onTextureWillBeInvalidated] is invoked.
144+
* The current texture is allocated lazily and replaced when the target changes size. Callers
145+
* may register or attach it to another framebuffer, but must not delete it, reallocate its
146+
* storage, or change its texture parameters. Callers must stop using it when
147+
* [onTextureWillBeInvalidated] is invoked. Access this property after the callback returns to
148+
* obtain the replacement.
148149
*/
149-
val webGlTexture: WebGLTexture by lazy {
150-
webGLContext.createTexture() ?: error("gl.createTexture() returned null")
151-
}
150+
val webGlTexture: WebGLTexture
151+
get() {
152+
val current = currentTexture
153+
if (current != null) return current
154+
155+
val created = webGLContext.createTexture() ?: error("gl.createTexture() returned null")
156+
currentTexture = created
157+
return created
158+
}
152159

153160
/** The depth/stencil attachment of [framebuffer]; like it, created once and only resized. */
154161
private val depthStencil: WebGLRenderbuffer by lazy {
@@ -181,7 +188,7 @@ class WebGLRenderTarget internal constructor(
181188

182189
/**
183190
* Called before the current texture-backed render resource becomes unavailable.
184-
* It happens when the texture is about to be reconfigured for a new size or the
191+
* It happens when the texture is about to be replaced for a new size or the
185192
* [WebGLRenderTarget] is being disposed.
186193
*/
187194
var onTextureWillBeInvalidated: (() -> Unit)? = null
@@ -265,14 +272,17 @@ class WebGLRenderTarget internal constructor(
265272
if (current != null && current.size == size) return
266273

267274
if (current != null) {
268-
onTextureWillBeInvalidated?.invoke()
269-
current.dispose()
275+
releaseTexture()
270276
}
271277

272-
adoptedTexture = null
273-
274-
webGLContext.configureWebGLTexture(webGlTexture, size)
275-
val adopted = webGLContext.adoptNewTexture(context, size, webGlTexture)
278+
val newTexture = webGlTexture
279+
webGLContext.configureWebGLTexture(newTexture, size)
280+
val adopted = try {
281+
webGLContext.adoptNewTexture(context, size, newTexture)
282+
} catch (error: Throwable) {
283+
currentTexture = null
284+
throw error
285+
}
276286
this.adoptedTexture = adopted
277287

278288
webGLContext.bindRenderbuffer(RENDERBUFFER, depthStencil)
@@ -284,7 +294,7 @@ class WebGLRenderTarget internal constructor(
284294
FRAMEBUFFER,
285295
COLOR_ATTACHMENT0,
286296
TEXTURE_2D,
287-
adopted.texture,
297+
newTexture,
288298
0,
289299
)
290300
webGLContext.framebufferRenderbuffer(
@@ -303,6 +313,22 @@ class WebGLRenderTarget internal constructor(
303313
generation++
304314
}
305315

316+
private fun releaseTexture() {
317+
val current = currentTexture ?: return
318+
try {
319+
onTextureWillBeInvalidated?.invoke()
320+
} finally {
321+
val adopted = adoptedTexture
322+
adoptedTexture = null
323+
currentTexture = null
324+
if (adopted == null) {
325+
webGLContext.deleteTexture(current)
326+
} else {
327+
adopted.dispose()
328+
}
329+
}
330+
}
331+
306332
/**
307333
* Disposes the target's GPU resources.
308334
*
@@ -312,16 +338,15 @@ class WebGLRenderTarget internal constructor(
312338
internal fun dispose() {
313339
if (isDisposed) return
314340
isDisposed = true
315-
if (adoptedTexture != null) {
316-
onTextureWillBeInvalidated?.invoke()
317-
adoptedTexture?.dispose()
318-
adoptedTexture = null
341+
try {
342+
releaseTexture()
343+
} finally {
344+
size = IntSize.Zero
345+
webGLContext.deleteFramebuffer(framebuffer)
346+
webGLContext.deleteRenderbuffer(depthStencil)
347+
webGLContext.bindFramebuffer(FRAMEBUFFER, null)
348+
directContext()?.resetAll()
319349
}
320-
size = IntSize.Zero
321-
webGLContext.deleteFramebuffer(framebuffer)
322-
webGLContext.deleteRenderbuffer(depthStencil)
323-
webGLContext.bindFramebuffer(FRAMEBUFFER, null)
324-
directContext()?.resetAll()
325350
}
326351
}
327352

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

Lines changed: 59 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,11 @@ import androidx.compose.ui.unit.IntSize
3838
import androidx.compose.ui.unit.dp
3939
import kotlin.test.Test
4040
import kotlin.test.assertEquals
41+
import kotlin.test.assertFailsWith
4142
import kotlin.test.assertFalse
4243
import kotlin.test.assertNotEquals
4344
import kotlin.test.assertNotNull
45+
import kotlin.test.assertNotSame
4446
import kotlin.test.assertSame
4547
import kotlin.test.assertTrue
4648
import org.khronos.webgl.WebGLRenderingContext
@@ -52,13 +54,18 @@ import org.khronos.webgl.WebGLRenderingContext.Companion.NO_ERROR
5254

5355
/** Opaque red as `0xRRGGBBAA`, chosen because every channel is exact in RGBA8. */
5456
private const val OPAQUE_RED = (255 shl 24) or 255
57+
private const val OPAQUE_GREEN = (255 shl 16) or 255
5558

56-
/** The whole "renderer": clear the target to opaque red, like the ColorPulse demo. */
57-
private fun clearToRed(target: WebGLRenderTarget): Boolean = target.render {
58-
target.webGLContext.viewport(0, 0, target.size.width, target.size.height)
59-
target.webGLContext.clearColor(1f, 0f, 0f, 1f)
60-
target.webGLContext.clear(COLOR_BUFFER_BIT)
61-
}
59+
private fun clearTo(target: WebGLRenderTarget, red: Float, green: Float, blue: Float): Boolean =
60+
target.render {
61+
target.webGLContext.viewport(0, 0, target.size.width, target.size.height)
62+
target.webGLContext.clearColor(red, green, blue, 1f)
63+
target.webGLContext.clear(COLOR_BUFFER_BIT)
64+
}
65+
66+
private fun clearToRed(target: WebGLRenderTarget): Boolean = clearTo(target, 1f, 0f, 0f)
67+
68+
private fun clearToGreen(target: WebGLRenderTarget): Boolean = clearTo(target, 0f, 1f, 0f)
6269

6370
class WebGLRenderTargetTests : OnCanvasTests {
6471

@@ -173,17 +180,23 @@ class WebGLRenderTargetTests : OnCanvasTests {
173180
}
174181

175182
val target = renderTarget ?: return@runApplicationTest skipWithoutWebGL2()
176-
var invalidationCount = 0
177-
target.onTextureWillBeInvalidated = { invalidationCount++ }
178-
179183
awaitAnimationFrame()
180184
awaitIdle()
181185

182186
assertTrue(clearToRed(target), "the first render() did not run")
183-
assertEquals(0, invalidationCount, "the first render unexpectedly invalidated the texture")
184187
assertEquals(IntSize(32, 32), target.size, "unexpected initial size")
185188
val generationBefore = target.generation
186189
val framebufferBefore = target.framebuffer
190+
val textureBefore = target.webGlTexture
191+
var invalidationCount = 0
192+
target.onTextureWillBeInvalidated = {
193+
invalidationCount++
194+
assertSame(textureBefore, target.webGlTexture, "the old texture was replaced too early")
195+
assertTrue(
196+
target.webGLContext.isTexture(textureBefore),
197+
"the old texture was deleted before the invalidation callback",
198+
)
199+
}
187200

188201
requestedSize.value = IntSize(48, 24)
189202
awaitAnimationFrame()
@@ -202,7 +215,28 @@ class WebGLRenderTargetTests : OnCanvasTests {
202215
target.framebuffer,
203216
"the framebuffer itself was replaced by the size change"
204217
)
218+
assertNotSame(
219+
textureBefore,
220+
target.webGlTexture,
221+
"the texture adopted and deleted by Skia was reused after the size change",
222+
)
205223
assertEquals(NO_ERROR, target.webGLContext.getError(), "reallocation reported a GL error")
224+
225+
val textureAfterResize = target.webGlTexture
226+
target.onTextureWillBeInvalidated = { error("expected invalidation failure") }
227+
requestedSize.value = IntSize(24, 48)
228+
awaitAnimationFrame()
229+
awaitIdle()
230+
assertFailsWith<IllegalStateException> { clearToRed(target) }
231+
assertEquals(null, target.adoptedTexture, "a failing callback kept the adopted image")
232+
assertNotSame(
233+
textureAfterResize,
234+
target.webGlTexture,
235+
"a failing callback kept the old texture as the current texture",
236+
)
237+
target.onTextureWillBeInvalidated = null
238+
assertTrue(clearToGreen(target), "render() did not recover after the callback failed")
239+
assertEquals(IntSize(24, 48), target.size, "the size was not applied after recovery")
206240
}
207241

208242
/**
@@ -213,13 +247,15 @@ class WebGLRenderTargetTests : OnCanvasTests {
213247
* once the browser has composited it.
214248
*/
215249
@Test
216-
fun theRenderedFrameReachesTheComposeCanvas() = runApplicationTest {
250+
fun theRenderedFrameReachesTheComposeCanvasBeforeAndAfterResize() = runApplicationTest {
217251
assertTrue(forcePreserveDrawingBuffer(), "could not force preserveDrawingBuffer")
218252
try {
219253
var renderTarget: WebGLRenderTarget? = null
254+
val requestedSize = mutableStateOf(IntSize(64, 64))
220255

221256
createComposeWindow {
222-
val target = rememberWebGLRenderTarget(IntSize(64, 64))
257+
val size by requestedSize
258+
val target = rememberWebGLRenderTarget(size)
223259
renderTarget = target
224260
if (target != null) {
225261
LaunchedEffect(target) {
@@ -261,6 +297,17 @@ class WebGLRenderTargetTests : OnCanvasTests {
261297
readCanvasPixelRgba8(gl, 600, outsideY).toHexString(),
262298
"the texture was drawn outside the composable"
263299
)
300+
301+
requestedSize.value = IntSize(48, 24)
302+
awaitAnimationFrame()
303+
awaitIdle()
304+
assertTrue(clearToGreen(target), "render() did not run after the size change")
305+
val resized = awaitCanvasPixel(gl, x = 100, y = insideY, expected = OPAQUE_GREEN)
306+
assertEquals(
307+
OPAQUE_GREEN.toHexString(),
308+
resized.toHexString(),
309+
"the resized texture did not reach the canvas inside the composable",
310+
)
264311
} finally {
265312
restorePreserveDrawingBuffer()
266313
}

0 commit comments

Comments
 (0)