Implement RTreeObject for Geometry enum

[urschrei]

Fixes #982

Still draft as I haven't written any tests yet.

• 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.

---

[michaelkirk]

It makes me (only a little) itchy that we now have a separate, mostly equivalent, implementation of bounding_rect for geometry in geo and now in geo_types for the RTree integration.

I had a quick go at seeing what it'd look like to share a backing implementation here, based on your branch: https://github.com/georust/geo/tree/mkirk/rtreeobject

On the one hand, that approach is not a small endeavor. On the other hand, it's nice to dedupe the implementations and also, IMO solves at least the majority of the need for new tests since the existing bounding_rect has decent testing.

It's not clear to me which is better, so if you prefer to have the duplicated implementation, I'm on board.

[michaelkirk]

Whoops, I left comments on the commit rather than in the PR.

Let me know if you want me to recreate them: 93dce71

[urschrei]

I vastly prefer the shared version, so that's incorporated now. In terms of tests, I want to make sure we're testing

1. GeometryCollections with empty envelopes, since I think that's the only real new logic we've added.
2. Adding and Querying a mixture of geometries as Geometry enum members

1 is probably a bit trickier.

[michaelkirk]

So now that we have the BoundingRect trait in geo-types, I was thinking that the impl for Geometry and all enum members (including GeometryCollection here) could be something like:

fn envelope(&self) -> Self::Envelope {
    self.bounding_rect()
        .map(|geo_rect| geo_rect_to_rstar_aabb(geo_rect))
        .unwrap_or(|| Envelope::new_empty())
}

And for the types whose bounding_rect is not-optional (like point, line, triangle, rect):

fn envelope(&self) -> Self::Envelope {
    geo_rect_to_rstar_aabb(self.bounding_rect())
}

[michaelkirk]

I didn't get as far as testing whether it would work out though 😬

[urschrei]

I have just realised that we also need to impl PointDistance which has really ruined my day.

[michaelkirk]

Are you sure we need to implement PointDistance for everything?

I'm AFK so can't easily verify, but I seem to recall we could get some useful functionality with only the RTreeObject impl.

Like maybe we could get "intersection candidates", but not "nearest neighbor", which still seems like useful progress.

[urschrei]

It certainly works, but everything other than the envelope-based queries require PointDistance. So I don't know where to go from here. I'm happy to add some more tests, write the geo_rect_to_rstar_aabb stuff (which I think will work), but I see no easy to way to implement PointDistance using the current crate architecture. Even re-implementing just Point-[geom] distance wholesale isn't enough because individual impls require e.g. Intersects.

[JosiahParry]

Let me know if I can help contribute to this PR as it would be very helpful.

[JosiahParry]

Given #1029 it can be quite simple to implement PointDistance for all geometry type such as

impl PointDistance for Geometry {
    fn distance_2(
            &self,
            point: &<Self::Envelope as rstar::Envelope>::Point,
        ) -> <<Self::Envelope as rstar::Envelope>::Point as rstar::Point>::Scalar {
            let pnt = geo_types::point!(geo_types::coord!{x: point[0], y: point[1]});
            let d = &self.geom.euclidean_distance(&pnt);
            d.powi(2)
    }
}

[urschrei]

Given #1029 it can be quite simple to implement PointDistance for all geometry type such as

impl PointDistance for Geometry {
    fn distance_2(
            &self,
            point: &<Self::Envelope as rstar::Envelope>::Point,
        ) -> <<Self::Envelope as rstar::Envelope>::Point as rstar::Point>::Scalar {
            let pnt = geo_types::point!(geo_types::coord!{x: point[0], y: point[1]});
            let d = &self.geom.euclidean_distance(&pnt);
            d.powi(2)
    }
}

It's been a long time, but the problem with this approach is that RTreeObject has to be implemented in geo-types, not geo. That means that none of the algorithms required to calculate distance_2 are available to us: they all reside in geo.

[JosiahParry]

@urschrei thanks for explaining! Perhaps, it could be feature gated in geo_types?

[urschrei]

It's not a feature issue, it's a circular dependency issue: there are no algorithms in geo-types (well, there are some secret ones for internal use only) and we can't pull in any from geo, because geo depends on geo-types. We've kinda painted ourselves into a corner by separating types from algorithms, and nobody is really sure how we get out of it.

[michaelkirk]

Let's merge geo and geo-types back together and have the --no-default-features build be types only.

[urschrei]