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

Limiting Zoom by style values #180

Closed
wants to merge 10 commits into from
Closed

Conversation

DerKarlos
Copy link

To solve the issue, I added a clamp to Zoom
and used it in ViewState.update_zoom.
I had to add maxzoom/minzoom as parameter/members to ViewState
Not yet changed: style-spec-v8.json
Todo: Error messages
(And it is my first Pull Request ever)

💻 Examples

Set the max/minzoom in the map style to the values, you want to test

🚨 Test instructions

Just zoom in/out to level 0/20. Zooming ends.
But the map sliding not! To be investigated

✔️ PR Todo

@maxammann
Copy link
Collaborator

maxammann commented Oct 8, 2022

style-spec-v8.json

That file should be removed soon. It is not used.

(And it is my first Pull Request ever)

Awensome! Please more! :D

maplibre/src/context.rs Outdated Show resolved Hide resolved
maplibre/src/style/style.rs Outdated Show resolved Hide resolved
Comment on lines 38 to 39
maxzoom: Some(20),
minzoom: Some( 0), // karlos: ok??? Detault values in extra file?
Copy link
Collaborator

Choose a reason for hiding this comment

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

There should be an option in your IDE to run Rust fmt when saving the file.

Copy link
Author

Choose a reason for hiding this comment

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

I do ussually, but your rustfmt.toml causes:
Warning: can't set imports_granularity = Crate, unstable features are only available in nightly channel.
Warning: can't set group_imports = StdExternalCrate, unstable features are only available in nightly channel.
And with commenting out the toml, noting happens.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh yes, you will need nightly fmt. So you will continue to use stable Rust, but you can use nightly formatter.

Try to run: just format or just fmt

Copy link
Author

@DerKarlos DerKarlos Oct 9, 2022

Choose a reason for hiding this comment

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

Not found: just. Is a windows package manager? I use macOS.
rustup install nightly-x86_64-apple-darwin
rustup component add rustfmt --toolchain nightly
info: component 'rustfmt' for target 'x86_64-apple-darwin' is up to date :( no nightly-x86

Copy link
Collaborator

Choose a reason for hiding this comment

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

In that case try to run cargo +nightly fmt

You probably have it already installed :)

Copy link
Author

Choose a reason for hiding this comment

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

Yes! So I have to add "+nightly". I will try to change this in the VS-Code

Comment on lines 48 to 49
// if maxzoom < zoom { error("mapstype error: maxzoom must be > zoom"); }
// if minzoom > zoom { error("mapstype error: minzoom must be < zoom"); }
Copy link
Collaborator

Choose a reason for hiding this comment

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

This brings up a great question: Where do we validate the style? There are probably more "invariants" we could check.
It should not happen in this file, because then we would need to copy&paste this code to the headless.rs file.

Ideally, it should be impossible to construct an invalid Style in the first place. Maybe we can add a constructor to the Style and validate it there.

For now it makes sense to a add a self.validate() function to the Style. We can call this then from anywhere.

Copy link
Author

Choose a reason for hiding this comment

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

I started, but far from perfect. Update soon

Co-authored-by: Max Ammann <max@maxammann.org>
@@ -13,6 +13,8 @@ use crate::{
/// Stores the camera configuration.
pub struct ViewState {
pub zoom: ChangeObserver<Zoom>,
pub maxZoom: u8,
pub minzoom: u8,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same goes for this attribute. Also both should be f64. Zoom is a float. A zoom level is u8.

Copy link
Author

Choose a reason for hiding this comment

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

Found the u8 in style.rs and used it. But yes, vector renderers never should force a "jumping" zoom.

@maxammann
Copy link
Collaborator

@DerKarlos I pushed a fix for the jumping. Basically the issue was that with every zoom there is also a translation of the map (such that the exact coordinates where the mouse points to, stays where the mouse pointer is)

changed max/min to f64
@DerKarlos
Copy link
Author

DerKarlos commented Oct 8, 2022

Two editing the same thing may make trouble.
I could merge it but can't build:
maplibre-winit/src/input/zoom_handler.rs:22:56
|
22 | let zoom_changed = actual_next_zoom != current_zoom;
| ^^^^^^^^^^^^ expected (), found struct Zoom

Uh! I can't merge, can't pul, just lost between all the tools.

Bordbuch nachtrag:

Your Zoom.ne needs an epsilon. So this causes the error?: let zoom_changed = actual_next_zoom != current_zoom;
update_zoom has no return but used like it does
May be I made a mistake while merging.
I added the -> Zoom to update_zoom and now it builds runs and zoom limit works without sliding the map.
It is merged and should be visible. May be the draft state could be removed

@maxammann
Copy link
Collaborator

@DerKarlos merged in main to fix builds

@maxammann maxammann changed the title limiting Zoom by mapstyle values (not done) Limiting Zoom by style values Oct 29, 2022
@maxammann
Copy link
Collaborator

@DerKarlos Hi! Are you still interested in pushing this forward? I may be able to help resolving conflicts.

@maxammann
Copy link
Collaborator

@DerKarlos Still wanna merge this? I think this is close to being finished

@DerKarlos
Copy link
Author

Sorry, live put me to other priorities. You may finish it or delete it.
May be I will be back some day ...

@maxammann maxammann closed this Jul 26, 2024
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