Changeset 17350 in josm for trunk/src


Ignore:
Timestamp:
2020-11-24T07:29:28+01:00 (4 years ago)
Author:
GerdP
Message:

fix #19465: Make "Overlapping ways" less aggressive

  • new preference overlapping-ways.ignore-layer with default false
  • new preference overlapping-ways.only-known-linear with default true
  • new message texts
  • improved list in method hasAreaTags(). Probably many more candidates could be added.
  • still produces info message Ways share segment when overlapping-ways.only-known-linear is changed to false. I tend to think that this test can be removed. It's kind of a mop up message for ways which are not clearly linear or area.
  • fix typo in TestError javadoc
Location:
trunk/src/org/openstreetmap/josm/data
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitive.java

    r17333 r17350  
    11181118     */
    11191119    public final boolean hasAreaTags() {
    1120         return hasKey("landuse", "amenity", "building", "building:part")
     1120        return hasKey("building", "landuse", "amenity", "shop", "building:part", "boundary", "historic", "place")
    11211121                || hasTag("area", OsmUtils.TRUE_VALUE)
    11221122                || hasTag("waterway", "riverbank")
     1123                || hasTag("highway", "rest_area", "services", "platform")
     1124                || hasTag("railway", "platform")
    11231125                || hasTagDifferent("leisure", "picnic_table", "slipway", "firepit")
    11241126                || hasTag("natural", "water", "wood", "scrub", "wetland", "grassland", "heath", "rock", "bare_rock",
    11251127                                     "sand", "beach", "scree", "bay", "glacier", "shingle", "fell", "reef", "stone",
    1126                                      "mud", "landslide", "sinkhole", "crevasse", "desert");
     1128                                     "mud", "landslide", "sinkhole", "crevasse", "desert")
     1129                || hasTag("aeroway", "aerodrome");
    11271130    }
    11281131

  • trunk/src/org/openstreetmap/josm/data/validation/TestError.java

    r17261 r17350  
    106106         * Sets the error message.
    107107         *
    108          * @param message The the message of this error group
     108         * @param message The message of this error group
    109109         * @param marktrDescription The {@linkplain I18n#marktr prepared for i18n} description of this error
    110110         * @param args The description arguments to be applied in {@link I18n#tr(String, Object...)}

  • trunk/src/org/openstreetmap/josm/data/validation/tests/OverlappingWays.java

    r15369 r17350  
    44import static org.openstreetmap.josm.data.validation.tests.CrossingWays.HIGHWAY;
    55import static org.openstreetmap.josm.data.validation.tests.CrossingWays.RAILWAY;
     6import static org.openstreetmap.josm.data.validation.tests.CrossingWays.WATERWAY;
    67import static org.openstreetmap.josm.tools.I18n.tr;
    78
    … …  
    1213import java.util.HashMap;
    1314import java.util.HashSet;
     15import java.util.LinkedHashSet;
    1416import java.util.List;
    1517import java.util.Map;
    … …  
    1719import java.util.TreeSet;
    1820import java.util.function.Predicate;
     21import java.util.stream.Collectors;
    1922
    2023import org.openstreetmap.josm.data.osm.Node;
    … …  
    2528import org.openstreetmap.josm.data.osm.WaySegment;
    2629import org.openstreetmap.josm.data.preferences.ListProperty;
     30import org.openstreetmap.josm.data.preferences.sources.ValidatorPrefHelper;
    2731import org.openstreetmap.josm.data.validation.Severity;
    2832import org.openstreetmap.josm.data.validation.Test;
    2933import org.openstreetmap.josm.data.validation.TestError;
    3034import org.openstreetmap.josm.gui.progress.ProgressMonitor;
     35import org.openstreetmap.josm.spi.preferences.Config;
    3136import org.openstreetmap.josm.tools.MultiMap;
    3237import org.openstreetmap.josm.tools.Pair;
    … …  
    4348    private MultiMap<Pair<Node, Node>, WaySegment> nodePairs;
    4449
     50    private boolean onlyKnwonLinear;
     51    private boolean includeOther;
     52    private boolean ignoreLayer;
     53
    4554    protected static final int OVERLAPPING_HIGHWAY = 101;
    4655    protected static final int OVERLAPPING_RAILWAY = 102;
    4756    protected static final int OVERLAPPING_WAY = 103;
     57    protected static final int OVERLAPPING_WATERWAY = 104;
    4858    protected static final int OVERLAPPING_HIGHWAY_AREA = 111;
    4959    protected static final int OVERLAPPING_RAILWAY_AREA = 112;
    5060    protected static final int OVERLAPPING_WAY_AREA = 113;
     61    protected static final int OVERLAPPING_WATERWAY_AREA = 114;
    5162    protected static final int DUPLICATE_WAY_SEGMENT = 121;
     63    protected static final int OVERLAPPING_HIGHWAY_LINEAR_WAY = 131;
     64    protected static final int OVERLAPPING_RAILWAY_LINEAR_WAY = 132;
     65    protected static final int OVERLAPPING_WATERWAY_LINEAR_WAY = 133;
    5266
    5367    protected static final ListProperty IGNORED_KEYS = new ListProperty(
    … …  
    6983        super.startTest(monitor);
    7084        nodePairs = new MultiMap<>(1000);
     85        includeOther = isBeforeUpload ? ValidatorPrefHelper.PREF_OTHER_UPLOAD.get() : ValidatorPrefHelper.PREF_OTHER.get();
     86        onlyKnwonLinear = Config.getPref().getBoolean("overlapping-ways.only-known-linear", true);
     87        ignoreLayer = Config.getPref().getBoolean("overlapping-ways.ignore-layer", false);
    7188    }
    7289
    7390    private static boolean parentMultipolygonConcernsArea(OsmPrimitive p) {
    7491        return p.referrers(Relation.class)
    75                 .anyMatch(Relation::concernsArea);
     92                .anyMatch(Relation::isMultipolygon);
    7693    }
    7794
    … …  
    8097        Map<List<Way>, Set<WaySegment>> seenWays = new HashMap<>(500);
    8198
    82         Collection<TestError> preliminaryErrors = new ArrayList<>();
    8399        for (Set<WaySegment> duplicated : nodePairs.values()) {
    84             int ways = duplicated.size();
    85 
    86             if (ways > 1) {
    87                 List<OsmPrimitive> prims = new ArrayList<>();
    88                 List<Way> currentWays = new ArrayList<>();
    89                 Collection<WaySegment> highlight;
    90                 int highway = 0;
    91                 int railway = 0;
    92                 int area = 0;
    93 
     100            if (duplicated.size() <= 1)
     101                continue;
     102            if (ignoreLayer) {
     103                analyseOverlaps(duplicated, seenWays);
     104            } else {
     105                // group by layer tag value (which is very likely null)
     106                Map<String, Set<WaySegment>> grouped = new HashMap<>();
    94107                for (WaySegment ws : duplicated) {
    95                     if (ws.way.hasKey(HIGHWAY)) {
    96                         highway++;
    97                     } else if (ws.way.hasKey(RAILWAY)) {
    98                         railway++;
    99                     }
    100                     Boolean ar = OsmUtils.getOsmBoolean(ws.way.get("area"));
    101                     if (ar != null && ar) {
    102                         area++;
    103                     }
    104                     if (ws.way.concernsArea() || parentMultipolygonConcernsArea(ws.way)) {
    105                         area++;
    106                         ways--;
    107                     }
    108 
    109                     prims.add(ws.way);
    110                     currentWays.add(ws.way);
     108                    // order in set is important
     109                    grouped.computeIfAbsent(OsmUtils.getLayer(ws.way), k -> new LinkedHashSet<>()).add(ws);
    111110                }
    112                 // These ways not seen before
    113                 // If two or more of the overlapping ways are highways or railways mark a separate error
    114                 if ((highlight = seenWays.get(currentWays)) == null) {
    115                     String errortype;
    116                     int type;
    117 
    118                     if (area > 0) {
    119                         if (ways == 0 || duplicated.size() == area) {
    120                             continue; // We previously issued an annoying "Areas share segment" warning
    121                         } else if (highway == ways) {
    122                             errortype = tr("Highways share segment with area");
    123                             type = OVERLAPPING_HIGHWAY_AREA;
    124                         } else if (railway == ways) {
    125                             errortype = tr("Railways share segment with area");
    126                             type = OVERLAPPING_RAILWAY_AREA;
    127                         } else {
    128                             errortype = tr("Ways share segment with area");
    129                             type = OVERLAPPING_WAY_AREA;
    130                         }
    131                     } else if (highway == ways) {
    132                         errortype = tr("Overlapping highways");
    133                         type = OVERLAPPING_HIGHWAY;
    134                     } else if (railway == ways) {
    135                         errortype = tr("Overlapping railways");
    136                         type = OVERLAPPING_RAILWAY;
    137                     } else {
    138                         errortype = tr("Overlapping ways");
    139                         type = OVERLAPPING_WAY;
    140                     }
    141 
    142                     Severity severity = type < OVERLAPPING_HIGHWAY_AREA ? Severity.WARNING : Severity.OTHER;
    143                     preliminaryErrors.add(TestError.builder(this, severity, type)
    144                             .message(errortype)
    145                             .primitives(prims)
    146                             .highlightWaySegments(duplicated)
    147                             .build());
    148                     seenWays.put(currentWays, duplicated);
    149                 } else { /* way seen, mark highlight layer only */
    150                     highlight.addAll(duplicated);
     111                grouped.values().forEach(group -> analyseOverlaps(group, seenWays));
     112            }
     113        }
     114        nodePairs = null;
     115
     116        super.endTest();
     117    }
     118
     119    private void analyseOverlaps(Set<WaySegment> duplicated, Map<List<Way>, Set<WaySegment>> seenWays) {
     120        int ways = duplicated.size();
     121        if (ways <= 1)
     122            return;
     123
     124        List<Way> currentWays = duplicated.stream().map(ws -> ws.way).collect(Collectors.toList());
     125        Collection<WaySegment> highlight;
     126        if ((highlight = seenWays.get(currentWays)) != null) {
     127            /* this combination of ways was seen before, just add highlighted segment */
     128            highlight.addAll(duplicated);
     129        } else {
     130            int countHighway = 0;
     131            int countRailway = 0;
     132            int countWaterway = 0;
     133            int countOther = 0;
     134            int numAreas = 0;
     135            for (WaySegment ws : duplicated) {
     136                boolean isArea = ws.way.concernsArea();
     137                if (ws.way.hasKey(HIGHWAY)) {
     138                    if (!isArea) {
     139                        countHighway++;
     140                    }
     141                } else if (ws.way.hasKey(RAILWAY)) {
     142                    if (!isArea) {
     143                        countRailway++;
     144                    }
     145                } else if (ws.way.hasKey(WATERWAY)) {
     146                    if (!isArea) {
     147                        countWaterway++;
     148                    }
     149                } else {
     150                    if (ws.way.getInterestingTags().isEmpty() && parentMultipolygonConcernsArea(ws.way))
     151                        isArea = true;
     152                    if (!isArea && isOtherLinear(ws.way)) {
     153                        countOther++;
     154                    }
    151155                }
    152             }
    153         }
    154 
    155         // see ticket #9598 - only report if at least 3 segments are shared, except for overlapping ways, i.e warnings (see #9820)
    156         for (TestError error : preliminaryErrors) {
    157             if (error.getSeverity() == Severity.WARNING || error.getHighlighted().size() / error.getPrimitives().size() >= 3) {
    158                 boolean ignore = error.getPrimitives().stream().anyMatch(IGNORED);
    159                 if (!ignore) {
    160                     errors.add(error);
     156                if (isArea) {
     157                    numAreas++;
    161158                }
    162159            }
    163         }
    164 
    165         super.endTest();
    166         nodePairs = null;
     160            if (numAreas == ways) {
     161                // no linear object, we don't care when areas share segments
     162                return;
     163            }
     164
     165
     166            // If two or more of the overlapping ways are highways or railways mark a separate error
     167            String errortype;
     168            int type;
     169            int allKnownLinear = countHighway + countRailway + countWaterway + countOther;
     170            final Severity severity;
     171            if (countHighway > 1) {
     172                errortype = tr("Overlapping highways");
     173                type = OVERLAPPING_HIGHWAY;
     174                severity = Severity.ERROR;
     175            } else if (countRailway > 1) {
     176                errortype = tr("Overlapping railways");
     177                type = OVERLAPPING_RAILWAY;
     178                severity = Severity.ERROR;
     179            } else if (countWaterway > 1) {
     180                errortype = tr("Overlapping waterways");
     181                type = OVERLAPPING_WATERWAY;
     182                severity = Severity.ERROR;
     183            } else if (countHighway > 0 && countHighway < allKnownLinear) {
     184                errortype = tr("Highway shares segment with linear way");
     185                type = OVERLAPPING_HIGHWAY_LINEAR_WAY;
     186                severity = Severity.WARNING;
     187            } else if (countRailway > 0 && countRailway < allKnownLinear) {
     188                errortype = tr("Railway shares segment with linear way");
     189                type = OVERLAPPING_HIGHWAY_LINEAR_WAY;
     190                severity = Severity.WARNING;
     191            } else if (countWaterway > 0 && countWaterway < allKnownLinear) {
     192                errortype = tr("Waterway shares segment with linear way");
     193                type = OVERLAPPING_WATERWAY_LINEAR_WAY;
     194                severity = Severity.WARNING;
     195            } else if (!includeOther || onlyKnwonLinear) {
     196                return;
     197            } else if (countHighway > 0) {
     198                errortype = tr("Highway shares segment with other way");
     199                type = OVERLAPPING_HIGHWAY_AREA;
     200                severity = Severity.OTHER;
     201            } else if (countRailway > 0) {
     202                errortype = tr("Railway shares segment with other way");
     203                type = OVERLAPPING_RAILWAY_AREA;
     204                severity = Severity.OTHER;
     205            } else if (countWaterway > 0) {
     206                errortype = tr("Waterway shares segment with other way");
     207                type = OVERLAPPING_WATERWAY_AREA;
     208                severity = Severity.OTHER;
     209            } else {
     210                errortype = tr("Ways share segment");
     211                type = OVERLAPPING_WAY;
     212                severity = Severity.OTHER;
     213            }
     214
     215            List<OsmPrimitive> prims = new ArrayList<>(currentWays);
     216            errors.add(TestError.builder(this, severity, type)
     217                    .message(errortype)
     218                    .primitives(prims)
     219                    .highlightWaySegments(duplicated)
     220                    .build());
     221            seenWays.put(currentWays, duplicated);
     222        }
     223    }
     224
     225    private static boolean isOtherLinear(Way way) {
     226        // it is assumed that area=* was evaluated before and is false
     227        return (way.hasKey("barrier", "addr:interpolation", "route", "ford")
     228                || way.hasTag("natural", "tree_row", "cliff", "ridge")
     229                || way.hasTag("power", "line", "minor_line", "cable", "portal")
     230                || way.hasTag("man_made", "pipeline"));
    167231    }
    168232
    … …  
    203267        }
    204268
     269        if (IGNORED.test(w))
     270            return;
     271
     272        if (onlyKnwonLinear && (w.concernsArea() || w.getInterestingTags().isEmpty()))
     273            return;
     274
    205275        Node lastN = null;
    206276        int i = -2;
Note: See TracChangeset for help on using the changeset viewer.