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

View constraints system review #219

Closed
elemoine opened this issue Feb 20, 2013 · 10 comments
Closed

View constraints system review #219

elemoine opened this issue Feb 20, 2013 · 10 comments

Comments

@elemoine
Copy link
Member

The ol.View2D constructor creates constraints. (We currently just create a resolution constraint). These constraints aims to limit the value space for view properties (center, resolution, rotation).

Constraints are applied in high-level functions like view.zoom and view.rotate. Low-level functions like setResolution and setRotation, onto which the high-level functions are based, do not apply constraints.

Currently, when no resolution constraint is specified in the view options, the ol.View2D constructor creates a resolution constraint that snaps to the OSM resolutions, plus 4 (or 3?) resolutions between consecutive OSM resolutions. Total, this is 4*29 = 116 resolutions.

I think the value for the number of steps between consecutive OSM resolutions is arbitrary, and I'm not sure it should be our default. Instead, I tend to think that should not snap to the OSM resolutions, with no intermediary levels.

This has implications. The ol.Map.createControls_ and ol.Map.createInteractions_ functions currently have default values for zoomDelta that were chosen based on this default number of steps between consecutive OSM resolutions. More specifically zoomDelta is 4 for the Zoom control and the DblClickZoom interaction, and 1 for the MouseWheelZoom interaction.

So if we decide to remove the intermediary levels, which I'm suggesting, we will need to change the default values for zoomDelta.

There are implications on the (not merged yet) TouchRotateAndZoom interaction (#168) as well. I think that pinching should not be constrained on the resolutions. The resolution constraint should be applied on touchend to zoom (with an animation) to the target resolution.

Interested in others' opinions.

@elemoine
Copy link
Member Author

See https://github.com/elemoine/ol3/compare/openlayers:master...zoomfactor.

I'll provide a PR if people agree with these changes. I'm also happy to make other changes based on the feedback I get. Thanks.

@twpayne
Copy link
Contributor

twpayne commented Feb 20, 2013

Looks good. The animation jumps when you use the scroll wheel quickly, and the zoom is far too fast when using the trackpad on OS X. Reducing the number of intermediate zoom steps means that the effective zoom is much faster. I think we need to debounce the mousewheel events as discussed before this can be merged.

@tschaub
Copy link
Member

tschaub commented Feb 20, 2013

Instead, I tend to think that should not snap to the OSM resolutions, with no intermediary levels.

Can you clarify this? If you're saying we should effectively snap to a zoom factor of 2, I agree.

The resolution constraint should be applied on touchend to zoom (with an animation) to the target resolution.

Agreed. I think the pinch behavior in OL2 is good. In OL2 you can also configure your map so that it doesn't snap to a configured resolution at touch end.

Regarding the mouse wheel, it's clear to me that we need to accumulate deltas over some interval and then limit zooming instead of strictly obeying mousewheel events. You could start by accumulating for ~40ms and then limiting to +/- 4 levels (assuming a 2x zoom factor).

@elemoine
Copy link
Member Author

Can you clarify this? If you're saying we should effectively snap to a zoom factor of 2, I agree.

Yes. Check out my branch to verify we're in agreement.

@elemoine
Copy link
Member Author

Regarding the mouse wheel, it's clear to me that we need to accumulate deltas over some interval and then limit zooming instead of strictly obeying mousewheel events. You could start by accumulating for ~40ms and then limiting to +/- 4 levels (assuming a 2x zoom factor).

That's exactly what I started implementing.

@elemoine
Copy link
Member Author

I managed to squeeze a few commits between two fever sessions. My zoomfactor branch now includes code for debouncing mousewheel events (accumulating for 40ms, and limiting to +/- 4 levels as in Leaflet).

I'd be interested to know how it works in various browsers.

https://github.com/elemoine/ol3/compare/openlayers:master...zoomfactor

@twpayne
Copy link
Contributor

twpayne commented Feb 21, 2013

This is a definite improvement. On Mac OS X / Chrome the zooming is better, but still extremely fast, especially with the track pad. This will need some tuning...

@elemoine
Copy link
Member Author

Thanks for testing. I'd need to see it. The maxDelta should protect us from extremely fast zooming.

@elemoine
Copy link
Member Author

http://erilem.net/ol3/examples/full-screen.html?renderer=canvas

Uses maxDelta=1, which gives pretty good results when using my trackpad in FF and Chrome.

@elemoine
Copy link
Member Author

elemoine commented Apr 2, 2013

Addressed with #261.

@elemoine elemoine closed this as completed Apr 2, 2013
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

No branches or pull requests

3 participants