Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix TileLayer lag during fling/animated movement #1247

Merged
merged 11 commits into from
May 28, 2022

Conversation

rorystephenson
Copy link
Contributor

@rorystephenson rorystephenson commented May 27, 2022

Fixes #1245

When flinging or animating the moment of the map the TileLayer would lag behind which resulted in either jerkiness or other layers moving out of sync with the TileLayer depending on how fast the device is.

This PR should address that issue by moving the Tile translation calculation in to the builder rather than in a stream listener which called setState(). This means that translation is calculated during the build whilst updating/loading of tiles can still happen in the stream listener to prevent them from slowing down the build too much.

As part of these changes I heavily refactored the TileLayer code as it was difficult for me to follow.

…ld as it's fast anyways and just complicates the code
Previously we listened to the movement stream, calculated the
translations and then called setState(). This introduced noticible
jank/lag where when flinging the map or moving it with an
AnimatedController other layers (e.g. markers) would not move together
with the map.
 * Run dart format
 * Re-fix the tile ordering function, it was putting lower-zoom tiles.
 above higher zoom ones.
 * Add the StreamBuilder to trigger builds as a result of map movement.
@rorystephenson rorystephenson mentioned this pull request May 27, 2022
11 tasks
@JaffaKetchup
Copy link
Member

Wow, this looks much better than it did before and was one of our main aims. I'll test as soon as I can :)
Huge thanks!

Copy link
Member

@JaffaKetchup JaffaKetchup left a comment

Choose a reason for hiding this comment

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

Haven't tested/run anything, but this is all I could see from scrolling through the changes! You've really done a great job, thanks :)
All the suggestions are just slight optimisations and tidy up, and I'm open to discussion if you disagree with any of them.

lib/src/layer/tile_layer/tile_manager.dart Outdated Show resolved Hide resolved
lib/src/layer/tile_layer/tile_layer_options.dart Outdated Show resolved Hide resolved
lib/src/layer/tile_layer/tile_layer.dart Outdated Show resolved Hide resolved
lib/src/layer/tile_layer/tile_layer.dart Outdated Show resolved Hide resolved
The Tile loading was initiated before adding the Tile to the TileManager
which meant that with a very fast connection or cached Tile images the
load callback would be called before the Tile was in the TileManager.
Therefore when we tried to set the loaded value of the Tile in the
callback we would not find the Tile and the Tile was never marked as
loaded.

This caused the tile to be removed sooner than it should be since we
prune tiles that are not loaded yet when zooming in.
@rorystephenson
Copy link
Contributor Author

Thanks @JaffaKetchup I've made those changes 👌 !

@JaffaKetchup
Copy link
Member

Thanks. I'll do a full test at some point today!

…he exports since it's needed for custom TileProvider implementations
Copy link
Collaborator

@ibrierley ibrierley left a comment

Choose a reason for hiding this comment

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

All looks good to me. Great stuff, and thanks for all the work done on this.

@JaffaKetchup
Copy link
Member

Testing now...

@JaffaKetchup JaffaKetchup self-requested a review May 28, 2022 11:12
Copy link
Member

@JaffaKetchup JaffaKetchup left a comment

Choose a reason for hiding this comment

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

LGTM. Many thanks once again!

@JaffaKetchup JaffaKetchup merged commit 77c91eb into fleaflet:master May 28, 2022
@rorystephenson
Copy link
Contributor Author

Awesome I wasn't expecting it to get reviewed and merged so quick, thanks to both of you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] TileLayer lag/jitter
3 participants