From 414af2236fc54ec24e4da102019bd754311363c4 Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Tue, 29 Sep 2026 00:41:04 -0400 Subject: [PATCH 1/8] Take screenshots on the web beta, and of cloud strategies anywhere The screenshot button read the open strategy back from the local Hive box after a forced save. Cloud strategies are never in that box, so a screenshot of one silently did nothing, on desktop as well as web. It then wrote the PNG with dart:io, which a browser cannot do, which is why the web beta hid the button behind "desktop-only". The capture now reads the page as it is on the canvas (the editor's live providers, drawings copied so their cached paths stay the editor's), so local and cloud strategies capture the same way and nothing waits on a save or a sync. The PNG goes out through FilePicker.saveFile(bytes:), which writes the chosen file on desktop and downloads it in a browser. Images: the capture renders in its own provider container, which has no live cloud page, upload queue or pending bytes. Before it starts, every image on the page is resolved to what it will paint and handed in through captureImageSourcesProvider: files and in-memory bytes as they are, cloud URLs fetched to bytes (downloadCloudImageBytes, shared with the desktop media cache, including its one signed-URL refresh), and images the editor shows as unavailable stay unavailable. An image still loading or a fetch that fails stops the capture with a message instead of saving a picture with the image missing. PendingImageBytes becomes ImageBytes, since bytes fetched for a capture are not an upload. Screenshot leaves PlatformPolicy.webBeta.desktopOnly. Co-Authored-By: Claude Opus 5.5 (1M context) --- lib/config/platform_policy.dart | 1 - .../collab/cloud_media_cache_provider.dart | 75 ++-- lib/providers/strategy_image_source.dart | 58 +++- lib/screenshot/capture_images.dart | 60 ++++ lib/screenshot/page_screenshot.dart | 169 +++++++++ .../draggable_widgets/image/image_widget.dart | 2 +- lib/widgets/editor_toolbar.dart | 109 ++---- test/capture_images_test.dart | 70 ++++ test/page_screenshot_test.dart | 322 ++++++++++++++++++ test/screenshot_export_failure_test.dart | 48 --- test/strategy_image_source_test.dart | 4 +- test/widgets/web_beta_library_test.dart | 2 +- 12 files changed, 732 insertions(+), 188 deletions(-) create mode 100644 lib/screenshot/capture_images.dart create mode 100644 lib/screenshot/page_screenshot.dart create mode 100644 test/capture_images_test.dart create mode 100644 test/page_screenshot_test.dart delete mode 100644 test/screenshot_export_failure_test.dart diff --git a/lib/config/platform_policy.dart b/lib/config/platform_policy.dart index e388301f..c5b228c3 100644 --- a/lib/config/platform_policy.dart +++ b/lib/config/platform_policy.dart @@ -47,7 +47,6 @@ class PlatformPolicy { desktopOnly: { PlatformFeature.exportFiles, PlatformFeature.importFiles, - PlatformFeature.screenshot, PlatformFeature.videoExport, PlatformFeature.fileDrop, }, diff --git a/lib/providers/collab/cloud_media_cache_provider.dart b/lib/providers/collab/cloud_media_cache_provider.dart index 424f5fac..08c6b0f0 100644 --- a/lib/providers/collab/cloud_media_cache_provider.dart +++ b/lib/providers/collab/cloud_media_cache_provider.dart @@ -1,5 +1,6 @@ import 'dart:async'; import 'dart:io'; +import 'dart:typed_data'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:http/http.dart' as http; @@ -39,6 +40,34 @@ class CloudMediaCacheState { } } +class CloudImageDownloadException implements Exception { + const CloudImageDownloadException(this.statusCode); + final int statusCode; + + @override + String toString() => 'Failed to download image ($statusCode).'; +} + +/// Downloads a cloud image's bytes from [url]. A signed URL the host refuses +/// (401, 403, or 404: it expired) is swapped once for [freshUrl] from the +/// server. Throws [CloudImageDownloadException] when the bytes don't come. +Future downloadCloudImageBytes( + String url, { + required Future Function() freshUrl, +}) async { + var response = await http.get(Uri.parse(url)); + if (const {401, 403, 404}.contains(response.statusCode)) { + final refreshed = await freshUrl(); + if (refreshed != null && refreshed.isNotEmpty) { + response = await http.get(Uri.parse(refreshed)); + } + } + if (response.statusCode < 200 || response.statusCode >= 300) { + throw CloudImageDownloadException(response.statusCode); + } + return response.bodyBytes; +} + final cloudMediaCacheProvider = NotifierProvider( CloudMediaCacheNotifier.new, @@ -122,32 +151,20 @@ class CloudMediaCacheNotifier extends Notifier { _markInFlight(asset.publicId, strategyPublicId); try { - var response = await http.get(Uri.parse(asset.url!)); - if (_shouldRefreshSignedUrl(response.statusCode)) { - final linkView = ref.read(shareLinkViewProvider); - final refreshed = await ref - .read(convexStrategyRepositoryProvider) - .getImageAssetUrl( - strategyPublicId: strategyPublicId, - assetPublicId: asset.publicId, - // A signed-out reader's only access is the link they opened. - shareToken: linkView?.strategyPublicId == strategyPublicId - ? linkView!.token - : null, - ); - if (refreshed != null && refreshed.isNotEmpty) { - response = await http.get(Uri.parse(refreshed)); - } - } - - if (response.statusCode < 200 || response.statusCode >= 300) { - _recordError( - asset.publicId, - 'Failed to cache asset (${response.statusCode}).', - ); - return null; - } - + final bytes = await downloadCloudImageBytes( + asset.url!, + freshUrl: () { + final linkView = ref.read(shareLinkViewProvider); + return ref.read(convexStrategyRepositoryProvider).getImageAssetUrl( + strategyPublicId: strategyPublicId, + assetPublicId: asset.publicId, + // A signed-out reader's only access is the link they opened. + shareToken: linkView?.strategyPublicId == strategyPublicId + ? linkView!.token + : null, + ); + }, + ); final output = File( await localAssetPath( strategyId: strategyId, @@ -156,7 +173,7 @@ class CloudMediaCacheNotifier extends Notifier { ), ); await output.parent.create(recursive: true); - await output.writeAsBytes(response.bodyBytes, flush: true); + await output.writeAsBytes(bytes, flush: true); _markCached(asset.publicId); return output; } catch (error) { @@ -190,10 +207,6 @@ class CloudMediaCacheNotifier extends Notifier { return true; } - bool _shouldRefreshSignedUrl(int statusCode) { - return statusCode == 401 || statusCode == 403 || statusCode == 404; - } - void resetStrategy(String? strategyPublicId) { state = CloudMediaCacheState(strategyPublicId: strategyPublicId); } diff --git a/lib/providers/strategy_image_source.dart b/lib/providers/strategy_image_source.dart index ec3eb24c..9326b74c 100644 --- a/lib/providers/strategy_image_source.dart +++ b/lib/providers/strategy_image_source.dart @@ -25,7 +25,7 @@ sealed class StrategyImageSource { // browser fetch the bytes. webHtmlElementStrategy: WebHtmlElementStrategy.fallback, ), - PendingImageBytes(:final bytes) => MemoryImage(bytes), + ImageBytes(:final bytes) => MemoryImage(bytes), ImageLoading() || ImageFailed() => null, }; } @@ -42,10 +42,11 @@ final class RemoteImageUrl extends StrategyImageSource { final String url; } -/// Bytes this device is still uploading, painted until the cloud URL -/// arrives. Only where images are not files (web). -final class PendingImageBytes extends StrategyImageSource { - const PendingImageBytes(this.bytes); +/// Bytes in memory: an image this device is still uploading, painted until +/// the cloud URL arrives (only where images are not files, on web), or one +/// fetched ahead of an offscreen capture. +final class ImageBytes extends StrategyImageSource { + const ImageBytes(this.bytes); final Uint8List bytes; } @@ -63,6 +64,16 @@ final class ImageFailed extends StrategyImageSource { typedef StrategyImageKey = ({String id, String? fileExtension}); +/// What each image paints in an offscreen capture, by image id, and null +/// everywhere else. +/// +/// A capture renders in its own provider container, which has no live cloud +/// page, upload queue, or pending bytes to resolve an image from. The +/// capture resolves every image it paints before it starts and overrides +/// this provider with the result. +final captureImageSourcesProvider = + Provider?>((ref) => null); + /// Where the bytes for [image] come from, read from a widget's build. /// /// The file check runs on every build, so a file written or removed while @@ -71,12 +82,29 @@ typedef StrategyImageKey = ({String id, String? fileExtension}); StrategyImageSource watchStrategyImageSource( WidgetRef ref, StrategyImageKey image, +) => + _strategyImageSource(ref.watch, image); + +/// Where the bytes for [image] come from right now, read once, as the +/// editor would paint it. +StrategyImageSource readStrategyImageSource( + WidgetRef ref, + StrategyImageKey image, +) => + _strategyImageSource(ref.read, image); + +StrategyImageSource _strategyImageSource( + T Function(ProviderListenable provider) watch, + StrategyImageKey image, ) { - final (storageDirectory, source, strategyId) = ref.watch( + final captured = watch(captureImageSourcesProvider); + if (captured != null) return captured[image.id] ?? const ImageFailed(); + + final (storageDirectory, source, strategyId) = watch( strategyProvider .select((s) => (s.storageDirectory, s.source, s.strategyId)), ); - final (assetsLoaded, remoteAsset) = ref.watch( + final (assetsLoaded, remoteAsset) = watch( remoteEditorSnapshotProvider.select((snapshot) { final page = snapshot.valueOrNull?.activePage; return (page != null, page?.assetsById[image.id]); @@ -87,8 +115,8 @@ StrategyImageSource watchStrategyImageSource( // account is known, or while the outbox holds records it could not read, // the queue cannot rule a queued upload out. final uploadMayBeQueuedHere = isCloudStrategy && - (ref.watch(cloudMediaAccountIdProvider) == null || - ref.watch( + (watch(cloudMediaAccountIdProvider) == null || + watch( cloudMediaUploadQueueProvider.select( (queue) => !queue.outboxIsReliable || @@ -97,21 +125,21 @@ StrategyImageSource watchStrategyImageSource( ), ), )); - ref.watch(cloudMediaCacheProvider); + watch(cloudMediaCacheProvider); // Bytes this browser is still uploading for the signed-in account. They // only paint when neither the file check below nor the cloud URL has // anything. Where images are files there are never pending bytes. Uint8List? pendingBytes; - if (!ref.watch(imageFilesOnDeviceProvider)) { - final accountId = ref.watch(cloudMediaAccountIdProvider); + if (!watch(imageFilesOnDeviceProvider)) { + final accountId = watch(cloudMediaAccountIdProvider); if (accountId != null && strategyId != null) { final key = pendingMediaStorageKey(( accountId: accountId, strategyPublicId: strategyId, assetPublicId: image.id, )); - pendingBytes = ref - .watch(pendingMediaBytesProvider.select((pending) => pending[key])); + pendingBytes = + watch(pendingMediaBytesProvider.select((pending) => pending[key])); } } @@ -148,7 +176,7 @@ StrategyImageSource resolveStrategyImageSource({ if (localFilePath != null) return LocalImageFile(localFilePath); final url = remoteAsset?.url; if (url != null && url.isNotEmpty) return RemoteImageUrl(url); - if (pendingBytes != null) return PendingImageBytes(pendingBytes); + if (pendingBytes != null) return ImageBytes(pendingBytes); if (!isCloudStrategy) return const ImageFailed(); if (remoteAsset != null) { return remoteAsset.uploadStatus == 'failed' diff --git a/lib/screenshot/capture_images.dart b/lib/screenshot/capture_images.dart new file mode 100644 index 00000000..81c357aa --- /dev/null +++ b/lib/screenshot/capture_images.dart @@ -0,0 +1,60 @@ +import 'dart:typed_data'; + +import 'package:icarus/providers/strategy_image_source.dart'; + +/// An image a capture would paint is not ready: its cloud copy is still +/// loading, or its bytes could not be fetched. The capture stops rather than +/// save a picture with the image missing. +class CaptureImagesUnavailable implements Exception { + const CaptureImagesUnavailable.stillLoading() : cause = null; + const CaptureImagesUnavailable.fetchFailed(this.cause); + + /// Why the fetch failed; null while an image is still loading. + final Object? cause; + + String get userMessage => cause == null + ? 'Images on this page are still loading. Try again in a moment.' + : "Couldn't load the images on this page. Check your connection and " + 'try again.'; + + @override + String toString() => cause == null + ? 'CaptureImagesUnavailable: an image is still loading' + : 'CaptureImagesUnavailable: $cause'; +} + +/// Fetches the bytes behind a cloud image's [url]. +typedef CaptureImageFetcher = Future Function( + String imageId, + String url, +); + +/// Turns where each image's bytes come from, by image id, into what an +/// offscreen capture can paint ([captureImageSourcesProvider]). +/// +/// A capture has no network reads of its own, so cloud URLs are fetched here +/// and painted from memory. Files and bytes already in memory paint as they +/// are, and an image the editor shows as unavailable is captured that way +/// too. Throws [CaptureImagesUnavailable] while an image is still loading or +/// when a fetch fails. +Future> resolveCaptureImageSources( + Map sources, { + required CaptureImageFetcher fetch, +}) async { + final resolved = {}; + for (final MapEntry(key: imageId, value: source) in sources.entries) { + switch (source) { + case RemoteImageUrl(:final url): + try { + resolved[imageId] = ImageBytes(await fetch(imageId, url)); + } catch (error) { + throw CaptureImagesUnavailable.fetchFailed(error); + } + case ImageLoading(): + throw const CaptureImagesUnavailable.stillLoading(); + case LocalImageFile() || ImageBytes() || ImageFailed(): + resolved[imageId] = source; + } + } + return resolved; +} diff --git a/lib/screenshot/page_screenshot.dart b/lib/screenshot/page_screenshot.dart new file mode 100644 index 00000000..1a6e5315 --- /dev/null +++ b/lib/screenshot/page_screenshot.dart @@ -0,0 +1,169 @@ +import 'dart:typed_data'; + +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:hive_ce/hive.dart'; +import 'package:icarus/collab/convex_strategy_repository.dart'; +import 'package:icarus/const/coordinate_system.dart'; +import 'package:icarus/const/hive_boxes.dart'; +import 'package:icarus/const/line_provider.dart'; +import 'package:icarus/providers/ability_provider.dart'; +import 'package:icarus/providers/agent_provider.dart'; +import 'package:icarus/providers/collab/cloud_media_cache_provider.dart'; +import 'package:icarus/providers/collab/remote_strategy_snapshot_provider.dart'; +import 'package:icarus/providers/drawing_provider.dart'; +import 'package:icarus/providers/image_provider.dart'; +import 'package:icarus/providers/map_provider.dart'; +import 'package:icarus/providers/strategy_image_source.dart'; +import 'package:icarus/providers/strategy_page.dart'; +import 'package:icarus/providers/strategy_page_session_provider.dart'; +import 'package:icarus/providers/strategy_provider.dart'; +import 'package:icarus/providers/strategy_settings_provider.dart'; +import 'package:icarus/providers/text_provider.dart'; +import 'package:icarus/providers/user_preferences_provider.dart'; +import 'package:icarus/providers/utility_provider.dart'; +import 'package:icarus/screenshot/capture_geometry.dart'; +import 'package:icarus/screenshot/capture_images.dart'; +import 'package:icarus/screenshot/offscreen_capture.dart'; +import 'package:icarus/screenshot/persistent_offscreen_renderer.dart'; +import 'package:icarus/screenshot/screenshot_view.dart'; +import 'package:icarus/strategy/strategy_page_models.dart'; + +/// Renders the page open in the editor, as it is on screen, to a PNG. +/// +/// The capture reads the editor's live state rather than a saved copy, so a +/// cloud strategy captures exactly like a local one and nothing waits on a +/// save or a sync. Throws [CaptureImagesUnavailable] rather than return a +/// picture with an image missing. +Future captureEditorPage(WidgetRef ref) async { + final strategyState = ref.read(strategyProvider); + final strategyId = strategyState.strategyId; + if (strategyId == null) { + throw StateError('No strategy is open to capture.'); + } + final page = _editorPage(ref); + final mapState = ref.read(mapProvider); + final theme = ref.read(strategyThemeProvider); + + final images = await resolveCaptureImageSources( + { + for (final image in page.imageData) + image.id: readStrategyImageSource( + ref, + (id: image.id, fileExtension: image.fileExtension), + ), + }, + fetch: (imageId, url) => downloadCloudImageBytes( + url, + freshUrl: () => + ref.read(convexStrategyRepositoryProvider).getImageAssetUrl( + strategyPublicId: strategyId, + assetPublicId: imageId, + ), + ), + ); + + final view = ScreenshotView( + isAttack: page.isAttack, + mapValue: mapState.currentMap, + showSpawnBarrier: mapState.showSpawnBarrier, + showRegionNames: mapState.showRegionNames, + showUltOrbs: mapState.showUltOrbs, + agents: page.agentData, + abilities: page.abilityData, + text: page.textData, + images: page.imageData, + drawings: page.drawingData, + utilities: page.utilityData, + strategySettings: page.settings, + strategyState: strategyState, + pageName: page.name, + lineUpGraph: page.lineUpGraph, + themeProfileId: theme.profileId, + themeOverridePalette: theme.overridePalette, + ); + + final container = ProviderContainer( + overrides: [captureImageSourcesProvider.overrideWithValue(images)], + ); + CaptureGeometryLease? geometry; + try { + // The sightline models load asynchronously; the capture waits for the + // page's geometry before the first frame is taken. + geometry = await prepareCaptureGeometry( + container, + mapState.currentMap, + [page], + ); + final waitForFrame = geometry?.waitForFrame; + return await withScreenshotCoordinates(() async { + view.hydrateProviders(container); + final renderer = PersistentOffscreenRenderer( + targetSize: CoordinateSystem.screenShotSize, + waitForFrameData: waitForFrame, + wrapWidget: (child) => + wrapForOffscreenCapture(child, container: container), + ); + try { + await renderer.prepare( + view, + settleDuration: const Duration(milliseconds: 800), + ); + return await renderer.capture(view); + } finally { + await renderer.dispose(); + } + }); + } finally { + geometry?.close(); + container.dispose(); + } +} + +/// The page on the canvas right now. Drawings are copied: a capture +/// rebuilds their paths in screenshot coordinates, and the editor's own +/// strokes must keep theirs. +StrategyPage _editorPage(WidgetRef ref) { + final lineUps = ref.read(lineUpProvider).graph; + return StrategyPage( + id: ref.read(strategyPageSessionProvider).activePageId ?? '', + name: _activePageName(ref) ?? '', + drawingData: DrawingProvider.fromJson( + DrawingProvider.objectToJson(ref.read(drawingProvider).elements), + ), + agentData: ref.read(agentProvider), + abilityData: ref.read(abilityProvider), + textData: ref.read(textProvider.notifier).snapshotForPersistence(), + imageData: ref.read(placedImageProvider).images, + utilityData: ref.read(utilityProvider), + sortIndex: 0, + isAttack: ref.read(mapProvider).isAttack, + settings: ref.read(strategySettingsProvider), + lineUpOrigins: lineUps.origins, + lineUpLandings: lineUps.landings, + lineUpLinks: lineUps.links, + ); +} + +/// The open page's name, from wherever its strategy lives, as the pages bar +/// shows it. +String? _activePageName(WidgetRef ref) { + final strategy = ref.read(strategyProvider); + final pageId = ref.read(strategyPageSessionProvider).activePageId; + if (pageId == null) return null; + return switch (strategy.source) { + StrategySource.cloud => ref + .read(remoteEditorSnapshotProvider) + .valueOrNull + ?.pages + .where((page) => page.publicId == pageId) + .firstOrNull + ?.name, + StrategySource.local => Hive.box(HiveBoxNames.strategiesBox) + .get(strategy.strategyId) + ?.pages + .where((page) => page.id == pageId) + .firstOrNull + ?.name, + null => null, + }; +} diff --git a/lib/widgets/draggable_widgets/image/image_widget.dart b/lib/widgets/draggable_widgets/image/image_widget.dart index 132b4286..f32a87ce 100644 --- a/lib/widgets/draggable_widgets/image/image_widget.dart +++ b/lib/widgets/draggable_widgets/image/image_widget.dart @@ -169,7 +169,7 @@ class _ImageWidgetState extends ConsumerState { // Gapless, keyed by asset: this image's pending bytes keep painting // while its cloud URL loads, and no other image's frame carries // over. - LocalImageFile() || RemoteImageUrl() || PendingImageBytes() => Image( + LocalImageFile() || RemoteImageUrl() || ImageBytes() => Image( key: ValueKey(widget.id), image: image!, fit: BoxFit.contain, diff --git a/lib/widgets/editor_toolbar.dart b/lib/widgets/editor_toolbar.dart index e6a7a650..afd9353e 100644 --- a/lib/widgets/editor_toolbar.dart +++ b/lib/widgets/editor_toolbar.dart @@ -1,25 +1,17 @@ -import 'dart:io'; - import 'package:file_picker/file_picker.dart'; import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; -import 'package:hive_ce/hive.dart'; import 'package:icarus/const/coordinate_system.dart'; -import 'package:icarus/const/hive_boxes.dart'; import 'package:icarus/const/settings.dart'; import 'package:icarus/providers/drawing_provider.dart'; -import 'package:icarus/providers/map_provider.dart'; import 'package:icarus/providers/screenshot_provider.dart'; import 'package:icarus/providers/share_link_provider.dart'; -import 'package:icarus/providers/strategy_page_session_provider.dart'; import 'package:icarus/providers/strategy_provider.dart'; -import 'package:icarus/screenshot/capture_geometry.dart'; -import 'package:icarus/screenshot/offscreen_capture.dart'; -import 'package:icarus/screenshot/persistent_offscreen_renderer.dart'; +import 'package:icarus/screenshot/capture_images.dart'; +import 'package:icarus/screenshot/page_screenshot.dart'; import 'package:icarus/services/app_error_reporter.dart'; import 'package:icarus/services/cloud_strategy_export.dart'; -import 'package:icarus/screenshot/screenshot_view.dart'; import 'package:icarus/strategy/strategy_import_export.dart'; import 'package:icarus/strategy/strategy_page_models.dart'; import 'package:icarus/widgets/cloud_sync_button.dart'; @@ -177,96 +169,41 @@ class _EditorToolbarState extends ConsumerState { Future _captureScreenshot() async { if (!ensureFeatureAvailable(ref, PlatformFeature.screenshot)) return; if (_isCapturingScreenshot) return; + final strategy = ref.read(strategyProvider); + if (strategy.strategyId == null) return; setState(() => _isCapturingScreenshot = true); - ProviderContainer? screenshotContainer; - CaptureGeometryLease? captureGeometry; try { - final id = ref.read(strategyProvider).strategyId; - if (id == null) return; - - await ref.read(strategyProvider.notifier).forceSaveNow(id); - if (!mounted) return; - - final newStrat = Hive.box( - HiveBoxNames.strategiesBox, - ).values.where((StrategyData strategy) => strategy.id == id).firstOrNull; - if (newStrat == null) return; - - final currentPageID = ref.read(strategyPageSessionProvider).activePageId; - final mapState = ref.read(mapProvider); - if (currentPageID == null) return; - - final activePage = newStrat.pages.firstWhere( - (p) => p.id == currentPageID, - orElse: () => newStrat.pages.first, - ); - final captureContainer = ProviderContainer(); - screenshotContainer = captureContainer; - - final screenshotView = ScreenshotView( - isAttack: activePage.isAttack, - mapValue: newStrat.mapData, - showSpawnBarrier: mapState.showSpawnBarrier, - showRegionNames: mapState.showRegionNames, - showUltOrbs: mapState.showUltOrbs, - agents: activePage.agentData, - abilities: activePage.abilityData, - text: activePage.textData, - images: activePage.imageData, - drawings: activePage.drawingData, - utilities: activePage.utilityData, - strategySettings: activePage.settings, - strategyState: ref.read(strategyProvider), - pageName: activePage.name, - lineUpGraph: activePage.lineUpGraph, - themeProfileId: newStrat.themeProfileId, - themeOverridePalette: newStrat.themeOverridePalette, - ); - // The sightline models load asynchronously; the capture waits for the - // page's geometry before the first frame is taken. - captureGeometry = await prepareCaptureGeometry( - captureContainer, - newStrat.mapData, - [activePage], - ); - late Uint8List image; + late final Uint8List image; try { - image = await withScreenshotCoordinates(() async { - screenshotView.hydrateProviders(captureContainer); - final renderer = PersistentOffscreenRenderer( - targetSize: CoordinateSystem.screenShotSize, - waitForFrameData: captureGeometry?.waitForFrame, - wrapWidget: (child) => - wrapForOffscreenCapture(child, container: captureContainer)); - try { - await renderer.prepare(screenshotView, - settleDuration: const Duration(milliseconds: 800)); - return await renderer.capture(screenshotView); - } finally { - await renderer.dispose(); - } - }); + image = await captureEditorPage(ref); } finally { if (mounted) { ref.read(screenshotProvider.notifier).setIsScreenShot(false); ref .read(drawingProvider.notifier) .rebuildAllPaths(CoordinateSystem.instance); + setState(() => _isCapturingScreenshot = false); } } if (!mounted) return; - setState(() => _isCapturingScreenshot = false); - String? outputFile = await FilePicker.platform.saveFile( + // Desktop asks where to save and writes the bytes there; the browser + // downloads them. + await FilePicker.platform.saveFile( type: FileType.custom, dialogTitle: 'Please select an output file:', fileName: - "${ref.read(strategyProvider).strategyName ?? "new image"}.png", + '${sanitizeStrategyFileName(strategy.strategyName ?? 'new image')}.png', allowedExtensions: ['png'], + bytes: image, + ); + } on CaptureImagesUnavailable catch (error, stackTrace) { + AppErrorReporter.reportWarning( + error.userMessage, + error: error, + stackTrace: stackTrace, + source: 'EditorToolbar.screenshot', + promptUser: true, ); - if (outputFile != null) { - final file = File(outputFile); - await file.writeAsBytes(image); - } } catch (error, stackTrace) { AppErrorReporter.reportError( 'Could not export the screenshot. Please try again.', @@ -274,12 +211,6 @@ class _EditorToolbarState extends ConsumerState { stackTrace: stackTrace, source: 'EditorToolbar.screenshot', ); - } finally { - captureGeometry?.close(); - screenshotContainer?.dispose(); - if (mounted && _isCapturingScreenshot) { - setState(() => _isCapturingScreenshot = false); - } } } } diff --git a/test/capture_images_test.dart b/test/capture_images_test.dart new file mode 100644 index 00000000..03889599 --- /dev/null +++ b/test/capture_images_test.dart @@ -0,0 +1,70 @@ +import 'dart:typed_data'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:icarus/providers/strategy_image_source.dart'; +import 'package:icarus/screenshot/capture_images.dart'; + +void main() { + final fetched = Uint8List.fromList([1, 2, 3]); + final pending = Uint8List.fromList([4, 5, 6]); + + test('cloud URLs are fetched; everything else paints as the editor would', + () async { + final requests = <(String, String)>[]; + final resolved = await resolveCaptureImageSources( + { + 'remote': const RemoteImageUrl('https://media.example.com/remote.png'), + 'file': const LocalImageFile('/images/file.png'), + 'uploading': ImageBytes(pending), + 'failed': const ImageFailed(), + }, + fetch: (imageId, url) async { + requests.add((imageId, url)); + return fetched; + }, + ); + + expect(requests, [('remote', 'https://media.example.com/remote.png')]); + expect((resolved['remote'] as ImageBytes).bytes, fetched); + expect((resolved['file'] as LocalImageFile).path, '/images/file.png'); + expect((resolved['uploading'] as ImageBytes).bytes, pending); + expect(resolved['failed'], isA()); + }); + + test('an image still loading stops the capture', () async { + await expectLater( + resolveCaptureImageSources( + {'loading': const ImageLoading()}, + fetch: (_, __) async => fetched, + ), + throwsA( + isA() + .having((error) => error.cause, 'cause', isNull) + .having( + (error) => error.userMessage, + 'userMessage', + contains('still loading'), + ), + ), + ); + }); + + test('a failed fetch stops the capture and keeps its cause', () async { + final failure = Exception('offline'); + await expectLater( + resolveCaptureImageSources( + {'remote': const RemoteImageUrl('https://media.example.com/r.png')}, + fetch: (_, __) async => throw failure, + ), + throwsA( + isA() + .having((error) => error.cause, 'cause', same(failure)) + .having( + (error) => error.userMessage, + 'userMessage', + contains('connection'), + ), + ), + ); + }); +} diff --git a/test/page_screenshot_test.dart b/test/page_screenshot_test.dart new file mode 100644 index 00000000..a9aa2ab5 --- /dev/null +++ b/test/page_screenshot_test.dart @@ -0,0 +1,322 @@ +import 'dart:async'; +import 'dart:io'; +import 'dart:typed_data'; + +import 'package:flutter/material.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:hive_ce/hive.dart'; +import 'package:http/http.dart' as http; +import 'package:http/testing.dart'; +import 'package:icarus/collab/collab_models.dart'; +import 'package:icarus/collab/pending_media_bytes_store.dart'; +import 'package:icarus/const/app_provider_container.dart'; +import 'package:icarus/const/coordinate_system.dart'; +import 'package:icarus/const/hive_boxes.dart'; +import 'package:icarus/const/maps.dart'; +import 'package:icarus/const/placed_classes.dart'; +import 'package:icarus/const/image_scale_policy.dart'; +import 'package:icarus/hive/hive_registration.dart'; +import 'package:icarus/providers/collab/cloud_media_upload_queue_provider.dart'; +import 'package:icarus/providers/collab/media_bytes_source.dart'; +import 'package:icarus/providers/collab/remote_strategy_snapshot_provider.dart'; +import 'package:icarus/providers/collab/strategy_op_queue_provider.dart'; +import 'package:icarus/providers/image_provider.dart'; +import 'package:icarus/providers/strategy_page_session_provider.dart'; +import 'package:icarus/providers/strategy_provider.dart'; +import 'package:icarus/providers/user_preferences_provider.dart'; +import 'package:icarus/screenshot/capture_images.dart'; +import 'package:icarus/screenshot/page_screenshot.dart'; +import 'package:icarus/strategy/strategy_page_models.dart'; +import 'package:icarus/widgets/editor_toolbar.dart'; +import 'package:image/image.dart' as img; +import 'package:shadcn_ui/shadcn_ui.dart'; + +const _strategyId = 'cloud-strategy'; +const _pageId = 'page-1'; +const _imageId = 'image-1'; +const _imageUrl = 'https://media.example.com/image-1.png'; + +/// A cloud strategy open in the editor. Cloud strategies are never in the +/// local Hive box, which is what a capture used to read. +class _CloudStrategy extends StrategyProvider { + @override + StrategyState build() => const StrategyState( + strategyId: _strategyId, + strategyName: 'Cloud strat', + source: StrategySource.cloud, + storageDirectory: null, + isOpen: true, + ); +} + +class _CloudSnapshot extends RemoteEditorSnapshotNotifier { + _CloudSnapshot({required this.imageUrl}); + + final String? imageUrl; + + @override + Future build() async { + final now = DateTime.utc(2026); + final page = RemotePage( + publicId: _pageId, + strategyPublicId: _strategyId, + name: 'Retake B', + sortIndex: 0, + isAttack: true, + revision: 1, + createdAt: now, + updatedAt: now, + ); + return RemoteEditorSnapshot( + shell: RemoteStrategyShell( + header: RemoteStrategyHeader( + publicId: _strategyId, + name: 'Cloud strat', + mapData: Maps.mapNames[MapValue.ascent]!, + revision: 1, + createdAt: now, + updatedAt: now, + role: 'owner', + ), + pages: [page], + ), + activePage: RemotePageSnapshot( + page: page, + content: RemotePageContent(revision: 1, createdAt: now, updatedAt: now), + elements: const [], + lineups: const [], + assetsById: { + _imageId: RemoteImageAsset( + publicId: _imageId, + fileExtension: '.png', + width: 64, + height: 36, + url: imageUrl, + legacyStoragePath: null, + uploadStatus: imageUrl == null ? 'pending' : 'active', + ), + }, + ), + ); + } +} + +class _IdleOpQueue extends StrategyOpQueueNotifier { + @override + StrategyOpQueueState build() => const StrategyOpQueueState(); +} + +class _NoUploads extends CloudMediaUploadQueueNotifier { + @override + CloudMediaUploadQueueState build() => + const CloudMediaUploadQueueState(jobs: [], isProcessing: false); +} + +/// Pure magenta, a colour no map or marker draws. +final _magentaPng = Uint8List.fromList( + img.encodePng( + img.fill( + img.Image(width: 64, height: 36), + color: img.ColorRgb8(255, 0, 255), + ), + ), +); + +int _magentaPixels(Uint8List png) { + final decoded = img.decodePng(png)!; + var count = 0; + for (final pixel in decoded) { + if (pixel.r > 240 && pixel.g < 20 && pixel.b > 240) count++; + } + return count; +} + +void main() { + late Directory hiveDir; + + setUpAll(() async { + hiveDir = await Directory.systemTemp.createTemp('icarus_page_screenshot'); + Hive.init(hiveDir.path); + registerIcarusAdapters(Hive); + await Hive.openBox(HiveBoxNames.strategiesBox); + await Hive.openBox(HiveBoxNames.mapThemeProfilesBox); + await Hive.openBox(HiveBoxNames.appPreferencesBox); + await MapThemeProfilesProvider.bootstrap(); + }); + + tearDownAll(() async { + await Hive.close(); + await hiveDir.delete(recursive: true); + }); + + setUp(() { + CoordinateSystem(playAreaSize: const Size(1600, 900)); + CoordinateSystem.instance.setIsScreenshot(false); + }); + + /// Opens the cloud page in a web-like editor (no image files on this + /// device) and returns a ref to capture it with. + Future openCloudPage( + WidgetTester tester, { + required String? imageUrl, + }) async { + final container = ProviderContainer(overrides: [ + strategyProvider.overrideWith(_CloudStrategy.new), + remoteEditorSnapshotProvider + .overrideWith(() => _CloudSnapshot(imageUrl: imageUrl)), + imageFilesOnDeviceProvider.overrideWithValue(false), + pendingMediaBytesStoreProvider + .overrideWithValue(MemoryPendingMediaBytesStore()), + cloudMediaAccountIdProvider.overrideWithValue('account-a'), + cloudMediaUploadQueueProvider.overrideWith(_NoUploads.new), + strategyOpQueueProvider.overrideWith(_IdleOpQueue.new), + ]); + addTearDown(container.dispose); + await tester.runAsync( + () => container.read(remoteEditorSnapshotProvider.future), + ); + container.read(strategyPageSessionProvider.notifier).setStateForTest( + container + .read(strategyPageSessionProvider) + .copyWith(activePageId: _pageId), + ); + container.read(placedImageProvider.notifier).fromHive([ + PlacedImage( + position: const Offset(500, 500), + id: _imageId, + aspectRatio: 64 / 36, + scale: ImageScalePolicy.defaultWidth, + fileExtension: '.png', + sizeVersion: PlacedImage.currentSizeVersion, + ), + ]); + + late WidgetRef ref; + await tester.pumpWidget(UncontrolledProviderScope( + container: container, + child: Consumer(builder: (context, widgetRef, _) { + ref = widgetRef; + return const SizedBox.shrink(); + }), + )); + return ref; + } + + /// Runs [capture] as the app would: on real time, with the app's frames + /// still coming. Offscreen layout builders wait for the app's next frame + /// before rebuilding, so a capture that never sees one never paints what + /// arrived after its first build. + Future captureWithFrames( + WidgetTester tester, + Future Function() capture, + ) async { + Object? outcome; + var done = false; + await tester.runAsync(() async { + unawaited( + capture().then((png) => outcome = png, onError: (Object error) { + outcome = error; + }).whenComplete(() => done = true), + ); + }); + while (!done) { + await tester.runAsync( + () => Future.delayed(const Duration(milliseconds: 16)), + ); + await tester.pump(); + } + return outcome; + } + + testWidgets('a cloud page captures with its image fetched from the cloud URL', + (tester) async { + final ref = await openCloudPage(tester, imageUrl: _imageUrl); + final requested = []; + + final png = await captureWithFrames( + tester, + () => http.runWithClient( + () => captureEditorPage(ref), + () => MockClient((request) async { + requested.add(request.url); + return http.Response.bytes(_magentaPng, 200); + }), + ), + ); + + expect(png, isA()); + expect(requested, [Uri.parse(_imageUrl)]); + final decoded = img.decodePng(png! as Uint8List)!; + expect(decoded.width, CoordinateSystem.screenShotSize.width); + expect(decoded.height, CoordinateSystem.screenShotSize.height); + expect(_magentaPixels(png as Uint8List), greaterThan(500)); + // The capture never touches the editor's own coordinate mode. + expect(CoordinateSystem.instance.isScreenshot, isFalse); + }); + + testWidgets('an image whose URL has not arrived stops the capture', + (tester) async { + final ref = await openCloudPage(tester, imageUrl: null); + + final error = await captureWithFrames( + tester, + () => captureEditorPage(ref), + ); + + expect( + error, + isA() + .having((error) => error.cause, 'cause', isNull), + ); + }); + + testWidgets('an image that cannot be fetched stops the capture', + (tester) async { + final ref = await openCloudPage(tester, imageUrl: _imageUrl); + + final error = await captureWithFrames( + tester, + () => http.runWithClient( + () => captureEditorPage(ref), + () => MockClient((_) async => http.Response('down', 500)), + ), + ); + + expect( + error, + isA() + .having((error) => error.cause, 'cause', isNotNull), + ); + }); + + testWidgets( + 'a failed screenshot clears the spinner and restores canvas coordinates', + (tester) async { + await openCloudPage(tester, imageUrl: _imageUrl); + final container = ProviderScope.containerOf( + tester.element(find.byType(Consumer)), + ); + appProviderContainer = container; + final response = Completer(); + + await tester.pumpWidget(UncontrolledProviderScope( + container: container, + child: const ShadApp(home: Scaffold(body: EditorToolbar())), + )); + await http.runWithClient( + () => tester.tap(find.byIcon(LucideIcons.camera200)), + () => MockClient((_) => response.future), + ); + await tester.pump(); + // The camera turns into a spinner while the image is fetched. + expect(find.byIcon(LucideIcons.camera200), findsNothing); + expect(CoordinateSystem.instance.isScreenshot, isFalse); + + response.complete(http.Response('down', 500)); + await tester.pump(); + expect(find.byIcon(LucideIcons.camera200), findsOneWidget); + expect(CoordinateSystem.instance.isScreenshot, isFalse); + expect(tester.takeException(), isNull); + }); +} diff --git a/test/screenshot_export_failure_test.dart b/test/screenshot_export_failure_test.dart deleted file mode 100644 index 465ecf32..00000000 --- a/test/screenshot_export_failure_test.dart +++ /dev/null @@ -1,48 +0,0 @@ -import 'dart:async'; - -import 'package:flutter/material.dart'; -import 'package:flutter_riverpod/flutter_riverpod.dart'; -import 'package:flutter_test/flutter_test.dart'; -import 'package:icarus/const/app_provider_container.dart'; -import 'package:icarus/const/coordinate_system.dart'; -import 'package:icarus/providers/strategy_provider.dart'; -import 'package:icarus/widgets/editor_toolbar.dart'; -import 'package:shadcn_ui/shadcn_ui.dart'; - -class _FailingSave extends StrategyProvider { - final pending = Completer(); - - @override - StrategyState build() => StrategyState( - isSaved: true, stratName: 'Test', id: 'test', storageDirectory: null); - - @override - Future forceSaveNow(String id) => pending.future; -} - -void main() { - testWidgets( - 'failed screenshot preparation clears the spinner and restores canvas coordinates', - (tester) async { - CoordinateSystem(playAreaSize: const Size(1600, 900)); - final saver = _FailingSave(); - final container = ProviderContainer(overrides: [ - strategyProvider.overrideWith(() => saver), - ]); - appProviderContainer = container; - addTearDown(container.dispose); - await tester.pumpWidget(UncontrolledProviderScope( - container: container, - child: const ShadApp(home: Scaffold(body: EditorToolbar())))); - await tester.tap(find.byIcon(LucideIcons.camera200)); - await tester.pump(); - expect(find.byType(CircularProgressIndicator), findsOneWidget); - expect(CoordinateSystem.instance.isScreenshot, isFalse); - saver.pending.completeError(StateError('test save failure')); - await tester.pump(); - expect(find.byType(CircularProgressIndicator), findsNothing); - expect(find.byIcon(LucideIcons.camera200), findsOneWidget); - expect(CoordinateSystem.instance.isScreenshot, isFalse); - expect(tester.takeException(), isNull); - }); -} diff --git a/test/strategy_image_source_test.dart b/test/strategy_image_source_test.dart index a970eb04..55373476 100644 --- a/test/strategy_image_source_test.dart +++ b/test/strategy_image_source_test.dart @@ -334,7 +334,7 @@ void main() { remoteAsset: _asset(url: null, status: 'pending'), pendingBytes: _pendingPng, ); - expect((uploading as PendingImageBytes).bytes, _pendingPng); + expect((uploading as ImageBytes).bytes, _pendingPng); expect(uploading.imageProvider, isA()); // A failed attempt is retried from the same bytes; keep showing them. @@ -347,7 +347,7 @@ void main() { remoteAsset: _asset(url: null, status: 'failed'), pendingBytes: _pendingPng, ), - isA(), + isA(), ); expect( diff --git a/test/widgets/web_beta_library_test.dart b/test/widgets/web_beta_library_test.dart index 2de38e3f..c6151725 100644 --- a/test/widgets/web_beta_library_test.dart +++ b/test/widgets/web_beta_library_test.dart @@ -272,7 +272,7 @@ void main() { expect(find.text('Desktop-only for now'), findsOneWidget); expect( find.text( - 'Export · Import · Screenshot · Video export · Drag and drop', + 'Export · Import · Video export · Drag and drop', ), findsOneWidget, ); From f1cfe1de48cd14b5620a56c742eb243b3c24ff85 Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Tue, 29 Sep 2026 00:49:02 -0400 Subject: [PATCH 2/8] Copy the page and decode its images before a screenshot waits Review follow-up. A capture awaits image downloads before it renders, and the editor keeps editing its own objects in place meanwhile, so the page snapshot now clones agents, abilities, text, images and utilities (and deep-copies the lineup graph) instead of sharing them; an edit made while a slow image downloads can no longer land in half the picture. Fetched bytes were also never proven to be an image: a 200 with truncated bytes, or a decode slower than the 800 ms settle, would have saved a PNG with the picture missing. resolveCaptureImages now decodes every image that paints into the image cache and holds it there (keepAlive) until the capture releases it, so the capture's widgets find each picture already decoded, and a decode failure stops the capture with the same message as a failed fetch. Co-Authored-By: Claude Opus 5.5 (1M context) --- lib/screenshot/capture_images.dart | 98 +++++++++++++++++++++++------ lib/screenshot/page_screenshot.dart | 40 +++++++----- test/capture_images_test.dart | 86 ++++++++++++++++++++++--- test/page_screenshot_test.dart | 38 ++++++++++- 4 files changed, 214 insertions(+), 48 deletions(-) diff --git a/lib/screenshot/capture_images.dart b/lib/screenshot/capture_images.dart index 81c357aa..560e6ea4 100644 --- a/lib/screenshot/capture_images.dart +++ b/lib/screenshot/capture_images.dart @@ -1,15 +1,17 @@ +import 'dart:async'; import 'dart:typed_data'; +import 'package:flutter/painting.dart'; import 'package:icarus/providers/strategy_image_source.dart'; /// An image a capture would paint is not ready: its cloud copy is still -/// loading, or its bytes could not be fetched. The capture stops rather than -/// save a picture with the image missing. +/// loading, or its bytes could not be fetched or decoded. The capture stops +/// rather than save a picture with the image missing. class CaptureImagesUnavailable implements Exception { const CaptureImagesUnavailable.stillLoading() : cause = null; const CaptureImagesUnavailable.fetchFailed(this.cause); - /// Why the fetch failed; null while an image is still loading. + /// Why the image could not be loaded; null while it is still loading. final Object? cause; String get userMessage => cause == null @@ -29,32 +31,88 @@ typedef CaptureImageFetcher = Future Function( String url, ); +/// What each image in a capture paints, by image id, decoded and held in +/// the image cache so the capture's first frame already has every picture. +/// Call [release] once the capture is done. +class CaptureImages { + CaptureImages._(this.sources, this._handles); + + /// Feeds [captureImageSourcesProvider] in the capture's container. + final Map sources; + final List _handles; + + void release() { + for (final handle in _handles) { + handle.dispose(); + } + _handles.clear(); + } +} + /// Turns where each image's bytes come from, by image id, into what an -/// offscreen capture can paint ([captureImageSourcesProvider]). +/// offscreen capture can paint. /// /// A capture has no network reads of its own, so cloud URLs are fetched here /// and painted from memory. Files and bytes already in memory paint as they /// are, and an image the editor shows as unavailable is captured that way -/// too. Throws [CaptureImagesUnavailable] while an image is still loading or -/// when a fetch fails. -Future> resolveCaptureImageSources( +/// too. Every image that paints is decoded before this returns. Throws +/// [CaptureImagesUnavailable] while an image is still loading, or when a +/// fetch or a decode fails. +Future resolveCaptureImages( Map sources, { required CaptureImageFetcher fetch, }) async { final resolved = {}; - for (final MapEntry(key: imageId, value: source) in sources.entries) { - switch (source) { - case RemoteImageUrl(:final url): - try { - resolved[imageId] = ImageBytes(await fetch(imageId, url)); - } catch (error) { - throw CaptureImagesUnavailable.fetchFailed(error); - } - case ImageLoading(): - throw const CaptureImagesUnavailable.stillLoading(); - case LocalImageFile() || ImageBytes() || ImageFailed(): - resolved[imageId] = source; + final handles = []; + try { + for (final MapEntry(key: imageId, value: source) in sources.entries) { + final paintable = switch (source) { + RemoteImageUrl(:final url) => ImageBytes( + await _guard(() => fetch(imageId, url)), + ), + ImageLoading() => throw const CaptureImagesUnavailable.stillLoading(), + LocalImageFile() || ImageBytes() || ImageFailed() => source, + }; + final image = paintable.imageProvider; + if (image != null) handles.add(await _guard(() => _decode(image))); + resolved[imageId] = paintable; } + } catch (_) { + for (final handle in handles) { + handle.dispose(); + } + rethrow; + } + return CaptureImages._(resolved, handles); +} + +Future _guard(Future Function() load) async { + try { + return await load(); + } catch (error) { + throw CaptureImagesUnavailable.fetchFailed(error); } - return resolved; +} + +/// Decodes [image]'s first frame into the image cache and keeps it there +/// until the returned handle is disposed. +Future _decode(ImageProvider image) { + final decoded = Completer(); + final stream = image.resolve(ImageConfiguration.empty); + late final ImageStreamListener listener; + listener = ImageStreamListener( + (info, _) { + info.dispose(); + if (!decoded.isCompleted) { + decoded.complete(stream.completer!.keepAlive()); + } + stream.removeListener(listener); + }, + onError: (Object error, StackTrace? stackTrace) { + if (!decoded.isCompleted) decoded.completeError(error, stackTrace); + stream.removeListener(listener); + }, + ); + stream.addListener(listener); + return decoded.future; } diff --git a/lib/screenshot/page_screenshot.dart b/lib/screenshot/page_screenshot.dart index 1a6e5315..48021a55 100644 --- a/lib/screenshot/page_screenshot.dart +++ b/lib/screenshot/page_screenshot.dart @@ -1,5 +1,4 @@ -import 'dart:typed_data'; - +import 'package:flutter/foundation.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:hive_ce/hive.dart'; import 'package:icarus/collab/convex_strategy_repository.dart'; @@ -7,6 +6,7 @@ import 'package:icarus/const/coordinate_system.dart'; import 'package:icarus/const/hive_boxes.dart'; import 'package:icarus/const/line_provider.dart'; import 'package:icarus/providers/ability_provider.dart'; +import 'package:icarus/providers/action_history_models.dart'; import 'package:icarus/providers/agent_provider.dart'; import 'package:icarus/providers/collab/cloud_media_cache_provider.dart'; import 'package:icarus/providers/collab/remote_strategy_snapshot_provider.dart'; @@ -40,11 +40,11 @@ Future captureEditorPage(WidgetRef ref) async { if (strategyId == null) { throw StateError('No strategy is open to capture.'); } - final page = _editorPage(ref); + final page = editorPageSnapshot(ref); final mapState = ref.read(mapProvider); final theme = ref.read(strategyThemeProvider); - final images = await resolveCaptureImageSources( + final images = await resolveCaptureImages( { for (final image in page.imageData) image.id: readStrategyImageSource( @@ -83,7 +83,9 @@ Future captureEditorPage(WidgetRef ref) async { ); final container = ProviderContainer( - overrides: [captureImageSourcesProvider.overrideWithValue(images)], + overrides: [ + captureImageSourcesProvider.overrideWithValue(images.sources), + ], ); CaptureGeometryLease? geometry; try { @@ -116,25 +118,33 @@ Future captureEditorPage(WidgetRef ref) async { } finally { geometry?.close(); container.dispose(); + images.release(); } } -/// The page on the canvas right now. Drawings are copied: a capture -/// rebuilds their paths in screenshot coordinates, and the editor's own -/// strokes must keep theirs. -StrategyPage _editorPage(WidgetRef ref) { - final lineUps = ref.read(lineUpProvider).graph; +/// A copy of the page on the canvas right now. The editor keeps editing +/// its own objects in place while the capture fetches images, and a capture +/// rebuilds drawing paths in screenshot coordinates; neither may reach the +/// other. +@visibleForTesting +StrategyPage editorPageSnapshot(WidgetRef ref) { + final lineUps = ref.read(lineUpProvider).graph.deepCopy(); return StrategyPage( id: ref.read(strategyPageSessionProvider).activePageId ?? '', name: _activePageName(ref) ?? '', drawingData: DrawingProvider.fromJson( DrawingProvider.objectToJson(ref.read(drawingProvider).elements), ), - agentData: ref.read(agentProvider), - abilityData: ref.read(abilityProvider), - textData: ref.read(textProvider.notifier).snapshotForPersistence(), - imageData: ref.read(placedImageProvider).images, - utilityData: ref.read(utilityProvider), + agentData: ref.read(agentProvider).map(clonePlacedAgentNode).toList(), + abilityData: ref.read(abilityProvider).map(clonePlacedAbility).toList(), + textData: ref + .read(textProvider.notifier) + .snapshotForPersistence() + .map(clonePlacedText) + .toList(), + imageData: + ref.read(placedImageProvider).images.map(clonePlacedImage).toList(), + utilityData: ref.read(utilityProvider).map(clonePlacedUtility).toList(), sortIndex: 0, isAttack: ref.read(mapProvider).isAttack, settings: ref.read(strategySettingsProvider), diff --git a/test/capture_images_test.dart b/test/capture_images_test.dart index 03889599..1e0dd587 100644 --- a/test/capture_images_test.dart +++ b/test/capture_images_test.dart @@ -1,20 +1,45 @@ +import 'dart:io'; import 'dart:typed_data'; +import 'package:flutter/painting.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:icarus/providers/strategy_image_source.dart'; import 'package:icarus/screenshot/capture_images.dart'; +import 'package:image/image.dart' as img; +import 'package:path/path.dart' as path; + +Uint8List _png(int red) => Uint8List.fromList( + img.encodePng( + img.fill( + img.Image(width: 4, height: 4), + color: img.ColorRgb8(red, 0, 0), + ), + ), + ); void main() { - final fetched = Uint8List.fromList([1, 2, 3]); - final pending = Uint8List.fromList([4, 5, 6]); + TestWidgetsFlutterBinding.ensureInitialized(); + + final fetched = _png(10); + final pending = _png(20); + late Directory dir; + late String filePath; + + setUpAll(() async { + dir = await Directory.systemTemp.createTemp('icarus_capture_images'); + filePath = path.join(dir.path, 'file.png'); + await File(filePath).writeAsBytes(_png(30)); + }); + + tearDownAll(() => dir.delete(recursive: true)); test('cloud URLs are fetched; everything else paints as the editor would', () async { final requests = <(String, String)>[]; - final resolved = await resolveCaptureImageSources( + final images = await resolveCaptureImages( { 'remote': const RemoteImageUrl('https://media.example.com/remote.png'), - 'file': const LocalImageFile('/images/file.png'), + 'file': LocalImageFile(filePath), 'uploading': ImageBytes(pending), 'failed': const ImageFailed(), }, @@ -23,17 +48,45 @@ void main() { return fetched; }, ); + addTearDown(images.release); expect(requests, [('remote', 'https://media.example.com/remote.png')]); - expect((resolved['remote'] as ImageBytes).bytes, fetched); - expect((resolved['file'] as LocalImageFile).path, '/images/file.png'); - expect((resolved['uploading'] as ImageBytes).bytes, pending); - expect(resolved['failed'], isA()); + final sources = images.sources; + expect((sources['remote'] as ImageBytes).bytes, fetched); + expect((sources['file'] as LocalImageFile).path, filePath); + expect((sources['uploading'] as ImageBytes).bytes, pending); + expect(sources['failed'], isA()); + }); + + test('every image that paints is decoded and held until released', () async { + final cache = PaintingBinding.instance.imageCache; + final images = await resolveCaptureImages( + { + 'remote': const RemoteImageUrl('https://media.example.com/remote.png'), + 'file': LocalImageFile(filePath), + }, + fetch: (_, __) async => fetched, + ); + + // The capture's widgets ask for the same keys and find them decoded. + final keys = [ + for (final source in images.sources.values) + await source.imageProvider!.obtainKey(ImageConfiguration.empty), + ]; + for (final key in keys) { + expect(cache.statusForKey(key).keepAlive, isTrue); + } + + images.release(); + cache.clear(); + for (final key in keys) { + expect(cache.statusForKey(key).untracked, isTrue); + } }); test('an image still loading stops the capture', () async { await expectLater( - resolveCaptureImageSources( + resolveCaptureImages( {'loading': const ImageLoading()}, fetch: (_, __) async => fetched, ), @@ -52,7 +105,7 @@ void main() { test('a failed fetch stops the capture and keeps its cause', () async { final failure = Exception('offline'); await expectLater( - resolveCaptureImageSources( + resolveCaptureImages( {'remote': const RemoteImageUrl('https://media.example.com/r.png')}, fetch: (_, __) async => throw failure, ), @@ -67,4 +120,17 @@ void main() { ), ); }); + + test('bytes that do not decode stop the capture', () async { + await expectLater( + resolveCaptureImages( + {'remote': const RemoteImageUrl('https://media.example.com/r.png')}, + fetch: (_, __) async => Uint8List.fromList([1, 2, 3, 4]), + ), + throwsA( + isA() + .having((error) => error.cause, 'cause', isNotNull), + ), + ); + }); } diff --git a/test/page_screenshot_test.dart b/test/page_screenshot_test.dart index a9aa2ab5..648fdd7d 100644 --- a/test/page_screenshot_test.dart +++ b/test/page_screenshot_test.dart @@ -251,7 +251,7 @@ void main() { expect(decoded.width, CoordinateSystem.screenShotSize.width); expect(decoded.height, CoordinateSystem.screenShotSize.height); expect(_magentaPixels(png as Uint8List), greaterThan(500)); - // The capture never touches the editor's own coordinate mode. + // The capture leaves the editor's coordinate mode as it found it. expect(CoordinateSystem.instance.isScreenshot, isFalse); }); @@ -290,8 +290,40 @@ void main() { ); }); - testWidgets( - 'a failed screenshot clears the spinner and restores canvas coordinates', + testWidgets('bytes that are not an image stop the capture', (tester) async { + final ref = await openCloudPage(tester, imageUrl: _imageUrl); + + final error = await captureWithFrames( + tester, + () => http.runWithClient( + () => captureEditorPage(ref), + () => MockClient( + (_) async => http.Response.bytes([1, 2, 3, 4], 200), + ), + ), + ); + + expect( + error, + isA() + .having((error) => error.cause, 'cause', isNotNull), + ); + }); + + testWidgets('edits made while a capture waits never reach its copy', + (tester) async { + final ref = await openCloudPage(tester, imageUrl: _imageUrl); + final live = ref.read(placedImageProvider).images.single; + + final snapshot = editorPageSnapshot(ref); + live.scale = live.scale * 2; + + expect(snapshot.imageData.single, isNot(same(live))); + expect(snapshot.imageData.single.scale, live.scale / 2); + expect(snapshot.name, 'Retake B'); + }); + + testWidgets('a screenshot that fails while fetching clears the spinner', (tester) async { await openCloudPage(tester, imageUrl: _imageUrl); final container = ProviderScope.containerOf( From 625637ecee907f90540102aa7ecfdaac9a0fca81 Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Tue, 29 Sep 2026 01:09:49 -0400 Subject: [PATCH 3/8] Refresh a screenshot's image URLs through the reader's share link A signed-out reader who opened a strategy link (#225) can take a screenshot too. Their only access to its images is that link, so the capture's signed-URL refresh carries the link's token, as the media cache does. Co-Authored-By: Claude Opus 5.5 (1M context) --- lib/screenshot/page_screenshot.dart | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/screenshot/page_screenshot.dart b/lib/screenshot/page_screenshot.dart index 48021a55..5cd3a2ce 100644 --- a/lib/screenshot/page_screenshot.dart +++ b/lib/screenshot/page_screenshot.dart @@ -13,6 +13,7 @@ import 'package:icarus/providers/collab/remote_strategy_snapshot_provider.dart'; import 'package:icarus/providers/drawing_provider.dart'; import 'package:icarus/providers/image_provider.dart'; import 'package:icarus/providers/map_provider.dart'; +import 'package:icarus/providers/share_link_provider.dart'; import 'package:icarus/providers/strategy_image_source.dart'; import 'package:icarus/providers/strategy_page.dart'; import 'package:icarus/providers/strategy_page_session_provider.dart'; @@ -54,11 +55,17 @@ Future captureEditorPage(WidgetRef ref) async { }, fetch: (imageId, url) => downloadCloudImageBytes( url, - freshUrl: () => - ref.read(convexStrategyRepositoryProvider).getImageAssetUrl( - strategyPublicId: strategyId, - assetPublicId: imageId, - ), + freshUrl: () { + final linkView = ref.read(shareLinkViewProvider); + return ref.read(convexStrategyRepositoryProvider).getImageAssetUrl( + strategyPublicId: strategyId, + assetPublicId: imageId, + // A signed-out reader's only access is the link they opened. + shareToken: linkView?.strategyPublicId == strategyId + ? linkView!.token + : null, + ); + }, ), ); From 971b6aa3d399fcf158d441159f6afb4872bdb5a7 Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Tue, 29 Sep 2026 01:34:16 -0400 Subject: [PATCH 4/8] Keep a capture's images live until it is done, and bound their downloads Review follow-up. A keepAlive handle keeps a decoded image's completer alive, but once the last listener goes the image cache stops tracking it as live, so with enough large images an earlier one could be evicted and decoded again mid-capture, which is the missing-image timing the decode step exists to prevent. Each image now stays listened to until the capture releases it, which keeps it among the cache's live images. A stalled image request could also hold a capture forever: each download now times out after 60 s and fails the capture like any failed fetch. resolveCaptureImages takes a checkpoint, run before each image, so a caller can stop the work between images. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../collab/cloud_media_cache_provider.dart | 12 ++- lib/screenshot/capture_images.dart | 79 +++++++++++-------- test/capture_images_test.dart | 4 +- 3 files changed, 59 insertions(+), 36 deletions(-) diff --git a/lib/providers/collab/cloud_media_cache_provider.dart b/lib/providers/collab/cloud_media_cache_provider.dart index 08c6b0f0..f6f0679c 100644 --- a/lib/providers/collab/cloud_media_cache_provider.dart +++ b/lib/providers/collab/cloud_media_cache_provider.dart @@ -48,18 +48,24 @@ class CloudImageDownloadException implements Exception { String toString() => 'Failed to download image ($statusCode).'; } +/// How long one image download may take before it counts as failed. +const cloudImageDownloadTimeout = Duration(seconds: 60); + /// Downloads a cloud image's bytes from [url]. A signed URL the host refuses /// (401, 403, or 404: it expired) is swapped once for [freshUrl] from the -/// server. Throws [CloudImageDownloadException] when the bytes don't come. +/// server. Throws [CloudImageDownloadException] when the bytes don't come, +/// and a [TimeoutException] when a request stalls. Future downloadCloudImageBytes( String url, { required Future Function() freshUrl, }) async { - var response = await http.get(Uri.parse(url)); + Future get(String url) => + http.get(Uri.parse(url)).timeout(cloudImageDownloadTimeout); + var response = await get(url); if (const {401, 403, 404}.contains(response.statusCode)) { final refreshed = await freshUrl(); if (refreshed != null && refreshed.isNotEmpty) { - response = await http.get(Uri.parse(refreshed)); + response = await get(refreshed); } } if (response.statusCode < 200 || response.statusCode >= 300) { diff --git a/lib/screenshot/capture_images.dart b/lib/screenshot/capture_images.dart index 560e6ea4..d4326e78 100644 --- a/lib/screenshot/capture_images.dart +++ b/lib/screenshot/capture_images.dart @@ -35,17 +35,17 @@ typedef CaptureImageFetcher = Future Function( /// the image cache so the capture's first frame already has every picture. /// Call [release] once the capture is done. class CaptureImages { - CaptureImages._(this.sources, this._handles); + CaptureImages._(this.sources, this._holds); /// Feeds [captureImageSourcesProvider] in the capture's container. final Map sources; - final List _handles; + final List<_HeldImage> _holds; void release() { - for (final handle in _handles) { - handle.dispose(); + for (final hold in _holds) { + hold.release(); } - _handles.clear(); + _holds.clear(); } } @@ -57,15 +57,18 @@ class CaptureImages { /// are, and an image the editor shows as unavailable is captured that way /// too. Every image that paints is decoded before this returns. Throws /// [CaptureImagesUnavailable] while an image is still loading, or when a -/// fetch or a decode fails. +/// fetch or a decode fails. [checkpoint] runs before each image; whatever it +/// throws stops the work and releases what was held. Future resolveCaptureImages( Map sources, { required CaptureImageFetcher fetch, + void Function()? checkpoint, }) async { final resolved = {}; - final handles = []; + final holds = <_HeldImage>[]; try { for (final MapEntry(key: imageId, value: source) in sources.entries) { + checkpoint?.call(); final paintable = switch (source) { RemoteImageUrl(:final url) => ImageBytes( await _guard(() => fetch(imageId, url)), @@ -74,16 +77,18 @@ Future resolveCaptureImages( LocalImageFile() || ImageBytes() || ImageFailed() => source, }; final image = paintable.imageProvider; - if (image != null) handles.add(await _guard(() => _decode(image))); + if (image != null) + holds.add(await _guard(() => _HeldImage.decode(image))); resolved[imageId] = paintable; } + checkpoint?.call(); } catch (_) { - for (final handle in handles) { - handle.dispose(); + for (final hold in holds) { + hold.release(); } rethrow; } - return CaptureImages._(resolved, handles); + return CaptureImages._(resolved, holds); } Future _guard(Future Function() load) async { @@ -94,25 +99,35 @@ Future _guard(Future Function() load) async { } } -/// Decodes [image]'s first frame into the image cache and keeps it there -/// until the returned handle is disposed. -Future _decode(ImageProvider image) { - final decoded = Completer(); - final stream = image.resolve(ImageConfiguration.empty); - late final ImageStreamListener listener; - listener = ImageStreamListener( - (info, _) { - info.dispose(); - if (!decoded.isCompleted) { - decoded.complete(stream.completer!.keepAlive()); - } - stream.removeListener(listener); - }, - onError: (Object error, StackTrace? stackTrace) { - if (!decoded.isCompleted) decoded.completeError(error, stackTrace); - stream.removeListener(listener); - }, - ); - stream.addListener(listener); - return decoded.future; +/// A decoded image kept listened to, which keeps it among the image cache's +/// live images: a widget asking for the same image finds it decoded, however +/// full the cache gets, until [release]. +class _HeldImage { + _HeldImage._(this._stream, this._listener); + + final ImageStream _stream; + final ImageStreamListener _listener; + + /// Completes once [image]'s first frame is decoded. + static Future<_HeldImage> decode(ImageProvider image) { + final decoded = Completer<_HeldImage>(); + final stream = image.resolve(ImageConfiguration.empty); + late final ImageStreamListener listener; + listener = ImageStreamListener( + (info, _) { + info.dispose(); + if (!decoded.isCompleted) { + decoded.complete(_HeldImage._(stream, listener)); + } + }, + onError: (Object error, StackTrace? stackTrace) { + stream.removeListener(listener); + if (!decoded.isCompleted) decoded.completeError(error, stackTrace); + }, + ); + stream.addListener(listener); + return decoded.future; + } + + void release() => _stream.removeListener(_listener); } diff --git a/test/capture_images_test.dart b/test/capture_images_test.dart index 1e0dd587..c025c504 100644 --- a/test/capture_images_test.dart +++ b/test/capture_images_test.dart @@ -73,8 +73,10 @@ void main() { for (final source in images.sources.values) await source.imageProvider!.obtainKey(ImageConfiguration.empty), ]; + // Live, so no amount of cache pressure evicts them before the capture. + cache.clear(); for (final key in keys) { - expect(cache.statusForKey(key).keepAlive, isTrue); + expect(cache.statusForKey(key).live, isTrue); } images.release(); From d316f081982aad77ec1f7398363d336009213f55 Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Tue, 29 Sep 2026 01:45:55 -0400 Subject: [PATCH 5/8] Let go of a capture image only once Review follow-up: an image whose later frame failed to decode let go of its stream in the error handler, and release() then removed the listener again, which throws once the stream is disposed and would have stopped the cleanup of the images after it. Letting go is now idempotent. Co-Authored-By: Claude Opus 5.5 (1M context) --- lib/screenshot/capture_images.dart | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/lib/screenshot/capture_images.dart b/lib/screenshot/capture_images.dart index d4326e78..4dab88f0 100644 --- a/lib/screenshot/capture_images.dart +++ b/lib/screenshot/capture_images.dart @@ -103,31 +103,36 @@ Future _guard(Future Function() load) async { /// live images: a widget asking for the same image finds it decoded, however /// full the cache gets, until [release]. class _HeldImage { - _HeldImage._(this._stream, this._listener); + _HeldImage._(this._stream); final ImageStream _stream; - final ImageStreamListener _listener; + late final ImageStreamListener _listener; + bool _attached = false; /// Completes once [image]'s first frame is decoded. static Future<_HeldImage> decode(ImageProvider image) { + final hold = _HeldImage._(image.resolve(ImageConfiguration.empty)); final decoded = Completer<_HeldImage>(); - final stream = image.resolve(ImageConfiguration.empty); - late final ImageStreamListener listener; - listener = ImageStreamListener( + hold._listener = ImageStreamListener( (info, _) { info.dispose(); - if (!decoded.isCompleted) { - decoded.complete(_HeldImage._(stream, listener)); - } + if (!decoded.isCompleted) decoded.complete(hold); }, onError: (Object error, StackTrace? stackTrace) { - stream.removeListener(listener); + hold.release(); if (!decoded.isCompleted) decoded.completeError(error, stackTrace); }, ); - stream.addListener(listener); + hold._attached = true; + hold._stream.addListener(hold._listener); return decoded.future; } - void release() => _stream.removeListener(_listener); + /// Lets go of the image. Safe to call again: a later frame that fails to + /// decode lets go first, and the stream may be gone by the second call. + void release() { + if (!_attached) return; + _attached = false; + _stream.removeListener(_listener); + } } From 3cc0350333087399e33cee8eebe043eeb8accaab Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Tue, 29 Sep 2026 02:01:27 -0400 Subject: [PATCH 6/8] Keep a capture from signing the editor out of cloud sync Found in the browser run: each screenshot was followed by "Cloud connection lost". A capture renders in its own provider container, and hydrating its strategy provider builds that container's own auth provider, since the strategy provider listens to auth. A real auth provider configures the one Convex client the app syncs through: built with no session it clears that client's auth, with one it replaces the editor's token fetcher, and disposing it tears the auth down. Either way the editor's session ends. createCaptureContainer now builds every capture's container with an inert signed-out auth provider (nothing a capture paints needs auth) and the capture's image sources. A test records every call a capture makes to the Convex auth API and requires none; before this change it saw clearAuth. Co-Authored-By: Claude Opus 5.5 (1M context) --- lib/screenshot/offscreen_capture.dart | 26 +++++++++++ lib/screenshot/page_screenshot.dart | 6 +-- test/page_screenshot_test.dart | 67 +++++++++++++++++++++++++++ 3 files changed, 94 insertions(+), 5 deletions(-) diff --git a/lib/screenshot/offscreen_capture.dart b/lib/screenshot/offscreen_capture.dart index c6c03fdd..73f6032f 100644 --- a/lib/screenshot/offscreen_capture.dart +++ b/lib/screenshot/offscreen_capture.dart @@ -3,8 +3,34 @@ import 'package:flutter_portal/flutter_portal.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:icarus/const/coordinate_system.dart'; import 'package:icarus/const/settings.dart'; +import 'package:icarus/providers/auth_provider.dart'; +import 'package:icarus/providers/strategy_image_source.dart'; import 'package:shadcn_ui/shadcn_ui.dart'; +/// The provider container one offscreen capture renders in, cut off from the +/// editor's, with [images] as what its images paint. +/// +/// A capture's providers are built fresh here, and a real auth provider +/// among them would reach the one Convex client the app syncs through: +/// building it sets or clears that client's auth, and disposing it tears +/// the auth down, signing the editor out of sync. The strategy provider +/// listens to auth, so auth here is an inert signed-out state; nothing a +/// capture paints needs more. +ProviderContainer createCaptureContainer({ + Map images = const {}, +}) => + ProviderContainer( + overrides: [ + authProvider.overrideWith(_CaptureAuth.new), + captureImageSourcesProvider.overrideWithValue(images), + ], + ); + +class _CaptureAuth extends AuthProvider { + @override + AppAuthState build() => AppAuthState.fromSession(null); +} + /// Keep screenshot coordinates confined to pixel capture. Asset loading and /// output dialogs must leave the live editor's coordinate conversions intact. Future withScreenshotCoordinates(Future Function() capture) async { diff --git a/lib/screenshot/page_screenshot.dart b/lib/screenshot/page_screenshot.dart index 5cd3a2ce..a0259f1e 100644 --- a/lib/screenshot/page_screenshot.dart +++ b/lib/screenshot/page_screenshot.dart @@ -89,11 +89,7 @@ Future captureEditorPage(WidgetRef ref) async { themeOverridePalette: theme.overridePalette, ); - final container = ProviderContainer( - overrides: [ - captureImageSourcesProvider.overrideWithValue(images.sources), - ], - ); + final container = createCaptureContainer(images: images.sources); CaptureGeometryLease? geometry; try { // The sightline models load asynchronously; the capture waits for the diff --git a/test/page_screenshot_test.dart b/test/page_screenshot_test.dart index 648fdd7d..26f81112 100644 --- a/test/page_screenshot_test.dart +++ b/test/page_screenshot_test.dart @@ -17,6 +17,7 @@ import 'package:icarus/const/maps.dart'; import 'package:icarus/const/placed_classes.dart'; import 'package:icarus/const/image_scale_policy.dart'; import 'package:icarus/hive/hive_registration.dart'; +import 'package:icarus/providers/auth_provider.dart'; import 'package:icarus/providers/collab/cloud_media_upload_queue_provider.dart'; import 'package:icarus/providers/collab/media_bytes_source.dart'; import 'package:icarus/providers/collab/remote_strategy_snapshot_provider.dart'; @@ -31,6 +32,7 @@ import 'package:icarus/strategy/strategy_page_models.dart'; import 'package:icarus/widgets/editor_toolbar.dart'; import 'package:image/image.dart' as img; import 'package:shadcn_ui/shadcn_ui.dart'; +import 'package:supabase_flutter/supabase_flutter.dart' show AuthState, Session; const _strategyId = 'cloud-strategy'; const _pageId = 'page-1'; @@ -102,6 +104,45 @@ class _CloudSnapshot extends RemoteEditorSnapshotNotifier { } } +/// Records every call a capture makes to the app's one Convex client. +class _RecordingConvexAuth extends Fake implements AuthProviderConvexApi { + final calls = []; + + @override + Stream get authState => const Stream.empty(); + + @override + bool get isAuthenticated => true; + + @override + Future setAuthWithRefresh({ + required Future Function() fetchToken, + void Function(bool isAuthenticated)? onAuthChange, + }) async { + calls.add('setAuthWithRefresh'); + return _RecordingHandle(calls); + } + + @override + Future clearAuth() async => calls.add('clearAuth'); +} + +class _RecordingHandle implements AuthProviderAuthHandle { + _RecordingHandle(this.calls); + final List calls; + + @override + void dispose() => calls.add('dispose'); +} + +class _NoSupabase extends Fake implements AuthProviderSupabaseApi { + @override + Session? get currentSession => null; + + @override + Stream get onAuthStateChange => const Stream.empty(); +} + class _IdleOpQueue extends StrategyOpQueueNotifier { @override StrategyOpQueueState build() => const StrategyOpQueueState(); @@ -255,6 +296,32 @@ void main() { expect(CoordinateSystem.instance.isScreenshot, isFalse); }); + testWidgets('a capture never touches the app\'s cloud session', + (tester) async { + // A capture's own container builds a strategy provider, which listens + // to auth. A real auth provider there would set, and on dispose tear + // down, auth on the one Convex client the editor syncs through. + final convex = _RecordingConvexAuth(); + AuthProvider.debugConvexApi = convex; + AuthProvider.debugSupabaseApi = _NoSupabase(); + addTearDown(AuthProvider.resetTestOverrides); + final ref = await openCloudPage(tester, imageUrl: _imageUrl); + + final png = await captureWithFrames( + tester, + () => http.runWithClient( + () => captureEditorPage(ref), + () => MockClient((_) async => http.Response.bytes(_magentaPng, 200)), + ), + ); + await tester.runAsync( + () => Future.delayed(const Duration(milliseconds: 50)), + ); + + expect(png, isA()); + expect(convex.calls, isEmpty); + }); + testWidgets('an image whose URL has not arrived stops the capture', (tester) async { final ref = await openCloudPage(tester, imageUrl: null); From 3d1dc7f773af5dc085b176cdc341921e3b3554da Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Tue, 29 Sep 2026 02:30:04 -0400 Subject: [PATCH 7/8] Download a capture's images together, and take one screenshot at a time Greptile follow-ups. A page's cloud images downloaded one after another, so the wait was the sum of them; they now start together and decode in order, with the same checkpoints and cleanup, and an image still loading stops the capture before any download starts. And the screenshot guard cleared before the save dialog closed, so a second click could capture again and open a second dialog; the guard now holds until the save is done. Both have tests; the second fails without the fix. Co-Authored-By: Claude Opus 5.5 (1M context) --- lib/screenshot/capture_images.dart | 23 ++++++---- lib/widgets/editor_toolbar.dart | 9 +++- test/capture_images_test.dart | 41 +++++++++++++++++ test/page_screenshot_test.dart | 72 ++++++++++++++++++++++++++++++ 4 files changed, 136 insertions(+), 9 deletions(-) diff --git a/lib/screenshot/capture_images.dart b/lib/screenshot/capture_images.dart index 4dab88f0..e8ec3520 100644 --- a/lib/screenshot/capture_images.dart +++ b/lib/screenshot/capture_images.dart @@ -64,21 +64,28 @@ Future resolveCaptureImages( required CaptureImageFetcher fetch, void Function()? checkpoint, }) async { + if (sources.values.any((source) => source is ImageLoading)) { + throw const CaptureImagesUnavailable.stillLoading(); + } + // Downloads run together, so a page's wait is its slowest image rather + // than the sum of them. A download nobody awaits (the capture stopped + // first) must not surface as an unhandled error. + final downloads = { + for (final MapEntry(key: imageId, value: source) in sources.entries) + if (source case RemoteImageUrl(:final url)) + imageId: _guard(() => fetch(imageId, url))..ignore(), + }; final resolved = {}; final holds = <_HeldImage>[]; try { for (final MapEntry(key: imageId, value: source) in sources.entries) { checkpoint?.call(); - final paintable = switch (source) { - RemoteImageUrl(:final url) => ImageBytes( - await _guard(() => fetch(imageId, url)), - ), - ImageLoading() => throw const CaptureImagesUnavailable.stillLoading(), - LocalImageFile() || ImageBytes() || ImageFailed() => source, - }; + final download = downloads[imageId]; + final paintable = download == null ? source : ImageBytes(await download); final image = paintable.imageProvider; - if (image != null) + if (image != null) { holds.add(await _guard(() => _HeldImage.decode(image))); + } resolved[imageId] = paintable; } checkpoint?.call(); diff --git a/lib/widgets/editor_toolbar.dart b/lib/widgets/editor_toolbar.dart index afd9353e..eca8856c 100644 --- a/lib/widgets/editor_toolbar.dart +++ b/lib/widgets/editor_toolbar.dart @@ -48,6 +48,10 @@ class EditorToolbar extends ConsumerStatefulWidget { class _EditorToolbarState extends ConsumerState { bool _isCapturingScreenshot = false; + /// From the click until the PNG is saved or the save dialog closes: a + /// second click meanwhile would capture again and open a second dialog. + bool _screenshotInProgress = false; + @override Widget build(BuildContext context) { const style = kEditorToolbarButtonStyle; @@ -168,9 +172,10 @@ class _EditorToolbarState extends ConsumerState { Future _captureScreenshot() async { if (!ensureFeatureAvailable(ref, PlatformFeature.screenshot)) return; - if (_isCapturingScreenshot) return; + if (_screenshotInProgress) return; final strategy = ref.read(strategyProvider); if (strategy.strategyId == null) return; + _screenshotInProgress = true; setState(() => _isCapturingScreenshot = true); try { late final Uint8List image; @@ -211,6 +216,8 @@ class _EditorToolbarState extends ConsumerState { stackTrace: stackTrace, source: 'EditorToolbar.screenshot', ); + } finally { + _screenshotInProgress = false; } } } diff --git a/test/capture_images_test.dart b/test/capture_images_test.dart index c025c504..b3d20276 100644 --- a/test/capture_images_test.dart +++ b/test/capture_images_test.dart @@ -86,6 +86,47 @@ void main() { } }); + test('cloud images download together, not one after another', () async { + final started = []; + final watch = Stopwatch()..start(); + final images = await resolveCaptureImages( + { + for (final id in ['a', 'b', 'c', 'd']) + id: RemoteImageUrl('https://media.example.com/$id.png'), + }, + fetch: (imageId, _) async { + started.add(imageId); + await Future.delayed(const Duration(milliseconds: 200)); + return _png(imageId.codeUnitAt(0)); + }, + ); + addTearDown(images.release); + + expect(started, ['a', 'b', 'c', 'd']); + // Four 200 ms downloads in series would take 800 ms. + expect(watch.elapsedMilliseconds, lessThan(600)); + expect(images.sources.values, everyElement(isA())); + }); + + test('an image still loading stops the capture before any download', + () async { + final started = []; + await expectLater( + resolveCaptureImages( + { + 'remote': const RemoteImageUrl('https://media.example.com/r.png'), + 'loading': const ImageLoading(), + }, + fetch: (imageId, _) async { + started.add(imageId); + return fetched; + }, + ), + throwsA(isA()), + ); + expect(started, isEmpty); + }); + test('an image still loading stops the capture', () async { await expectLater( resolveCaptureImages( diff --git a/test/page_screenshot_test.dart b/test/page_screenshot_test.dart index 26f81112..beb390d1 100644 --- a/test/page_screenshot_test.dart +++ b/test/page_screenshot_test.dart @@ -2,6 +2,7 @@ import 'dart:async'; import 'dart:io'; import 'dart:typed_data'; +import 'package:file_picker/file_picker.dart'; import 'package:flutter/material.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_test/flutter_test.dart'; @@ -143,6 +144,26 @@ class _NoSupabase extends Fake implements AuthProviderSupabaseApi { Stream get onAuthStateChange => const Stream.empty(); } +/// A save dialog the test closes when it chooses. +class _PendingPicker extends FilePicker { + final saves = []; + final closed = Completer(); + + @override + Future saveFile({ + String? dialogTitle, + String? fileName, + String? initialDirectory, + FileType type = FileType.any, + List? allowedExtensions, + Uint8List? bytes, + bool lockParentWindow = false, + }) { + saves.add(bytes); + return closed.future; + } +} + class _IdleOpQueue extends StrategyOpQueueNotifier { @override StrategyOpQueueState build() => const StrategyOpQueueState(); @@ -390,6 +411,57 @@ void main() { expect(snapshot.name, 'Retake B'); }); + testWidgets('a second click while the save dialog is open does nothing', + (tester) async { + final ref = await openCloudPage(tester, imageUrl: _imageUrl); + final picker = _PendingPicker(); + FilePicker.platform = picker; + final container = ProviderScope.containerOf( + tester.element(find.byType(Consumer)), + ); + await tester.pumpWidget(UncontrolledProviderScope( + container: container, + child: const ShadApp(home: Scaffold(body: EditorToolbar())), + )); + expect(ref, isNotNull); + + Future clickCamera() => tester.runAsync( + () => http.runWithClient( + () => tester.tap(find.byIcon(LucideIcons.camera200)), + () => MockClient( + (_) async => http.Response.bytes(_magentaPng, 200), + ), + ), + ); + + await clickCamera(); + // Real time with the app's frames coming, until the dialog opens. + for (var i = 0; i < 400 && picker.saves.isEmpty; i++) { + await tester.runAsync( + () => Future.delayed(const Duration(milliseconds: 16)), + ); + await tester.pump(); + } + expect(picker.saves, hasLength(1)); + expect(picker.saves.single, isNotEmpty); + + // The spinner is gone while the dialog is open, but the button waits. + expect(find.byIcon(LucideIcons.camera200), findsOneWidget); + await clickCamera(); + for (var i = 0; i < 30; i++) { + await tester.runAsync( + () => Future.delayed(const Duration(milliseconds: 16)), + ); + await tester.pump(); + } + expect(picker.saves, hasLength(1)); + expect(find.byIcon(LucideIcons.camera200), findsOneWidget); + + picker.closed.complete(null); + await tester.pump(); + expect(tester.takeException(), isNull); + }); + testWidgets('a screenshot that fails while fetching clears the spinner', (tester) async { await openCloudPage(tester, imageUrl: _imageUrl); From 40edb70e7b155439e64a86aae5c28a46d5c4f031 Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Tue, 29 Sep 2026 02:45:33 -0400 Subject: [PATCH 8/8] Abort a capture's other downloads when it stops early Greptile follow-up: when one image failed or the capture was cancelled, the downloads already started for the others kept running, and a retry started them again alongside. A capture's downloads now share one HTTP client that is closed when resolution ends, which aborts whatever is still in flight (BrowserClient and IOClient both abort on close). A test fails one image while another waits and requires the client closed. Co-Authored-By: Claude Opus 5.5 (1M context) --- lib/screenshot/capture_images.dart | 14 ++++++++-- test/capture_images_test.dart | 45 ++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/lib/screenshot/capture_images.dart b/lib/screenshot/capture_images.dart index e8ec3520..36cc554e 100644 --- a/lib/screenshot/capture_images.dart +++ b/lib/screenshot/capture_images.dart @@ -2,6 +2,7 @@ import 'dart:async'; import 'dart:typed_data'; import 'package:flutter/painting.dart'; +import 'package:http/http.dart' as http; import 'package:icarus/providers/strategy_image_source.dart'; /// An image a capture would paint is not ready: its cloud copy is still @@ -68,12 +69,17 @@ Future resolveCaptureImages( throw const CaptureImagesUnavailable.stillLoading(); } // Downloads run together, so a page's wait is its slowest image rather - // than the sum of them. A download nobody awaits (the capture stopped - // first) must not surface as an unhandled error. + // than the sum of them. They share one client, closed when this returns, + // so a capture that stops early aborts the downloads it no longer needs; + // a download nobody awaits must not surface as an unhandled error. + final client = http.Client(); final downloads = { for (final MapEntry(key: imageId, value: source) in sources.entries) if (source case RemoteImageUrl(:final url)) - imageId: _guard(() => fetch(imageId, url))..ignore(), + imageId: http.runWithClient( + () => _guard(() => fetch(imageId, url)), + () => client, + )..ignore(), }; final resolved = {}; final holds = <_HeldImage>[]; @@ -94,6 +100,8 @@ Future resolveCaptureImages( hold.release(); } rethrow; + } finally { + client.close(); } return CaptureImages._(resolved, holds); } diff --git a/test/capture_images_test.dart b/test/capture_images_test.dart index b3d20276..ae519098 100644 --- a/test/capture_images_test.dart +++ b/test/capture_images_test.dart @@ -1,3 +1,4 @@ +import 'dart:async'; import 'dart:io'; import 'dart:typed_data'; @@ -5,6 +6,7 @@ import 'package:flutter/painting.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:icarus/providers/strategy_image_source.dart'; import 'package:icarus/screenshot/capture_images.dart'; +import 'package:http/http.dart' as http; import 'package:image/image.dart' as img; import 'package:path/path.dart' as path; @@ -17,6 +19,26 @@ Uint8List _png(int red) => Uint8List.fromList( ), ); +/// Answers only when closed, which is what aborting looks like to a caller. +class _SlowClient extends http.BaseClient { + final requested = []; + var closed = false; + final _closing = Completer(); + + @override + Future send(http.BaseRequest request) async { + requested.add(request.url.toString()); + await _closing.future; + throw http.ClientException('Client closed', request.url); + } + + @override + void close() { + closed = true; + if (!_closing.isCompleted) _closing.complete(); + } +} + void main() { TestWidgetsFlutterBinding.ensureInitialized(); @@ -108,6 +130,29 @@ void main() { expect(images.sources.values, everyElement(isA())); }); + test('a capture that fails aborts the downloads it no longer needs', + () async { + final client = _SlowClient(); + await expectLater( + http.runWithClient( + () => resolveCaptureImages( + { + 'broken': const RemoteImageUrl('https://media.example.com/x.png'), + 'slow': const RemoteImageUrl('https://media.example.com/s.png'), + }, + fetch: (imageId, url) async { + if (imageId == 'broken') throw Exception('offline'); + return (await http.get(Uri.parse(url))).bodyBytes; + }, + ), + () => client, + ), + throwsA(isA()), + ); + expect(client.requested, ['https://media.example.com/s.png']); + expect(client.closed, isTrue); + }); + test('an image still loading stops the capture before any download', () async { final started = [];