fix #24046 - improve speed of multipolygon validator - patch by taylo…

…r.smock

git-svn-id: https://josm.openstreetmap.de/svn/trunk@19336 0c6e7542-c601-0410-84e7-c038aed88b3b

src/org/openstreetmap/josm/data/osm/MultipolygonBuilder.java

@@ -154,10 +154,39 @@ public JoinedPolygonCreationException(String message) {
      * @throws JoinedPolygonCreationException if the creation fails.
      */
     public static Pair<List<JoinedPolygon>, List<JoinedPolygon>> joinWays(Relation multipolygon) {
+        return joinWays(null, multipolygon);
+    }
+
+    /**
+     * Joins the given {@code multipolygon} to a pair of outer and inner multipolygon rings.
+     *
+     * @param multipolygon the multipolygon to join.
+     * @return a pair of outer and inner multipolygon rings.
+     * @throws JoinedPolygonCreationException if the creation fails.
+     * @since xxx
+     */
+    public static Pair<List<JoinedPolygon>, List<JoinedPolygon>> joinWays(
+            Map<IRelation<?>, Pair<List<JoinedPolygon>, List<JoinedPolygon>>> cache, Relation multipolygon) {
+        if (cache != null) {
+            return cache.computeIfAbsent(multipolygon, MultipolygonBuilder::joinWaysActual);
+        }
+        return joinWaysActual(multipolygon);
+    }
+
+    /**
+     * Perform the actual join ways calculation
+     *
+     * @param multipolygon the multipolygon to join.
+     * @return a pair of outer and inner multipolygon rings.
+     * @throws JoinedPolygonCreationException if the creation fails.
+     */
+    private static Pair<List<JoinedPolygon>, List<JoinedPolygon>> joinWaysActual(IRelation<?> multipolygon) {
         CheckParameterUtil.ensureThat(multipolygon.isMultipolygon(), "multipolygon.isMultipolygon");
-        final Map<String, Set<Way>> members = multipolygon.getMembers().stream()
-                .filter(RelationMember::isWay)
-                .collect(Collectors.groupingBy(RelationMember::getRole, Collectors.mapping(RelationMember::getWay, Collectors.toSet())));
+        CheckParameterUtil.ensureThat(multipolygon instanceof Relation,
+                "This method currently only supports Relation objects due to potential breakage");
+        final Map<String, Set<Way>> members = ((Relation) multipolygon).getMembers().stream()
+                .filter(IRelationMember::isWay)
+                .collect(Collectors.groupingBy(IRelationMember::getRole, Collectors.mapping(RelationMember::getWay, Collectors.toSet())));
         final List<JoinedPolygon> outerRings = joinWays(members.getOrDefault("outer", Collections.emptySet()));
         final List<JoinedPolygon> innerRings = joinWays(members.getOrDefault("inner", Collections.emptySet()));
         return Pair.create(outerRings, innerRings);

src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java

@@ -24,6 +24,8 @@
 import java.util.stream.Stream;
 
 import org.openstreetmap.josm.data.osm.IPrimitive;
+import org.openstreetmap.josm.data.osm.IRelation;
+import org.openstreetmap.josm.data.osm.MultipolygonBuilder.JoinedPolygon;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.preferences.BooleanProperty;
 import org.openstreetmap.josm.data.preferences.CachingProperty;
@@ -49,6 +51,7 @@
 import org.openstreetmap.josm.tools.I18n;
 import org.openstreetmap.josm.tools.Logging;
 import org.openstreetmap.josm.tools.MultiMap;
+import org.openstreetmap.josm.tools.Pair;
 import org.openstreetmap.josm.tools.Stopwatch;
 import org.openstreetmap.josm.tools.Utils;
 
@@ -60,6 +63,7 @@ public class MapCSSTagChecker extends Test.TagTest {
     private MapCSSStyleIndex indexData;
     private final Map<MapCSSRule, MapCSSTagCheckerAndRule> ruleToCheckMap = new HashMap<>();
     private static final Map<IPrimitive, Area> mpAreaCache = new HashMap<>();
+    private static final Map<IRelation<?>, Pair<List<JoinedPolygon>, List<JoinedPolygon>>> mpJoinedAreaCache = new HashMap<>();
     private static final Set<IPrimitive> toMatchForSurrounding = new HashSet<>();
     static final boolean ALL_TESTS = true;
     static final boolean ONLY_SELECTED_TESTS = false;
@@ -163,6 +167,7 @@ public synchronized Collection<TestError> getErrorsForPrimitive(OsmPrimitive p,
 
         final Environment env = new Environment(p, new MultiCascade(), Environment.DEFAULT_LAYER, null);
         env.mpAreaCache = mpAreaCache;
+        env.mpJoinedAreaCache = mpJoinedAreaCache;
         env.toMatchForSurrounding = toMatchForSurrounding;
 
         Iterator<MapCSSRule> candidates = indexData.getRuleCandidates(p);
@@ -221,6 +226,7 @@ static Collection<TestError> getErrorsForPrimitive(
         final List<TestError> r = new ArrayList<>();
         final Environment env = new Environment(p, new MultiCascade(), Environment.DEFAULT_LAYER, null);
         env.mpAreaCache = mpAreaCache;
+        env.mpJoinedAreaCache = mpJoinedAreaCache;
         env.toMatchForSurrounding = toMatchForSurrounding;
         for (Set<MapCSSTagCheckerRule> schecks : checksCol) {
             for (MapCSSTagCheckerRule check : schecks) {
@@ -376,6 +382,7 @@ public synchronized void endTest() {
         // always clear the cache to make sure that we catch changes in geometry
         mpAreaCache.clear();
         ruleToCheckMap.clear();
+        mpJoinedAreaCache.clear();
         toMatchForSurrounding.clear();
         super.endTest();
     }
@@ -396,6 +403,7 @@ void visit(Collection<OsmPrimitive> selection, Predicate<String> urlPredicate) {
         }
 
         mpAreaCache.clear();
+        mpJoinedAreaCache.clear();
         toMatchForSurrounding.clear();
 
         Set<OsmPrimitive> surrounding = new HashSet<>();

src/org/openstreetmap/josm/gui/mappaint/Environment.java

@@ -8,13 +8,16 @@
 import java.util.Set;
 
 import org.openstreetmap.josm.data.osm.IPrimitive;
+import org.openstreetmap.josm.data.osm.IRelation;
+import org.openstreetmap.josm.data.osm.MultipolygonBuilder;
 import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.data.osm.Way;
 import org.openstreetmap.josm.data.osm.WaySegment;
 import org.openstreetmap.josm.gui.mappaint.mapcss.Condition.Context;
 import org.openstreetmap.josm.gui.mappaint.mapcss.Selector;
 import org.openstreetmap.josm.gui.mappaint.mapcss.Selector.LinkSelector;
 import org.openstreetmap.josm.tools.CheckParameterUtil;
+import org.openstreetmap.josm.tools.Pair;
 
 /**
  * Environment is a data object to provide access to various "global" parameters.
@@ -90,6 +93,10 @@ public class Environment {
      * Cache for multipolygon areas, can be null, used with CrossingFinder
      */
     public Map<IPrimitive, Area> mpAreaCache;
+    /**
+     * Cache for multipolygon areas as calculated by {@link MultipolygonBuilder#joinWays(Relation)}, can be {@code null}
+     */
+    public Map<IRelation<?>, Pair<List<MultipolygonBuilder.JoinedPolygon>, List<MultipolygonBuilder.JoinedPolygon>>> mpJoinedAreaCache;
 
     /**
      * Can be null, may contain primitives when surrounding objects of the primitives are tested
@@ -162,6 +169,7 @@ private Environment(Environment other, Selector selector) {
         this.intersections = other.intersections;
         this.crossingWaysMap = other.crossingWaysMap;
         this.mpAreaCache = other.mpAreaCache;
+        this.mpJoinedAreaCache = other.mpJoinedAreaCache;
         this.toMatchForSurrounding = other.toMatchForSurrounding;
         this.selector = selector;
     }

src/org/openstreetmap/josm/gui/mappaint/mapcss/Selector.java

@@ -490,7 +490,7 @@ public void visit(IWay<?> w) {
             public void visit(IRelation<?> r) {
                 if (r instanceof Relation && r.isMultipolygon() && r.getBBox().bounds(e.osm.getBBox())
                         && left.matches(new Environment(r).withParent(e.osm))
-                        && !Geometry.filterInsideMultipolygon(Collections.singletonList(e.osm), (Relation) r).isEmpty()) {
+                        && !Geometry.filterInsideMultipolygon(Collections.singletonList(e.osm), (Relation) r, e.mpJoinedAreaCache).isEmpty()) {
                     addToChildren(e, r);
                 }
             }

src/org/openstreetmap/josm/tools/Geometry.java

@@ -17,6 +17,7 @@
 import java.util.Iterator;
 import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
 import java.util.function.Predicate;
@@ -32,6 +33,7 @@
 import org.openstreetmap.josm.data.osm.DataSet;
 import org.openstreetmap.josm.data.osm.INode;
 import org.openstreetmap.josm.data.osm.IPrimitive;
+import org.openstreetmap.josm.data.osm.IRelation;
 import org.openstreetmap.josm.data.osm.IWay;
 import org.openstreetmap.josm.data.osm.MultipolygonBuilder;
 import org.openstreetmap.josm.data.osm.MultipolygonBuilder.JoinedPolygon;
@@ -1191,7 +1193,7 @@ public static List<IPrimitive> filterInsidePolygon(Collection<IPrimitive> primit
                 if (polygonArea == null) {
                     polygonArea = getArea(polygon.getNodes());
                 }
-                Multipolygon mp = new Multipolygon((Relation) p);
+                Multipolygon mp = p.getDataSet() != null ? MultipolygonCache.getInstance().get((Relation) p) : 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()) {
@@ -1220,13 +1222,27 @@ public static List<IPrimitive> filterInsidePolygon(Collection<IPrimitive> primit
      * @since 15069
      */
     public static List<IPrimitive> filterInsideMultipolygon(Collection<IPrimitive> primitives, Relation multiPolygon) {
+        return filterInsideMultipolygon(primitives, multiPolygon, null);
+    }
+
+    /**
+     * Find all primitives in the given collection which are inside the given multipolygon. Members of the multipolygon are
+     * ignored. Unclosed ways and multipolygon relations with unclosed outer rings are ignored.
+     * @param primitives the primitives
+     * @param multiPolygon the multipolygon relation
+     * @param cache The cache to avoid calculating joined inner/outer ways multiple times (see {@link MultipolygonBuilder#joinWays(Relation)})
+     * @return a new list containing the found primitives, empty if multipolygon is invalid or nothing was found.
+     * @since xxx
+     */
+    public static List<IPrimitive> filterInsideMultipolygon(Collection<IPrimitive> primitives, Relation multiPolygon,
+                                                            Map<IRelation<?>, Pair<List<JoinedPolygon>, List<JoinedPolygon>>> cache) {
         List<IPrimitive> res = new ArrayList<>();
         if (primitives.isEmpty())
             return res;
 
         final Pair<List<JoinedPolygon>, List<JoinedPolygon>> outerInner;
         try {
-            outerInner = MultipolygonBuilder.joinWays(multiPolygon);
+            outerInner = MultipolygonBuilder.joinWays(cache, multiPolygon);
         } catch (MultipolygonBuilder.JoinedPolygonCreationException ex) {
             Logging.trace(ex);
             Logging.debug("Invalid multipolygon " + multiPolygon);

test/unit/org/openstreetmap/josm/tools/GeometryTest.java

@@ -367,15 +367,15 @@ void testFilterInsideMultiPolygon() {
         mp2.addMember(new RelationMember("inner", inner));
         mp2.put("type", "multipolygon");
         assertFalse(Geometry.isPolygonInsideMultiPolygon(w1.getNodes(), mp2, null));
-        assertFalse(Geometry.filterInsideMultipolygon(Collections.singletonList(w1), mp2).contains(w1));
+        assertFalse(Geometry.filterInsideMultipolygon(Collections.singletonList(w1), mp2, null).contains(w1));
 
         node4.setCoor(new LatLon(1.006, 0.99));
         // now w1 is inside
         assertTrue(Geometry.isPolygonInsideMultiPolygon(w1.getNodes(), mp2, null));
-        assertTrue(Geometry.filterInsideMultipolygon(Collections.singletonList(w1), mp2).contains(w1));
-        assertTrue(Geometry.filterInsideMultipolygon(Collections.singletonList(mp1), mp2).contains(mp1));
-        assertTrue(Geometry.filterInsideMultipolygon(Arrays.asList(w1, mp1), mp2).contains(w1));
-        assertTrue(Geometry.filterInsideMultipolygon(Arrays.asList(w1, mp1), mp2).contains(mp1));
+        assertTrue(Geometry.filterInsideMultipolygon(Collections.singletonList(w1), mp2, null).contains(w1));
+        assertTrue(Geometry.filterInsideMultipolygon(Collections.singletonList(mp1), mp2, null).contains(mp1));
+        assertTrue(Geometry.filterInsideMultipolygon(Arrays.asList(w1, mp1), mp2, null).contains(w1));
+        assertTrue(Geometry.filterInsideMultipolygon(Arrays.asList(w1, mp1), mp2, null).contains(mp1));
     }
 
     /**