android: per-view swap chain bookkeeping (multi-viewer fixes)#169
Open
mushogenshin wants to merge 3 commits into
Open
android: per-view swap chain bookkeeping (multi-viewer fixes)#169mushogenshin wants to merge 3 commits into
mushogenshin wants to merge 3 commits into
Conversation
…ying its own swapchain
Real cause of the SIGABRT-on-mount on Android. Not a destroy-vs-
endFrame race (the previous flush() patch was wrong). It's a
straight-up API bug in FFIFilamentApp.getSwapChains():
Future<Iterable<SwapChain>> getSwapChains() async {
return _swapChains; // ← live reference, not a snapshot
}
In `thermion_flutter_plugin_native.dart` the Android branch of
createTextureAndBindToView reads this list, creates a new swap
chain, then destroys whatever was in the snapshot:
final swapChains = await FilamentApp.instance!.getSwapChains();
final swapChain = await FilamentApp.instance!.createSwapChain(...);
if (swapChains.isNotEmpty) {
await FilamentApp.instance!.destroySwapChain(swapChains.first);
}
await FilamentApp.instance!.renderManager.attach(view, swapChain);
The intent reads correctly: "snapshot existing chains; create the
new one; tear down old chains". But because getSwapChains returned
the live list reference rather than a copy, `swapChains` aliased
`_swapChains`. createSwapChain appended to _swapChains, so
swapChains.first immediately referred to the *new* swap chain we
just created. The plugin then destroyed its own new swap chain
and attached the view to a freed pointer. The next render hit
Filament's `SwapChain must remain valid until endFrame is called`
precondition and the process aborted with SIGABRT.
Reproduced on every viewer mount on Android — including
examples/flutter/quickstart, ARM64 emulator and physical device.
iOS / macOS / Windows are unaffected because their plugin paths
don't iterate getSwapChains().
Fix: return List.unmodifiable(_swapChains). Defensive at the API
layer; the call-site logic in plugin_native.dart is now correct
without modification.
The Android branch of `createTextureAndBindToView` (in `thermion_flutter_plugin_native.dart`) destroys the *previous* swap chain for the size-change case so the renderer doesn't keep an old, wrong-sized surface around. The previous version chose what to destroy by iterating FilamentApp's global swap chain list and picking `swapChains.first`. After the upstream getSwapChains() snapshot fix (nmfisher#167) the list is correctly snapshot, but in a multi-viewer app `swapChains.first` is whichever viewer's swap chain happens to be earliest in the list — usually a *different* viewer's, not this one's. So mounting viewer #N destroyed viewer #(N-1)'s swap chain. The renderer for #(N-1) then ran with a freed pointer; symptom on screen was middle viewers freezing while only the most recently mounted one (whose swap chain nothing destroyed) kept rendering. Fix: track the swap chain currently bound to each view in a `<View, SwapChain>` map and destroy *that* one, scoped to this view only. Other viewers' swap chains stay live. Order also rearranged: attach the new swap chain *before* destroying the old one, so the view never has a window of being unattached. Other ordering inside the function is unchanged. `_viewSwapChains` mirrors the pattern of the existing `_viewRenderTargets` map a few lines above. No cleanup hook on view-destroy, same as `_viewRenderTargets`'s existing convention; that's a separate lifecycle concern. Reproduces with the spike's stress-test screen at 4 / 8 viewers on Android emulator. Single-viewer is unaffected (map starts empty, no destroy on first mount). iOS / macOS / Windows are unaffected — only the Android plugin branch ever ran the destroy logic.
After the previous patch (per-view swap chain tracking) the Android plugin correctly created and reused a SwapChain bound to each View. Disposal of the chain when the widget unmounted, however, was not wired up — the SwapChain stayed in `_viewSwapChains` and stayed attached to the RenderManager, while `ThermionWidget.dispose()` released the underlying SurfaceTexture (and thus the native window). Result on the next frame: Filament's render thread iterates the attached swap chains, hits the now-orphaned one, and calls `eglSwapBuffers` on the freed native window. The emulator's EGL backend logs: E/EGL_emulation: egl_window_surface_t::swapBuffers called with NULL buffer E/EGL_emulation: tid <n>: swapBuffers(764): error 0x300d (EGL_BAD_SURFACE) repeatedly until the next frame after the SwapChain is collected elsewhere. On the spike's stress-test screen at 4 / 8 viewers, even brief widget rebuilds (e.g. toggling viewer count) flood the log and cause visible frame drops because every render iteration attempts the bad swap. Fix: add a public `releaseTextureBindingForView(View view)` method to `ThermionFlutterPlugin` that releases plugin-side resources bound to a view (currently: the Android per-view SwapChain). The native plugin removes the entry from `_viewSwapChains` and calls `destroySwapChain` (which detaches from RenderManager and destroys the Filament SwapChain). Web plugin is a no-op — it doesn't track per-view bindings. `ThermionWidget.dispose()` now calls this BEFORE destroying the texture descriptor. The two are sequenced inside an unawaited async closure so dispose() stays synchronous for the framework but the swap chain destroy completes before the SurfaceTexture release. Other platforms aren't affected — releaseTextureBinding ForView is a no-op when no per-view binding exists.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three related fixes to the Android plugin's swap-chain handling. All three are needed for multi-viewer Flutter apps on Android to mount, render, and unmount cleanly. Single-viewer behaviour is unchanged.
Background
`thermion_flutter_plugin_native.dart`'s Android branch of `createTextureAndBindToView` recreates a swap chain whenever the surface size changes — that's the documented intent (`// On Android, we recreate the swapchain whenever the size changes`). The implementation works for single-viewer apps but has three latent bugs that surface immediately when more than one viewer is mounted.
Fix 1: `getSwapChains()` returns a snapshot, not the live list
`FFIFilamentApp.getSwapChains()` returned the live `_swapChains` list reference. The plugin code does:
```dart
final swapChains = await FilamentApp.instance!.getSwapChains(); // aliases _swapChains
final swapChain = await FilamentApp.instance!.createSwapChain(...); // appends to _swapChains
if (swapChains.isNotEmpty) { // now [newSwapChain]!
await FilamentApp.instance!.destroySwapChain(swapChains.first); // destroys the NEW one
}
```
The intent reads as "snapshot existing chains, create the new one, tear down the old one" — but because `swapChains` aliased `_swapChains`, `createSwapChain` appended into the very list the plugin was about to iterate. `swapChains.first` immediately referred to the newly-created swap chain. The plugin destroyed its own new swap chain and attached the view to a freed pointer; the next render hit Filament's `endFrame:490` precondition and aborted on every viewer mount.
Returning `List.unmodifiable(_swapChains)` defends at the API layer.
Fix 2: per-view swap chain tracking (multi-viewer freeze)
After Fix 1, the snapshot is correct, but `swapChains.first` is still the first swap chain in the list — which in a multi-viewer app is another viewer's swap chain. Mounting viewer #N destroyed viewer #(N-1)'s swap chain, freezing it. The most-recently-mounted viewer always rendered fine; everyone else froze in cascade.
Track the swap chain currently bound to each view in a `<View, SwapChain>` map (mirroring the existing `_viewRenderTargets` pattern in the same file), and destroy that one — scoped to this view only. Other viewers' swap chains stay live.
Fix 3: release per-view swap chain on widget dispose (EGL_BAD_SURFACE flood)
After Fix 2, the swap chain is correctly created and reused per view, but never released when the widget unmounts. `ThermionWidget.dispose()` released the underlying SurfaceTexture (freeing the native window), while the SwapChain stayed in `_viewSwapChains` and stayed attached to the RenderManager. The next render iteration would then call `eglSwapBuffers` on the now-orphaned native window, logging:
```
E/EGL_emulation: egl_window_surface_t::swapBuffers called with NULL buffer
E/EGL_emulation: tid : swapBuffers(764): error 0x300d (EGL_BAD_SURFACE)
```
repeatedly per render. On a multi-viewer stress test, even brief widget rebuilds (toggling viewer count) flooded the log and cost visible frame budget.
Add a public `releaseTextureBindingForView(View view)` API on `ThermionFlutterPlugin`. Native impl removes the entry from `_viewSwapChains` and calls `destroySwapChain` (which detaches from RenderManager and destroys the Filament SwapChain). Web impl is a no-op — it doesn't track per-view bindings. `ThermionWidget.dispose()` calls it before tearing down the descriptor, sequenced inside an unawaited async closure so `dispose()` stays synchronous for the framework while the destroy still completes before the SurfaceTexture release.
Verification
Related