Changeset 18034 in josm for trunk

Timestamp:

2021-07-15T22:02:36+02:00 (3 years ago)

Author:

Don-vip

Message:

fix #20795 - fix #20797 - fix wrong correlate to gpx behaviour (patch by Bjoeni)

Location:

trunk/src/org/openstreetmap/josm

Files:

• data/gpx/GpxImageCorrelation.java (modified) (1 diff)
• gui/layer/geoimage/CorrelateGpxWithImages.java (modified) (24 diffs)
• gui/layer/geoimage/GeoImageLayer.java (modified) (4 diffs)

trunk/src/org/openstreetmap/josm/data/gpx/GpxImageCorrelation.java

@@ -169 +169 @@
             }
         }
-        if (trkTag) {
+        if (trkTag && prevWp != null) {
             ret += matchPoints(images, prevWp, prevWpTime, prevWp, prevWpTime, offset, false, trkTagTime, true);
         }

trunk/src/org/openstreetmap/josm/gui/layer/geoimage/CorrelateGpxWithImages.java

@@ -22 +22 @@
 import java.awt.event.WindowAdapter;
 import java.awt.event.WindowEvent;
+import java.beans.PropertyChangeEvent;
+import java.beans.PropertyChangeListener;
 import java.io.File;
 import java.io.IOException;
@@ -40 +42 @@
 import java.util.TimeZone;
 import java.util.concurrent.TimeUnit;
+import java.util.function.Consumer;
 import java.util.stream.Collectors;
 
@@ -99 +102 @@
 import org.openstreetmap.josm.spi.preferences.Config;
 import org.openstreetmap.josm.spi.preferences.IPreferences;
+import org.openstreetmap.josm.tools.Destroyable;
 import org.openstreetmap.josm.tools.GBC;
 import org.openstreetmap.josm.tools.ImageProvider;
@@ -112 +116 @@
  * @since 2566
  */
-public class CorrelateGpxWithImages extends AbstractAction {
-
-    private static final List<GpxData> loadedGpxData = new ArrayList<>();
+public class CorrelateGpxWithImages extends AbstractAction implements Destroyable {
+
+    private static MutableComboBoxModel<GpxDataWrapper> gpxModel;
+    private static boolean forceTags;
 
     private final transient GeoImageLayer yLayer;
     private transient GpxTimezone timezone;
     private transient GpxTimeOffset delta;
-    private static boolean forceTags;
 
     /**
@@ -129 +133 @@
         new ImageProvider("dialogs/geoimage/gpx2img").getResource().attachImageIcon(this, true);
         this.yLayer = layer;
-        MainApplication.getLayerManager().addLayerChangeListener(new GpxLayerAddedListener());
     }
 
@@ -232 +235 @@
 
     private static class GpxDataWrapper {
-        private final String name;
+        private String name;
         private final GpxData data;
         private final File file;
@@ -242 +245 @@
         }
 
+        void setName(String name) {
+            this.name = name;
+            forEachLayer(CorrelateGpxWithImages::repaintCombobox);
+        }
+
         @Override
         public String toString() {
@@ -248 +256 @@
     }
 
+    private static class NoGpxDataWrapper extends GpxDataWrapper {
+        NoGpxDataWrapper() {
+            super(null, null, null);
+        }
+
+        @Override
+        public String toString() {
+            return tr("<No GPX track loaded yet>");
+        }
+    }
+
     private ExtendedDialog syncDialog;
-    private MutableComboBoxModel<GpxDataWrapper> gpxModel;
     private JPanel outerPanel;
     private JosmComboBox<GpxDataWrapper> cbGpx;
@@ -279 +297 @@
             try {
                 outerPanel.setCursor(Cursor.getPredefinedCursor(Cursor.WAIT_CURSOR));
-                for (int i = gpxModel.getSize() - 1; i >= 0; i--) {
-                    GpxDataWrapper wrapper = gpxModel.getElementAt(i);
-                    if (sel.equals(wrapper.file)) {
-                        gpxModel.setSelectedItem(wrapper);
-                        if (!sel.getName().equals(wrapper.name)) {
-                            JOptionPane.showMessageDialog(
-                                    MainApplication.getMainFrame(),
-                                    tr("File {0} is loaded yet under the name \"{1}\"", sel.getName(), wrapper.name),
-                                    tr("Error"),
-                                    JOptionPane.ERROR_MESSAGE
-                            );
-                        }
-                        return;
-                    }
-                }
+                removeDuplicates(sel);
                 GpxData data = null;
                 try (InputStream iStream = Compression.getUncompressedFileInputStream(sel)) {
@@ -321 +325 @@
                 }
 
-                loadedGpxData.add(data);
-                if (gpxModel.getElementAt(0).file == null) {
-                    gpxModel.removeElementAt(0);
-                }
                 GpxDataWrapper elem = new GpxDataWrapper(sel.getName(), data, sel);
                 gpxModel.addElement(elem);
                 gpxModel.setSelectedItem(elem);
+                statusBarUpdater.matchAndUpdateStatusBar();
             } finally {
                 outerPanel.setCursor(Cursor.getDefaultCursor());
@@ -526 +527 @@
                 forceTags = cForce.isSelected(); // This setting is not supposed to be saved permanently
 
-                statusBarUpdater.updateStatusBar();
+                statusBarUpdater.matchAndUpdateStatusBar();
                 yLayer.updateBufferAndRepaint();
             }
@@ -767 +768 @@
 
             }
-            statusBarUpdater.updateStatusBar();
+            statusBarUpdater.matchAndUpdateStatusBar();
             yLayer.updateBufferAndRepaint();
         }
@@ -790 +791 @@
     }
 
-    private class GpxLayerAddedListener implements LayerChangeListener {
+    private static class GpxLayerAddedListener implements LayerChangeListener {
         @Override
         public void layerAdded(LayerAddEvent e) {
-            if (syncDialog != null && syncDialog.isVisible()) {
-                Layer layer = e.getAddedLayer();
-                if (layer instanceof GpxLayer) {
-                    GpxLayer gpx = (GpxLayer) layer;
-                    GpxDataWrapper gdw = new GpxDataWrapper(gpx.getName(), gpx.data, gpx.data.storageFile);
-                    if (gpxModel.getElementAt(0).file == null) {
-                        gpxModel.removeElementAt(0);
-                    }
-                    gpxModel.addElement(gdw);
-                }
+            Layer layer = e.getAddedLayer();
+            if (layer instanceof GpxLayer) {
+                GpxLayer gpx = (GpxLayer) layer;
+                File file = gpx.data.storageFile;
+                removeDuplicates(file);
+                GpxDataWrapper gdw = new GpxDataWrapper(gpx.getName(), gpx.data, file);
+                gpx.addPropertyChangeListener(new GpxLayerRenamedListener(gdw));
+                gpxModel.addElement(gdw);
+                forEachLayer(CorrelateGpxWithImages::repaintCombobox);
             }
         }
@@ -817 +817 @@
     }
 
+    private static class GpxLayerRenamedListener implements PropertyChangeListener {
+        private GpxDataWrapper gdw;
+        GpxLayerRenamedListener(GpxDataWrapper gdw) {
+            this.gdw = gdw;
+        }
+
+        @Override
+        public void propertyChange(PropertyChangeEvent e) {
+            if (Layer.NAME_PROP.equals(e.getPropertyName())) {
+                gdw.setName(e.getNewValue().toString());
+            }
+        }
+    }
+
     @Override
     public void actionPerformed(ActionEvent ae) {
-        // Construct the list of loaded GPX tracks
-        gpxModel = new DefaultComboBoxModel<>();
-        GpxDataWrapper defaultItem = null;
-        for (GpxLayer cur : MainApplication.getLayerManager().getLayersOfType(GpxLayer.class).stream()
-                .filter(GpxLayer::isLocalFile).collect(Collectors.toList())) {
-            GpxDataWrapper gdw = new GpxDataWrapper(cur.getName(), cur.data, cur.data.storageFile);
-            gpxModel.addElement(gdw);
-            if (cur == yLayer.gpxLayer || (defaultItem == null && gdw.file != null)) {
-                defaultItem = gdw;
-            }
-        }
-        for (GpxData data : loadedGpxData) {
-            GpxDataWrapper gdw = new GpxDataWrapper(data.storageFile.getName(), data, data.storageFile);
-            gpxModel.addElement(gdw);
-            if (defaultItem == null && gdw.file != null) { // select first GPX track associated to a file
-                defaultItem = gdw;
-            }
-        }
-
-        GpxDataWrapper nogdw = new GpxDataWrapper(tr("<No GPX track loaded yet>"), null, null);
-        if (gpxModel.getSize() == 0) {
-            gpxModel.addElement(nogdw);
-        } else if (defaultItem != null) {
-            gpxModel.setSelectedItem(defaultItem);
+        NoGpxDataWrapper nogdw = new NoGpxDataWrapper();
+        if (gpxModel == null) {
+            // Construct the list of loaded GPX tracks
+            gpxModel = new DefaultComboBoxModel<>();
+            GpxDataWrapper defaultItem = null;
+            for (GpxLayer cur : MainApplication.getLayerManager().getLayersOfType(GpxLayer.class)) {
+                GpxDataWrapper gdw = new GpxDataWrapper(cur.getName(), cur.data, cur.data.storageFile);
+                cur.addPropertyChangeListener(new GpxLayerRenamedListener(gdw));
+                gpxModel.addElement(gdw);
+                if (cur == yLayer.gpxLayer || defaultItem == null) {
+                    defaultItem = gdw;
+                }
+            }
+
+            if (gpxModel.getSize() == 0) {
+                gpxModel.addElement(nogdw);
+            } else if (defaultItem != null) {
+                gpxModel.setSelectedItem(defaultItem);
+            }
+            MainApplication.getLayerManager().addLayerChangeListener(new GpxLayerAddedListener());
         }
 
@@ -1010 +1020 @@
         cbTaggedImg.addItemListener(statusBarUpdaterWithRepaint);
 
-        statusBarUpdater.updateStatusBar();
+        statusBarUpdater.matchAndUpdateStatusBar();
         yLayer.updateBufferAndRepaint();
 
@@ -1017 +1027 @@
 
         if (!GraphicsEnvironment.isHeadless()) {
+            forEachLayer(CorrelateGpxWithImages::closeDialog);
             syncDialog = new ExtendedDialog(
                     MainApplication.getMainFrame(),
@@ -1034 +1045 @@
     }
 
+    private static void removeDuplicates(File file) {
+        for (int i = gpxModel.getSize() - 1; i >= 0; i--) {
+            GpxDataWrapper wrapper = gpxModel.getElementAt(i);
+            if (wrapper instanceof NoGpxDataWrapper || (file != null && file.equals(wrapper.file))) {
+                gpxModel.removeElement(wrapper);
+            }
+        }
+    }
+
+    private static void forEachLayer(Consumer<CorrelateGpxWithImages> action) {
+        MainApplication.getLayerManager().getLayersOfType(GeoImageLayer.class)
+                .forEach(geo -> action.accept(geo.getGpxCorrelateAction()));
+    }
+
     private final transient StatusBarUpdater statusBarUpdater = new StatusBarUpdater(false);
     private final transient StatusBarUpdater statusBarUpdaterWithRepaint = new StatusBarUpdater(true);
@@ -1046 +1071 @@
         @Override
         public void insertUpdate(DocumentEvent ev) {
-            updateStatusBar();
+            matchAndUpdateStatusBar();
         }
 
         @Override
         public void removeUpdate(DocumentEvent ev) {
-            updateStatusBar();
+            matchAndUpdateStatusBar();
         }
 
@@ -1061 +1086 @@
         @Override
         public void itemStateChanged(ItemEvent e) {
-            updateStatusBar();
+            matchAndUpdateStatusBar();
         }
 
         @Override
         public void actionPerformed(ActionEvent e) {
-            updateStatusBar();
-        }
-
-        public void updateStatusBar() {
-            statusBarText.setText(statusText());
-            if (doRepaint) {
-                yLayer.updateBufferAndRepaint();
-            }
-        }
-
-        private String statusText() {
+            matchAndUpdateStatusBar();
+        }
+
+        public void matchAndUpdateStatusBar() {
+            if (syncDialog != null && syncDialog.isVisible()) {
+                statusBarText.setText(matchAndGetStatusText());
+                if (doRepaint) {
+                    yLayer.updateBufferAndRepaint();
+                }
+            }
+        }
+
+        private String matchAndGetStatusText() {
             try {
                 timezone = GpxTimezone.parseTimezone(tfTimezone.getText().trim());
@@ -1199 +1226 @@
                             "Time difference of {0} days", Math.abs(dayOffset), Math.abs(dayOffset)));
 
-                    statusBarUpdater.updateStatusBar();
+                    statusBarUpdater.matchAndUpdateStatusBar();
                     yLayer.updateBufferAndRepaint();
                 }
@@ -1251 +1278 @@
 
     static class NoGpxTimestamps extends Exception {
+    }
+
+    void closeDialog() {
+        if (syncDialog != null) {
+            syncDialog.setVisible(false);
+            new SyncDialogWindowListener().windowDeactivated(null);
+            syncDialog.dispose();
+            syncDialog = null;
+        }
+    }
+
+    void repaintCombobox() {
+        if (cbGpx != null) {
+            cbGpx.repaint();
+        }
     }
 
@@ -1318 +1360 @@
             tfOffset.getDocument().addDocumentListener(statusBarUpdater);
 
-            statusBarUpdater.updateStatusBar();
+            statusBarUpdater.matchAndUpdateStatusBar();
             yLayer.updateBufferAndRepaint();
         }
@@ -1346 +1388 @@
         Object item = gpxModel.getSelectedItem();
 
-        if (item == null || ((GpxDataWrapper) item).file == null) {
+        if (item == null || ((GpxDataWrapper) item).data == null) {
             if (complain) {
                 JOptionPane.showMessageDialog(MainApplication.getMainFrame(), tr("You should select a GPX track"),
@@ -1356 +1398 @@
     }
 
+    @Override
+    public void destroy() {
+        if (cbGpx != null) {
+            // Force the JCombobox to remove its eventListener from the static GpxDataWrapper
+            cbGpx.setModel(new DefaultComboBoxModel<GpxDataWrapper>());
+            cbGpx = null;
+        }
+        closeDialog();
+    }
+
 }

trunk/src/org/openstreetmap/josm/gui/layer/geoimage/GeoImageLayer.java

@@ -82 +82 @@
     GpxLayer gpxFauxLayer;
 
+    private CorrelateGpxWithImages gpxCorrelateAction;
+
     private final Icon icon = ImageProvider.get("dialogs/geoimage/photo-marker");
     private final Icon selectedIcon = ImageProvider.get("dialogs/geoimage/photo-marker-selected");
@@ -406 +408 @@
         entries.add(new RenameLayerAction(null, this));
         entries.add(SeparatorLayerAction.INSTANCE);
-        entries.add(new CorrelateGpxWithImages(this));
+        entries.add(getGpxCorrelateAction());
         entries.add(new ShowThumbnailAction(this));
         if (!menuAdditions.isEmpty()) {
@@ -852 +854 @@
         super.destroy();
         stopLoadThumbs();
+        if (gpxCorrelateAction != null) {
+            gpxCorrelateAction.destroy();
+            gpxCorrelateAction = null;
+        }
         MapView mapView = MainApplication.getMap().mapView;
         mapView.removeMouseListener(mouseAdapter);
@@ -942 +948 @@
     public GpxLayer getGpxLayer() {
         return gpxLayer;
+    }
+
+    /**
+     * Returns the gpxCorrelateAction
+     * @return the gpxCorrelateAction
+     */
+    public CorrelateGpxWithImages getGpxCorrelateAction() {
+        if (gpxCorrelateAction == null) {
+            gpxCorrelateAction = new CorrelateGpxWithImages(this);
+        }
+        return gpxCorrelateAction;
     }