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

FIX MultiPoint#dimensions #561

Merged
merged 2 commits into from
Dec 8, 2020
Merged

FIX MultiPoint#dimensions #561

merged 2 commits into from
Dec 8, 2020

Conversation

michaelkirk
Copy link
Member

@michaelkirk michaelkirk commented Dec 7, 2020

Simply wrong.

  • I agree to follow the project's code of conduct.
  • I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

@@ -283,7 +283,7 @@ impl<C: CoordinateType> HasDimensions for MultiPoint<C> {
return Dimensions::Empty;
}

Dimensions::OneDimensional
Dimensions::ZeroDimensional
Copy link
Member Author

Choose a reason for hiding this comment

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

straight up bug. 😞

@martinfrances107
Copy link
Contributor

This looks like a good change to me.

Copy link
Member

@frewsxcv frewsxcv left a comment

Choose a reason for hiding this comment

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

good catch 🐛

@frewsxcv
Copy link
Member

frewsxcv commented Dec 8, 2020

bors r+

@bors
Copy link
Contributor

bors bot commented Dec 8, 2020

Build succeeded:

@bors bors bot merged commit 4540562 into master Dec 8, 2020
@michaelkirk michaelkirk deleted the mkirk/fix-multipoint-dims branch February 10, 2021 18:04
bors bot added a commit that referenced this pull request Apr 13, 2021
639: Introduce the geomgraph module for DE-9IM Relate trait r=michaelkirk a=michaelkirk

- [x] I agree to follow the project's [code of conduct](https://github.com/georust/geo/blob/master/CODE_OF_CONDUCT.md).
- [x] I added an entry to `CHANGES.md` if knowledge of this change could be valuable to users.
---

Fixes #513, #515

(I'm sorry it's so large)

~~I'm going to leave it as a draft (edit: 🤦 I failed to actually open the PR as a draft) while I wait to merge #636 and #638 and then do some rebasing, but I don't anticipate doing other large changes before review.~~ *update: ready for review!*

Here's some of the earlier work in pursuit of this:

#514
#516
#523
#524 
#538
#552
#561
#611
#628
#629
#636 

Primarily, this introduces the geomgraph module for a DE-9IM `Relate` trait.

geomgraph implements a topology graph largely inspired by JTS's module of the same name:
https://github.com/locationtech/jts/tree/jts-1.18.1/modules/core/src/main/java/org/locationtech/jts/geomgraph

You can see some of the reference code if you omit the "REMOVE JTS COMMENTS" commit. In some places the implementation is quite close to the JTS source. 

The overall "flow" is pretty similar to that of JTS, but in the small, there were some divergences. It's not easy (or desirable) to literally translate a Java codebase making heavy use of inheritance and pointers to rust. Additionally, I chose to take advantage of `Option` and rust's enums with associated data to make some case analysis more explicit.

There is a corresponding PR in our [jts-test-runner](georust/jts-test-runner#6) crate which includes the bulk of the tests for the new Relate trait.

## Algorithm Overview 

This functionality is accessed on geometries, via the `Relate` trait, e.g. `line.relate(point)` which returns a DE-9IM [`IntersectionMatrix`](https://en.wikipedia.org/wiki/DE-9IM#Matrix_model).

The `Relate` trait is driven by the `RelateOperation`. The `RelateOperation` builds a `GeometryGraph` for each of the two geometries being related. 

A `GeometryGraph` is a systematic way to organize the "interesting" parts of a geometry's structure - e.g. where its vertices, lines, and areas lie relative to one another.

Once the `RelateOperation` has built the two `GeometryGraph`s, it uses them to efficiently compare the two Geometries's structures, outputting the `IntesectionMatrix`.





Co-authored-by: Michael Kirk <michael.code@endoftheworl.de>
Co-authored-by: bors[bot] <26634292+bors[bot]@users.noreply.github.com>
bors bot added a commit that referenced this pull request Apr 13, 2021
639: Introduce the geomgraph module for DE-9IM Relate trait r=frewsxcv,rmanoka a=michaelkirk

- [x] I agree to follow the project's [code of conduct](https://github.com/georust/geo/blob/master/CODE_OF_CONDUCT.md).
- [x] I added an entry to `CHANGES.md` if knowledge of this change could be valuable to users.
---

Fixes #513, #515

(I'm sorry it's so large)

~~I'm going to leave it as a draft (edit: 🤦 I failed to actually open the PR as a draft) while I wait to merge #636 and #638 and then do some rebasing, but I don't anticipate doing other large changes before review.~~ *update: ready for review!*

Here's some of the earlier work in pursuit of this:

#514
#516
#523
#524 
#538
#552
#561
#611
#628
#629
#636 

Primarily, this introduces the geomgraph module for a DE-9IM `Relate` trait.

geomgraph implements a topology graph largely inspired by JTS's module of the same name:
https://github.com/locationtech/jts/tree/jts-1.18.1/modules/core/src/main/java/org/locationtech/jts/geomgraph

You can see some of the reference code if you omit the "REMOVE JTS COMMENTS" commit. In some places the implementation is quite close to the JTS source. 

The overall "flow" is pretty similar to that of JTS, but in the small, there were some divergences. It's not easy (or desirable) to literally translate a Java codebase making heavy use of inheritance and pointers to rust. Additionally, I chose to take advantage of `Option` and rust's enums with associated data to make some case analysis more explicit.

There is a corresponding PR in our [jts-test-runner](georust/jts-test-runner#6) crate which includes the bulk of the tests for the new Relate trait.

## Algorithm Overview 

This functionality is accessed on geometries, via the `Relate` trait, e.g. `line.relate(point)` which returns a DE-9IM [`IntersectionMatrix`](https://en.wikipedia.org/wiki/DE-9IM#Matrix_model).

The `Relate` trait is driven by the `RelateOperation`. The `RelateOperation` builds a `GeometryGraph` for each of the two geometries being related. 

A `GeometryGraph` is a systematic way to organize the "interesting" parts of a geometry's structure - e.g. where its vertices, lines, and areas lie relative to one another.

Once the `RelateOperation` has built the two `GeometryGraph`s, it uses them to efficiently compare the two Geometries's structures, outputting the `IntesectionMatrix`.





Co-authored-by: Michael Kirk <michael.code@endoftheworl.de>
Co-authored-by: bors[bot] <26634292+bors[bot]@users.noreply.github.com>
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