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

First pass at creating correctly-oriented polygons. #117

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

urschrei
Copy link
Member

@urschrei urschrei commented May 7, 2019

See #53

urschrei added 3 commits May 7, 2019 15:16
While it's possible to use a crate name as a feature if it's optional,
This isn't possible if a feature uses more than one crate. Thus
renaming the geo-types feature to geotypes allows us to optionally use
the geo and geo-types crate
@urschrei
Copy link
Member Author

urschrei commented May 9, 2019

@frewsxcv I've had to rename the geo-types feature to geotypes in order to set up multiple crates as a feature (AFAIK you can only use a crate name as a feature name for a single crate). Lmk if there's a better / non-breaking way to accomplish it…

@urschrei urschrei requested a review from frewsxcv May 9, 2019 11:00
@frewsxcv
Copy link
Member

frewsxcv commented Jun 24, 2019

From the GeoJSON spec:

   o  A linear ring MUST follow the right-hand rule with respect to the
      area it bounds, i.e., exterior rings are counterclockwise, and
      holes are clockwise.

   Note: the [GJ2008] specification did not discuss linear ring winding
   order.  For backwards compatibility, parsers SHOULD NOT reject
   Polygons that do not follow the right-hand rule.

I'm having a little trouble comprehending this, but it to me it sounds like:

  • When reading GeoJSON, allow clockwise or counterclockwise linear rings
  • When writing GeoJSON, output counterclockwise exterior linear rings, clockwise interior rings

Does that sound right? 🤔


Also, separate thought, but instead of pulling in the entire geo crate, we do have the ability to just add a geo-winding-order crate as a dependency. Just an idea!

@urschrei
Copy link
Member Author

urschrei commented Jun 24, 2019

Does that sound right? 🤔

Yep, that's my understanding! Good idea on the winding-order crate if it gives us a compilation speedup…

@frewsxcv
Copy link
Member

frewsxcv commented Jul 9, 2019

Does that sound right? 🤔

Yep, that's my understanding! Good idea on the winding-order crate if it gives us a compilation speedup…

So considering we'll need to potentially reverse the orientation of linear rings when reading and writing GeoJSON, it seems like we'll need to make a dependency on geo (or geo-winding-order) regardless of whether they have geotypes feature enabled.

@frewsxcv frewsxcv removed their request for review September 18, 2020 11:59
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