Hi Gerd
Patch attached that:
- rewords the sentence is the Style Manual and changes the
highlighting; I need to check the next build/download to see if this is
clearer.
- fixes polygon 'any' method to also return true if exactly ON.
- merge polygons for 'any' so that line on shared boundary is "in"
rather than "on".
- change the test driver to try all methods relevant to the element,
checking they return true/false as appropriate. I decided that, rather
than introducing a new tag saying which methods should match, it was
clearer to use the 'expected' tag value as a description of how the
element interacted with the polygons and generate the methods that
should match from this and the non-matching from a list if all methods.
Ticker
On Thu, 2020-02-20 at 16:45 +0000, Ticker Berkin wrote:
> Hi Gerd
>
> I don't think the test data 'expected' values are wrong, it is just
> that they are more specific than the 'method' mechanism allows to be
> differentiated; eg a polygon can only be tested for ALL in or ANY in.
>
> At the moment I feel you have a reluctance about the whole concept of
> the methods. Once the principle is accepted, I'll go through the test
> data and add, as another tag, the list of methods that should match
> the
> element, then change the test driver to check that these match and
> the
> other applicable methods don't.
>
> Reg. b14: It isn't the stop-early code that causes the problems,
> isLineInShape is not giving the correct answer for a simple polygon
> produced by the MP cutter.
>
> It would be quite easy to introduce some POLYGON 'on' methods, that
> match the outer, inner or either edge of a polygon, but maybe this
> could wait until there is a call for it.
>
> Next mail:
> I'll change the sentence as you suggest.
>
> Please can you commit the patch as it stands; it has a lot of good
> stuff in it. Then I can do the IsInUtilTest and test data changes as
> the next stage. It's also handy to see how the Style Manual looks
> after
> each build into the download area, because I don't know how to
> generate
> it and am just guessing at the formatting.
>
> Thank you
> Ticker
>
> On Thu, 2020-02-20 at 15:41 +0000, Gerd Petermann wrote:
> > Hi Ticker,
> >
> > I see that you overwrite the expected value stored in the test data
> > in the unit test. Please don't do this. If you think that the
> > expected value in is-in-samples.osm
> > is wrong we should discuss the test data.
> > In my eyes b14 clearly has points on the edge (as it is part of the
> > edge) and is out.
> >
> > If you think the expected results are correct but your new code
> > doesn't allow to test them because of the early stop code please
> > add
> > a new tag to each object or maybe create a new file. The unit test
> > file is meant to document what the code does.
> >
> > Gerd
Index: doc/styles/rules.txt
===================================================================
--- doc/styles/rules.txt (revision 4456)
+++ doc/styles/rules.txt (working copy)
@@ -283,29 +283,29 @@
some element ids are changed and some have a faked id > 4611686018427387904.
|is_in(tag,value,method) | x | x | |
-+true+ if the element is in polygon(s) having the specified +tag+=+value+ according to the +method+, +false+ otherwise.
++true+ if the element is in polygon(s) having the specified *tag=value* according to the *method*, +false+ otherwise.
The methods available depend on the Style section:
-. polygons:
- +all+ - all of the closed Way is within the polygon(s).
- +any+ - some is within.
+polygons:
+ *all* - all of the closed 'way' is within the polygon(s).
+ *any* - some is within.
-. points:
- +in+ - the Node is within a polygon.
- +in_or_on+ - it is within or on the edge.
- +on+ - it is on the edge.
+points:
+ *in* - the 'node' is within a polygon.
+ *in_or_on* - it is within or on the edge.
+ *on* - it is on the edge.
-. lines:
- +all+ - part of the Way is within the polygon(s), none is outside; it might touch an edge.
- +all_in_or_on+ - none is outside.
- +on+ - it runs along the edge.
- +any+ - part is within.
- +none+ - part is outside, none is inside
+lines:
+ *all* - part of the 'way' is within the polygon(s), none is outside; it might touch an edge.
+ *all_in_or_on* - none is outside.
+ *on* - it runs along the edge.
+ *any* - part is within.
+ *none* - part is outside, none is inside.
A common case is a line outside the polygon that runs to the edge, joining a line that is inside.
-The method to match an outside line (+none+) allows part to be on the edge,
-likewise, the method to match an inside line (+all+) allows part to be on the edge.
-The method +all_in_or_on+ additionally matches lines are only on the edge of the polygon.
+The method to match an outside line (*none*) allows part to be on the edge,
+likewise, the method to match an inside line (*all*) allows part to be on the edge.
+Compared to *all*, the method *all_in_or_on* additionally matches lines which are only on the edge of the polygon.
|====
Index: src/uk/me/parabola/mkgmap/osmstyle/function/IsInFunction.java
===================================================================
--- src/uk/me/parabola/mkgmap/osmstyle/function/IsInFunction.java (revision 4456)
+++ src/uk/me/parabola/mkgmap/osmstyle/function/IsInFunction.java (working copy)
@@ -57,7 +57,7 @@
{ @Override public boolean mapFlags(boolean hasIn, boolean hasOn, boolean hasOut) {return !hasOut;} },
LINE_ALL_ON("on", FeatureKind.POLYLINE, true, false, true, true)
{ @Override public boolean mapFlags(boolean hasIn, boolean hasOn, boolean hasOut) {return !(hasIn || hasOut);} },
- LINE_ANY_IN("any", FeatureKind.POLYLINE, true, false, false, false)
+ LINE_ANY_IN("any", FeatureKind.POLYLINE, true, false, false, true)
{ @Override public boolean mapFlags(boolean hasIn, boolean hasOn, boolean hasOut) {return hasIn;} },
// LINE_ANY_IN_OR_ON("any_in_or_on", FeatureKind.POLYLINE, true, false, false, true)
// { @Override public boolean mapFlags(boolean hasIn, boolean hasOn, boolean hasOut) {return hasIn || !hasOut;} },
@@ -69,8 +69,27 @@
// POLYGON_ANY("any", FeatureKind.POLYGON, true, false, false, false)
// problem with test b14 on the cut polygons and isLineInShape that goes away when merged. TODO: investigate sometime
POLYGON_ANY("any", FeatureKind.POLYGON, true, false, false, true)
- { @Override public boolean mapFlags(boolean hasIn, boolean hasOn, boolean hasOut) {return hasIn;} };
+ { @Override public boolean mapFlags(boolean hasIn, boolean hasOn, boolean hasOut) {return hasIn || !hasOut;} };
+/* thoughts for ON methods for polyons and the hasOn flag
+
+possible methods:
+ on_outer / on
+ on_inner / hole
+ on_either
+ all_or_inner - to match, say building, even when cut out of area
+
+on_outer is ok, with just ON.
+on_inner would be logical to represent as ON|OUT
+but, at the moment, an outside line/poly touching an outer will also set this combination
+
+Could:
+ don't hasOn() when isLineInShape returns IN|ON|OUT (in setHasFromFlags)
+ other places where currently call hasOn(), test kind for poly and don't when in comb. with IN or OUT
+
+actually, would be safe not to call hasOn() even for POLYLINE, because none of the methods test it
+*/
+
public abstract boolean mapFlags(boolean hasIn, boolean hasOn, boolean hasOut);
private final String methodName;
Index: test/uk/me/parabola/util/IsInUtilTest.java
===================================================================
--- test/uk/me/parabola/util/IsInUtilTest.java (revision 4456)
+++ test/uk/me/parabola/util/IsInUtilTest.java (working copy)
@@ -27,6 +27,8 @@
import java.util.Set;
import java.util.Arrays;
import java.util.stream.Collectors;
+import java.util.Map;
+import java.util.HashMap;
import org.junit.Test;
@@ -44,8 +46,6 @@
/*
Source: test/resources/in/osm/is-in-samples.osm
errors: test-reports/uk/me/parabola/util/62_IsInUtilTest-err.html
-
-b14 failed, expected: 4 got 5
*/
public class IsInUtilTest {
@@ -52,6 +52,46 @@
Area testSourceBbox = null;
+ private static final String allPointMethods = "in,in_or_on,on";
+ private static final String allLineMethods = "all,all_in_or_on,on,any,none";
+ private static final String allPolygonMethods = "all,any";
+
+ private static final Map<Integer, String> pointMethods = new HashMap<>();
+ private static final Map<Integer, String> lineMethods = new HashMap<>();
+ private static final Map<Integer, String> polygonMethods = new HashMap<>();
+
+ public IsInUtilTest() {
+ // set up the methods that should return true for the 'expected' value
+ pointMethods.put(1, "in,in_or_on");
+ pointMethods.put(2, "in_or_on,on");
+ pointMethods.put(4, "");
+
+/* all=someInNoneOut, any=anyIn, none=someOutNoneIn
+ 1 2 4
+a 1) IN all allInOrOn any
+b 3) IN ON all allInOrOn any
+c 7) IN ON OUT any
+d 2) ON allInOrOn on
+e 6) ON OUT none
+f 4) OUT none
+*/
+ lineMethods.put(1, "all,all_in_or_on,any");
+ lineMethods.put(2, "all_in_or_on,on");
+ lineMethods.put(3, "all,all_in_or_on,any");
+ lineMethods.put(4, "none");
+ //lineMethods.put(5, "");
+ lineMethods.put(6, "none");
+ lineMethods.put(7, "any");
+
+ polygonMethods.put(1, "all,any");
+ polygonMethods.put(2, "all,any");
+ polygonMethods.put(3, "all,any");
+ polygonMethods.put(4, "");
+ //polygonMethods.put(5, "");
+ polygonMethods.put(6, "");
+ polygonMethods.put(7, "any");
+ }
+
private static boolean invokeMethod(IsInFunction anInst, String method, FeatureKind kind, Element el) {
anInst.setParams(Arrays.asList("landuse", "residential", method), kind); // tag key/value don't matter
String rslt = anInst.calcImpl(el);
@@ -58,118 +98,93 @@
return "true".equals(rslt);
}
- private int calcInsideness(FeatureKind kind, Element el, Set<Way> polygons) {
- int result = 0;
- if (polygons == null || polygons.isEmpty()) {
- return IsInUtil.OUT;
- }
+ public List<String> testWithVariants(FeatureKind kind, Element el, String name, Set<Way> polygons) {
+ List<String> errors = new ArrayList<>();
+
IsInFunction anInst = new IsInFunction();
List<Element> matchingPolygons = new ArrayList<>();
for (Way polygon : polygons)
matchingPolygons.add(polygon);
anInst.unitTestAugment(new ElementQuadTree(testSourceBbox, matchingPolygons));
- switch (kind) {
- case POINT:
- if (invokeMethod(anInst, "in_or_on", kind, el))
- if (invokeMethod(anInst, "in", kind, el))
- result = IsInUtil.IN;
- else
- result = IsInUtil.ON;
- else
- result = IsInUtil.OUT;
- break;
- case POLYLINE:
- /* all=someInNoneOut, any=anyIn, none=someOutNoneIn
-a) IN all allInOrOn any
-b) IN ON all allInOrOn any
-c) IN ON OUT any
-d) ON allInOrOn on
-e) ON OUT none
-f) OUT none
- */
- if (invokeMethod(anInst, "all", kind, el)) /*a,b*/
- result = IsInUtil.IN | IsInUtil.ON; // methods won't say if also ON
- else /*c,d,e,f*/ if (invokeMethod(anInst, "on", kind, el)) /*d*/
- result = IsInUtil.ON;
- else /*c,e,f*/ if (invokeMethod(anInst, "any", kind, el)) /*c*/
- result = IsInUtil.IN | IsInUtil.ON | IsInUtil.OUT;
- else /*e,f*/
- result = IsInUtil.OUT | IsInUtil.ON; // methods won't say if also ON
- break;
- case POLYGON: // ON is meaningless for polygons
- if (invokeMethod(anInst, "all", kind, el))
- result = IsInUtil.IN;
- else if (invokeMethod(anInst, "any", kind, el))
- result = IsInUtil.IN | IsInUtil.OUT;
- else
- result = IsInUtil.OUT;
- break;
- }
- return result;
- }
-
- public List<String> testWithVariants(FeatureKind kind, Element el, String name, Set<Way> polygons) {
- List<String> errors = new ArrayList<>();
- int res = calcInsideness(kind, el, polygons);
String expectedVal = el.getTag("expected");
if (expectedVal != null && !"?".equals(expectedVal)) {
int expected = Integer.parseInt(expectedVal);
+ String allMethods = "";
+ Map<Integer, String> methods = null;
+ switch (kind) {
+ case POINT:
+ allMethods = allPointMethods;
+ methods = pointMethods;
+ break;
+ case POLYLINE:
+ allMethods = allLineMethods;
+ methods = lineMethods;
+ break;
+ case POLYGON:
+ allMethods = allPolygonMethods;
+ methods = polygonMethods;
+ break;
+ }
+ if (!methods.containsKey(expected)) {
+ errors.add(name + " failed, no methods for expected: " + expectedVal);
+ return errors;
+ }
+ String[] trueMethods = methods.get(expected).split(",");
+ if (trueMethods[0].isEmpty())
+ trueMethods = new String[0];
+ List<String> falseMethods = new ArrayList<>();
+ for (String tstMethod : allMethods.split(",")) {
+ boolean inList = false;
+ for (String trueMethod : trueMethods)
+ if (tstMethod.equals(trueMethod)) {
+ inList = true;
+ break;
+ }
+ if (!inList)
+ falseMethods.add(tstMethod);
+ }
-/*
-Using the "method" interface to emulate the old version of IsInUtil.calcInsideness and try and deduce
-the IN/ON/OUT flags to compare with the 'expected' tag isn't quite possible:
- For POLYGONs the ON flag is meaningless - it can be wholly or partially within
- For LINEs, the methods don't distinguish between a line that is totally ON and one that is IN but touches the edge,
- Similarly a line that is OUT and one that touches the edge.
-So here we adjust the expected value to match what can be tested.
-*/
- if (kind == FeatureKind.POLYGON)
- expected &= ~IsInUtil.ON;
- else if (kind == FeatureKind.POLYLINE)
- if (expected == IsInUtil.IN || expected == IsInUtil.OUT)
- expected |= IsInUtil.ON;
+ for (String tstMethod : trueMethods)
+ if (!invokeMethod(anInst, tstMethod, kind, el))
+ errors.add(name + " failed, expected: " + expectedVal + ". " + tstMethod + " should be true");
+ for (String tstMethod : falseMethods)
+ if (invokeMethod(anInst, tstMethod, kind, el))
+ errors.add(name + " failed, expected: " + expectedVal + ". " + tstMethod + " should be false");
- if (expected != res) {
- errors.add(name + " failed, expected: " + expected + " got "+ res);
+ if (!errors.isEmpty() || !(el instanceof Way))
return errors;
+ Way w2 = (Way) el.copy();
+ Collections.reverse(w2.getPoints());
+ for (String tstMethod : trueMethods)
+ if (!invokeMethod(anInst, tstMethod, kind, w2))
+ errors.add(name + " failed reversed, expected: " + expectedVal + ". " + tstMethod + " should be true");
+ for (String tstMethod : falseMethods)
+ if (invokeMethod(anInst, tstMethod, kind, w2))
+ errors.add(name + " failed reversed, expected: " + expectedVal + ". " + tstMethod + " should be false");
+
+ if (!errors.isEmpty() || !w2.hasIdenticalEndPoints())
+ return errors;
+ List<Coord> points = w2.getPoints();
+ for (int i = 1; i < w2.getPoints().size(); i++) {
+ points.remove(points.size() - 1);
+ Collections.rotate(points, 1);
+ points.add(points.get(0));
+ for (String tstMethod : trueMethods)
+ if (!invokeMethod(anInst, tstMethod, kind, w2))
+ errors.add(name + " failed rotated, expected: " + expectedVal + ". " + tstMethod + " should be true");
+ for (String tstMethod : falseMethods)
+ if (invokeMethod(anInst, tstMethod, kind, w2))
+ errors.add(name + " failed rotated, expected: " + expectedVal + ". " + tstMethod + " should be false");
}
- if (el instanceof Way) {
- Way w2 = (Way) el.copy();
- Collections.reverse(w2.getPoints());
- int res2 = calcInsideness(kind, w2, polygons);
- if (expected != res2) {
- errors.add(name + " failed reversed, expected: " + expected + " got " + res2);
- }
- if (w2.hasIdenticalEndPoints()) {
- List<Coord> points = w2.getPoints();
- for (int i = 1; i < w2.getPoints().size(); i++) {
- points.remove(points.size() - 1);
- Collections.rotate(points, 1);
- points.add(points.get(0));
- res2 = calcInsideness(kind, w2, polygons);
- if (expected != res2) {
- errors.add(name + " failed rotated " + i + " , expected: " + expected + " got " + res2);
- }
- }
- }
- }
}
return errors;
}
-
- /**
- * A very basic check that the size of all the sections has not changed.
- * This can be used to make sure that a change that is not expected to
- * change the output does not do so.
- *
- * The sizes will have to be always changed when the output does change
- * though.
- */
+
@Test
public void testBasic() throws FileNotFoundException {
- // just loads the file
+ // just loads the file
class TestSource extends OsmMapDataSource {
@Override
public Set<String> getUsedTags() {
_______________________________________________
mkgmap-dev mailing list
mkgmap-dev@lists.mkgmap.org.uk
http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
