Skip to content
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

Add parameter to fit to integer zoom levels #1367

Merged
merged 3 commits into from
Sep 26, 2022

Conversation

Robbendebiene
Copy link
Contributor

Since flutter map is mostly used with pixel tiles, it is often desirable to use integer zoom levels in order to avoid any pixel tile upscaling. Currently this isn't possible using the fitBounds and centerZoomFitBounds methods.

This PR adds an additional parameter to the FitBoundsOptions called forceIntegerZoomLevel in order to force the calculation of applicable integer zoom levels of the aforementioned functions.

Copy link
Contributor

@TesteurManiak TesteurManiak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, if everybody is okay with this feature it can be merged (cc @JaffaKetchup)

Copy link
Contributor

@ibrierley ibrierley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have any code to show the effectiveness of this at all ?

@Robbendebiene
Copy link
Contributor Author

Robbendebiene commented Sep 19, 2022

@ibrierley I require this in a project I'm working on, which is also the place where I've tested it. What kind of example do you have in mind? The degree of effectiveness is similar to the inside parameter.
Normally one can simply round the zoom level outside of the flutter map code basis, but this doesn't work when fitting to bounds.

@ibrierley
Copy link
Contributor

I'm just thinking of a way to test it is all, so maybe future code changes can use to check its still working (don't really mean a full example page, but just something people can use to highlight its working or not.

@Robbendebiene
Copy link
Contributor Author

@ibrierley I've added a test that checks the fit bounds methods for some parameters. The change of the expected zoom values and the consistent center point give an impression of what is going on. At least this test can fail in the future if any internal implementation details change.

@JaffaKetchup
Copy link
Member

@ibrierley This good for you?

@JaffaKetchup JaffaKetchup added the feature This issue requests a new feature label Sep 22, 2022
Copy link
Contributor

@ibrierley ibrierley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't see any obvious problems if others are happy with it!

@JaffaKetchup
Copy link
Member

Sure, you merge it if you're happy.

@JaffaKetchup JaffaKetchup merged commit fb3f270 into fleaflet:master Sep 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature This issue requests a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants