Skip to content

Marker dimensions using meters #2106

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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

LeonTenorio
Copy link
Contributor

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
Copy link
Member

JaffaKetchup commented Jun 8, 2025

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor Author

@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

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.

3 participants