Skip to content

Fix gray tiles when tile image is already avaible#715

Merged
johnpryan merged 3 commits intofleaflet:masterfrom
maRci002:feature/async-tile-load
Aug 4, 2020
Merged

Fix gray tiles when tile image is already avaible#715
johnpryan merged 3 commits intofleaflet:masterfrom
maRci002:feature/async-tile-load

Conversation

@maRci002
Copy link
Copy Markdown
Contributor

@maRci002 maRci002 commented Aug 4, 2020

Hello, this PR make sure the loadTileImage's listeners runs after the tile is in _tiles map .

  /// Adds a listener callback that is called whenever a new concrete [ImageInfo]
  /// object is available or an error is reported. If a concrete image is
  /// already available, or if an error has been already reported, this object
  /// will notify the listener synchronously.
  ///
  /// If the [ImageStreamCompleter] completes multiple images over its lifetime,
  /// this listener's [ImageStreamListener.onImage] will fire multiple times.
  ///
  /// {@macro flutter.painting.imageStream.addListener}
  void addListener(ImageStreamListener listener) {

If a concrete image is already available, or if an error has been already reported, this object will notify the listener synchronously.

Since this code:

var key = _tileCoordsToKey(coords);
tile = _tiles[key];
if (null == tile) {
  return;
}

skipped fade-in part, the tiles remained gray. This only happaned if image object was in phone's ram so there wasn't an async callback since image was instantly avaible, however this fix doesn't assume async image load anymore.

closes #707
closes #713
closes #636
closes #608

@maRci002
Copy link
Copy Markdown
Contributor Author

maRci002 commented Aug 4, 2020

How to test, just change flutter_map dependecy to:

  flutter_map:
    git:
      url: https://github.com/fleaflet/flutter_map.git
      ref: 7413bba7b91f6697f7912aa2660465a940d7c9e4

@maRci002 maRci002 changed the title Make sure tile loads asynchronously Fix gray tiles when tile image is already avaible Aug 4, 2020
tileReady: _tileReady,
);

tile.loadTileImage();
Copy link
Copy Markdown
Collaborator

@johnpryan johnpryan Aug 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nitpick) This code could use the cascade operator to simplify:

_tiles[tileCoordsToKey] = Tile(
  // ...
)..loadTileImage();

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried it but didn't work. I'll give it another try maybe hotreload bugged.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I retried it didn't work. I think this happens under hood when cascade operator is used.

  1. Tile object created
  2. loadTileImage method called on Tile
  3. Tile added to _tiles[tileCoordsToKey]

With temp variable this happens:

  1. Tile object created
  2. Tile added to _tiles[tileCoordsToKey]
  3. loadTileImage method called on Tile

So even if loadTileImage's callback is synchronous we are sure it is already in _tiles map.

@johnpryan johnpryan merged commit 975cae2 into fleaflet:master Aug 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants