Index: trunk/src/org/openstreetmap/josm/gui/mappaint/mapcss/Selector.java =================================================================== --- trunk/src/org/openstreetmap/josm/gui/mappaint/mapcss/Selector.java (revision 15068) +++ trunk/src/org/openstreetmap/josm/gui/mappaint/mapcss/Selector.java (revision 15069) @@ -5,4 +5,5 @@ import java.text.MessageFormat; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -313,4 +314,6 @@ private class ContainsFinder extends AbstractFinder { + protected List toCheck; + protected ContainsFinder(Environment e) { super(e); @@ -319,22 +322,27 @@ @Override - public void visit(INode n) { - if (left.matches(new Environment(n).withParent(e.osm)) - && ((e.osm instanceof IWay && Geometry.nodeInsidePolygon(n, ((IWay) e.osm).getNodes())) - || (e.osm instanceof Relation && ( - (Relation) e.osm).isMultipolygon() && Geometry.isNodeInsideMultiPolygon(n, (Relation) e.osm, null)))) { - addToChildren(e, n); - } - } - - @Override - public void visit(IWay w) { - if (left.matches(new Environment(w).withParent(e.osm)) - && ((e.osm instanceof IWay && Geometry.PolygonIntersection.FIRST_INSIDE_SECOND.equals( - Geometry.polygonIntersection(w.getNodes(), ((IWay) e.osm).getNodes()))) - || (e.osm instanceof Relation && ( - (Relation) e.osm).isMultipolygon() - && Geometry.isPolygonInsideMultiPolygon(w.getNodes(), (Relation) e.osm, null)))) { - addToChildren(e, w); + public void visit(Collection primitives) { + for (IPrimitive p : primitives) { + if (p != e.osm && isPrimitiveUsable(p) && left.matches(new Environment(p).withParent(e.osm))) { + if (toCheck == null) { + toCheck = new ArrayList<>(); + } + toCheck.add(p); + } + } + } + + void execGeometryTests() { + if (toCheck == null || toCheck.isEmpty()) + return; + + if (e.osm instanceof IWay) { + for (IPrimitive p : Geometry.filterInsidePolygon(toCheck, (IWay) e.osm)) { + addToChildren(e, p); + } + } else if (e.osm instanceof Relation) { + for (IPrimitive p : Geometry.filterInsideMultipolygon(toCheck, (Relation) e.osm)) { + addToChildren(e, p); + } } } @@ -364,9 +372,12 @@ containsFinder.visit(e.osm.getDataSet().searchWays(e.osm.getBBox())); } + if (((OptimizedGeneralSelector) left).matchesBase(OsmPrimitiveType.RELATION)) { + containsFinder.visit(e.osm.getDataSet().searchRelations(e.osm.getBBox())); + } } else { // use slow test containsFinder.visit(e.osm.getDataSet().allPrimitives()); } - + containsFinder.execGeometryTests(); return e.children != null; Index: trunk/src/org/openstreetmap/josm/tools/Geometry.java =================================================================== --- trunk/src/org/openstreetmap/josm/tools/Geometry.java (revision 15068) +++ trunk/src/org/openstreetmap/josm/tools/Geometry.java (revision 15069) @@ -30,4 +30,5 @@ import org.openstreetmap.josm.data.osm.INode; import org.openstreetmap.josm.data.osm.IPrimitive; +import org.openstreetmap.josm.data.osm.IWay; import org.openstreetmap.josm.data.osm.MultipolygonBuilder; import org.openstreetmap.josm.data.osm.MultipolygonBuilder.JoinedPolygon; @@ -39,4 +40,5 @@ import org.openstreetmap.josm.data.osm.WaySegment; import org.openstreetmap.josm.data.osm.visitor.paint.relations.Multipolygon; +import org.openstreetmap.josm.data.osm.visitor.paint.relations.Multipolygon.PolyData; import org.openstreetmap.josm.data.osm.visitor.paint.relations.MultipolygonCache; import org.openstreetmap.josm.data.projection.Projection; @@ -965,4 +967,5 @@ * Tests if the {@code node} is inside the multipolygon {@code multiPolygon}. The nullable argument * {@code isOuterWayAMatch} allows to decide if the immediate {@code outer} way of the multipolygon is a match. + * For repeated tests against {@code multiPolygon} better use {@link Geometry#filterInsideMultipolygon}. * @param node node * @param multiPolygon multipolygon @@ -977,4 +980,5 @@ * Tests if the polygon formed by {@code nodes} is inside the multipolygon {@code multiPolygon}. The nullable argument * {@code isOuterWayAMatch} allows to decide if the immediate {@code outer} way of the multipolygon is a match. + * For repeated tests against {@code multiPolygon} better use {@link Geometry#filterInsideMultipolygon}. *

* If {@code nodes} contains exactly one element, then it is checked whether that one node is inside the multipolygon. @@ -982,8 +986,121 @@ * @param multiPolygon multipolygon * @param isOuterWayAMatch allows to decide if the immediate {@code outer} way of the multipolygon is a match - * @return {@code true} if the polygon formed by nodes is inside the multipolygon + * @return {@code true} if the multipolygon is valid and the polygon formed by nodes is inside the multipolygon */ public static boolean isPolygonInsideMultiPolygon(List nodes, Relation multiPolygon, Predicate isOuterWayAMatch) { - // Extract outer/inner members from multipolygon + try { + return isPolygonInsideMultiPolygon(nodes, MultipolygonBuilder.joinWays(multiPolygon), isOuterWayAMatch); + } catch (MultipolygonBuilder.JoinedPolygonCreationException ex) { + Logging.trace(ex); + Logging.debug("Invalid multipolygon " + multiPolygon); + return false; + } + } + + /** + * Tests if the polygon formed by {@code nodes} is inside the multipolygon {@code multiPolygon}. The nullable argument + * {@code isOuterWayAMatch} allows to decide if the immediate {@code outer} way of the multipolygon is a match. + * For repeated tests against {@code multiPolygon} better use {@link Geometry#filterInsideMultipolygon}. + *

+ * If {@code nodes} contains exactly one element, then it is checked whether that one node is inside the multipolygon. + * @param nodes nodes forming the polygon + * @param outerInner result of {@link MultipolygonBuilder#joinWays(Relation)} + * @param isOuterWayAMatch allows to decide if the immediate {@code outer} way of the multipolygon is a match + * @return {@code true} if the multipolygon is valid and the polygon formed by nodes is inside the multipolygon + * @since 15069 + */ + public static boolean isPolygonInsideMultiPolygon(List nodes, Pair, + List> outerInner, Predicate isOuterWayAMatch) { + Area a1 = nodes.size() == 1 ? null : getArea(nodes); + // Test if object is inside an outer member + for (JoinedPolygon out : outerInner.a) { + if (a1 == null + ? nodeInsidePolygon(nodes.get(0), out.getNodes()) + : PolygonIntersection.FIRST_INSIDE_SECOND == polygonIntersection(a1, out.area)) { + boolean insideInner = false; + // If inside an outer, check it is not inside an inner + for (JoinedPolygon in : outerInner.b) { + if (a1 == null ? nodeInsidePolygon(nodes.get(0), in.getNodes()) + : in.area.getBounds2D().contains(a1.getBounds2D()) + && polygonIntersection(a1, in.area) == PolygonIntersection.FIRST_INSIDE_SECOND + && polygonIntersection(in.area, out.area) == PolygonIntersection.FIRST_INSIDE_SECOND) { + insideInner = true; + break; + } + } + if (!insideInner) { + // Final check using predicate + if (isOuterWayAMatch == null || isOuterWayAMatch.test(out.ways.get(0) + /* TODO give a better representation of the outer ring to the predicate */)) { + return true; + } + } + } + } + return false; + } + + /** + * Find all primitives in the given collection which are inside the given polygon. + * @param primitives the primitives + * @param polygon the polygon + * @return a new list containing the found primitives, empty if polygon is invalid or nothing was found. + * @since 15069 + */ + + public static List filterInsidePolygon(List primitives, IWay polygon) { + List res = new ArrayList<>(); + if (!polygon.isClosed() || polygon.getNodesCount() <= 3) + return res; + /** polygon area in east north space, calculated only when really needed */ + Area polygonArea = null; + for (IPrimitive p : primitives) { + if (p instanceof INode) { + if (nodeInsidePolygon((INode) p, polygon.getNodes())) { + res.add(p); + } + } else if (p instanceof IWay) { + if (polygonArea == null) { + polygonArea = getArea(polygon.getNodes()); + } + if (PolygonIntersection.FIRST_INSIDE_SECOND == polygonIntersection(getArea(((IWay) p).getNodes()), + polygonArea)) { + res.add(p); + } + } else if (p.isMultipolygon()) { + if (polygonArea == null) { + polygonArea = getArea(polygon.getNodes()); + } + Multipolygon mp = new Multipolygon((Relation) p); + boolean inside = true; + // a (valid) multipolygon is inside the polygon if all outer rings are inside + for (PolyData outer : mp.getOuterPolygons()) { + if (PolygonIntersection.FIRST_INSIDE_SECOND != polygonIntersection(getArea(outer.getNodes()), + polygonArea)) { + inside = false; + break; + } + } + if (inside) { + res.add(p); + } + } + } + return res; + } + + /** + * Find all primitives in the given collection which are inside the given multipolygon. Members of the multipolygon are + * ignored. + * @param primitives the primitives + * @param multiPolygon the multipolygon relation + * @return a new list containing the found primitives, empty if multipolygon is invalid or nothing was found. + * @since 15069 + */ + public static List filterInsideMultipolygon(Collection primitives, Relation multiPolygon) { + List res = new ArrayList<>(); + if (primitives.isEmpty()) + return res; + final Pair, List> outerInner; try { @@ -992,34 +1109,37 @@ Logging.trace(ex); Logging.debug("Invalid multipolygon " + multiPolygon); - return false; - } - // Test if object is inside an outer member - for (JoinedPolygon out : outerInner.a) { - if (nodes.size() == 1 - ? nodeInsidePolygon(nodes.get(0), out.getNodes()) - : PolygonIntersection.FIRST_INSIDE_SECOND == polygonIntersection(nodes, out.getNodes())) { - boolean insideInner = false; - // If inside an outer, check it is not inside an inner - for (JoinedPolygon in : outerInner.b) { - if (nodes.size() == 1 ? nodeInsidePolygon(nodes.get(0), in.getNodes()) - : polygonIntersection(nodes, in.getNodes()) == PolygonIntersection.FIRST_INSIDE_SECOND - && polygonIntersection(in.getNodes(), - out.getNodes()) == PolygonIntersection.FIRST_INSIDE_SECOND) { - insideInner = true; + return res; + } + + Set members = multiPolygon.getMemberPrimitives(); + for (IPrimitive p : primitives) { + if (members.contains(p)) + continue; + if (p instanceof Node) { + if (isPolygonInsideMultiPolygon(Collections.singletonList((Node) p), outerInner, null)) { + res.add(p); + } + } else if (p instanceof Way) { + if (isPolygonInsideMultiPolygon(((Way) p).getNodes(), outerInner, null)) { + res.add(p); + } + } else if (p.isMultipolygon()) { + Multipolygon mp = new Multipolygon((Relation) p); + boolean inside = true; + // a (valid) multipolygon is inside multiPolygon if all outer rings are inside + for (PolyData outer : mp.getOuterPolygons()) { + if (!isPolygonInsideMultiPolygon(outer.getNodes(), outerInner, null)) { + inside = false; break; } } - // Inside outer but not inside inner -> the polygon appears to be inside a the multipolygon - if (!insideInner) { - // Final check using predicate - if (isOuterWayAMatch == null || isOuterWayAMatch.test(out.ways.get(0) - /* TODO give a better representation of the outer ring to the predicate */)) { - return true; - } - } - } - } - return false; - } + if (inside) { + res.add(p); + } + } + } + return res; + } + /** Index: trunk/test/data/regress/17695/bib2.osm =================================================================== --- trunk/test/data/regress/17695/bib2.osm (revision 15069) +++ trunk/test/data/regress/17695/bib2.osm (revision 15069) @@ -0,0 +1,242 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + Index: trunk/test/unit/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerTest.java =================================================================== --- trunk/test/unit/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerTest.java (revision 15068) +++ trunk/test/unit/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerTest.java (revision 15069) @@ -339,3 +339,21 @@ assertEquals(1, errors.size()); } + + /** + * Non-regression test for Bug #17695. + * @throws Exception if an error occurs + */ + @Test + public void testTicket17695() throws Exception { + final MapCSSTagChecker test = buildTagChecker( + "*[building] ∈ *[building] {" + + "throwWarning: tr(\"Building inside building\");" + + "}"); + try (InputStream is = TestUtils.getRegressionDataStream(17695, "bib2.osm")) { + test.visit(OsmReader.parseDataSet(is, null).allPrimitives()); + List errors = test.getErrors(); + assertEquals(6, errors.size()); + } + } + } Index: trunk/test/unit/org/openstreetmap/josm/tools/GeometryTest.java =================================================================== --- trunk/test/unit/org/openstreetmap/josm/tools/GeometryTest.java (revision 15068) +++ trunk/test/unit/org/openstreetmap/josm/tools/GeometryTest.java (revision 15069) @@ -265,4 +265,46 @@ /** + * Test of {@link Geometry#filterInsideMultipolygon} + */ + @Test + public void testFilterInsideMultiPolygon() { + Node node1 = new Node(new LatLon(1.01, 1.0)); + Node node2 = new Node(new LatLon(1.01, 1.1)); + Node node3 = new Node(new LatLon(1.02, 1.05)); + Way w1 = new Way(); + w1.setNodes(Arrays.asList(node1, node2, node3, node1)); + w1.put("building", "yes"); + Relation mp1 = new Relation(); + mp1.addMember(new RelationMember("outer", w1)); + mp1.put("type", "multipolygon"); + + Node node4 = new Node(new LatLon(1.0, 1.09)); + Node node5 = new Node(new LatLon(1.0, 1.12)); + Node node6 = new Node(new LatLon(1.1, 1.12)); + Node node7 = new Node(new LatLon(1.1, 1.09)); + Way outer = new Way(); + outer.setNodes(Arrays.asList(node4, node5, node6, node7, node4)); + Node node8 = new Node(new LatLon(1.04, 1.1)); + Node node9 = new Node(new LatLon(1.04, 1.11)); + Node node10 = new Node(new LatLon(1.06, 1.105)); + Way inner = new Way(); + inner.setNodes(Arrays.asList(node8, node9, node10, node8)); + Relation mp2 = new Relation(); + mp2.addMember(new RelationMember("outer", outer)); + mp2.addMember(new RelationMember("inner", inner)); + mp2.put("type", "multipolygon"); + assertFalse(Geometry.isPolygonInsideMultiPolygon(w1.getNodes(), mp2, null)); + assertFalse(Geometry.filterInsideMultipolygon(Arrays.asList(w1), mp2).contains(w1)); + + node4.setCoor(new LatLon(1.006, 0.99)); + // now w1 is inside + assertTrue(Geometry.isPolygonInsideMultiPolygon(w1.getNodes(), mp2, null)); + assertTrue(Geometry.filterInsideMultipolygon(Arrays.asList(w1), mp2).contains(w1)); + assertTrue(Geometry.filterInsideMultipolygon(Arrays.asList(mp1), mp2).contains(mp1)); + assertTrue(Geometry.filterInsideMultipolygon(Arrays.asList(w1, mp1), mp2).contains(w1)); + assertTrue(Geometry.filterInsideMultipolygon(Arrays.asList(w1, mp1), mp2).contains(mp1)); + } + + /** * Test of {@link Geometry#getDistance} method. */