diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9fc0e906..16a54880 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -5,7 +5,6 @@ on: push: branches: [main] pull_request: - branches: [main] concurrency: group: test-${{ github.ref }} @@ -17,13 +16,13 @@ jobs: name: Test timeout-minutes: 12 steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Install FVM shell: bash run: | curl -fsSL https://fvm.app/install.sh | bash - echo "/home/runner/fvm/bin" >> $GITHUB_PATH + echo "/home/runner/fvm/bin" >> "$GITHUB_PATH" - uses: kuhnroyal/flutter-fvm-config-action@v2 id: fvm-config-action @@ -71,7 +70,7 @@ jobs: name: Integration Tests timeout-minutes: 25 steps: - - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - name: Install Linux Desktop Dependencies timeout-minutes: 8 @@ -87,7 +86,7 @@ jobs: shell: bash run: | curl -fsSL https://fvm.app/install.sh | bash - echo "/home/runner/fvm/bin" >> $GITHUB_PATH + echo "/home/runner/fvm/bin" >> "$GITHUB_PATH" - uses: kuhnroyal/flutter-fvm-config-action@v2 id: fvm-config-action @@ -137,7 +136,7 @@ jobs: shell: bash run: | curl -fsSL https://fvm.app/install.sh | bash - echo "/home/runner/fvm/bin" >> $GITHUB_PATH + echo "/home/runner/fvm/bin" >> "$GITHUB_PATH" - uses: kuhnroyal/flutter-fvm-config-action@v2 id: fvm-config-action diff --git a/packages/superdeck/lib/src/capture/slide_capture_service.dart b/packages/superdeck/lib/src/capture/slide_capture_service.dart index 7c342f70..71fcd8d9 100644 --- a/packages/superdeck/lib/src/capture/slide_capture_service.dart +++ b/packages/superdeck/lib/src/capture/slide_capture_service.dart @@ -206,19 +206,33 @@ class SlideCaptureService { } Future _imageToUint8List(ui.Image image) async { - final byteData = await image.toByteData(format: ui.ImageByteFormat.png); - image.dispose(); - return byteData!.buffer.asUint8List(); + try { + final byteData = await image.toByteData(format: ui.ImageByteFormat.png); + + return byteData!.buffer.asUint8List(); + } finally { + image.dispose(); + } } /// Converts a Flutter widget to a [ui.Image] via an isolated render pipeline. /// /// Sets up a complete render context (theme, media query, material app), /// drives a bounded settle loop for async/delayed widgets, then rasterises. + /// Releases the temporary element, render, and focus resources on success, + /// on the settle limit, and on failure. Future _fromWidgetToImage( Widget widget, RenderConfig config, ) async { + RenderRepaintBoundary? repaintBoundary; + RenderPositionedBox? rootBox; + RenderView? renderView; + PipelineOwner? pipelineOwner; + FocusManager? focusManager; + BuildOwner? buildOwner; + RenderObjectToWidgetElement? rootElement; + try { final mixScope = MixScope.maybeOf(config.context); final readiness = SlideCaptureReadiness(); @@ -242,7 +256,11 @@ class SlideCaptureService { ), ); - final repaintBoundary = RenderRepaintBoundary(); + repaintBoundary = RenderRepaintBoundary(); + rootBox = RenderPositionedBox( + alignment: Alignment.center, + child: repaintBoundary, + ); final platformDispatcher = WidgetsBinding.instance.platformDispatcher; final view = @@ -251,12 +269,9 @@ class SlideCaptureService { config.targetSize ?? view.physicalSize / view.devicePixelRatio; final physicalSize = logicalSize * config.pixelRatio; - final renderView = RenderView( + renderView = RenderView( view: view, - child: RenderPositionedBox( - alignment: Alignment.center, - child: repaintBoundary, - ), + child: rootBox, configuration: ViewConfiguration( logicalConstraints: BoxConstraints.tight(logicalSize), physicalConstraints: BoxConstraints.tight(physicalSize), @@ -265,18 +280,17 @@ class SlideCaptureService { ); var isDirty = false; - final pipelineOwner = PipelineOwner( - onNeedVisualUpdate: () => isDirty = true, - ); - final buildOwner = BuildOwner( - focusManager: FocusManager(), + pipelineOwner = PipelineOwner(onNeedVisualUpdate: () => isDirty = true); + focusManager = FocusManager(); + buildOwner = BuildOwner( + focusManager: focusManager, onBuildScheduled: () => isDirty = true, ); pipelineOwner.rootNode = renderView; renderView.prepareInitialFrame(); - final rootElement = RenderObjectToWidgetAdapter( + rootElement = RenderObjectToWidgetAdapter( container: repaintBoundary, child: Directionality(textDirection: TextDirection.ltr, child: child), ).attachToRenderTree(buildOwner); @@ -331,6 +345,61 @@ class SlideCaptureService { } catch (e) { log('Error finalizing tree: $e'); rethrow; + } finally { + _releaseCaptureTree( + buildOwner: buildOwner, + rootElement: rootElement, + repaintBoundary: repaintBoundary, + rootBox: rootBox, + renderView: renderView, + pipelineOwner: pipelineOwner, + focusManager: focusManager, + ); + } + } + + /// Unmounts the temporary capture subtree and releases its owned resources. + /// + /// Rebuilding the root adapter without a child deactivates the captured + /// widgets, and [BuildOwner.finalizeTree] then unmounts them so their + /// [State.dispose] and render object disposal run. The render pipeline, + /// the render objects this service created, and the focus manager are + /// released afterwards. + void _releaseCaptureTree({ + required BuildOwner? buildOwner, + required RenderObjectToWidgetElement? rootElement, + required RenderRepaintBoundary? repaintBoundary, + required RenderPositionedBox? rootBox, + required RenderView? renderView, + required PipelineOwner? pipelineOwner, + required FocusManager? focusManager, + }) { + if (buildOwner != null && rootElement != null && repaintBoundary != null) { + try { + RenderObjectToWidgetAdapter( + container: repaintBoundary, + ).attachToRenderTree(buildOwner, rootElement); + buildOwner + ..buildScope(rootElement) + ..finalizeTree(); + } catch (e, stackTrace) { + log('Error unmounting capture tree: $e', stackTrace: stackTrace); + } + } + + if (pipelineOwner != null) { + pipelineOwner.rootNode = null; + pipelineOwner.dispose(); + } + + repaintBoundary?.dispose(); + rootBox?.dispose(); + renderView?.dispose(); + + if (focusManager != null) { + // Unmounting focus nodes schedules a focus update microtask. Disposing + // in a later microtask lets that update run against a live manager. + scheduleMicrotask(focusManager.dispose); } } } diff --git a/packages/superdeck/lib/src/deck/deck_presentation_state.dart b/packages/superdeck/lib/src/deck/deck_presentation_state.dart index 616717e0..ac320f28 100644 --- a/packages/superdeck/lib/src/deck/deck_presentation_state.dart +++ b/packages/superdeck/lib/src/deck/deck_presentation_state.dart @@ -19,6 +19,8 @@ final class DeckPresentationState { final _thumbnails = signal>({}); EffectCleanup? _indexClampEffect; + EffectCleanup? _thumbnailPruneEffect; + int _transitionOperation = 0; bool _disposed = false; late final GoRouter router = GoRouter( @@ -57,6 +59,11 @@ final class DeckPresentationState { _currentIndex.value = clamped; } }); + // Thumbnail cleanup follows the slide collection, not thumbnail warmup, + // so obsolete handles are released even when the deck becomes empty. + _thumbnailPruneEffect = effect(() { + _pruneThumbnails(_slideKeys(_slides.value)); + }); } ReadonlySignal get isMenuOpen => _isMenuOpen; @@ -96,10 +103,13 @@ final class DeckPresentationState { Future goToSlide(int index) async { if (_disposed || index < 0 || index >= _slides.value.length) return; + // Only the latest transition may clear the transitioning state, so an + // earlier delay cannot end a transition that started after it. + final operation = ++_transitionOperation; _isTransitioning.value = true; router.go('/slides/$index'); await Future.delayed(_transitionDuration); - if (_disposed) return; + if (_disposed || operation != _transitionOperation) return; _isTransitioning.value = false; } @@ -121,28 +131,14 @@ final class DeckPresentationState { bool force = false, }) { if (_disposed) return; - if (slides.isEmpty) return; - - final validKeys = slides.map((s) => s.key).toSet(); - final current = _thumbnails.value; - final staleKeys = current.keys - .where((k) => !validKeys.contains(k)) - .toList(growable: false); - final cache = staleKeys.isEmpty - ? current - : Map.from(current); - for (final key in staleKeys) { - cache.remove(key)?.dispose(); - } - if (staleKeys.isNotEmpty) { - _thumbnails.value = cache; - } + _pruneThumbnails(_slideKeys(slides)); + if (slides.isEmpty) return; _thumbnailService.generateThumbnails( slides: slides, context: context, - cache: cache, + cache: _thumbnails.peek(), onCacheUpdate: (updated) { if (_disposed) return; _thumbnails.value = updated; @@ -174,6 +170,7 @@ final class DeckPresentationState { void dispose() { _disposed = true; _indexClampEffect?.call(); + _thumbnailPruneEffect?.call(); router.routeInformationProvider.removeListener(_syncCurrentIndexFromRouter); router.dispose(); for (final thumbnail in _thumbnails.value.values) { @@ -190,6 +187,23 @@ final class DeckPresentationState { currentSlide.dispose(); } + /// Disposes and drops every thumbnail whose slide is no longer present. + void _pruneThumbnails(Set validKeys) { + if (_disposed) return; + + final current = _thumbnails.peek(); + final staleKeys = current.keys + .where((key) => !validKeys.contains(key)) + .toList(growable: false); + if (staleKeys.isEmpty) return; + + final cache = Map.from(current); + for (final key in staleKeys) { + cache.remove(key)?.dispose(); + } + _thumbnails.value = cache; + } + void _syncCurrentIndexFromRouter() { if (_disposed) return; final path = router.routeInformationProvider.value.uri.path; @@ -205,6 +219,9 @@ final class DeckPresentationState { } } + static Set _slideKeys(List slides) => + slides.map((slide) => slide.key).toSet(); + static int _clampIndex(int index, int totalSlides) { final maxIndex = totalSlides > 0 ? totalSlides - 1 : 0; return index.clamp(0, maxIndex); diff --git a/packages/superdeck/test/src/capture/slide_capture_teardown_test.dart b/packages/superdeck/test/src/capture/slide_capture_teardown_test.dart new file mode 100644 index 00000000..11fe88f8 --- /dev/null +++ b/packages/superdeck/test/src/capture/slide_capture_teardown_test.dart @@ -0,0 +1,177 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:superdeck/superdeck.dart'; +import 'package:superdeck_core/superdeck_core.dart'; + +/// Records the mount lifecycle of one widget inside a capture subtree. +class _LifecycleRecord { + var mounted = false; + var disposed = false; +} + +class _LifecycleWidget extends StatefulWidget { + final _LifecycleRecord record; + final bool trackReadiness; + final bool failBuild; + + const _LifecycleWidget({ + required this.record, + this.trackReadiness = false, + this.failBuild = false, + }); + + @override + State<_LifecycleWidget> createState() => _LifecycleWidgetState(); +} + +class _LifecycleWidgetState extends State<_LifecycleWidget> { + SlideCaptureReadinessHandle? _readiness; + + @override + void initState() { + super.initState(); + widget.record.mounted = true; + } + + @override + void didChangeDependencies() { + super.didChangeDependencies(); + if (widget.trackReadiness) { + _readiness ??= SlideCaptureReadiness.track( + context, + label: 'teardown-test-widget', + ); + } + } + + @override + void dispose() { + widget.record.disposed = true; + super.dispose(); + } + + @override + Widget build(BuildContext context) { + if (widget.failBuild) { + throw StateError('captured widget failed to build'); + } + + return const SizedBox.expand(); + } +} + +SlideConfiguration _lifecycleSlide( + _LifecycleRecord record, { + bool trackReadiness = false, + bool failBuild = false, +}) { + return SlideConfiguration( + slideIndex: 0, + style: SlideStyler(), + slide: Slide( + key: 'lifecycle', + sections: [ + SectionBlock([WidgetBlock(name: 'lifecycle', args: const {})]), + ], + ), + widgets: { + 'lifecycle': (_) => _LifecycleWidget( + record: record, + trackReadiness: trackReadiness, + failBuild: failBuild, + ), + }, + thumbnailKey: 'thumbnail_lifecycle.png', + ); +} + +Future _pumpContext(WidgetTester tester) async { + final key = GlobalKey(); + await tester.pumpWidget(MaterialApp(home: SizedBox(key: key))); + + return key.currentContext!; +} + +void main() { + group('Slide capture teardown', () { + testWidgets('unmounts the capture subtree after a successful capture', ( + tester, + ) async { + final context = await _pumpContext(tester); + final record = _LifecycleRecord(); + + await tester.runAsync(() async { + final bytes = await SlideCaptureService().capture( + slide: _lifecycleSlide(record), + context: context, + ); + + expect(bytes, isNotEmpty); + }); + + expect(record.mounted, isTrue); + expect(record.disposed, isTrue); + }); + + testWidgets('unmounts the capture subtree after the settle limit', ( + tester, + ) async { + final context = await _pumpContext(tester); + final record = _LifecycleRecord(); + + await tester.runAsync(() async { + final bytes = await SlideCaptureService().capture( + slide: _lifecycleSlide(record, trackReadiness: true), + context: context, + ); + + expect(bytes, isNotEmpty); + }); + + expect(record.mounted, isTrue); + expect(record.disposed, isTrue); + }); + + testWidgets('unmounts the capture subtree when a captured widget fails', ( + tester, + ) async { + final context = await _pumpContext(tester); + final record = _LifecycleRecord(); + + await tester.runAsync(() async { + final bytes = await SlideCaptureService().capture( + slide: _lifecycleSlide(record, failBuild: true), + context: context, + ); + + expect(bytes, isNotEmpty); + }); + + expect(tester.takeException(), isStateError); + expect(record.mounted, isTrue); + expect(record.disposed, isTrue); + }); + + testWidgets('releases the subtree of every capture in a sequence', ( + tester, + ) async { + final context = await _pumpContext(tester); + final records = List.generate(3, (_) => _LifecycleRecord()); + final service = SlideCaptureService(); + + await tester.runAsync(() async { + for (final record in records) { + final bytes = await service.capture( + slide: _lifecycleSlide(record), + context: context, + ); + + expect(bytes, isNotEmpty); + } + }); + + expect(records.every((record) => record.mounted), isTrue); + expect(records.every((record) => record.disposed), isTrue); + }); + }); +} diff --git a/packages/superdeck/test/src/deck/deck_presentation_state_test.dart b/packages/superdeck/test/src/deck/deck_presentation_state_test.dart index d79b407a..df431dd0 100644 --- a/packages/superdeck/test/src/deck/deck_presentation_state_test.dart +++ b/packages/superdeck/test/src/deck/deck_presentation_state_test.dart @@ -114,6 +114,117 @@ void main() { expect(service.receivedCacheKeys.last, equals({'slide-0'})); }); + testWidgets('slide removal disposes stale thumbnails without warmup', ( + tester, + ) async { + final slides = signal>(createTestSlides(2)); + addTearDown(slides.dispose); + + final service = _RecordingThumbnailService(); + final state = DeckPresentationState( + thumbnailService: service, + slides: slides, + transitionDuration: Duration.zero, + ); + addTearDown(state.dispose); + + final context = await _pumpContext(tester); + state.generateThumbnails(context, slides.value); + final staleThumbnail = service.trackedThumbnails['slide-1']!; + final callsBeforeRemoval = service.callCount; + + slides.value = [slides.value.first]; + + expect(service.callCount, callsBeforeRemoval); + expect(staleThumbnail.disposed, isTrue); + expect(state.getThumbnail('slide-0'), isNotNull); + expect(state.getThumbnail('slide-1'), isNull); + }); + + testWidgets('an empty slide collection disposes every thumbnail', ( + tester, + ) async { + final slides = signal>(createTestSlides(2)); + addTearDown(slides.dispose); + + final service = _RecordingThumbnailService(); + final state = DeckPresentationState( + thumbnailService: service, + slides: slides, + transitionDuration: Duration.zero, + ); + addTearDown(state.dispose); + + final context = await _pumpContext(tester); + state.generateThumbnails(context, slides.value); + final slide0 = service.trackedThumbnails['slide-0']!; + final slide1 = service.trackedThumbnails['slide-1']!; + + slides.value = const []; + + expect(slide0.disposed, isTrue); + expect(slide1.disposed, isTrue); + expect(state.getThumbnail('slide-0'), isNull); + expect(state.getThumbnail('slide-1'), isNull); + }); + + testWidgets('an empty warmup list disposes every thumbnail', ( + tester, + ) async { + final slides = signal>(createTestSlides(2)); + addTearDown(slides.dispose); + + final service = _RecordingThumbnailService(); + final state = DeckPresentationState( + thumbnailService: service, + slides: slides, + transitionDuration: Duration.zero, + ); + addTearDown(state.dispose); + + final context = await _pumpContext(tester); + state.generateThumbnails(context, slides.value); + final slide0 = service.trackedThumbnails['slide-0']!; + final slide1 = service.trackedThumbnails['slide-1']!; + + state.generateThumbnails(context, const []); + + expect(slide0.disposed, isTrue); + expect(slide1.disposed, isTrue); + expect(state.getThumbnail('slide-0'), isNull); + expect(state.getThumbnail('slide-1'), isNull); + }); + + testWidgets('a superseded transition keeps the latest transition open', ( + tester, + ) async { + const transitionDuration = Duration(milliseconds: 300); + final slides = signal>(createTestSlides(3)); + addTearDown(slides.dispose); + + final service = _RecordingThumbnailService(); + final state = DeckPresentationState( + thumbnailService: service, + slides: slides, + transitionDuration: transitionDuration, + ); + addTearDown(state.dispose); + + final first = state.goToSlide(1); + await tester.pump(const Duration(milliseconds: 200)); + final second = state.goToSlide(2); + + // The first transition's delay elapses while the second one is running. + await tester.pump(const Duration(milliseconds: 150)); + expect(state.isTransitioning.value, isTrue); + + await tester.pump(const Duration(milliseconds: 200)); + await first; + await second; + + expect(state.isTransitioning.value, isFalse); + }); + testWidgets('deleteAllThumbnails disposes cache and clears getThumbnail', ( tester, ) async {