Reduce use of sqrt() calls when determining distances between points/lines/shapes

[bretlambert]

New distance calculation functions which don't call sqrt(), for use when speed is important (i.e., nested loops). User must call sqrt() on resulting value.

[dmorissette]

Alan will take care of this one.

[aboudreault]

bretlambert, I think you modified the msDistancePointToSegment to msSquareDistancePointToSegment by mistake, am I right ? Or there is a part of the patch that is missing ?

[aboudreault]

Replying to aboudreault:

bretlambert, I think you modified the msDistancePointToSegment to msSquareDistancePointToSegment by mistake, am I right ? Or there is a part of the patch that is missing ?

forgot to point the place in the code: mapsearch.c: line 417, 482.

[bretlambert]

Replying to aboudreault:

Replying to aboudreault:

bretlambert, I think you modified the msDistancePointToSegment to msSquareDistancePointToSegment by mistake, am I right ? Or there is a part of the patch that is missing ?

forgot to point the place in the code: mapsearch.c: line 417, 482.

No; I reduced the use of sqrt() within that function from either 2 or 3 calls to only 1 per invocation. I should've made that clear in my description.

Changing l -> l_squared means:

1) you don't have to do l*l 2) you don't call sqrt() when you just want to check for 0 3) defers the actual calculation of the square root to the end of the function where the actual square root is needed to calculate the return value

Changing the lines under the if (r > 1 < 0) tests to only call sqrt() on the value returned from the MS_MIN() macro removes a superfluous sqrt() call.

[aboudreault]

Yes, but there is no msSquareDistancePointToSegment function.

[bretlambert]

Replying to aboudreault:

Yes, but there is no msSquareDistancePointToSegment function.

No, there is not. I didn't implement that in this iteration, as it wasn't immediately apparent to me at 1 am as to how to do so ("Math is hard!" --Barbie).

[aboudreault]

Ok, should I consider your patch as incomplete or, for the moment, modify it by reversing your changes of the line 471 and 482 ?

[aboudreault]

As discussed on IRC, we will take care of that as soon as the final patch is done.

[bretlambert]

reduce sqrt() usage in msPolygonLabelPoint() (incorrect version uploaded previously)

[aboudreault]

The patch looks good to me. However, I did a simple modification. In your msPolygonLabelPoint.sqrt.diff patch, I've modified:

dist = sqrt(min_dist);

for

min_dist = sqrt(min_dist);

bretlambert, does that change look good to you too?

[bretlambert]

Replying to aboudreault:

The patch looks good to me. However, I did a simple modification. In your

Yes; I had uploaded the incorrect version initially, and uploaded the corrected version (which contains the change you made to the original) when I noticed the issue.

[aboudreault]

Emm... your corrected version contains the issue (it uses dist instead of min_dist). Anyway.. I fixed it.

Thanks for the patch!!

Fixed and committed in r9353.

[bretlambert]

Replying to aboudreault:

Emm... your corrected version contains the issue (it uses dist instead of min_dist). Anyway.. I fixed it.

Thanks for the patch!! Fixed and committed in r9353.

When I look at the second diff I placed on trac, it shows the correct version. In any case, sorry for the mixup.

[aboudreault]

Just to show you my point: check the line 1116 of  http://trac.osgeo.org/mapserver/attachment/ticket/3134/msPolygonLabelPoint.sqrt.diff , that line is incorrect, since you do a dist = instead of min_dist =.

[bretlambert]

Wow, that's embarassingly bad. Sorry for the noise.

[aboudreault]

hehe, np bretlambert, thanks again for your patch!