Changeset 15959 in josm


Ignore:
Timestamp:
2020-02-29T08:07:39+01:00 (4 weeks ago)
Author:
GerdP
Message:

fix #13165 Validator did not warn about overlapping multipolygons

  • let CrossingFinder accept multipolygons
  • improve highlighting of overlapping areas
  • if incomplete multipolygons are tested the area calculation is used unless open ends are found, else the same algortihm as in CrossingWays is used to hilite the crossing segments. In the later case overlaps with shared nodes are not (yet) found.
Location:
trunk
Files:
3 added
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/src/org/openstreetmap/josm/data/validation/tests/CrossingWays.java

    r15928 r15959  
    428428        return cells;
    429429    }
     430
     431    /**
     432     * Find ways which are crossing without sharing a node.
     433     * @param w way that is to be checked
     434     * @param cellSegments map with already collected way segments
     435     * @param crossingWays map to collect crossing ways and related segments
     436     * @param findSharedWaySegments true: find shared way segments instead of crossings
     437     */
     438    public static void findIntersectingWay(Way w, Map<Point2D, List<WaySegment>> cellSegments,
     439            Map<List<Way>, List<WaySegment>> crossingWays, boolean findSharedWaySegments) {
     440        int nodesSize = w.getNodesCount();
     441        for (int i = 0; i < nodesSize - 1; i++) {
     442            final WaySegment es1 = new WaySegment(w, i);
     443            final EastNorth en1 = es1.getFirstNode().getEastNorth();
     444            final EastNorth en2 = es1.getSecondNode().getEastNorth();
     445            if (en1 == null || en2 == null) {
     446                Logging.warn("Crossing ways test skipped " + es1);
     447                continue;
     448            }
     449            for (List<WaySegment> segments : CrossingWays.getSegments(cellSegments, en1, en2)) {
     450                for (WaySegment es2 : segments) {
     451
     452                    List<WaySegment> highlight;
     453                    if (es2.way == w // reported by CrossingWays.SelfIntersection
     454                            || (findSharedWaySegments && !es1.isSimilar(es2))
     455                            || (!findSharedWaySegments && !es1.intersects(es2)))
     456                        continue;
     457
     458                    List<Way> prims = Arrays.asList(es1.way, es2.way);
     459                    if ((highlight = crossingWays.get(prims)) == null) {
     460                        highlight = new ArrayList<>();
     461                        highlight.add(es1);
     462                        highlight.add(es2);
     463                        crossingWays.put(prims, highlight);
     464                    } else {
     465                        highlight.add(es1);
     466                        highlight.add(es2);
     467                    }
     468                }
     469                segments.add(es1);
     470            }
     471        }
     472    }
     473
    430474}
  • trunk/src/org/openstreetmap/josm/data/validation/tests/MapCSSTagChecker.java

    r15938 r15959  
    4444import org.openstreetmap.josm.data.osm.Tag;
    4545import org.openstreetmap.josm.data.osm.Way;
     46import org.openstreetmap.josm.data.osm.WaySegment;
    4647import org.openstreetmap.josm.data.preferences.sources.SourceEntry;
    4748import org.openstreetmap.josm.data.preferences.sources.ValidatorPrefHelper;
     
    9394 */
    9495public class MapCSSTagChecker extends Test.TagTest {
    95     MapCSSTagCheckerIndex indexData;
    96     final Set<OsmPrimitive> tested = new HashSet<>();
     96    private MapCSSTagCheckerIndex indexData;
     97    private final Set<OsmPrimitive> tested = new HashSet<>();
     98    final Map<IPrimitive, Area> mpAreaCache = new HashMap<>();
    9799
    98100    /**
     
    202204                @Override
    203205                public Command createCommand(OsmPrimitive p, Selector matchingSelector) {
    204                     final Tag tag = Tag.ofString(evaluateObject(obj, p, matchingSelector));
     206                    final Tag tag = Tag.ofString(FixCommand.evaluateObject(obj, p, matchingSelector));
    205207                    return new ChangePropertyCommand(p, tag.getKey(), tag.getValue());
    206208                }
     
    223225                @Override
    224226                public Command createCommand(OsmPrimitive p, Selector matchingSelector) {
    225                     final String key = evaluateObject(obj, p, matchingSelector);
     227                    final String key = FixCommand.evaluateObject(obj, p, matchingSelector);
    226228                    return new ChangePropertyCommand(p, key, "");
    227229                }
     
    620622                                errorBuilder = errorBuilder.fix(() -> fix);
    621623                            }
     624                            // check if we have special information about highlighted objects */
     625                            boolean hiliteFound = false;
    622626                            if (env.intersections != null) {
    623627                                Area is = env.intersections.get(c);
    624628                                if (is != null) {
    625629                                    errorBuilder = errorBuilder.highlight(is);
     630                                    hiliteFound = true;
     631                                }
     632                            }
     633                            if (env.crossingWaysMap != null && !hiliteFound) {
     634                                Map<List<Way>, List<WaySegment>> is = env.crossingWaysMap.get(c);
     635                                if (is != null) {
     636                                    Set<WaySegment> toHilite = new HashSet<>();
     637                                    for (List<WaySegment> wsList : is.values()) {
     638                                        toHilite.addAll(wsList);
     639                                    }
     640                                    errorBuilder = errorBuilder.highlightWaySegments(toHilite);
    626641                                }
    627642                            }
     
    709724
    710725        Environment env = new Environment(p, new MultiCascade(), Environment.DEFAULT_LAYER, null);
     726        env.mpAreaCache = mpAreaCache;
     727
    711728        // the declaration indices are sorted, so it suffices to save the last used index
    712729        Declaration lastDeclUsed = null;
     
    775792    }
    776793
    777     private static Collection<TestError> getErrorsForPrimitive(OsmPrimitive p, boolean includeOtherSeverity,
     794    private Collection<TestError> getErrorsForPrimitive(OsmPrimitive p, boolean includeOtherSeverity,
    778795            Collection<Set<TagCheck>> checksCol) {
     796        // this variant is only used by the assertion tests
    779797        final List<TestError> r = new ArrayList<>();
    780798        final Environment env = new Environment(p, new MultiCascade(), Environment.DEFAULT_LAYER, null);
     799        env.mpAreaCache = mpAreaCache;
    781800        for (Set<TagCheck> schecks : checksCol) {
    782801            for (TagCheck check : schecks) {
     
    9921011        }
    9931012        tested.clear();
     1013        mpAreaCache.clear();
    9941014    }
    9951015
     
    10231043            tested.clear();
    10241044        }
    1025         super.endTest();
    10261045        // no need to keep the index, it is quickly build and doubles the memory needs
    10271046        indexData = null;
     1047        // always clear the cache to make sure that we catch changes in geometry
     1048        mpAreaCache.clear();
     1049        super.endTest();
    10281050    }
    10291051}
  • trunk/src/org/openstreetmap/josm/data/validation/tests/MultipolygonTest.java

    r15586 r15959  
    3737import org.openstreetmap.josm.tools.Geometry;
    3838import org.openstreetmap.josm.tools.Geometry.PolygonIntersection;
    39 import org.openstreetmap.josm.tools.Logging;
    4039
    4140/**
     
    626625        for (Way w: r.getMemberPrimitives(Way.class)) {
    627626            if (!w.hasIncompleteNodes()) {
    628                 findIntersectingWay(w, cellSegments, crossingWays, findSharedWaySegments);
     627                CrossingWays.findIntersectingWay(w, cellSegments, crossingWays, findSharedWaySegments);
    629628            }
    630629        }
    631630        return crossingWays;
    632     }
    633 
    634     /**
    635      * Find ways which are crossing without sharing a node.
    636      * @param w way that is member of the relation
    637      * @param cellSegments map with already collected way segments
    638      * @param crossingWays map to collect crossing ways and related segments
    639      * @param findSharedWaySegments true: find shared way segments instead of crossings
    640      */
    641     private static void findIntersectingWay(Way w, Map<Point2D, List<WaySegment>> cellSegments,
    642             Map<List<Way>, List<WaySegment>> crossingWays, boolean findSharedWaySegments) {
    643         int nodesSize = w.getNodesCount();
    644         for (int i = 0; i < nodesSize - 1; i++) {
    645             final WaySegment es1 = new WaySegment(w, i);
    646             final EastNorth en1 = es1.getFirstNode().getEastNorth();
    647             final EastNorth en2 = es1.getSecondNode().getEastNorth();
    648             if (en1 == null || en2 == null) {
    649                 Logging.warn("Crossing ways test (MP) skipped " + es1);
    650                 continue;
    651             }
    652             for (List<WaySegment> segments : CrossingWays.getSegments(cellSegments, en1, en2)) {
    653                 for (WaySegment es2 : segments) {
    654 
    655                     List<WaySegment> highlight;
    656                     if (es2.way == w // reported by CrossingWays.SelfIntersection
    657                             || (findSharedWaySegments && !es1.isSimilar(es2))
    658                             || (!findSharedWaySegments && !es1.intersects(es2)))
    659                         continue;
    660 
    661                     List<Way> prims = Arrays.asList(es1.way, es2.way);
    662                     if ((highlight = crossingWays.get(prims)) == null) {
    663                         highlight = new ArrayList<>();
    664                         highlight.add(es1);
    665                         highlight.add(es2);
    666                         crossingWays.put(prims, highlight);
    667                     } else {
    668                         highlight.add(es1);
    669                         highlight.add(es2);
    670                     }
    671                 }
    672                 segments.add(es1);
    673             }
    674         }
    675631    }
    676632
  • trunk/src/org/openstreetmap/josm/gui/mappaint/Environment.java

    r15938 r15959  
    33
    44import java.awt.geom.Area;
    5 import java.util.HashMap;
    65import java.util.LinkedHashSet;
     6import java.util.List;
    77import java.util.Map;
    88import java.util.Set;
     
    1010import org.openstreetmap.josm.data.osm.IPrimitive;
    1111import org.openstreetmap.josm.data.osm.Relation;
     12import org.openstreetmap.josm.data.osm.Way;
     13import org.openstreetmap.josm.data.osm.WaySegment;
    1214import org.openstreetmap.josm.gui.mappaint.mapcss.Condition.Context;
    1315import org.openstreetmap.josm.gui.mappaint.mapcss.Selector.LinkSelector;
     
    7274
    7375    /**
     76     * Crossing ways result from CrossingFinder, filled for incomplete ways/relations
     77    */
     78    public Map<IPrimitive, Map<List<Way>, List<WaySegment>>> crossingWaysMap;
     79
     80    /**
    7481     * Intersection areas (only filled with CrossingFinder if children is not null)
    7582     */
    7683    public Map<IPrimitive, Area> intersections;
     84
     85    /**
     86     * Cache for multipolygon areas, can be null, used with CrossingFinder
     87     */
     88    public Map<IPrimitive, Area> mpAreaCache;
    7789
    7890    /**
     
    126138        this.context = other.getContext();
    127139        this.children = other.children == null ? null : new LinkedHashSet<>(other.children);
    128         this.intersections = other.intersections == null ? null : new HashMap<>(other.intersections);
     140        this.intersections = other.intersections;
     141        this.crossingWaysMap = other.crossingWaysMap;
     142        this.mpAreaCache = other.mpAreaCache;
    129143    }
    130144
     
    294308        children = null;
    295309        intersections = null;
     310        crossingWaysMap = null;
    296311    }
    297312
  • trunk/src/org/openstreetmap/josm/gui/mappaint/mapcss/Selector.java

    r15938 r15959  
    55
    66import java.awt.geom.Area;
     7import java.awt.geom.Point2D;
    78import java.text.MessageFormat;
    89import java.util.ArrayList;
     
    1213import java.util.LinkedHashSet;
    1314import java.util.List;
     15import java.util.Map;
    1416import java.util.Objects;
    1517import java.util.function.IntFunction;
     
    2628import org.openstreetmap.josm.data.osm.OsmUtils;
    2729import org.openstreetmap.josm.data.osm.Relation;
     30import org.openstreetmap.josm.data.osm.Way;
     31import org.openstreetmap.josm.data.osm.WaySegment;
    2832import org.openstreetmap.josm.data.osm.visitor.PrimitiveVisitor;
    2933import org.openstreetmap.josm.data.osm.visitor.paint.relations.MultipolygonCache;
     34import org.openstreetmap.josm.data.validation.tests.CrossingWays;
    3035import org.openstreetmap.josm.gui.mappaint.Environment;
    3136import org.openstreetmap.josm.gui.mappaint.Range;
     
    303308            private final String layer;
    304309            private Area area;
     310            /** Will contain all way segments, grouped by cells */
     311            Map<Point2D, List<WaySegment>> cellSegments;
    305312
    306313            private CrossingFinder(Environment e) {
    307314                super(e);
    308                 CheckParameterUtil.ensureThat(e.osm instanceof IWay, "Only ways are supported");
     315                CheckParameterUtil.ensureThat(isArea(e.osm), "Only areas are supported");
    309316                layer = OsmUtils.getLayer(e.osm);
    310317            }
    311318
    312             @Override
    313             public void visit(IWay<?> w) {
    314                 if (Objects.equals(layer, OsmUtils.getLayer(w))
    315                     && left.matches(new Environment(w).withParent(e.osm))
    316                     && e.osm instanceof IWay) {
    317                     if (area == null) {
    318                         area = Geometry.getAreaEastNorth(e.osm);
    319                     }
    320                     Pair<PolygonIntersection, Area> is = Geometry.polygonIntersectionResult(
    321                             Geometry.getAreaEastNorth(w), area, Geometry.INTERSECTION_EPS_EAST_NORTH);
    322                     if (Geometry.PolygonIntersection.CROSSING == is.a) {
    323                         addToChildren(e, w);
    324                         // store intersection area to improve highlight and zoom to problem
    325                         if (e.intersections == null) {
    326                             e.intersections = new HashMap<>();
     319            private Area getAreaEastNorth(IPrimitive p, Environment e) {
     320                if (e.mpAreaCache != null && p.isMultipolygon()) {
     321                    Area a = e.mpAreaCache.get(p);
     322                    if (a == null) {
     323                        a = Geometry.getAreaEastNorth(p);
     324                        e.mpAreaCache.put(p, a);
     325                    }
     326                    return a;
     327                }
     328                return Geometry.getAreaEastNorth(p);
     329            }
     330
     331            private Map<List<Way>, List<WaySegment>> findCrossings(IPrimitive area,
     332                    Map<Point2D, List<WaySegment>> cellSegments) {
     333                /** The detected crossing ways */
     334                Map<List<Way>, List<WaySegment>> crossingWays = new HashMap<>(50);
     335                if (area instanceof Way) {
     336                    CrossingWays.findIntersectingWay((Way) area, cellSegments, crossingWays, false);
     337                } else if (area instanceof Relation && area.isMultipolygon()) {
     338                    Relation r = (Relation) area;
     339                    for (Way w : r.getMemberPrimitives(Way.class)) {
     340                        if (!w.hasIncompleteNodes()) {
     341                            CrossingWays.findIntersectingWay(w, cellSegments, crossingWays, false);
    327342                        }
    328                         e.intersections.put(w, is.b);
     343                    }
     344                }
     345                return crossingWays;
     346            }
     347
     348            @Override
     349            public void visit(Collection<? extends IPrimitive> primitives) {
     350                List<? extends IPrimitive> toIgnore;
     351                if (e.osm instanceof Relation) {
     352                    toIgnore = ((IRelation<?>) e.osm).getMemberPrimitivesList();
     353                } else {
     354                    toIgnore = null;
     355                }
     356
     357                for (IPrimitive p : primitives) {
     358                    if (isPrimitiveUsable(p) && Objects.equals(layer, OsmUtils.getLayer(p))
     359                            && left.matches(new Environment(p).withParent(e.osm)) && isArea(p)
     360                            && (toIgnore == null || !toIgnore.contains(p))) {
     361                        if (area == null) {
     362                            area = getAreaEastNorth(e.osm, e);
     363                        }
     364                        Area otherArea = getAreaEastNorth(p, e);
     365                        if (area.isEmpty() || otherArea.isEmpty()) {
     366                            if (cellSegments == null) {
     367                                // lazy initialisation
     368                                cellSegments = new HashMap<>();
     369                                findCrossings(e.osm, cellSegments); // ignore self intersections etc. here
     370                            }
     371                            // need a copy
     372                            final Map<Point2D, List<WaySegment>> tmpCellSegments = new HashMap<>(cellSegments);
     373                            // calculate all crossings between e.osm and p
     374                            Map<List<Way>, List<WaySegment>> crossingWays = findCrossings(p, tmpCellSegments);
     375                            if (!crossingWays.isEmpty()) {
     376                                addToChildren(e, p);
     377                                if (e.crossingWaysMap == null) {
     378                                    e.crossingWaysMap = new HashMap<>();
     379                                }
     380                                e.crossingWaysMap.put(p, crossingWays);
     381                            }
     382                        } else {
     383                            // we have complete data. This allows to find intersections with shared nodes
     384                            // See #16707
     385                            Pair<PolygonIntersection, Area> is = Geometry.polygonIntersectionResult(
     386                                    otherArea, area, Geometry.INTERSECTION_EPS_EAST_NORTH);
     387                            if (Geometry.PolygonIntersection.CROSSING == is.a) {
     388                                addToChildren(e, p);
     389                                // store intersection area to improve highlight and zoom to problem
     390                                if (e.intersections == null) {
     391                                    e.intersections = new HashMap<>();
     392                                }
     393                                e.intersections.put(p, is.b);
     394                            }
     395                        }
    329396                    }
    330397                }
     
    453520                return ChildOrParentSelectorType.SUPERSET_OR_EQUAL == type ? e.children != null : e.children == null;
    454521
    455             } else if (ChildOrParentSelectorType.CROSSING == type && e.osm instanceof IWay) {
     522            } else if (ChildOrParentSelectorType.CROSSING == type) {
    456523                e.parent = e.osm;
    457                 if (right instanceof OptimizedGeneralSelector
    458                         && e.osm.getDataSet() != null
    459                         && ((OptimizedGeneralSelector) right).matchesBase(OsmPrimitiveType.WAY)) {
     524                if (e.osm.getDataSet() != null && isArea(e.osm)) {
    460525                    final CrossingFinder crossingFinder = new CrossingFinder(e);
    461                     crossingFinder.visit(e.osm.getDataSet().searchWays(e.osm.getBBox()));
     526                    visitBBox(e, crossingFinder);
     527                    return e.children != null;
    462528                }
    463529                return e.children != null;
  • trunk/src/org/openstreetmap/josm/tools/Geometry.java

    r15942 r15959  
    562562            return Geometry.getArea(((Way) p).getNodes());
    563563        }
    564         if (p instanceof Relation && p.isMultipolygon() && !p.isIncomplete()
    565                 && !((Relation) p).hasIncompleteMembers()) {
     564        if (p instanceof Relation && p.isMultipolygon() && !p.isIncomplete()) {
    566565            Multipolygon mp = MultipolygonCache.getInstance().get((Relation) p);
    567             Path2D path = new Path2D.Double();
    568             path.setWindingRule(Path2D.WIND_EVEN_ODD);
    569             for (PolyData pd : mp.getCombinedPolygons()) {
    570                 path.append(pd.get(), false);
    571             }
    572             return new Area(path);
     566            if (mp.getOpenEnds().isEmpty()) {
     567                Path2D path = new Path2D.Double();
     568                path.setWindingRule(Path2D.WIND_EVEN_ODD);
     569                for (PolyData pd : mp.getCombinedPolygons()) {
     570                    path.append(pd.get(), false);
     571                }
     572                return new Area(path);
     573            }
    573574        }
    574575        return new Area();
  • trunk/test/unit/org/openstreetmap/josm/data/validation/tests/MapCSSTagCheckerTest.java

    r15073 r15959  
    357357    }
    358358
     359    /**
     360     * Non-regression test for <a href="https://josm.openstreetmap.de/ticket/13165">Bug #13165</a>.
     361     * @throws Exception if an error occurs
     362     */
     363    @Test
     364    public void testTicket13165() throws Exception {
     365        final MapCSSTagChecker test = buildTagChecker(
     366                "area:closed[tag(\"landuse\") = parent_tag(\"landuse\")] ⧉ area:closed[landuse] {"
     367                        + "throwWarning: tr(\"Overlapping Identical Landuses\");"
     368                        + "}");
     369        try (InputStream is = TestUtils.getRegressionDataStream(13165, "13165.osm")) {
     370            test.visit(OsmReader.parseDataSet(is, null).allPrimitives());
     371            List<TestError> errors = test.getErrors();
     372            assertEquals(3, errors.size());
     373        }
     374    }
     375
     376    /**
     377     * Non-regression test for <a href="https://josm.openstreetmap.de/ticket/13165">Bug #13165</a>.
     378     * @throws Exception if an error occurs
     379     */
     380    @Test
     381    public void testTicket13165IncompleteMP() throws Exception {
     382        final MapCSSTagChecker test = buildTagChecker(
     383                "area:closed[tag(\"landuse\") = parent_tag(\"landuse\")] ⧉ area:closed[landuse] {"
     384                        + "throwWarning: tr(\"Overlapping Identical Landuses\");"
     385                        + "}");
     386        try (InputStream is = TestUtils.getRegressionDataStream(13165, "13165-incomplete.osm.bz2")) {
     387            test.visit(OsmReader.parseDataSet(is, null).allPrimitives());
     388            List<TestError> errors = test.getErrors();
     389            assertEquals(3, errors.size());
     390        }
     391    }
     392
    359393}
Note: See TracChangeset for help on using the changeset viewer.