perf: preserve `Marker` widget state when list of visible markers changes

[ReinisSprogis]

Added ValueKey and RepaintBoundary to preserve Marker state. m.key was always null. However, might be that implementation can produce duplicate Keys if coordinates of the two markers are the same. Need a better sollution for key, but this is start.

[ReinisSprogis]

I found out that m.key is null for all markers, So as soon as children of Stack changed they all rebuild. You can see that in this video with highlite repaints how they all repaint.

https://github.com/user-attachments/assets/9061be11-a2d5-4ed7-919b-67d793aab605

So now with non null key and RepaintBoundary we preserve the state.

https://github.com/user-attachments/assets/05523848-04b6-4f49-9345-4756311ff518

[ReinisSprogis]

Ahh. I missclicked and closed PR. So reopened. It looks to improve with this fix as there is no more of that jank on ui thread.

https://github.com/user-attachments/assets/14dd12a9-27b6-4b63-bebf-da6c8eef4a1a

Unlike before it was lagging as soon as any marker left the screen.

https://github.com/user-attachments/assets/1cd451c7-2666-4988-82db-3fcd6625374e

But need to figure out how to give it a stable and uniqe Key. If you have any ideas, welcome to contribute.

[ReinisSprogis]

Prehaps can add UniqueKey but then cannot make Marker const

Marker({
    Key? key,
    required this.point,
    required this.child,
    this.width = 30,
    this.height = 30,
    this.alignment,
    this.rotate,
  }) : key = key ?? UniqueKey();

[JaffaKetchup]

This is a nice fix, thanks. But I would first like to find out when this issue was introduced, since I don't remember it always being there. Is this ready for review?

[ReinisSprogis]

Hi. Not yet. Still need to figure out what key to use. Right now, the key is based on the marker’s location, but if two markers share the same location, it throws a “Duplicate keys found” exception.

As I mentioned, I’m not sure what the best key would be in that case. One option might be to use a UniqueKey when creating each marker, which would avoid the duplicate issue, but then I wouldn’t be able to declare the Marker as const. That could reduce some of the performance optimizations Flutter provides.

[JaffaKetchup]

I don't know if it's possible, but that might suggest using keys isn't the best way around this.

At the moment, we run a function/generator to return widgets. I guess Stack is smart enough somehow to avoid rebuilding itself and its children when its children don't 'change' (even though the generator does re-run on every frame of panning/zooming/etc.)? Or maybe that's completely wrong (I'm not sure how it would do that necessarily, given the identity of the returned list I think would change on every generation?).

So maybe we need to go a little more complex than Stack?

---

For example, see #2134.

• I was originally intending to use it to remove the width & height of Markers, until I realised that was necessary for culling prior to build. But using a RO still means that each marker is built only once across multiple worlds, just rendered multiple times.
• caveat: even here the RO docs say to use a key potentially - but we can use the Marker itself as the key value.
• point: when testing right now with the many marker example, culling doesn't seem to be necessary because each marker seems to be built only once ever, not on each rebuild. Need to look into if this is stable in a variety of usage conditions. Maybe just because Marker is const, so an ObjectKey depending on it works correctly - and we also just list out the markers once in the example, then take slices? Idk: more research required.
• so maybe it fixes this?
• maybe: if it is indeed only building each marker once across a variety of usage situations (and we're not just lucky with how we've done it) - then maybe width/height for culling is indeed unnecessary and we can remove it?
• it doesn't yet forward hit testing, this is tricky with how I've done it so far

[ReinisSprogis]

Ah yes ObjectKey can be used. #2134 looks good, Not sure I understand it all. I don't have much experiance with custom RenderObjects. Regarding culling, need to test and see whats better. Maybe checking if outside the screen and toggle visibility in Offstage widget https://api.flutter.dev/flutter/widgets/Offstage-class.html I think It would be better to rename Marker to MapWidget and WidgetLayer (or similar) as it has nothing to do with Marker as such. If user want's to put marker as child, thats ok.

[JaffaKetchup]

@ReinisSprogis For the moment, #2134 is nice but low priority for me right now (I've got limited time for the next while). I think that's the "correct" way to do things - moreso the "right" way to support multi-worlds (as it just renders multiple times instead of building multiple times) - but it's a lot to figure out and I'm struggling with the rotation stuff & coordinate maths (as I mentioned there). It's close to working, but not there just yet. + research is needed into culling need.

Therefore, for the time being, this might be worth perusing if you're still interested.

Renaming stuff is not possible for the time being. If a user wants to overlay a widget more precisely, there's a plugin for that which we could consider incorporating in the future, but the marker API & usage is pretty standardised across mapping libraries and so doesn't need changing.

But if using the Marker as the ObjectKey(...) works, then I think this could be a very good improvement.

[ReinisSprogis]

Hi @JaffaKetchup. I am bit busy at the moment too. I can check on this some time next week.

[ReinisSprogis]

I don't know why, but seems like performance is overal reduced with this fix. But removes that lag when markers are culled.

[JaffaKetchup]

Hey, unfortunately I haven't had much time to review this in any detail and I'm away for a while soon, so I'm not sure when I'll get round to reviewing this, but I'll try not to leave it too long!

[ReinisSprogis]

Hi! @JaffaKetchup No worries. I have been busy myself. I too suspect it's RepaintBoundry. Not sure what can be done about it. Widgets are just not a good way to draw markers to be honest. Adding/removing thousands of widgets to widget, element and render tree I think is what slows it all down. The CircleLayer was not performant enough either. I ended up making a custom solution that covers all my needs. I use it in production as it is good enough, but there still are some bugs and things I would want to change if I publish as a package. But I have been bit hesitant to release it as package also because this puts a responsibility for little to no reward and I have not much experience with maintaining one. Maybe this could be the "reward". It is very hard to make a "one fits all" marker solution. But I think I am on the right track with this. You can check it out here. I uploaded as unlisted. https://www.youtube.com/watch?v=xLGqlNFpHOE It's a typical use case for me where I want a custom marker shape with Icon and I can dynamically change colors and icons. So I replicated it in flutter_map using Marker widget. This is huge amount of markers and not a typical use case, but might be useful as in like plane or ship tracker style maps. On Raster thread performance is similar but significantly better on UI thread. Towards the end you can see I will toggle on "magic" mode that takes performance to another planet :D. But it has some limitations and still needs work.

[JaffaKetchup]

Hey, I've had a look at the video, and yeah it does look like the UI thread is doing a lot better.

The tiled display mode looks very interesting as well - that could definitely be a good option for some people. Although does it only support static markers?

[JaffaKetchup]

For the time being, I'm going to close this PR - but that's not to say we're not interested in improving the marker performance. I'd be very interested in seeing a plugin - as you said there is not much reward and it takes a lot of effort, but still sharing knowledge is never a bad thing. And ofc if there's something more similar to this PR which can be done (by which I mean changes on a smaller scale), we'd be more than happy to accept another PR or re-open this one.

I am wondering whether there's something simpler we can do to prevent this issue of lagging when markers are entering and exiting the viewport - at the moment we're using a generator because it was easy to implement, but I guess the problem is it has to regenerate all the existing markers for every new marker. Maybe the key is to use keys similar to this PR, but maybe not at the widget level. The polygon/line layers use the same approach as is currently used with markers, but I guess we don't see it there because there's usually fewer polygons? Or maybe I just need to have a dig.

Anyway, thanks for this PR and working on this for your own project at least - and interesting to see where it's got you! Maybe once the modern tile layer is done, using tiles for things like this could be a little easier.