refactor resizing to address race condition on Windows

This commit is contained in:
Nick Fisher
2023-10-16 17:02:24 +11:00
parent 025bdf662e
commit 6cf8e58bed
10 changed files with 920 additions and 739 deletions

View File

@@ -8,11 +8,18 @@ typedef FilamentEntity = int;
enum ToneMapper { ACES, FILMIC, LINEAR }
class TextureDetails {
final int textureId;
final int width;
final int height;
TextureDetails({required this.textureId, required this.width, required this.height});
}
abstract class FilamentController {
// the current target size of the viewport, in logical pixels
ui.Size size = ui.Size.zero;
Stream<int?> get textureId;
Future get isReadyForScene;
///
@@ -54,6 +61,7 @@ abstract class FilamentController {
///
Future destroyViewer();
///
/// Destroys the backing texture. You probably want to call [destroy] instead of this; this is exposed mostly for lifecycle changes which are handled by FilamentWidget.
///
@@ -71,13 +79,13 @@ abstract class FilamentController {
/// 5) The FilamentWidget will replace the empty Container with a Texture widget
/// If you need to wait until a FilamentViewer has been created, [await] the [isReadyForScene] Future.
///
Future createViewer(int width, int height);
Future<TextureDetails> createViewer(int width, int height);
///
/// Resize the viewport & backing texture.
/// This is called by FilamentWidget; you shouldn't need to invoke this manually.
///
Future resize(int width, int height, {double scaleFactor = 1.0});
Future<TextureDetails> resize(int width, int height, {double scaleFactor = 1.0});
///
/// Set the background image to [path] (which should have a file extension .png, .jpg, or .ktx).

View File

@@ -11,14 +11,13 @@ import 'package:polyvox_filament/generated_bindings.dart';
const FilamentEntity _FILAMENT_ASSET_ERROR = 0;
class FilamentControllerFFI extends FilamentController {
late MethodChannel _channel = MethodChannel("app.polyvox.filament/event");
double _pixelRatio = 1.0;
int? _textureId;
final _textureIdController = StreamController<int?>.broadcast();
Stream<int?> get textureId => _textureIdController.stream;
Completer _isReadyForScene = Completer();
Future get isReadyForScene => _isReadyForScene.future;
@@ -105,20 +104,21 @@ class FilamentControllerFFI extends FilamentController {
@override
Future destroyTexture() async {
if(_textureId != null) {
throw Exception("No texture available");
}
print("Destroying texture");
// we need to flush all references to the previous texture ID before calling destroy, otherwise the Texture widget will attempt to render a non-existent texture and crash.
// however, this is not a synchronous stream, so we need to ensure the Texture widget has been removed from the hierarchy before destroying
_textureId = null;
_textureIdController.add(null);
await _channel.invokeMethod("destroyTexture");
await _channel.invokeMethod("destroyTexture", _textureId!);
print("Texture destroyed");
}
///
/// Called by `FilamentWidget`. You do not need to call this yourself.
///
Future createViewer(int width, int height) async {
Future<TextureDetails> createViewer(int width, int height) async {
if (_viewer != null) {
throw Exception(
"Viewer already exists, make sure you call destroyViewer first");
@@ -133,8 +133,6 @@ class FilamentControllerFFI extends FilamentController {
throw Exception("Failed to get resource loader");
}
print("Using loader ${loader.address}");
size = ui.Size(width * _pixelRatio, height * _pixelRatio);
print("Creating viewer with size $size");
@@ -195,45 +193,99 @@ class FilamentControllerFFI extends FilamentController {
_assetManager = _lib.get_asset_manager(_viewer!);
_textureIdController.add(_textureId);
_isReadyForScene.complete(true);
return TextureDetails(textureId: _textureId!, width: width, height:height);
}
///
/// I'm not exactly sure how to resize the backing textures on all platforms.
/// So for now, I'm sticking with the safe option when the widget is resized: destroying the swapchain, recreating the textures, and creating a new swapchain.
/// When a FilamentWidget is resized, it will call [resize]. This method will tear down/recreate the swapchain and propagate a new texture ID back to the FilamentWidget.
/// For "once-off" resizes, this is fine.
/// However, this can be problematic for consecutive resizes (e.g. dragging to expand/contract the parent window on desktop, or animating the size of the FilamentWidget itself).
/// It is too expensive to recreate the swapchain multiple times per second.
/// We therefore add a timer to FilamentWidget so that the call to [resize] is delayed (e.g. 50ms).
/// Any subsequent resizes before the delay window elapses will cancel the earlier call.
///
/// The overall process looks like this:
/// 1) the window is resized
/// 2) (Windows only) PixelBufferTexture is requested to provide a new pixel buffer with a new size, and we return an empty texture
/// 3) After Xms, [resize] is invoked
/// 4) the viewer is instructed to stop rendering (synchronous)
/// 5) the existing Filament swapchain is destroyed (synchronous)
/// 6) the Flutter texture is unregistered
/// a) this is asynchronous, but
/// b) *** SEE NOTE BELOW ON WINDOWS *** by passing the method channel result through to the callback, we make this synchronous from the Flutter side,
// c) in this async callback, the glTexture is destroyed
/// 7) a new Flutter/OpenGL texture is created (synchronous)
/// 8) a new swapchain is created (synchronous)
/// 9) if the viewer was rendering prior to the resize, the viewer is instructed to recommence rendering
/// 10) the new texture ID is pushed to the FilamentWidget
/// 11) the FilamentWidget updates the Texture widget with the new texture.
///
/// #### (Windows-only) ############################################################
/// # As soon as the widget/window is resized, the PixelBufferTexture will be
/// # requested to provide a new pixel buffer for the new size.
/// # Even with zero delay to the call to [resize], this will be triggered *before*
/// # we have had a chance to anything else (like tear down the swapchain).
/// # On the backend, we deal with this by simply returning an empty texture as soon
/// # as the size changes, and will rely on the followup call to [resize] to actually
/// # destroy/recreate the pixel buffer and Flutter texture.
///
/// NOTE RE ASYNC CALLBACK
/// # The bigger problem is a race condition when resize is called multiple times in quick succession (e.g dragging to resize on Windows).
/// # It looks like occasionally, the backend OpenGL texture is being destroyed while its corresponding swapchain is still active, causing a crash.
/// # I'm not exactly sure how/where this is occurring, but something clearly isn't synchronized between destroy_swap_chain_ffi and
/// # the asynchronous callback passed to FlutterTextureRegistrar::UnregisterTexture.
/// # Theoretically this could occur if resize_2 starts before resize_1 completes, i.e.
/// # 1) resize_1 destroys swapchain/texture and creates new texture
/// # 2) resize_2 destroys swapchain/texture
/// # 3) resize_1 creates new swapchain but texture isn't available, ergo crash
/// #
/// # I don't think this should happen if:
/// # 1) we add a flag on the Flutter side to ensure only one call to destroy/recreate the swapchain/texture is active at any given time, and
/// # 2) on the Flutter side, we are sure that calling destroyTexture only returns once the async callback on the native side has completed.
/// # For (1), checking if textureId is null at the entrypoint should be sufficient.
/// # For (2), we invoke flutter::MethodResult<flutter::EncodableValue>->Success in the UnregisterTexture callback.
/// #
/// # Maybe (2) doesn't actually make Flutter wait?
/// #
/// # The other possibility is that both (1) and (2) are fine and the issue is elsewhere.
/// #
/// # Either way, the current solution is to basically setup a double-buffer on resize.
/// # When destroyTexture is called, the active texture isn't destroyed yet, it's only marked as inactive.
/// # On subsequent calls to destroyTexture, the inactive texture is destroyed.
/// # This seems to work fine.
///
/// # Another option is to only use a single large (e.g. 4k) texture and simply crop whenever a resize is requested.
/// # This might be preferable for other reasons (e.g. don't need to destroy/recreate the pixel buffer or swapchain).
/// # Given we don't do this on other platforms, I'm OK to stick with the existing solution for the time being.
/// ############################################################################
///
///
///
/// Other options:
/// 1) never destroy the texture, simply allocate a large (4k?) texture and crop as needed
/// 2) double-buffering?
@override
Future resize(int width, int height, {double scaleFactor = 1.0}) async {
Future<TextureDetails> resize(int width, int height, {double scaleFactor = 1.0}) async {
if (_textureId == null) {
print("No texture created, ignoring call to resize.");
return;
throw Exception("No texture created, ignoring call to resize.");
}
if (_resizing) {
print("Resize currently underway, ignoring");
return;
}
bool wasRendering = _rendering;
if (_viewer != null && _rendering) {
await setRendering(false);
}
_resizing = true;
var textureId = _textureId;
_textureId = null;
_lib.set_rendering_ffi(_viewer!, false);
if (_viewer != null) {
_lib.destroy_swap_chain_ffi(_viewer!);
}
await destroyTexture();
await _channel.invokeMethod("destroyTexture", textureId);
size = ui.Size(width * _pixelRatio, height * _pixelRatio);
var textures =
await _channel.invokeMethod("createTexture", [size.width, size.height]);
print("Created new texture");
var flutterTextureId = textures[0];
_textureId = flutterTextureId;
// void* on iOS (pointer to pixel buffer), void* on Android (pointer to native window), null on Windows/macOS
var surfaceAddress = textures[1] as int? ?? 0;
@@ -256,11 +308,10 @@ class FilamentControllerFFI extends FilamentController {
_lib.update_viewport_and_camera_projection_ffi(
_viewer!, size.width.toInt(), size.height.toInt(), 1.0);
_textureIdController.add(_textureId);
_resizing = false;
if (wasRendering) {
setRendering(true);
}
await setRendering(_rendering);
_textureId = textures[0];
return TextureDetails(textureId: _textureId!, width: width, height:height);
}
@override

View File

@@ -91,7 +91,8 @@ class FilamentControllerMethodChannel extends FilamentController {
/// 3) Initially, this widget will only contain an empty Container. After the first frame is rendered, the widget itself will automatically call [createViewer] with the width/height from its constraints
/// 4) The FilamentWidget will replace the empty Container with the Texture widget.
///
Future createViewer(int width, int height) async {
Future<TextureDetails> createViewer(int width, int height) async {
throw Exception();
if (_viewer != null) {
throw Exception(
"Viewer already exists, make sure you call destroyViewer first");
@@ -119,7 +120,9 @@ class FilamentControllerMethodChannel extends FilamentController {
bool _resizing = false;
Future resize(int width, int height, {double scaleFactor = 1.0}) async {
Future<TextureDetails> resize(int width, int height, {double scaleFactor = 1.0}) async {
throw Exception();
_resizing = true;
_textureId = await _channel.invokeMethod(
"resize", [width * _pixelRatio, height * _pixelRatio, scaleFactor]);

View File

@@ -56,10 +56,8 @@ class FilamentWidget extends StatefulWidget {
/// The default is a solid red Container, intentionally chosen to make it clear that there will be at least one frame where the Texture widget is not being rendered.
///
final Widget? initial;
final void Function()? onResize;
const FilamentWidget(
{Key? key, required this.controller, this.onResize, this.initial})
const FilamentWidget({Key? key, required this.controller, this.initial})
: super(key: key);
@override
@@ -67,23 +65,21 @@ class FilamentWidget extends StatefulWidget {
}
class _FilamentWidgetState extends State<FilamentWidget> {
StreamSubscription? _textureIdListener;
int? _textureId;
TextureDetails? _textureDetails;
late final AppLifecycleListener _listener;
AppLifecycleState? _lastState;
bool _resizing = false;
String? _error;
Timer? _resizeTimer;
int? _width;
int? _height;
void _handleStateChange(AppLifecycleState state) async {
switch (state) {
case AppLifecycleState.detached:
print("Detached");
_textureId = null;
_textureDetails = null;
await widget.controller.destroyViewer();
await widget.controller.destroyTexture();
@@ -91,7 +87,7 @@ class _FilamentWidgetState extends State<FilamentWidget> {
case AppLifecycleState.hidden:
print("Hidden");
if (Platform.isIOS) {
_textureId = null;
_textureDetails = null;
await widget.controller.destroyViewer();
await widget.controller.destroyTexture();
}
@@ -104,12 +100,16 @@ class _FilamentWidgetState extends State<FilamentWidget> {
break;
case AppLifecycleState.resumed:
print("Resumed");
if (_textureId == null) {
var size = ((context.findRenderObject()) as RenderBox).size;
print("Size after resuming : $size");
await widget.controller
.createViewer(size.width.toInt(), size.height.toInt());
print("Created viewer Size after resuming");
if (!Platform.isWindows) {
if (_textureDetails == null) {
var size = ((context.findRenderObject()) as RenderBox).size;
print("Size after resuming : $size");
_height = size.height.ceil();
_width = size.width.ceil();
await widget.controller
.createViewer(_width!, _height!);
print("Created viewer Size after resuming");
}
}
break;
}
@@ -130,9 +130,12 @@ class _FilamentWidgetState extends State<FilamentWidget> {
await Future.delayed(Duration(seconds: 2));
}
var size = ((context.findRenderObject()) as RenderBox).size;
_width = size.width.ceil();
_height = size.height.ceil();
try {
await widget.controller.createViewer(size.width.toInt(), size.height.toInt());
_textureDetails = await widget.controller
.createViewer(_width!, _height!);
} catch (err) {
setState(() {
_error = err.toString();
@@ -140,28 +143,20 @@ class _FilamentWidgetState extends State<FilamentWidget> {
}
});
_textureIdListener = widget.controller.textureId.listen((int? textureId) {
var size = ((context.findRenderObject()) as RenderBox).size;
print(
"Received new texture ID $textureId at size $size (current textureID $_textureId)");
setState(() {
_textureId = textureId;
});
});
super.initState();
}
@override
void dispose() {
_textureIdListener?.cancel();
_listener.dispose();
_resizeTimer?.cancel();
super.dispose();
}
Timer? _resizeTimer;
@override
Widget build(BuildContext context) {
// if an error was encountered in creating a viewer, display the error message and don't even try to display a Texture widget.
if (_error != null) {
return Container(
color: Colors.white,
@@ -170,51 +165,51 @@ class _FilamentWidgetState extends State<FilamentWidget> {
Text(_error!)
]));
}
return LayoutBuilder(builder: ((context, constraints) {
if (_textureId == null) {
return widget.initial ?? Container(color: Colors.red);
}
var texture = Texture(
key: ObjectKey("texture_$_textureId"),
textureId: _textureId!,
// if no texture ID is available, display the [initial] widget (solid red by default)
late Widget content;
if ( _textureDetails == null || _textureDetails!.height != _height || _textureDetails!.width != _width) {
content = widget.initial ?? Container(color: Colors.red);
} else {
content = Texture(
key: ObjectKey("texture_${_textureDetails!.textureId}"),
textureId: _textureDetails!.textureId,
filterQuality: FilterQuality.none,
freeze: false,
);
return SizedBox(
height: constraints.maxHeight,
width: constraints.maxWidth,
child: ResizeObserver(
onResized: (Size oldSize, Size newSize) async {
WidgetsBinding.instance.addPostFrameCallback((_) async {
_resizeTimer?.cancel();
}
_resizeTimer = Timer(Duration(milliseconds: 500), () async {
// setState(() {
// _resizing = true;
// });
// see [FilamentControllerFFI.resize] for an explanation of how we deal with resizing
return ResizeObserver(
onResized: (Size oldSize, Size newSize) async {
// TODO - we could snapshot the widget to display while we resize?
_resizeTimer?.cancel();
print("Resizing to $newSize");
// await widget.controller
// .resize(newSize.width.toInt(), newSize.height.toInt());
// WidgetsBinding.instance.addPostFrameCallback((_) async {
// setState(() {
// _resizing = false;
// widget.onResize?.call();
// });
// });
});
WidgetsBinding.instance.addPostFrameCallback((_) {
_resizeTimer = Timer(const Duration(milliseconds:50), () async {
var newWidth = newSize.width.ceil();
var newHeight = newSize.height.ceil();
_textureDetails = await widget.controller
.resize(newWidth, newHeight);
WidgetsBinding.instance.addPostFrameCallback((_) {
setState(() {
_width = newWidth;
_height = newHeight;
});
},
child: _resizing
? Container()
: Platform.isLinux || Platform.isWindows
? Transform(
alignment: Alignment.center,
transform: Matrix4.rotationX(
pi), // TODO - this rotation is due to OpenGL texture coordinate working in a different space from Flutter, can we move this to the C++ side somewhere?
child: texture)
: texture));
}));
});
});
});
},
child: Stack(children: [
Positioned.fill(
child: Platform.isLinux || Platform.isWindows
? Transform(
alignment: Alignment.center,
transform: Matrix4.rotationX(
pi), // TODO - this rotation is due to OpenGL texture coordinate working in a different space from Flutter, can we move this to the C++ side somewhere?
child: content)
: content)
]));
}
}