### Eclipse Workspace Patch 1.0 #P josm Index: test/unit/org/openstreetmap/josm/data/gpx/GpxImageCorrelationTest.java =================================================================== --- test/unit/org/openstreetmap/josm/data/gpx/GpxImageCorrelationTest.java (revision 17884) +++ test/unit/org/openstreetmap/josm/data/gpx/GpxImageCorrelationTest.java (working copy) @@ -12,7 +12,6 @@ import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.MethodOrderer; import org.junit.jupiter.api.MethodOrderer.MethodName; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInstance; Index: src/org/openstreetmap/josm/gui/layer/geoimage/GeoImageLayer.java =================================================================== --- src/org/openstreetmap/josm/gui/layer/geoimage/GeoImageLayer.java (revision 17884) +++ src/org/openstreetmap/josm/gui/layer/geoimage/GeoImageLayer.java (working copy) @@ -81,6 +81,8 @@ GpxLayer gpxLayer; 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"); @@ -405,7 +407,7 @@ entries.add(LayerListDialog.getInstance().createMergeLayerAction(this)); 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()) { entries.add(SeparatorLayerAction.INSTANCE); @@ -851,6 +853,10 @@ public synchronized void destroy() { super.destroy(); stopLoadThumbs(); + if (gpxCorrelateAction != null) { + gpxCorrelateAction.destroy(); + gpxCorrelateAction = null; + } MapView mapView = MainApplication.getMap().mapView; mapView.removeMouseListener(mouseAdapter); mapView.removeMouseMotionListener(mouseMotionAdapter); @@ -944,6 +950,17 @@ } /** + * Returns the gpxCorrelateAction + * @return the gpxCorrelateAction + */ + public CorrelateGpxWithImages getGpxCorrelateAction() { + if (gpxCorrelateAction == null) { + gpxCorrelateAction = new CorrelateGpxWithImages(this); + } + return gpxCorrelateAction; + } + + /** * Returns a faux GPX layer built from the images or the associated GPX layer. * @return A faux GPX layer or the associated GPX layer * @since 14802 Index: src/org/openstreetmap/josm/data/gpx/GpxImageCorrelation.java =================================================================== --- src/org/openstreetmap/josm/data/gpx/GpxImageCorrelation.java (revision 17884) +++ src/org/openstreetmap/josm/data/gpx/GpxImageCorrelation.java (working copy) @@ -168,7 +168,7 @@ } } } - if (trkTag) { + if (trkTag && prevWp != null) { ret += matchPoints(images, prevWp, prevWpTime, prevWp, prevWpTime, offset, false, trkTagTime, true); } return ret; Index: src/org/openstreetmap/josm/gui/layer/geoimage/CorrelateGpxWithImages.java =================================================================== --- src/org/openstreetmap/josm/gui/layer/geoimage/CorrelateGpxWithImages.java (revision 17884) +++ src/org/openstreetmap/josm/gui/layer/geoimage/CorrelateGpxWithImages.java (working copy) @@ -21,6 +21,8 @@ import java.awt.event.ItemListener; 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; import java.io.InputStream; @@ -41,6 +43,7 @@ import java.util.Objects; import java.util.TimeZone; import java.util.concurrent.TimeUnit; +import java.util.function.Consumer; import java.util.stream.Collectors; import javax.swing.AbstractAction; @@ -100,6 +103,7 @@ import org.openstreetmap.josm.io.nmea.NmeaReader; 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; import org.openstreetmap.josm.tools.JosmRuntimeException; @@ -113,14 +117,14 @@ * Then it correlates the images of the layer with that GPX file. * @since 2566 */ -public class CorrelateGpxWithImages extends AbstractAction { +public class CorrelateGpxWithImages extends AbstractAction implements Destroyable { - private static final List loadedGpxData = new ArrayList<>(); + private static MutableComboBoxModel gpxModel; + private static boolean forceTags; private final transient GeoImageLayer yLayer; private transient GpxTimezone timezone; private transient GpxTimeOffset delta; - private static boolean forceTags; /** * Constructs a new {@code CorrelateGpxWithImages} action. @@ -130,7 +134,6 @@ super(tr("Correlate to GPX")); new ImageProvider("dialogs/geoimage/gpx2img").getResource().attachImageIcon(this, true); this.yLayer = layer; - MainApplication.getLayerManager().addLayerChangeListener(new GpxLayerAddedListener()); } private final class SyncDialogWindowListener extends WindowAdapter { @@ -233,7 +236,7 @@ } private static class GpxDataWrapper { - private final String name; + private String name; private final GpxData data; private final File file; @@ -243,6 +246,11 @@ this.file = file; } + void setName(String name) { + this.name = name; + forEachLayer(CorrelateGpxWithImages::repaintCombobox); + } + @Override public String toString() { return name; @@ -249,8 +257,18 @@ } } + private static class NoGpxDataWrapper extends GpxDataWrapper { + NoGpxDataWrapper() { + super(null, null, null); + } + + @Override + public String toString() { + return tr(""); + } + } + private ExtendedDialog syncDialog; - private MutableComboBoxModel gpxModel; private JPanel outerPanel; private JosmComboBox cbGpx; private JosmTextField tfTimezone; @@ -280,21 +298,7 @@ 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)) { IGpxReader reader = gpxFilter.accept(sel) ? new GpxReader(iStream) : new NmeaReader(iStream); @@ -322,13 +326,10 @@ return; } - 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()); } @@ -527,7 +528,7 @@ forceTags = cForce.isSelected(); // This setting is not supposed to be saved permanently - statusBarUpdater.updateStatusBar(); + statusBarUpdater.matchAndUpdateStatusBar(); yLayer.updateBufferAndRepaint(); } } @@ -764,7 +765,7 @@ isOk = true; } - statusBarUpdater.updateStatusBar(); + statusBarUpdater.matchAndUpdateStatusBar(); yLayer.updateBufferAndRepaint(); } @@ -787,19 +788,18 @@ } } - 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); } } @@ -814,34 +814,44 @@ } } + 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; + 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; + } } - } - 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; + + if (gpxModel.getSize() == 0) { + gpxModel.addElement(nogdw); + } else if (defaultItem != null) { + gpxModel.setSelectedItem(defaultItem); } + MainApplication.getLayerManager().addLayerChangeListener(new GpxLayerAddedListener()); } - GpxDataWrapper nogdw = new GpxDataWrapper(tr(""), null, null); - if (gpxModel.getSize() == 0) { - gpxModel.addElement(nogdw); - } else if (defaultItem != null) { - gpxModel.setSelectedItem(defaultItem); - } - JPanel panelCb = new JPanel(); panelCb.add(new JLabel(tr("GPX track: "))); @@ -1007,7 +1017,7 @@ cbExifImg.addItemListener(statusBarUpdaterWithRepaint); cbTaggedImg.addItemListener(statusBarUpdaterWithRepaint); - statusBarUpdater.updateStatusBar(); + statusBarUpdater.matchAndUpdateStatusBar(); yLayer.updateBufferAndRepaint(); outerPanel = new JPanel(new BorderLayout()); @@ -1014,6 +1024,7 @@ outerPanel.add(statusBar, BorderLayout.PAGE_END); if (!GraphicsEnvironment.isHeadless()) { + forEachLayer(CorrelateGpxWithImages::closeDialog); syncDialog = new ExtendedDialog( MainApplication.getMainFrame(), tr("Correlate images with GPX track"), @@ -1031,6 +1042,20 @@ } } + 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 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); @@ -1043,12 +1068,12 @@ @Override public void insertUpdate(DocumentEvent ev) { - updateStatusBar(); + matchAndUpdateStatusBar(); } @Override public void removeUpdate(DocumentEvent ev) { - updateStatusBar(); + matchAndUpdateStatusBar(); } @Override @@ -1058,22 +1083,24 @@ @Override public void itemStateChanged(ItemEvent e) { - updateStatusBar(); + matchAndUpdateStatusBar(); } @Override public void actionPerformed(ActionEvent e) { - updateStatusBar(); + matchAndUpdateStatusBar(); } - public void updateStatusBar() { - statusBarText.setText(statusText()); - if (doRepaint) { - yLayer.updateBufferAndRepaint(); + public void matchAndUpdateStatusBar() { + if (syncDialog != null && syncDialog.isVisible()) { + statusBarText.setText(matchAndGetStatusText()); + if (doRepaint) { + yLayer.updateBufferAndRepaint(); + } } } - private String statusText() { + private String matchAndGetStatusText() { try { timezone = GpxTimezone.parseTimezone(tfTimezone.getText().trim()); delta = GpxTimeOffset.parseOffset(tfOffset.getText().trim()); @@ -1196,7 +1223,7 @@ lblMatches.setText(statusBarText.getText() + "
" + trn("(Time difference of {0} day)", "Time difference of {0} days", Math.abs(dayOffset), Math.abs(dayOffset))); - statusBarUpdater.updateStatusBar(); + statusBarUpdater.matchAndUpdateStatusBar(); yLayer.updateBufferAndRepaint(); } } @@ -1250,6 +1277,21 @@ 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(); + } + } + /** * Tries to auto-guess the timezone and offset. * @@ -1315,7 +1357,7 @@ tfTimezone.getDocument().addDocumentListener(statusBarUpdater); tfOffset.getDocument().addDocumentListener(statusBarUpdater); - statusBarUpdater.updateStatusBar(); + statusBarUpdater.matchAndUpdateStatusBar(); yLayer.updateBufferAndRepaint(); } } @@ -1343,7 +1385,7 @@ private GpxDataWrapper selectedGPX(boolean complain) { 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"), tr("No selected GPX track"), JOptionPane.ERROR_MESSAGE); @@ -1353,4 +1395,14 @@ return (GpxDataWrapper) item; } + @Override + public void destroy() { + if (cbGpx != null) { + // Force the JCombobox to remove its eventListener from the static GpxDataWrapper + cbGpx.setModel(new DefaultComboBoxModel()); + cbGpx = null; + } + closeDialog(); + } + }