feat: add ability to use meters to size Markers

[LeonTenorio]

Feature to render marker using size in meters.

Added 4 new parameters:

• useSizeInMeters: When true (default is false) the marker width and height will be used as meters
• maxWidthUsingMetersPixels: Optional parameter to control the max width in pixels when rendering the marker using size in meters (In that mode when the zoom increase the marker size also increase and we a lot of times want to limit that region)
• maxHeightUsingMetersPixels: Optional parameter to control the max height in pixels when rendering the marker using size in meters (In that mode when the zoom increase the marker size also increase and we a lot of times want to limit that region)
• minWidthUsingMetersPixels: Optional parameter to control the minimal width in pixels when rendering the marker using size in meters (Sometimes we will want that the marker appers in tha map with a minimal size)
• minHeightUsingMetersPixels: Optional parameter to control the minimal height in pixels when rendering the marker using size in meters (Sometimes we will want that the marker appers in tha map with a minimal size)

[JaffaKetchup]

Hi @LeonTenorio,
Thanks for the PR! I'm not sure whether we will include it in the next release, or whether it will be for the one after - it depends how much work it needs.
I think at the moment this adds a little bit too much complexity with the 4 extra params just to control sizing in edge cases. Would replacing the 4 params with a single BoxConstraints work better?
Over time we could also extend this to replace the width and height, and just have a constraints and useConstraintsInMeters parameter. But that's out of scope.
I am wondering whether it would be possible for us to remove the Marker.width and Marker.height completely, and infer from the child instead - perhaps using custom RenderObjects or something. I don't know how this would factor in - maybe we would need a builder with a ratio exposed (although that could be done to workaround this even now). But this is just an idea for now and very out of scope for this PR. (And ofc, that makes culling before build difficult).

[LeonTenorio]

Hi @JaffaKetchup
I updated the merge request with the box constraint suggestion and definitely is better that way without so many parameters.
I also updated my code with the changes in the package master branch code.
Waiting for another review and those features in the next release version of he package.

[ReinisSprogis]

Hi. Maybe need to guard a bit more, because setting BoxConstraints can be infinity but will not be null. For example I can provide BoxConstraints.expand(); A parameter is valid, but would crash (I think).

[LeonTenorio]

@ReinisSprogis I don't think it would crash with BoxConstraints.expand(), with that value the check if the height and width are in the interval will do nothing

[JaffaKetchup]

Sorry, this slipped under the radar! We're about to release v8.3 but I won't include this now. I do think it's a good feature though, and I'm no longer working on the other thing I mentioned about markers, so it's a good candidate for the next release, which hopefully won't be as long as it's been since the last version. Instead, hopefully we can get this reviewed and merged for v8.4, which might also include #2145, #2198, and #2188.

[JaffaKetchup]

LGTM! I've made some significant changes though, so I'll wait for another maintainer to review :) @fleaflet/maintainers

It now follows the caching of projections introduced in #2213, and provides a setting to optimize calculations, same as the circle layer.

[vinlet]

Sorry to revive the thread now, I just wanted to ask if this is considered a breaking change. I was testing flutter_map 8.4.0-dev.1 locally, and I also use flutter_map_marker_cluster 8.2.2 (latest version at the time of writing) which depends (with caret syntax) on flutter_map ^8.2.2, so it can resolve to flutter_map 8.4.0-dev.1. But flutter_map_marker_cluster has this class

class MarkerNode extends MarkerOrClusterNode implements Marker

The new useDimensionsInMeters field from Marker is obviously not implemented in this class which breaks compilation. People who use that package won't be able to upgrade to flutter_map 8.4.0 until its maintainer fixes this

[JaffaKetchup]

Technically yes, this should be a breaking change. Any time we add anything to a non-final class, it should be breaking.

In reality, that would be a bit silly because we'd have a million major versions and everyone would just spend their time upgrading their pubspec. We, for example, add new stuff to MapOptions and InteractionOptions fairly frequently without marking it breaking.

The class modifiers were introduced partially because of this very point. But our code predates these by a long way and so are not often used.

We did not expect Marker to be subclassed - it wasn't explicitly designed for that. Therefore, I would suggest that this will not be marked as a major release. Although it would be good for the other @fleaflet/maintainers to weigh in.

@lpongetti are you aware of this? Is there a way to implement your package without relying on subclassing Marker?