Opened 4 years ago

Closed 4 years ago

#3244 closed defect (fixed)

ST_3DClosestPoint returns garbage in Z for 3dz-2d arg

Reported by: strk Owned by: nicklas
Priority: high Milestone: PostGIS 2.2.0
Component: postgis Version: trunk
Keywords: Cc:

Description

strk=# select ST_AsText(ST_3DClosestPoint('POINT(0 0 0)', 'POINT(0 0)'));
NOTICE:  One or both of the geometries is missing z-value. The unknown z-value will be regarded as 'any value'
    st_astext
-----------------
 POINT Z (0 0 0)
(1 row)

strk=# select ST_AsText(ST_3DClosestPoint('POINT(0 0)', 'POINT(0 0 0)'));
NOTICE:  One or both of the geometries is missing z-value. The unknown z-value will be regarded as 'any value'
             st_astext
------------------------------------
 POINT Z (0 0 1.17549435082229e-38)
(1 row)

Change History (13)

comment:1 Changed 4 years ago by strk

See also #2033

comment:2 Changed 4 years ago by nicklas

This is about #2034.

If you look at my last comment I explain what is supposed to happen. Expected answer is point(0 0 0).

I think what you see is some rounding error.

But I will take a look when I get to a computor.

comment:3 Changed 4 years ago by pramsey

The problem, in lw_dist3d_distanceline and lw_dist3d_distancepoint, is the use of FLT_MIN as the bottom of the "big vertical line". It doesn't mean what you think, it's the smallest value greater than zero that can be represented. (In fact, it's the value of Z, seen above). You want -1 * FLT_MAX.

Except. I tried that fix, and unfortunately then we're constructing an edge that is so huge that the floating point math in your 3d seg-seg routine falls apart and can no longer correctly calculate the z value of the projection between the lines. So... there we are.

comment:4 Changed 4 years ago by strk

Shouldn't first arg always drive dimension space ? I mean, what's the point on FIRST ARG which is closest to SECOND ARG ? If FIRST ARG is 2d, we should just discard the Z from SECOND ARG, shouldn't we ?

comment:5 Changed 4 years ago by pramsey

That's not what was decided in #2034. Yes, for the closest point, probably a 2d 1st argument should imply a 2d return. But what about the shortest line between a 2d and a 3d? That was decided in #2034 to be a 3d return, and the issue above still applies, unfortunately.

comment:6 Changed 4 years ago by pramsey

Owner: changed from pramsey to nicklas

comment:7 Changed 4 years ago by nicklas

Thank Paul and strk for finding it and pointing at the problem

I suggest using a maximum/minimum int32 as max and min value for the vertical line.

I added it in r13946 and it seems to work.

That means that if the result should be outside that min/max values it will fall back to the max/min values.

Is that acceptable?

Should we detect that and give a warning?

I didn't dare using int64 since I don't know if that might give problems on some systems (didn't even test on mine actually)

But since I define the limit as 1<<32 for the max int32 we could use 1<<50 instead, which probably is safe.

But it is quite ugly to just "pick a number".

comment:8 Changed 4 years ago by strk

Would using the max Z value from the other geometry plus one be safer ? Also, could you please add this case to the regression suite ?

comment:9 Changed 4 years ago by nicklas

Yes, I thought about that to. But it would need another iteration of all points. But maybe it is worth it.

Wouldn't need +1 either. Max and min z-value is enough.

Yes, I will add it to the regression suite this evening.

comment:10 Changed 4 years ago by nicklas

New fix at r13964 using max and min z-value from 3D geometry to calculate vertical line.

Also added 2 regression tests catching the tickets example.

strk, close if the fix is ok

comment:11 Changed 4 years ago by strk

What about "for the closest point, probably a 2d 1st argument should imply a 2d return" ? Is anyone against that semantic ?

comment:12 Changed 4 years ago by nicklas

Hmm, I don't see why we shall treat (2d, 3d) different from (3d,2d) which from my understanding you agreed on making return 3d here http://trac.osgeo.org/postgis/ticket/2034#comment:17

Anyway, maybe a new discussion about this should be in a separate ticket?

comment:13 Changed 4 years ago by pramsey

Resolution: fixed
Status: newclosed

If there's further discussion on the semantics, start a new ticket, please.

Note: See TracTickets for help on using tickets.