refactor: use separate painters for each Polyline

[thomascoumau]

Change PolylineLayer logic to have a custom painter for each polyline, this allow the layer to not overtake the map tap on objects that are not part of the polylines

[thomascoumau]

Hello,
I have made this PR to improve the logic of the layer painter and in order to be able to have a tap on each polyline individually in the future

[JaffaKetchup]

Thanks @thomascoumau!

I'm wondering how having multiple custom painters will impact performance. Previously, CircleMarkers also used multiple painters, but #1679 found that this was pretty bad for performance. Is having multiple painters the only way to do this? If it is, then there should probably be some batching so that polylines without a tap can be bundled into one painter - but I'm not sure how that will work over this core and FMTP.

It's also important to note that this breaking (albeit only for a minority of users), and so may not appear in a release for a little while - we don't want to overwhelm users with too many breaking changes. This isn't too important though.

[thomascoumau]

Hey, sorry for the late response.
I dont really understand why having a multitude of painter could lead to having less performance as the number of paths you draw is the same, maybe there is a flutter issue about this ? Using a single painter for a layer I can improve the tap of all the polyline combined but I cant distinguish on which polyline the user is applying a tap.

[JaffaKetchup]

I can't really explain it myself, but I guess it's similar to the reason saveLayer and similar canvas operations can be expensive. They require more draw calls in the GPU and reduce its ability to batch. Or something like that, I don't really know.
Maybe we should do a little bit of testing just to ensure, such as checking the framerate with many polylines.

[thomascoumau]

Yeah, I think we need some testing on the performance of course. Do you want me to make some tests and send you a report of them. I never really have tested the performance on my app but I am eager to learn and I think this would be a good opportunity. If you have any tests I must do let me know, or if you don't want me to run the investigation let me know to 👌

[JaffaKetchup]

I don't mind you running the tests. Just need to not change anything else except this, then compare FPS based on number and length of polylines.

[ignatz]

Performance aside (Jaffa is right, drawing one path with many lines to a canvas is cheaper than drawing many lines to the same canvas. Each draw call just makes the CPU tell the GPU to draw eagerly), is the goal to make individual polylines tappable? Even with many small Canvases stacked on top, I'd expect this to be challenging. At the of the day, the canvas is rectangular and not line shaped. If it works for your use-case, I may suggest to just Stack many PolylineLayers rather than changing the PolylineLayer itself. The Result should be the same.

[thomascoumau]

Hello @ignatz,
I was thinking about stacking layer of polylines. But it would not be efficient for uses cases where you trigger a specific event for each polyline you build. As an example in the app I am working on, when you tap on a polyline, you have precise data, based on the polyline you tapped. If I were to stack different layer I would have to make a polyline layer for each polyline I want to display on screen.
However, I think that you are right and in some use cases stacking layers would be more logic and efficient.
I have made this PR here because I think that on the FMTP repo, we want to extend the polylines class and not create our own layer. I think I should check FMTP first to be sure I understand their vision and also see how this implementation drop performances first before making any decisions.

[ignatz]

Thanks for confirming that you want individual polylines to be tappable. As I was trying to explain before, I don't think either approach will work well. Think about the rectangular hit detection of the canvas:

To get the correct hit detection, you'd probably be better off stacking a bunch of positioned, rotated, box widgets

Performance-wise, I'm not sure why a Stack of layers with one polyline each vs stacking them internally would be very different. It seems to be more about the distribution of responsibilities 🤷

[thomascoumau]

Ok thank you for your response I understand a little better your position on why it should be only one canvas for all the polylines. However, I tested and using different canvas dont impact the detection since I override the path taken in the hitTestBehavior to match the drawing on screen. I think I should run the performance test before trying to change all the implementation on this matter.

PS: Sorry for the really late response

[ignatz]

Hi @thomascoumau, after you mentioned it I saw that you overwrote the hit test behavior in another branch, that makes sense. Couldn't use a similar approach for when there's more than one line on a canvas? FWIW, I do something similar in my app for polygons. I'd love to have tapable polylines.

[thomascoumau]

Hi @ignatz, it is possible to override the hitTest for all the polyline at once in one canvas. As I explained before, I want to test first if having a canvas per polyline is impacting the performance otherwise I think it is better to keep having a painter per polylines.

[ignatz]

it is possible to override the hitTest for all the polyline at once in one canvas.

In principle yes, you just have to do a bit more work to narrow down the corresponding lines. What I do for polygons is to first prune all the polygons that don't have an overlapping bounding box and then use a more complex algorithm to find whether the tap is inside the polygon. With the multiple canvases you basically get the bounding boxing and z-axis ordering for free.

As I explained before, I want to test first if having a canvas per polyline is impacting the performance otherwise I think it is better to keep having a painter per polylines.

You can estimate the impact at HEAD w/o any code changes. Draw your polylines once with a transparent color and once with an opaque color (the latter needs to be the same color for all lines). The transparent color will result in individual draw calls, alas into the same canvas but the performance should be similar. Whereas the opaque lines will all be drawn in one go. You should experience a significant performance difference.

[ignatz]

I ended up looking into this, since I also could use some tappable polylines. This is what I ended up with:

https://github.com/ignatz/flutter_map-clone/blob/polyline_tap/lib/src/layer/polyline_layer.dart

[JaffaKetchup]

Huge thanks for contributing, but after discussion, this has been replaced by #1728. We're still up for curved lines in future though!