georust / geo

Rust geospatial primitives & algorithms
https://crates.io/crates/geo
Other
1.58k stars 198 forks source link

IntersectionMatrix::is_covers() seems to be wrong? #1264

Closed pkerichang closed 2 weeks ago

pkerichang commented 2 weeks ago

So I have this simple test case, which fails:

#[test]
fn covers() {
    let box_1: Rect<f64> = Rect::new(Coord { x: -5.0, y: -4.0 }, Coord { x: 0.0, y: 0.0 });
    let box_2: Rect<f64> = Rect::new(Coord { x: -4.0, y: -4.0 }, Coord { x: -1.0, y: -4.0 });

    assert!(relate::Relate::relate(&box_1, &box_2).is_covers());
}

So box_2 is a 0-area rectangle (a degenerated line) that lies completely on the boundary of box_1. From the definition of "covers" on both the DE-9IM wiki article and also the georust documentation, it seems like this should return true, but currently it returns false.

Curiously, if box_2 is a point like Rect::new(Coord { x: -4.0, y: -4.0 }, Coord { x: --4.0, y: -4.0 }) instead, then is_covers() does return true. What's going on here?

urschrei commented 2 weeks ago

JTS doesn't think covers is true either:

input:

POLYGON ((-5 -4, 0 -4, 0 0, -5 0, -5 -4))
POLYGON ((-4 -4, -3 -4, -2 -4, -1 -4, -4 -4))

Screenshot 2024-11-06 at 11 51 37

Could you verify that the polygon points I used to reproduce the geometries match yours? You may have to call to_polygon() and then print them.

pkerichang commented 2 weeks ago

Yes, those polygons do indeed match. Is my understanding of what "covers" mean wrong? Because from how I interpreted the definitions it should return true here.

On Wed, Nov 6, 2024, 7:55 PM Stephan Hügel @.***> wrote:

JTS doesn't think covers is true either:

input:

POLYGON ((-5 -4, 0 -4, 0 0, -5 0, -5 -4)) POLYGON ((-4 -4, -3 -4, -2 -4, -1 -4, -4 -4))

Screenshot.2024-11-06.at.11.51.37.png (view on web) https://github.com/user-attachments/assets/4be7434e-5501-4550-8b88-adcde100493b

Could you verify that the polygon points I used to reproduce the geometries match yours? You may have to call to_polygon() and then print them.

— Reply to this email directly, view it on GitHub https://github.com/georust/geo/issues/1264#issuecomment-2459552862, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB3SJVB4OJAGEG2HESXUYLTZ7H7S3AVCNFSM6AAAAABRIFFANSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINJZGU2TEOBWGI . You are receiving this because you authored the thread.Message ID: @.***>

urschrei commented 2 weeks ago

This is related to the fact that it's a Polygon and degenerate (though I can't find a reference at the moment); if you change the second geometry to a LineString:

LINESTRING (-4 -4, -3 -4, -2 -4, -1 -4)

The covers predicate is what you expect (true).

PostGIS (so GEOS) reproduces the same false predicate:

WITH poly1 AS (
    SELECT ST_GeomFromText('POLYGON ((-5 -4, 0 -4, 0 0, -5 0, -5 -4))') AS geom1
),
poly2 AS (
    SELECT ST_GeomFromText('POLYGON ((-4 -4, -3 -4, -2 -4, -1 -4, -4 -4))') AS geom2
)
SELECT ST_Covers(p1.geom1, p2.geom2) AS is_covering
FROM poly1 p1, poly2 p2;
michaelkirk commented 2 weeks ago

For my own feeble brain, I simplified the case and drew it up.

IMG_4807

geo (mostly) follows OGC validity.

Rings may not self-intersect (they may neither touch nor cross themselves).

So this is an invalid polygon, and I don't really expect Topology algorithms to work with invalid geometries.

One work around for you would be to use HasDimensions::dimensions to detect degenerate Rects, and if encountered, convert it to a LineString and run Relate on that.

urschrei commented 2 weeks ago

For some reason JTS didn't make it obvious that Polygon B was invalid, but yes that's very much the issue. I'm going to close this.