Index: data/validator/unnecessary.mapcss =================================================================== --- data/validator/unnecessary.mapcss (revision 15416) +++ data/validator/unnecessary.mapcss (working copy) @@ -165,3 +165,12 @@ assertNoMatch: "way name=house building=house"; assertMatch: "way name=house building=yes"; } + +/* #2760 */ +*[/^gpx:/] { + throwWarning: tr("{0} should not be uploaded", "{0.key}"); + group: tr("unnecessary tag"); + fixRemove: "{0.key}"; + assertMatch: "node gpx:time=2018-01-01T12:00:00Z"; + assertNoMatch: "node source=gpx:foo"; +} Index: src/org/openstreetmap/josm/actions/SimplifyWayAction.java =================================================================== --- src/org/openstreetmap/josm/actions/SimplifyWayAction.java (revision 15416) +++ src/org/openstreetmap/josm/actions/SimplifyWayAction.java (working copy) @@ -5,6 +5,7 @@ import static org.openstreetmap.josm.tools.I18n.tr; import static org.openstreetmap.josm.tools.I18n.trn; +import java.awt.GridBagLayout; import java.awt.event.ActionEvent; import java.awt.event.KeyEvent; import java.util.ArrayList; @@ -17,7 +18,13 @@ import java.util.Set; import java.util.stream.Collectors; +import javax.swing.BorderFactory; +import javax.swing.JCheckBox; +import javax.swing.JLabel; import javax.swing.JOptionPane; +import javax.swing.JPanel; +import javax.swing.JSpinner; +import javax.swing.SpinnerNumberModel; import javax.swing.SwingUtilities; import org.openstreetmap.josm.command.ChangeCommand; @@ -24,6 +31,7 @@ import org.openstreetmap.josm.command.Command; import org.openstreetmap.josm.command.DeleteCommand; import org.openstreetmap.josm.command.SequenceCommand; +import org.openstreetmap.josm.data.SystemOfMeasurement; import org.openstreetmap.josm.data.UndoRedoHandler; import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.data.osm.Node; @@ -30,11 +38,14 @@ import org.openstreetmap.josm.data.osm.OsmPrimitive; import org.openstreetmap.josm.data.osm.Way; import org.openstreetmap.josm.data.projection.Ellipsoid; +import org.openstreetmap.josm.gui.ExtendedDialog; import org.openstreetmap.josm.gui.HelpAwareOptionPane; import org.openstreetmap.josm.gui.HelpAwareOptionPane.ButtonSpec; import org.openstreetmap.josm.gui.MainApplication; import org.openstreetmap.josm.gui.Notification; import org.openstreetmap.josm.spi.preferences.Config; +import org.openstreetmap.josm.spi.preferences.IPreferences; +import org.openstreetmap.josm.tools.GBC; import org.openstreetmap.josm.tools.ImageProvider; import org.openstreetmap.josm.tools.Shortcut; @@ -93,6 +104,68 @@ ); } + /** + * Asks the user for max-err value used to simplify ways, if not remembered before + * @param text the text being shown + * @param auto whether it's called automatically (conversion) or by the user + * @return the max-err value or -1 if canceled + */ + public static double askSimplifyWays(String text, boolean auto) { + IPreferences s = Config.getPref(); + String key = "simplify-way." + (auto ? "auto." : ""); + String keyRemember = key + "remember"; + String keyError = key + "max-error"; + + String r = s.get(keyRemember, "ask"); + if (auto && "no".equals(r)) { + return -1; + } else if ("yes".equals(r)) { + return s.getDouble(keyError, 3.0); + } + + JPanel p = new JPanel(new GridBagLayout()); + p.add(new JLabel("" + text + "

" + + tr("This reduces unnecessary nodes along the way and is especially recommended if GPS tracks were recorded by time " + + "(e.g. one point per second) or when the accuracy was low (reduces \"zigzag\" tracks).") + + ""), GBC.eol()); + p.setBorder(BorderFactory.createEmptyBorder(5, 10, 10, 5)); + JPanel q = new JPanel(new GridBagLayout()); + q.add(new JLabel(tr("Maximum error (meters): "))); + JSpinner n = new JSpinner(new SpinnerNumberModel( + s.getDouble(keyError, 3.0), 0.01, 100, 0.5)); + q.add(n); + q.setBorder(BorderFactory.createEmptyBorder(14, 0, 10, 0)); + p.add(q, GBC.eol()); + JCheckBox c = new JCheckBox(tr("Do not ask again")); + p.add(c, GBC.eol()); + + ExtendedDialog ed = new ExtendedDialog(MainApplication.getMainFrame(), + tr("Simplify way"), tr("Simplify"), + auto ? tr("Proceed without simplifying") : tr("Cancel")) + .setContent(p) + .configureContextsensitiveHelp(("Action/SimplifyWay"), true); + if (auto) { + ed.setButtonIcons("simplify", "ok"); + } else { + ed.setButtonIcons("ok", "cancel"); + } + + int ret = ed.showDialog().getValue(); + double val = (double) n.getValue(); + if (ret == 1) { + s.putDouble(keyError, val); + if (c.isSelected()) { + s.put(keyRemember, "yes"); + } + return val; + } else { + if (auto && c.isSelected()) { //do not remember cancel for manual simplify, otherwise nothing would happen + s.put(keyRemember, "no"); + } + return -1; + } + } + @Override public void actionPerformed(ActionEvent e) { DataSet ds = getLayerManager().getEditDataSet(); @@ -108,20 +181,20 @@ return; } - Collection allCommands = new LinkedList<>(); - for (Way way: ways) { - SequenceCommand simplifyCommand = simplifyWay(way); - if (simplifyCommand == null) { - continue; - } - allCommands.add(simplifyCommand); + String lengthstr = SystemOfMeasurement.getSystemOfMeasurement().getDistText( + ways.stream().collect( + Collectors.summingDouble(w -> { + return w.getLength(); + }))); + + double err = askSimplifyWays(trn( + "You are about to simplify {0} way with a total length of {1}.", + "You are about to simplify {0} ways with a total length of {1}.", + ways.size(), ways.size(), lengthstr), false); + + if (err > 0) { + simplifyWays(ways, err); } - if (allCommands.isEmpty()) return; - SequenceCommand rootCommand = new SequenceCommand( - trn("Simplify {0} way", "Simplify {0} ways", allCommands.size(), allCommands.size()), - allCommands - ); - UndoRedoHandler.getInstance().add(rootCommand); } finally { ds.endUpdate(); } @@ -156,17 +229,6 @@ } /** - * Simplifies a way with default threshold (read from preferences). - * - * @param w the way to simplify - * @return The sequence of commands to run - * @since 6411 - */ - public final SequenceCommand simplifyWay(Way w) { - return simplifyWay(w, Config.getPref().getDouble("simplify-way.max-error", 3.0)); - } - - /** * Calculate a set of nodes which occurs more than once in the way * @param w the way * @return a set of nodes which occurs more than once in the way @@ -182,14 +244,76 @@ } /** - * Simplifies a way with a given threshold. + * Runs the commands to simplify the ways with the given threshold * + * @param ways the ways to simplify + * @param threshold the max error threshold + * @since xxx + */ + public static void simplifyWays(List ways, double threshold) { + Collection allCommands = new LinkedList<>(); + for (Way way : ways) { + SequenceCommand simplifyCommand = createSimplifyCommand(way, threshold); + if (simplifyCommand == null) { + continue; + } + allCommands.add(simplifyCommand); + } + if (allCommands.isEmpty()) + return; + SequenceCommand rootCommand = new SequenceCommand( + trn("Simplify {0} way", "Simplify {0} ways", allCommands.size(), allCommands.size()), + allCommands); + UndoRedoHandler.getInstance().add(rootCommand); + } + + /** + * Creates the SequenceCommand to simplify a way with default threshold. + * @deprecated Replaced by {@link #createSimplifyCommand(Way)}. You can also use {@link #simplifyWays(List, double)} directly. + * * @param w the way to simplify + * @return The sequence of commands to run + * @since 6411 + */ + @Deprecated + public final SequenceCommand simplifyWay(Way w) { + return createSimplifyCommand(w); + } + + /** + * Creates the SequenceCommand to simplify a way with a given threshold. + * @deprecated Replaced by {@link #createSimplifyCommand(Way, double)}. You can also use {@link #simplifyWays(List, double)} directly. + * + * @param w the way to simplify * @param threshold the max error threshold * @return The sequence of commands to run * @since 6411 */ + @Deprecated public static SequenceCommand simplifyWay(Way w, double threshold) { + return createSimplifyCommand(w, threshold); + } + + /** + * Creates the SequenceCommand to simplify a way with default threshold. + * + * @param w the way to simplify + * @return The sequence of commands to run + * @since xxx + */ + public final SequenceCommand createSimplifyCommand(Way w) { + return createSimplifyCommand(w, Config.getPref().getDouble("simplify-way.max-error", 3.0)); + } + + /** + * Creates the SequenceCommand to simplify a way with a given threshold. + * + * @param w the way to simplify + * @param threshold the max error threshold + * @return The sequence of commands to run + * @since xxx + */ + public static SequenceCommand createSimplifyCommand(Way w, double threshold) { int lower = 0; int i = 0; Index: src/org/openstreetmap/josm/data/gpx/GpxConstants.java =================================================================== --- src/org/openstreetmap/josm/data/gpx/GpxConstants.java (revision 15416) +++ src/org/openstreetmap/josm/data/gpx/GpxConstants.java (working copy) @@ -14,6 +14,9 @@ */ public interface GpxConstants { + /** Prefix used for attributes when converting to OSM data */ + String GPX_PREFIX = "gpx:"; + /** GPS name of the element. This field will be transferred to and from the GPS. * GPX does not place restrictions on the length of this field or the characters contained in it. * It is up to the receiving application to validate the field before sending it to the GPS. */ Index: src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java =================================================================== --- src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java (revision 15416) +++ src/org/openstreetmap/josm/data/osm/AbstractPrimitive.java (working copy) @@ -20,6 +20,7 @@ import java.util.concurrent.atomic.AtomicLong; import java.util.function.BiPredicate; +import org.openstreetmap.josm.data.gpx.GpxConstants; import org.openstreetmap.josm.spi.preferences.Config; import org.openstreetmap.josm.tools.Utils; @@ -740,7 +741,7 @@ if (uninteresting == null) { List l = new LinkedList<>(Arrays.asList( "source", "source_ref", "source:", "comment", - "watch", "watch:", "description", "attribution")); + "watch", "watch:", "description", "attribution", GpxConstants.GPX_PREFIX)); l.addAll(getDiscardableKeys()); l.addAll(getWorkInProgressKeys()); uninteresting = new HashSet<>(Config.getPref().getList("tags.uninteresting", l)); Index: src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java =================================================================== --- src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java (revision 15416) +++ src/org/openstreetmap/josm/gui/layer/OsmDataLayer.java (working copy) @@ -31,6 +31,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.atomic.AtomicBoolean; @@ -779,7 +780,7 @@ Collection> trk = new ArrayList<>(); Map trkAttr = new HashMap<>(); - String name = w.get("name"); + String name = gpxVal(w, "name"); if (name != null) { trkAttr.put("name", name); } @@ -806,7 +807,7 @@ private static boolean containsOnlyGpxTags(Tagged t) { for (String key : t.getKeys().keySet()) { - if (!GpxConstants.WPT_KEYS.contains(key)) { + if (!GpxConstants.WPT_KEYS.contains(key) && !key.startsWith(GpxConstants.GPX_PREFIX)) { return false; } } @@ -814,6 +815,16 @@ } /** + * Reads the Gpx key from the given {@link OsmPrimitive}, with or without "gpx:" prefix + * @param node + * @param key + * @return the value or null if not present + */ + public static String gpxVal(OsmPrimitive node, String key) { + return Optional.ofNullable(node.get(GpxConstants.GPX_PREFIX + key)).orElse(node.get(key)); + } + + /** * @param n the {@code Node} to convert * @return {@code WayPoint} object * @since 13210 @@ -836,10 +847,11 @@ addDoubleIfPresent(wpt, n, GpxConstants.PT_ELE); try { + String v; if (time > Long.MIN_VALUE) { wpt.setTimeInMillis(time); - } else if (n.hasKey(GpxConstants.PT_TIME)) { - wpt.setTimeInMillis(DateUtils.tsFromString(n.get(GpxConstants.PT_TIME))); + } else if ((v = gpxVal(n, GpxConstants.PT_TIME)) != null) { + wpt.setTimeInMillis(DateUtils.tsFromString(v)); } else if (!n.isTimestampEmpty()) { wpt.setTime(Integer.toUnsignedLong(n.getRawTimestamp())); } @@ -859,7 +871,7 @@ Collection links = new ArrayList<>(); for (String key : new String[]{"link", "url", "website", "contact:website"}) { - String value = n.get(key); + String value = gpxVal(n, key); if (value != null) { links.add(new GpxLink(value)); } @@ -897,7 +909,7 @@ List possibleKeys = new ArrayList<>(Arrays.asList(osmKeys)); possibleKeys.add(0, gpxKey); for (String key : possibleKeys) { - String value = p.get(key); + String value = gpxVal(p, key); if (value != null) { try { int i = Integer.parseInt(value); @@ -918,7 +930,7 @@ List possibleKeys = new ArrayList<>(Arrays.asList(osmKeys)); possibleKeys.add(0, gpxKey); for (String key : possibleKeys) { - String value = p.get(key); + String value = gpxVal(p, key); if (value != null) { try { double d = Double.parseDouble(value); @@ -938,7 +950,7 @@ List possibleKeys = new ArrayList<>(Arrays.asList(osmKeys)); possibleKeys.add(0, gpxKey); for (String key : possibleKeys) { - String value = p.get(key); + String value = gpxVal(p, key); // Sanity checks if (value != null && (!GpxConstants.PT_FIX.equals(gpxKey) || GpxConstants.FIX_VALUES.contains(value))) { wpt.put(gpxKey, value); Index: src/org/openstreetmap/josm/gui/layer/gpx/ConvertFromGpxLayerAction.java =================================================================== --- src/org/openstreetmap/josm/gui/layer/gpx/ConvertFromGpxLayerAction.java (revision 15416) +++ src/org/openstreetmap/josm/gui/layer/gpx/ConvertFromGpxLayerAction.java (working copy) @@ -54,7 +54,7 @@ public DataSet convert() { final DataSet ds = new DataSet(); - List keys = new ArrayList<>(); + List keys = new ArrayList<>(); //note that items in this list don't have the GPX_PREFIX String convertTags = Config.getPref().get(GPX_SETTING, "ask"); boolean check = "list".equals(convertTags) || "ask".equals(convertTags); boolean none = "no".equals(convertTags); // no need to convert tags when no dialog will be shown anyways @@ -72,12 +72,12 @@ } if (!none && (obj instanceof String || obj instanceof Number)) { // only convert when required - n.put(key, obj.toString()); + n.put(GpxConstants.GPX_PREFIX + key, obj.toString()); } else if (obj instanceof Date && GpxConstants.PT_TIME.equals(key)) { // timestamps should always be converted Date date = (Date) obj; if (!none) { //... but the tag will only be set when required - n.put(key, DateUtils.fromDate(date)); + n.put(GpxConstants.GPX_PREFIX + key, DateUtils.fromDate(date)); } n.setTimestamp(date); } @@ -125,7 +125,7 @@ /** * Filters the tags of the given {@link DataSet} * @param ds The {@link DataSet} - * @param listPos A {@code List} containing the tags to be kept, can be {@code null} if all tags are to be removed + * @param listPos A {@code List} containing the tags (without prefix) to be kept, can be {@code null} if all tags are to be removed * @return The {@link DataSet} * @since 14103 */ @@ -133,7 +133,7 @@ Collection nodes = ds.getNodes(); for (Node n : nodes) { for (String key : n.keySet()) { - if (listPos == null || !listPos.contains(key)) { + if (listPos == null || !listPos.contains(key.substring(GpxConstants.GPX_PREFIX.length()))) { n.put(key, null); } } Index: src/org/openstreetmap/josm/gui/layer/gpx/ConvertToDataLayerAction.java =================================================================== --- src/org/openstreetmap/josm/gui/layer/gpx/ConvertToDataLayerAction.java (revision 15416) +++ src/org/openstreetmap/josm/gui/layer/gpx/ConvertToDataLayerAction.java (working copy) @@ -7,6 +7,7 @@ import java.awt.GridBagLayout; import java.awt.event.ActionEvent; import java.io.File; +import java.util.ArrayList; import javax.swing.AbstractAction; import javax.swing.JLabel; @@ -13,6 +14,7 @@ import javax.swing.JOptionPane; import javax.swing.JPanel; +import org.openstreetmap.josm.actions.SimplifyWayAction; import org.openstreetmap.josm.data.osm.DataSet; import org.openstreetmap.josm.gui.ConditionalOptionPaneUtil; import org.openstreetmap.josm.gui.MainApplication; @@ -62,6 +64,10 @@ } final DataSet ds = convert(); if (ds != null) { + double err = SimplifyWayAction.askSimplifyWays(tr("Would you like to simplify the ways in the converted layer?"), true); + if (err > 0) { + SimplifyWayAction.simplifyWays(new ArrayList<>(ds.getWays()), err); + } final OsmDataLayer osmLayer = new OsmDataLayer(ds, tr("Converted from: {0}", layer.getName()), null); if (layer.getAssociatedFile() != null) { osmLayer.setAssociatedFile(new File(layer.getAssociatedFile().getParentFile(), Index: test/unit/org/openstreetmap/josm/actions/SimplifyWayActionTest.java =================================================================== --- test/unit/org/openstreetmap/josm/actions/SimplifyWayActionTest.java (revision 15416) +++ test/unit/org/openstreetmap/josm/actions/SimplifyWayActionTest.java (working copy) @@ -129,7 +129,7 @@ final Way w = new Way(); Stream.of(n1, n2, n3, n4, w).forEach(ds::addPrimitive); Stream.of(n1, n2, n3, n4, n1).forEach(w::addNode); - final SequenceCommand command = action.simplifyWay(w); + final SequenceCommand command = SimplifyWayAction.createSimplifyCommand(w, 3); assertNotNull(command); assertEquals(2, command.getChildren().size()); final Collection deleteCommands = Utils.filteredCollection(command.getChildren(), DeleteCommand.class); Index: test/unit/org/openstreetmap/josm/gui/layer/OsmDataLayerTest.java =================================================================== --- test/unit/org/openstreetmap/josm/gui/layer/OsmDataLayerTest.java (revision 15416) +++ test/unit/org/openstreetmap/josm/gui/layer/OsmDataLayerTest.java (working copy) @@ -215,12 +215,12 @@ "\n" + "\n" + " \n" + - " \n" + - " \n" + + " \n" + + " \n" + " \n" + " \n" + " \n" + - " \n" + + " \n" + " \n" + " \n" + " \n" +