From d5a6cb715c6e64d748c892d861422430a698724e Mon Sep 17 00:00:00 2001 From: Rory Stephenson Date: Wed, 2 Aug 2023 18:16:50 +0200 Subject: [PATCH] Only create placeholder tiles where tiles are missing or not fully transitioned at the current zoom This reduces the number of placeholder widgets. The algorithm used to determine which coordinates to show placeholders for is a tradeoff between speed and the number of redundant placeholders created. It creates a placeholder for every tile at the current zoom which failed to load or is still transitioning. This means that if a tile from a lower zoom obscures the placeholder or multiple tiles from a higher zoom collectively obscure the placeholder it will be unnecessarily created. It would be possible to avoid this but it would require a more complex data structure or iterating all of the tiles for every potential placeholder. --- lib/src/layer/tile_layer/tile_image.dart | 39 ++++++------- .../layer/tile_layer/tile_image_manager.dart | 23 ++++---- lib/src/layer/tile_layer/tile_image_view.dart | 6 +- lib/src/layer/tile_layer/tile_layer.dart | 55 ++++++++++++++----- .../tile_layer/tile_image_view_test.dart | 10 ++-- 5 files changed, 79 insertions(+), 54 deletions(-) diff --git a/lib/src/layer/tile_layer/tile_image.dart b/lib/src/layer/tile_layer/tile_image.dart index d8c0a9205..393503fb2 100644 --- a/lib/src/layer/tile_layer/tile_image.dart +++ b/lib/src/layer/tile_layer/tile_image.dart @@ -8,8 +8,9 @@ class TileImage extends ChangeNotifier { // Controls fade-in opacity. AnimationController? _animationController; - // Whether the tile is displayable. See [readyToDisplay]. - bool _readyToDisplay = false; + // Whether the tile has both loaded and finished transitioning. See + // [transitionComplete]. + bool _transitionComplete = false; /// Used by animationController. Still required if animation is disabled in /// case the tile display is changed at a later point. @@ -67,7 +68,7 @@ class TileImage extends ChangeNotifier { double get opacity => _tileDisplay.when( instantaneous: (instantaneous) => - _readyToDisplay ? instantaneous.opacity : 0.0, + _transitionComplete ? instantaneous.opacity : 0.0, fadeIn: (fadeIn) => _animationController!.value, )!; @@ -75,14 +76,14 @@ class TileImage extends ChangeNotifier { String get coordinatesKey => coordinates.key; - /// Whether the tile is displayable. This means that either: - /// * Loading errored but an error image is configured. - /// * Loading succeeded and the fade animation has finished. - /// * Loading succeeded and there is no fade animation. + /// Whether the tile has loaded and finished fading in. This is true when + /// loading succeeded and: + /// * The fade animation has finished. + /// * There is no fade animation. /// /// Note that [opacity] can be less than 1 when this is true if instantaneous /// tile display is used with a maximum opacity less than 1. - bool get readyToDisplay => _readyToDisplay; + bool get transitionComplete => _transitionComplete; // Used to sort TileImages by their distance from the current zoom. double zIndex(double maxZoom, int currentZoom) => @@ -102,7 +103,7 @@ class TileImage extends ChangeNotifier { _animationController = AnimationController( duration: fadeIn.duration, vsync: vsync, - value: _readyToDisplay ? 1.0 : 0.0, + value: _transitionComplete ? 1.0 : 0.0, ); }, ); @@ -150,7 +151,7 @@ class TileImage extends ChangeNotifier { this.imageInfo = imageInfo; if (!_disposed) { - _display(); + _initiateTransition(); onLoadComplete(coordinates); } } @@ -164,16 +165,16 @@ class TileImage extends ChangeNotifier { } } - // Initiates fading in and marks this TileImage as readyToDisplay when fading - // finishes. If fading is disabled or a loading error occurred this TileImage - // becomes readyToDisplay immediately. - void _display() { + // Initiates fading in and sets [transitionComplete] to true when fading + // finishes. If fading is disabled [transitionComplete] is set to true + // immediately. + void _initiateTransition() { final previouslyLoaded = loadFinishedAt != null; loadFinishedAt = DateTime.now(); _tileDisplay.when( instantaneous: (_) { - _readyToDisplay = true; + _transitionComplete = true; if (!_disposed) notifyListeners(); }, fadeIn: (fadeIn) { @@ -181,12 +182,12 @@ class TileImage extends ChangeNotifier { previouslyLoaded ? fadeIn.reloadStartOpacity : fadeIn.startOpacity; if (fadeStartOpacity == 1.0) { - _readyToDisplay = true; + _transitionComplete = true; if (!_disposed) notifyListeners(); } else { _animationController!.reset(); _animationController!.forward(from: fadeStartOpacity).then((_) { - _readyToDisplay = true; + _transitionComplete = true; if (!_disposed) notifyListeners(); }); } @@ -212,7 +213,7 @@ class TileImage extends ChangeNotifier { } } - _readyToDisplay = false; + _transitionComplete = false; _animationController?.stop(canceled: false); _animationController?.value = 0.0; notifyListeners(); @@ -232,6 +233,6 @@ class TileImage extends ChangeNotifier { @override String toString() { - return 'TileImage($coordinates, readyToDisplay: $_readyToDisplay)'; + return 'TileImage($coordinates, transitionComplete: $_transitionComplete)'; } } diff --git a/lib/src/layer/tile_layer/tile_image_manager.dart b/lib/src/layer/tile_layer/tile_image_manager.dart index c46d7b672..1c5dbe021 100644 --- a/lib/src/layer/tile_layer/tile_image_manager.dart +++ b/lib/src/layer/tile_layer/tile_image_manager.dart @@ -19,19 +19,16 @@ class TileImageManager { bool get allLoaded => _tiles.values.none((tile) => tile.loadFinishedAt == null); - // Returns in the order in which they should be rendered: - // 1. Tiles at the current zoom. - // 2. Tiles at the current zoom +/- 1. - // 3. Tiles at the current zoom +/- 2. - // 4. ...etc - List inRenderOrder(double maxZoom, int currentZoom) { - final result = _tiles.values.toList() - ..sort((a, b) => a - .zIndex(maxZoom, currentZoom) - .compareTo(b.zIndex(maxZoom, currentZoom))); - - return result; - } + /// Returns in the order in which they should be rendered: + /// 1. Tiles at the current zoom. + /// 2. Tiles at the current zoom +/- 1. + /// 3. Tiles at the current zoom +/- 2. + /// 4. ...etc + List inRenderOrder(double maxZoom, int currentZoom) => + _tiles.values.toList() + ..sort((a, b) => a + .zIndex(maxZoom, currentZoom) + .compareTo(b.zIndex(maxZoom, currentZoom))); // Creates missing tiles in the given range. Does not initiate loading of the // tiles. diff --git a/lib/src/layer/tile_layer/tile_image_view.dart b/lib/src/layer/tile_layer/tile_image_view.dart index 7aca827e2..5de432385 100644 --- a/lib/src/layer/tile_layer/tile_image_view.dart +++ b/lib/src/layer/tile_layer/tile_image_view.dart @@ -33,7 +33,7 @@ class TileImageView { final retain = Set.from(tilesInKeepRange); for (final tile in tilesInKeepRange) { - if (!tile.readyToDisplay) { + if (!tile.transitionComplete) { final coords = tile.coordinates; if (!_retainAncestor( retain, coords.x, coords.y, coords.z, coords.z - 5)) { @@ -64,7 +64,7 @@ class TileImageView { final tile = _tileImages[coords2.key]; if (tile != null) { - if (tile.readyToDisplay) { + if (tile.transitionComplete) { retain.add(tile); return true; } else if (tile.loadFinishedAt != null) { @@ -94,7 +94,7 @@ class TileImageView { final tile = _tileImages[coords.key]; if (tile != null) { - if (tile.readyToDisplay || tile.loadFinishedAt != null) { + if (tile.transitionComplete || tile.loadFinishedAt != null) { retain.add(tile); } } diff --git a/lib/src/layer/tile_layer/tile_layer.dart b/lib/src/layer/tile_layer/tile_layer.dart index a5fe7968a..5a426813f 100644 --- a/lib/src/layer/tile_layer/tile_layer.dart +++ b/lib/src/layer/tile_layer/tile_layer.dart @@ -514,11 +514,18 @@ class _TileLayerState extends State with TickerProviderStateMixin { _tileScaleCalculator.clearCacheUnlessZoomMatches(map.zoom); + final tileImagesInRenderOrder = + _tileImageManager.inRenderOrder(widget.maxZoom, tileZoom); + return _addBackgroundColor( Stack( children: [ if (widget.placeholderImage != null) - ...visibleTileRange.coordinates.map( + ..._placeholderCoordinates( + tileImagesInRenderOrder.takeWhile( + (tileImage) => tileImage.coordinates.z == tileZoom), + visibleTileRange, + ).map( (tileCoordinates) => TilePlaceholder( key: ValueKey('placeholder-${tileCoordinates.key}'), tileCoordinates: tileCoordinates, @@ -530,25 +537,45 @@ class _TileLayerState extends State with TickerProviderStateMixin { placeholderImage: widget.placeholderImage!, ), ), - ..._tileImageManager.inRenderOrder(widget.maxZoom, tileZoom).map( - (tileImage) => Tile( - // Must be an ObjectKey, not a ValueKey using the coordinates, in - // case we remove and replace the TileImage with a different one. - key: ObjectKey(tileImage), - scaledTileSize: _tileScaleCalculator.scaledTileSize( - map.zoom, - tileImage.coordinates.z, - ), - currentPixelOrigin: currentPixelOrigin, - tileImage: tileImage, - tileBuilder: widget.tileBuilder, - ), + ...tileImagesInRenderOrder.map( + (tileImage) => Tile( + // Must be an ObjectKey, not a ValueKey using the coordinates, in + // case we remove and replace the TileImage with a different one. + key: ObjectKey(tileImage), + scaledTileSize: _tileScaleCalculator.scaledTileSize( + map.zoom, + tileImage.coordinates.z, ), + currentPixelOrigin: currentPixelOrigin, + tileImage: tileImage, + tileBuilder: widget.tileBuilder, + ), + ), ], ), ); } + /// Returns the visible coordinates from the current zoom for which there is + /// no TileImage which has finished loading and transitioning. It is possible + /// a returned placeholder coordinate will be for a placeholder which is + /// obscured by a tile from a lower zoom or a collection of tiles from higher + /// zooms, this simple algorithm is a tradeoff between minimising the number + /// of unnecessary placeholders and keeping the calculation fast. + Iterable _placeholderCoordinates( + Iterable tileImagesAtCurrentZoom, + DiscreteTileRange visibleTileRange, + ) { + final obscuredCoordinatesAtCurrentZoom = tileImagesAtCurrentZoom + .where((tileImage) => tileImage.transitionComplete) + .map((tileImage) => tileImage.coordinates) + .toSet(); + + return visibleTileRange.coordinates + .toSet() + .difference(obscuredCoordinatesAtCurrentZoom); + } + // This can be removed once the deprecated backgroundColor option is removed. Widget _addBackgroundColor(Widget child) { // ignore: deprecated_member_use_from_same_package diff --git a/test/layer/tile_layer/tile_image_view_test.dart b/test/layer/tile_layer/tile_image_view_test.dart index e6f5f36bb..0de3da9cc 100644 --- a/test/layer/tile_layer/tile_image_view_test.dart +++ b/test/layer/tile_layer/tile_image_view_test.dart @@ -59,7 +59,7 @@ void main() { test('ancestor tile is not stale if a tile has not loaded yet', () { final tileImages = tileImagesMappingFrom([ MockTileImage(0, 0, 0), - MockTileImage(0, 0, 1, loadFinished: false, readyToDisplay: false), + MockTileImage(0, 0, 1, loadFinished: false, transitionComplete: false), ]); final removalState = TileImageView( tileImages: tileImages, @@ -75,8 +75,8 @@ void main() { test('descendant tile is not stale if there is no loaded tile obscuring it', () { final tileImages = tileImagesMappingFrom([ - MockTileImage(0, 0, 0, loadFinished: false, readyToDisplay: false), - MockTileImage(0, 0, 1, loadFinished: false, readyToDisplay: false), + MockTileImage(0, 0, 0, loadFinished: false, transitionComplete: false), + MockTileImage(0, 0, 1, loadFinished: false, transitionComplete: false), MockTileImage(0, 0, 2), ]); final removalState = TileImageView( @@ -171,7 +171,7 @@ void main() { class MockTileImage extends TileImage { @override - final bool readyToDisplay; + final bool transitionComplete; @override final bool loadError; @@ -180,7 +180,7 @@ class MockTileImage extends TileImage { int x, int y, int zoom, { - this.readyToDisplay = true, + this.transitionComplete = true, bool loadFinished = true, this.loadError = false, void Function(TileCoordinates coordinates)? onLoadComplete,