jts icon indicating copy to clipboard operation
jts copied to clipboard

Fix ScaledNoder behaviour

Open jnh5y opened this issue 7 years ago • 4 comments

It was not consistent between scale=1 (no rounding) and other scales (rounding). Now, it skip scaling process for 0.0 scale, but not for 1.0 scale.

jnh5y avatar Mar 18 '17 19:03 jnh5y

No unit tests? Not sure what original problem was? Is this really a non-breaking change?

dr-jts avatar Jun 15 '17 20:06 dr-jts

Hi Martin,

Here is a small beanshell script written with OpenJUMP to demonstrate the problem 👍

import com.vividsolutions.jts.io.WKTReader;
import com.vividsolutions.jts.noding.*;
import com.vividsolutions.jts.noding.snapround.*;
import com.vividsolutions.jts.algorithm.RobustLineIntersector;

// create two intersecting linestrings with coordinates having a precision of 0.0001
l1 = new WKTReader().read("LINESTRING(0 0, 20.11111 30.11111)");
l2 = new WKTReader().read("LINESTRING(20.11111 0, 10.11111 30.11111)");
nss1 = new NodedSegmentString(l1.getCoordinates(),"");
nss2 = new NodedSegmentString(l2.getCoordinates(),"");

noder = new MCIndexSnapRounder(new PrecisionModel(1));
// Create a ScaledNoder with a scale factor of 100 
snoder = new ScaledNoder(noder, 100);

Collection coll = new ArrayList();
coll.add(nss1);
coll.add(nss2);

snoder.computeNodes(coll);
print(snoder.getNodedSubstrings());
// -> [LINESTRING (0.0 0.0, 13.43 20.11), LINESTRING (13.43 20.11, 20.11 30.11), LINESTRING (20.11 0.0, 13.43 20.11), LINESTRING (13.43 20.11, 10.11 30.11)]
// Intersection coordinate AND input coordinates are rounded to 0.01
// Note that here, the rounding performed by MCIndexSnapRounder is relative to already scaled coordinates, but this is not the point.

// now do the same with new ScaledNoder(noder, 1);
// -> [LINESTRING (0.0 0.0, 13.0 20.0), LINESTRING (13.0 20.0, 20.11111 30.11111), LINESTRING (20.11111 0.0, 13.0 20.0), LINESTRING (13.0 20.0, 10.11111 30.11111)]
// Intersection coordinate is rounded to 1 unit (13 20) BUT input coordinates are not rounded as in the previous case
// That's because the rounding is not performed when scaling is 1. I think scaling of input coordinates should behave the same way whatever the scale factor is.

Michaël

mukoki avatar Jul 03 '17 21:07 mukoki

Hi Martin, As stated in the above example, with a ScaledNoder with a scale different from 1, all input coordinates are rounded to the precision defined by the noder (not only intersection points) With a ScaledNoder with scale = 1 is rounding the intersection point, but other input coordinates are left with their original precision. My point is that the behaviour should not depend on the scale factor. Of course, it will change the result of some operations (computation made with a ScaleNoder of factor 1). If you confirm to me that a ScaledNoder with scale 1 should round all input coordinates to integers I can try to write a breaking unit test showing the problem.

mukoki avatar Oct 28 '17 18:10 mukoki

You are right. It is probably not very safe to remove a public method. I don't really change the meaning because I changed the method name, but we can also keep isIntegerPrecision() method to be on the safe side (I think it will not be used in the whole JTS API though). The motivation for this change is that I don't want a special behaviour for ScaledNoder when scaleFactor == 1.0. Hope it makes sense. I tried to demonstrate why I think the current behaviour is not correct in the case of scaledFactor == 1.0. If you or Martin agree about my explanations, I can try to add a unit test.

mukoki avatar Nov 15 '17 08:11 mukoki