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

improve autozooming on non-gpx background data layers #3826

Merged
merged 2 commits into from
Feb 6, 2017

Conversation

tyrasd
Copy link
Member

@tyrasd tyrasd commented Feb 6, 2017

The original implementation only worked for Points and LineStrings, but GeoJSON and KML can contain any geometry.

Aso, d3 is a bit special when working with GeoJSON Polygons: it does require clockwise winding, but typical GeoJSON uses either counterclockwise winding (RFC7946) or "winding doesn't matter" mode (old geojson.org spec). When inputing such GeoJSON (or KML) input, iD would zoom out all the way, which is not the intended behavior.

old implementation only worked for points and linestrings, but geojson and kml can contain any geometry.

aso, d3 is a bit special[1] when working with geojson polygons: it does require clockwise winding, but typical geojson is either counterclockwise winding (RFC7946) or "winding doesn't matter" (old geojson.org spec). When inputing such geojson (or kml) input, iD would zoom out all the way, which is not the intended behavior.

[1] https://github.com/d3/d3-geo#d3-geo
@bhousel
Copy link
Member

bhousel commented Feb 6, 2017

Also, d3 is a bit special when working with GeoJSON Polygons: it does require clockwise winding,

Whoa I did not know this, good catch.. All the GeoJSONs I tried this with contained LineStrings..

@bhousel bhousel merged commit 20b7994 into master Feb 6, 2017
@bhousel bhousel deleted the tyrasd-backgroundlayerAutozoom branch March 11, 2017 02:00
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.

2 participants