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

Prepared Contains mismatch with Contains #1007

Closed
pramsey opened this issue Dec 4, 2023 · 5 comments · Fixed by #1008
Closed

Prepared Contains mismatch with Contains #1007

pramsey opened this issue Dec 4, 2023 · 5 comments · Fixed by #1008
Labels
Milestone

Comments

@pramsey
Copy link
Member

pramsey commented Dec 4, 2023

As reported at https://trac.osgeo.org/postgis/ticket/5634. The issue is in GEOS prepared geometry.

A test for PreparedGeometryTest.cpp:

template<>
template<>
void object::test<3>
()
{
    g1 = reader.read( "MULTIPOLYGON(((-60 -50,-70 -50,-60 -40,-60 -50)))" );
    g2 = reader.read( "GEOMETRYCOLLECTION(MULTIPOINT((-60 -50),(-63 -49)))" );

    pg1 = prep::PreparedGeometryFactory::prepare(g1.get());

    ensure(  g1->contains(g2.get()) );
    ensure( pg1->contains(g2.get()) );
}
@pramsey pramsey added the Bug label Dec 4, 2023
@pramsey pramsey added this to the 3.9.6 milestone Dec 4, 2023
@pramsey
Copy link
Member Author

pramsey commented Dec 4, 2023

Seems to require the double nesting to exercise the bug, both

MULTIPOINT((-60 -50),(-63 -49))

and

GEOMETRYCOLLECTION(POINT(-60 -50),POINT(-63 -49))

work as variations on the second argument.

@pramsey
Copy link
Member Author

pramsey commented Dec 4, 2023

@cuteDen-ECNU as a general proposition, every prepared variant should return the same as the non-prepared variant, I imagine there's a lot of possible issues hiding in that proposition.

@dr-jts
Copy link
Contributor

dr-jts commented Dec 5, 2023

Tests expressed with geosop:

bin/geosop -a "MULTIPOLYGON(((-60 -50,-70 -50,-60 -40,-60 -50)))" -b "GEOMETRYCOLLECTION(MULTIPOINT((-60 -50),(-63 -49)))" contains
==> true

bin/geosop -a "MULTIPOLYGON(((-60 -50,-70 -50,-60 -40,-60 -50)))" -b "GEOMETRYCOLLECTION(MULTIPOINT((-60 -50),(-63 -49)))" containsPrep
==> false

I thought this might be related to #982, but in fact in this case the contains result is correct, but the preparedContains result is wrong. Also, "unwrapping" the MULTIPOINT produces the correct result:

bin/geosop -a "MULTIPOLYGON(((-60 -50,-70 -50,-60 -40,-60 -50)))" -b "MULTIPOINT((-60 -50),(-63 -49))" containsPrep 
==>true

Further investigation is required.

@cuteDen-ECNU
Copy link

as a general proposition, every prepared variant should return the same as the non-prepared variant, I imagine there's a lot of possible issues hiding in that proposition.

I agree with you. I am refining my tool to focus on this kind of issue.

@dr-jts
Copy link
Contributor

dr-jts commented Dec 5, 2023

The problem is caused by this line in AbstractPreparedPolygonContains:

if (geom->getNumGeometries() > 1) {

This does not handle GeometryCollections which contain one element which itself contains multiple elements.

A simple fix is to replace getNumGeometries with getNumPoints.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants