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

Make lwgeom_mindist3d solid aware #373

Closed
wants to merge 19 commits into from

Conversation

@Komzpa
Copy link
Member

commented Feb 11, 2019

Perform raycast to check if we're completely inside.

Lets us remove SFCGAL counterparts completely.

https://trac.osgeo.org/postgis/ticket/4278

Make lwgeom_mindist3d solid aware
Perform raycast to check if we're completely inside.
}

static inline int
lwgeom_solid_contains_lwgeom(const LWGEOM *solid, const LWGEOM *g)

This comment has been minimized.

Copy link
@Komzpa

Komzpa Feb 12, 2019

Author Member

@mhugo can you have a look, is there something obvious that I missed here?

@mhugo

This comment has been minimized.

Copy link

commented Feb 12, 2019

@Komzpa Thanks !
Isn't there a possible problem when the start point of the geometry to test has the same x,y as one of the vertex of the solid ?
In this case, lwpoly_contains_point will return true for each touching polygon, which could double the number of intersections. This will say a point is outside a solid when it is inside.

@Komzpa

This comment has been minimized.

Copy link
Member Author

commented Feb 12, 2019

@mhugo true, this can happen. Is there a simple way to fix this corner case? It's not enough to just check for vertex equality - a thing can be on some diagonal edge.

Technically ptarray_contains_point inside lwpoly_contains_point can output LW_BOUNDARY as hit test result. Probably I can teach lwpoly_contains_point to pass it outside and then count all the LW_BOUNDARY's as half-hits?

@mhugo

This comment has been minimized.

Copy link

commented Feb 12, 2019

@Komzpa

Probably I can teach lwpoly_contains_point to pass it outside and then count all the LW_BOUNDARY's as half-hits?

Half counting is not enough I think. There are situations where the ray may hit the intersection vertex of 3 polygons, or 4 polygons, or even more ...

In 2D, the crossing test for intersection uses an orientation to distinguish edge cases. See http://geomalgorithms.com/a03-_inclusion.html for example.

Another possible issue: what happens if the solid contains a polygon that is parallel to the Z axis and the ray intersects it ? Converting it to 2D will result in a point or linestring that will get ignored, right ?

@Komzpa

This comment has been minimized.

Copy link
Member Author

commented Feb 12, 2019

Okay. So how about just printing a warning on LW_BOUNDARY with a ticket link then, and deal with it when someone actually hits it? :)

Another option I see is to wrap it into a loop that will perform random rotation of both shapes each time LW_BOUNDARY happens. It feels like in finite-length solids with non-zero distance from interior point to edge there has to be a rotation that will hit only insides of polygons.

@Komzpa

This comment has been minimized.

Copy link
Member Author

commented Feb 12, 2019

@mhugo check it out - I'm now randomly skewing geometry if ray hits boundary until it doesn't. Do you maybe have a test case?

@mhugo

This comment has been minimized.

Copy link

commented Feb 13, 2019

@Komzpa I don't really like when an algorithm relies on randomness to finish ...
But, apparently CGAL does something similar (see https://doc.cgal.org/latest/Polygon_mesh_processing/classCGAL_1_1Side__of__triangle__mesh.html).

Some test cases that triggers the rotations would be welcomed indeed.

if (is_boundary)
{
/* randomly skew a bit and restart*/
double cx = lwrandom_uniform()-0.5;

This comment has been minimized.

Copy link
@mwtoews

mwtoews Feb 13, 2019

To make this random, lwrandom_set_seed(0) should be done once somewhere at the top of the while loop.

This comment has been minimized.

Copy link
@Algunenano

Algunenano Feb 13, 2019

Member

At the same time, ST_3DIntersects can't be IMMUTABLE if it relies on random behaviour. Could always setting the same seed be the way to go as it doesn't need true randomness, just a series of pseudo-random numbers?

This comment has been minimized.

Copy link
@mhugo

mhugo Feb 13, 2019

The randomness is not supposed to change the result. It will change the execution time, not the result. So immutable could stay I think.
However, I agree we do not need true randomness here.

Komzpa added 2 commits Feb 13, 2019
@Komzpa

This comment has been minimized.

Copy link
Member Author

commented Feb 13, 2019

@mhugo added a box with a box on top of it aligned with a box inside along Z. Skew is hit now and resolves the case. Any other cases?

@mhugo

This comment has been minimized.

Copy link

commented Feb 14, 2019

@Komzpa Thanks. I would say a last one when the ray hits a vertical face. Something like this (in 2D):
3dintersects_test

The solid could be:

select st_astext(st_extrude(st_translate('polygon((0 0 1,0 0 2,1 0 -1,0 0 -1,-1 0 -1,-1 0 1,0 0 1))',0,-1,0),0,2,0));
=>
POLYHEDRALSURFACE Z (((0 -1 1,-1 -1 1,-1 -1 -1,0 -1 -1,1 -1 -1,0 -1 2,0 -1 1)),((0 1 1,0 1 2,1 1 -1,0 1 -1,-1 1 -1,-1 1 1,0 1 1)),((0 -1 1,0 1 1,-1 1 1,-1 -1 1,0 -1 1)),((-1 -1 1,-1 1 1,-1 1 -1,-1 -1 -1,-1 -1 1)),((-1 -1 -1,-1 1 -1,0 1 -1,0 -1 -1,-1 -1 -1)),((0 -1 -1,0 1 -1,1 1 -1,1 -1 -1,0 -1 -1)),((1 -1 -1,1 1 -1,0 1 2,0 -1 2,1 -1 -1)),((0 -1 2,0 1 2,0 1 1,0 -1 1,0 -1 2)))

and the point: POINT(0 0 0)

@Komzpa

This comment has been minimized.

Copy link
Member Author

commented Feb 14, 2019

If I ST_Tesselate the input the code's not yet working, investigating.

@Komzpa

This comment has been minimized.

Copy link
Member Author

commented Feb 15, 2019

Okay. This works for polyhedrals but not TINS, about which it gets a message. Adding TIN can't be just simple "if POLYGONTYPE || TRIANGLETYPE" as memory layout of TRIANGLE and POLYGON is different (which is not obvious when you count (()) in representation that are aligned with POLYGON).

@mhugo

This comment has been minimized.

Copy link

commented Feb 18, 2019

@Komzpa Thanks for your work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.