Skip to content
New issue

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

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

Already on GitHub? # to your account

[BUG] Polylines lagging after upgrading from Flutter 2.5 to 2.10 #1217

Closed
3 of 8 tasks
Tracked by #1165
MeixDev opened this issue Apr 15, 2022 · 14 comments · Fixed by #1219
Closed
3 of 8 tasks
Tracked by #1165

[BUG] Polylines lagging after upgrading from Flutter 2.5 to 2.10 #1217

MeixDev opened this issue Apr 15, 2022 · 14 comments · Fixed by #1219
Labels
bug This issue reports broken functionality or another error

Comments

@MeixDev
Copy link

MeixDev commented Apr 15, 2022

Describe The Bug
After upgrading a project from Flutter 2.5.2 to Flutter 2.10.4, Polylines induce a lot more lag on the map, especially on Android (it seems fine on iOS). I saw other issues already called for this, but were either closed or focusing on a different issue.
In our current usecase, we got a list of 764 lines, for a total of 4354 points.
Zooming on the map obvisouly helps a bit, but isn't an acceptable solution. We do already use polylineCulling: true and didn't notice any significant amelioration.

Expected Behavior
To have the same amount of lag (which was a bit but not much when all the lines were displayed on-screen, and almost none with a bit of zooming in).

Screenshots & Recordings

Here is a video of the app in the 2.10.4 version of Flutter :
https://youtube.com/shorts/3O8CmaJ5cf4?feature=share

The same exact data, but this time in the 2.5.2 version of Flutter :
https://youtube.com/shorts/eyhn7JRGOtY?feature=share

Additional Information
The issue was also observed by my coworker @TesteurManiak. We tried to use both the pub.dev version and the git:master version of flutter_map, without any noticeable difference (Except the fact that the filling of polygons disappeared in the git version. Should we open another issue for that one ?)


Doctors Report

Doctor summary (to see all details, run flutter doctor -v):
[√] Flutter (Channel stable, 2.10.4, on Microsoft Windows [version 10.0.19044.1586], locale fr-FR)
[√] Android toolchain - develop for Android devices (Android SDK version 29.0.2)
[√] Chrome - develop for the web
[√] Visual Studio - develop for Windows (Visual Studio Professional 2019 16.11.3)
[√] Android Studio (version 2021.1)
[√] VS Code (version 1.66.2)
[√] Connected device (4 available)
[√] HTTP Host Availability

• No issues found!

To Reproduce
If necessary, it is possible for us to provide an extract of our List of Polylines as a json model, with a fromJson/toJson extension on the current model.


Severity
This will help us to label the issue quicker and decide what needs attention first. Only choose fatal if the entire app crashes, otherwise choose non-fatal.

  • Non-Fatal
  • Fatal / App Crashes

Frequency/Rarity
This will help us to label the issue quicker and decide what needs attention first.

  • Once
  • Uncommon
  • Common
  • Always

Applicable Platforms
Only select those that you've tested on - one or more. If possible, test on a variety of platforms.

  • Android
  • iOS
  • [?] Web
  • [?] Windows
  • [?] Others (beta platforms)
@MeixDev MeixDev added bug This issue reports broken functionality or another error needs triage This new bug report needs reproducing and prioritizing labels Apr 15, 2022
@ibrierley
Copy link
Contributor

If the performance drops with the exact same version of flutter_map but different versions of flutter, you may need to do some isolating. You could try and produce a minimal reproducable example without any plugins (even better if it can be without flutter_map, just using canvas/drawpath etc or whatever).

Couple of bits you could try hacking with to see if it makes any difference, try removing the opacity widgets in flutter_map and in the tile_layer (if using), and also try removing any saveLayer and restore from the polyline/gon layers.

Regarding poly fills that are missing in a different version, I'd raise that as a separate issue (assuming its not related to the flutter version change).

@MeixDev
Copy link
Author

MeixDev commented Apr 15, 2022

It seems to be unrelated to the Flutter version indeed. I'll try to take time to open the issue later in the weekend.

My friend @TesteurManiak apparently began working on a reproducible sample using a snapshot of our data. We'll see what we can do. It probably still will be using flutter_map for now, but should allow us to at least share something to work with.

@TesteurManiak
Copy link
Contributor

Here's a sample project which reproduce our issue: https://github.com/Floating-Dartists/flutter_map_sample
The same code compiled with Flutter 2.5.2 is much faster than when it is compiled with Flutter 2.10.4.

@TesteurManiak
Copy link
Contributor

I've also tested with Flutter 2.8.1 and the issue is the same as with version 2.10.4.

@ibrierley
Copy link
Contributor

There's a lot of moving parts with that example, using compute etc, 2 lists to parse etc. I would try and get a minimal example, without any of that processing at all, to try and isolate the issue. Do the basic polyline examples perform different in different versions (if you add more lines to test for example).

@ibrierley
Copy link
Contributor

Just did a quick test, this doesn't get down to the problem (flutter versions), but if you remove the 4 canvas.saveLayer/canvas.restore calls, it's certainly faster.

I can't easily test with flutter versions older than 2.8.1 to know if that brings it more into line with versions before that.

@ibrierley
Copy link
Contributor

Sorry, that was in the polyline_layer.dart code in case that wasn't clear.

@ibrierley
Copy link
Contributor

Note, if you do play with that, double check any polylines that have opacity and overlap, as those may not be quite right, as I think those were introduced to fix that.

This may all be a redherring, as it doesn't address flutter version issue, but I was just noticing the performance change when using canvas layers or not.

@TesteurManiak
Copy link
Contributor

TesteurManiak commented Apr 16, 2022

@ibrierley You're right, by disabling the canvas.saveLayer & canvas.restore the map became a lot faster, I'm going to keep testing to see if other modification can help.

@ibrierley
Copy link
Contributor

I'd be interested if you think you can equate old flutter to similar performance to newer flutter versions without saveLayer.

I've previously wondered if they "fixed" something, which caused added resources to be used, but I've never found anyone else really exhibiting new issues. I don't think there's many others using saveLayer as much as some of our code. Flutter_map I think is a bit odd, that it causes a saveLayer on every line (I think), which feels clunky, but is probably correct for people who use things like opacity or filters. If you don't it's maybe a hinderance. So one thing that's been pondered is to make saveLayer an option.

@TesteurManiak
Copy link
Contributor

Yeah definitely, for now I've created a new PolylineLayerOptions class with an optional parameter updateCanvas to quickly compare with it enabled or disabled. The best way would probably be to check beforehand if any polyline is relying on opacity or filters to set the option (and allows the developer to completely override the value).

@TesteurManiak
Copy link
Contributor

@ibrierley would you be interested in a PR which would add a parameter to disable calls to saveLayer & restore ?

I could also include this suggestion made last month: #1165 (comment)

@ibrierley
Copy link
Contributor

Yes, all PRs welcome! Only think I couldn't decide when thinking about it, was whether to have the default as on or off for saveLayer. Feels like it would be better disabled, but may catch a few out who already have opacity on lines for example, but I personally think its better disabled and if people shout it's suggested they enable it.. I'm happy to be convinced otherwise though!

@MeixDev
Copy link
Author

MeixDev commented Apr 19, 2022

It seems like the best solution to me as well. None of our usecases needed opacity on lines, which made the multiple calls to saveLayer unneeded. I tend to think that most other usecases don't necessarily need opacity either, which makes it a niche case.

Glad to see a solution found !

@JaffaKetchup JaffaKetchup linked a pull request Apr 19, 2022 that will close this issue
@JaffaKetchup JaffaKetchup removed the needs triage This new bug report needs reproducing and prioritizing label Apr 19, 2022
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug This issue reports broken functionality or another error
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants