Ticket #21605: 21605.5.patch

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

@@ -33 +33 @@
 
 import javax.swing.Action;
 import javax.swing.Icon;
+import javax.swing.ImageIcon;
 
 import org.openstreetmap.josm.actions.AutoScaleAction;
 import org.openstreetmap.josm.actions.ExpertToggleAction;
@@ -47 +48 @@
 import org.openstreetmap.josm.data.gpx.GpxData;
 import org.openstreetmap.josm.data.gpx.GpxImageEntry;
 import org.openstreetmap.josm.data.gpx.GpxTrack;
+import org.openstreetmap.josm.data.imagery.street_level.IImageEntry;
 import org.openstreetmap.josm.data.osm.visitor.BoundingXYVisitor;
+import org.openstreetmap.josm.data.preferences.NamedColorProperty;
 import org.openstreetmap.josm.gui.MainApplication;
 import org.openstreetmap.josm.gui.MapFrame;
 import org.openstreetmap.josm.gui.MapFrame.MapModeChangeListener;
@@ -62 +65 @@
 import org.openstreetmap.josm.gui.layer.JumpToMarkerActions.JumpToPreviousMarker;
 import org.openstreetmap.josm.gui.layer.Layer;
 import org.openstreetmap.josm.gui.layer.MainLayerManager.ActiveLayerChangeListener;
+import org.openstreetmap.josm.gui.util.GuiHelper;
 import org.openstreetmap.josm.gui.util.imagery.Vector3D;
 import org.openstreetmap.josm.tools.ImageProvider;
+import org.openstreetmap.josm.tools.ListenerList;
 import org.openstreetmap.josm.tools.Utils;
 
 /**
@@ -71 +76 @@
  * @since 99
  */
 public class GeoImageLayer extends AbstractModifiableLayer implements
-        JumpToMarkerLayer, NavigatableComponent.ZoomChangeListener, ImageDataUpdateListener {
+        JumpToMarkerLayer, NavigatableComponent.ZoomChangeListener, ImageDataUpdateListener,
+        IGeoImageLayer {
 
     private static final List<Action> menuAdditions = new LinkedList<>();
 
     private static volatile List<MapMode> supportedMapModes;
 
     private final ImageData data;
+    private final ListenerList<IGeoImageLayer.ImageChangeListener> imageChangeListeners = ListenerList.create();
     GpxData gpxData;
     GpxLayer gpxFauxLayer;
     GpxData gpxFauxData;
@@ -86 +93 @@
 
     private final Icon icon = ImageProvider.get("dialogs/geoimage/photo-marker");
     private final Icon selectedIcon = ImageProvider.get("dialogs/geoimage/photo-marker-selected");
+    private final Icon selectedIconNotImageViewer = generateSelectedIconNotImageViewer(this.selectedIcon);
+
+    private static Icon generateSelectedIconNotImageViewer(Icon selectedIcon) {
+        Color color = new NamedColorProperty("geoimage.selected.not.image.viewer", new Color(255, 170, 255)).get();
+        BufferedImage bi = new BufferedImage(selectedIcon.getIconWidth(), selectedIcon.getIconHeight(), BufferedImage.TYPE_INT_ARGB);
+        Graphics2D g2d = bi.createGraphics();
+        selectedIcon.paintIcon(null, g2d, 0, 0);
+        g2d.setComposite(AlphaComposite.getInstance(AlphaComposite.SRC_ATOP, 0.5f));
+        g2d.setColor(color);
+        g2d.fillRect(0, 0, selectedIcon.getIconWidth(), selectedIcon.getIconHeight());
+        g2d.dispose();
+        return new ImageIcon(bi);
+    }
 
     boolean useThumbs;
     private final ExecutorService thumbsLoaderExecutor =
@@ -172 +192 @@
         this.useThumbs = useThumbs;
         this.data.addImageDataUpdateListener(this);
         this.data.setLayer(this);
+        if (!ImageViewerDialog.hasInstance()) {
+            this.data.setSelectedImage(this.data.getFirstImage());
+            // This must be called *after* this layer is added to the layer manager.
+            // But it must also be run in the EDT. By adding this to the worker queue
+            // and then running the actual code in the EDT, we ensure that the layer
+            // will be added to the layer manager regardless of whether or not this
+            // was instantiated in the worker thread or the EDT thread.
+            MainApplication.worker.submit(() -> GuiHelper.runInEDT(() ->
+                    ImageViewerDialog.getInstance().displayImages(Collections.singletonList(this.data.getSelectedImage()))));
+        }
     }
 
     private final class ImageMouseListener extends MouseAdapter {
@@ -232 +262 @@
                     }
                 } else {
                     data.setSelectedImage(img);
-                    ImageViewerDialog.getInstance().displayImages(GeoImageLayer.this, Collections.singletonList(img));
+                    ImageViewerDialog.getInstance().displayImages(Collections.singletonList(img));
                 }
             }
         }
@@ -247 +277 @@
         MainApplication.worker.execute(new ImagesLoader(files, gpxLayer));
     }
 
+    @Override
+    public void clearSelection() {
+        this.getImageData().clearSelectedImage();
+    }
+
+    @Override
+    public boolean containsImage(IImageEntry<?> imageEntry) {
+        if (imageEntry instanceof ImageEntry) {
+            return this.data.getImages().contains(imageEntry);
+        }
+        return false;
+    }
+
     @Override
     public Icon getIcon() {
         return ImageProvider.get("dialogs/geoimage", ImageProvider.ImageSizes.LAYER);
     }
 
+    @Override
+    public List<ImageEntry> getSelection() {
+        return this.getImageData().getSelectedImages();
+    }
+
+    @Override
+    public List<IImageEntry<?>> getInvalidGeoImages() {
+        return this.getImageData().getImages().stream().filter(entry -> entry.getPos() == null || entry.getExifCoor() == null
+              || !entry.getExifCoor().isValid() || !entry.getPos().isValid()).collect(toList());
+    }
+
+    @Override
+    public void addImageChangeListener(ImageChangeListener listener) {
+        this.imageChangeListeners.addListener(listener);
+    }
+
+    @Override
+    public void removeImageChangeListener(ImageChangeListener listener) {
+        this.imageChangeListeners.removeListener(listener);
+    }
+
     /**
      * Register actions on the layer
      * @param addition the action to be added
@@ -451 +515 @@
             }
         }
 
+        final IImageEntry<?> currentImage = ImageViewerDialog.getCurrentImage();
         for (ImageEntry e: data.getSelectedImages()) {
             if (e != null && e.getPos() != null) {
                 Point p = mv.getPoint(e.getPos());
@@ -465 +530 @@
                 if (useThumbs && e.hasThumbnail()) {
                     g.setColor(new Color(128, 0, 0, 122));
                     g.fillRect(p.x - imgDim.width / 2, p.y - imgDim.height / 2, imgDim.width, imgDim.height);
+                } else if (e.equals(currentImage)) {
+                    selectedIcon.paintIcon(mv, g,
+                            p.x - imgDim.width / 2,
+                            p.y - imgDim.height / 2);
                 } else {
-                    selectedIcon.paintIcon(mv, g,
+                    selectedIconNotImageViewer.paintIcon(mv, g,
                             p.x - imgDim.width / 2,
                             p.y - imgDim.height / 2);
                 }
@@ -884 +953 @@
     @Override
     public void selectedImageChanged(ImageData data) {
         showCurrentPhoto();
+        this.imageChangeListeners.fireEvent(e -> e.imageChanged(this, null, data.getSelectedImages()));
     }
 
     @Override

new file src/org/openstreetmap/josm/gui/layer/geoimage/IGeoImageLayer.java

@@ +1 @@
+// License: GPL. For details, see LICENSE file.
+package org.openstreetmap.josm.gui.layer.geoimage;
+
+import java.util.Collections;
+import java.util.List;
+
+import org.openstreetmap.josm.data.imagery.street_level.IImageEntry;
+
+/**
+ * An interface for layers which want to show images
+ * @since xxx
+ */
+public interface IGeoImageLayer {
+    /**
+     * Clear the selection of the layer
+     */
+    void clearSelection();
+
+    /**
+     * Get the current selection
+     * @return The currently selected images
+     */
+    List<? extends IImageEntry<?>> getSelection();
+
+    /**
+     * Get the invalid geo images for this layer (specifically, those that <i>cannot</i> be displayed on the map)
+     * @return The list of invalid geo images
+     */
+    default List<IImageEntry<?>> getInvalidGeoImages() {
+        return Collections.emptyList();
+    }
+
+    /**
+     * Check if the layer contains the specified image
+     * @param imageEntry The entry to look for
+     * @return {@code true} if this layer contains the image
+     */
+    boolean containsImage(IImageEntry<?> imageEntry);
+
+    /**
+     * Add a listener for when images change
+     * @param listener The listener to call
+     */
+    void addImageChangeListener(ImageChangeListener listener);
+
+    /**
+     * Remove a listener for when images change
+     * @param listener The listener to remove
+     */
+    void removeImageChangeListener(ImageChangeListener listener);
+
+    /**
+     * Listen for image changes
+     */
+    interface ImageChangeListener {
+        /**
+         * Called when the selected image(s) change
+         * @param source The source of the change
+         * @param oldImages The previously selected image(s)
+         * @param newImages The newly selected image(s)
+         */
+        void imageChanged(IGeoImageLayer source, List<? extends IImageEntry<?>> oldImages, List<? extends IImageEntry<?>> newImages);
+    }
+}

src/org/openstreetmap/josm/gui/layer/geoimage/ImageEntry.java

@@ -147 +147 @@
         if (entry instanceof ImageEntry) {
             this.dataSet.setSelectedImage((ImageEntry) entry);
         }
-        imageViewerDialog.displayImages(this.dataSet.getLayer(), Collections.singletonList(entry));
+        imageViewerDialog.displayImages(Collections.singletonList(entry));
     }
 
     @Override

src/org/openstreetmap/josm/gui/layer/geoimage/ImageViewerDialog.java

@@ -11 +11 @@
 import java.awt.GridBagConstraints;
 import java.awt.GridBagLayout;
 import java.awt.event.ActionEvent;
-import java.awt.event.ActionListener;
 import java.awt.event.KeyEvent;
 import java.awt.event.WindowEvent;
 import java.io.IOException;
@@ -23 +22 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Comparator;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.concurrent.Future;
 import java.util.function.UnaryOperator;
 import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import java.util.stream.Stream;
 
 import javax.swing.AbstractAction;
+import javax.swing.AbstractButton;
 import javax.swing.Box;
 import javax.swing.JButton;
 import javax.swing.JLabel;
@@ -42 +42 @@
 import javax.swing.SwingConstants;
 import javax.swing.SwingUtilities;
 
+import org.openstreetmap.josm.actions.ExpertToggleAction;
 import org.openstreetmap.josm.actions.JosmAction;
 import org.openstreetmap.josm.data.ImageData;
-import org.openstreetmap.josm.data.ImageData.ImageDataUpdateListener;
 import org.openstreetmap.josm.data.imagery.street_level.IImageEntry;
 import org.openstreetmap.josm.gui.ExtendedDialog;
 import org.openstreetmap.josm.gui.MainApplication;
@@ -64 +64 @@
 import org.openstreetmap.josm.gui.layer.imagery.ImageryFilterSettings;
 import org.openstreetmap.josm.gui.util.GuiHelper;
 import org.openstreetmap.josm.gui.util.imagery.Vector3D;
+import org.openstreetmap.josm.gui.widgets.HideableTabbedPane;
+import org.openstreetmap.josm.spi.preferences.Config;
 import org.openstreetmap.josm.tools.ImageProvider;
 import org.openstreetmap.josm.tools.Logging;
 import org.openstreetmap.josm.tools.PlatformManager;
@@ -73 +75 @@
 /**
  * Dialog to view and manipulate geo-tagged images from a {@link GeoImageLayer}.
  */
-public final class ImageViewerDialog extends ToggleDialog implements LayerChangeListener, ActiveLayerChangeListener, ImageDataUpdateListener {
+public final class ImageViewerDialog extends ToggleDialog implements LayerChangeListener, ActiveLayerChangeListener {
     private static final String GEOIMAGE_FILLER = marktr("Geoimage: {0}");
     private static final String DIALOG_FOLDER = "dialogs";
 
@@ -124 +126 @@
         return dialog;
     }
 
+    /**
+     * Check if there is an instance for the {@link ImageViewerDialog}
+     * @return {@code true} if there is a static singleton instance of {@link ImageViewerDialog}
+     * @since xxx
+     */
+    public static boolean hasInstance() {
+        return dialog != null;
+    }
+
+    /**
+     * Destroy the current dialog
+     */
+    private static void destroyInstance() {
+        dialog = null;
+    }
+
     private JButton btnLast;
     private JButton btnNext;
     private JButton btnPrevious;
@@ -135 +153 @@
     private JButton btnDeleteFromDisk;
     private JToggleButton tbCentre;
     /** The layer tab (used to select images when multiple layers provide images, makes for easy switching) */
-    private JPanel layers;
+    private final HideableTabbedPane layers = new HideableTabbedPane();
 
     private ImageViewerDialog() {
         super(tr("Geotagged Images"), "geoimage", tr("Display geotagged images"), Shortcut.registerShortcut("tools:geotagged",
@@ -168 +186 @@
 
     private void build() {
         JPanel content = new JPanel(new BorderLayout());
-        this.layers = new JPanel(new GridBagLayout());
-        content.add(layers, BorderLayout.NORTH);
-
-        content.add(imgDisplay, BorderLayout.CENTER);
+        content.add(this.layers, BorderLayout.CENTER);
 
         Dimension buttonDim = new Dimension(26, 26);
 
@@ -187 +202 @@
         btnLast = createNavigationButton(imageLastAction, buttonDim);
 
         tbCentre = new JToggleButton(imageCenterViewAction);
+        tbCentre.setSelected(Config.getPref().getBoolean("geoimage.viewer.centre.on.image", false));
         tbCentre.setPreferredSize(buttonDim);
 
         JButton btnZoomBestFit = new JButton(imageZoomAction);
@@ -196 +212 @@
         btnCollapse.setAlignmentY(Component.TOP_ALIGNMENT);
 
         JPanel buttons = new JPanel();
-        buttons.add(btnFirst);
-        buttons.add(btnPrevious);
-        buttons.add(btnNext);
-        buttons.add(btnLast);
-        buttons.add(Box.createRigidArea(new Dimension(7, 0)));
-        buttons.add(tbCentre);
-        buttons.add(btnZoomBestFit);
-        buttons.add(Box.createRigidArea(new Dimension(7, 0)));
-        buttons.add(btnDelete);
-        buttons.add(btnDeleteFromDisk);
-        buttons.add(Box.createRigidArea(new Dimension(7, 0)));
-        buttons.add(btnCopyPath);
-        buttons.add(btnOpenExternal);
-        buttons.add(Box.createRigidArea(new Dimension(7, 0)));
-        buttons.add(createButton(visibilityAction, buttonDim));
+        addButtonGroup(buttons, this.btnFirst, this.btnPrevious, this.btnNext, this.btnLast);
+        addButtonGroup(buttons, this.tbCentre, btnZoomBestFit);
+        addButtonGroup(buttons, this.btnDelete, this.btnDeleteFromDisk);
+        addButtonGroup(buttons, this.btnCopyPath, this.btnOpenExternal);
+        addButtonGroup(buttons, createButton(visibilityAction, buttonDim));
 
         JPanel bottomPane = new JPanel(new GridBagLayout());
         GridBagConstraints gc = new GridBagConstraints();
@@ -231 +237 @@
         createLayout(content, false, null);
     }
 
-    private void updateLayers() {
-        if (this.tabbedEntries.size() <= 1) {
+    /**
+     * Add a button group to a panel
+     * @param buttonPanel The panel holding the buttons
+     * @param buttons The button group to add
+     */
+    private static void addButtonGroup(JPanel buttonPanel, AbstractButton... buttons) {
+        if (buttonPanel.getComponentCount() != 0) {
+            buttonPanel.add(Box.createRigidArea(new Dimension(7, 0)));
+        }
+
+        for (AbstractButton jButton : buttons) {
+            buttonPanel.add(jButton);
+        }
+    }
+
+    /**
+     * Update the tabs for the different image layers
+     * @param changed {@code true} if the tabs changed
+     */
+    private void updateLayers(boolean changed) {
+        MainLayerManager layerManager = MainApplication.getLayerManager();
+        List<IGeoImageLayer> geoImageLayers = layerManager.getLayers().stream()
+                .filter(IGeoImageLayer.class::isInstance).map(IGeoImageLayer.class::cast).collect(Collectors.toList());
+        if (geoImageLayers.isEmpty()) {
             this.layers.setVisible(false);
-            this.layers.removeAll();
         } else {
             this.layers.setVisible(true);
-            // Remove all old components
-            this.layers.removeAll();
-            MainLayerManager layerManager = MainApplication.getLayerManager();
-            List<Layer> invalidLayers = this.tabbedEntries.keySet().stream().filter(layer -> !layerManager.containsLayer(layer))
-                    .collect(Collectors.toList());
-            // `null` is for anything using the old methods, without telling us what layer it comes from.
-            invalidLayers.remove(null);
-            // We need to do multiple calls to avoid ConcurrentModificationExceptions
-            invalidLayers.forEach(this.tabbedEntries::remove);
-            addButtonsForImageLayers();
+            if (changed) {
+                addButtonsForImageLayers();
+            }
+            MoveImgDisplayPanel<?> selected = (MoveImgDisplayPanel<?>) this.layers.getSelectedComponent();
+            if ((this.imgDisplay.getParent() == null || this.imgDisplay.getParent().getParent() == null)
+                && selected != null && selected.layer.containsImage(this.currentEntry)) {
+                selected.setVisible(selected.isVisible());
+            } else if (selected != null && !selected.layer.containsImage(this.currentEntry)) {
+                this.getImageTabs().filter(m -> m.layer.containsImage(this.currentEntry)).mapToInt(this.layers::indexOfComponent).findFirst()
+                        .ifPresent(this.layers::setSelectedIndex);
+            }
             this.layers.invalidate();
         }
+        this.layers.getParent().invalidate();
         this.revalidate();
     }
 
@@ -256 +285 @@
      * Add the buttons for image layers
      */
     private void addButtonsForImageLayers() {
-        final IImageEntry<?> current;
-        synchronized (this) {
-            current = this.currentEntry;
-        }
-        List<JButton> layerButtons = new ArrayList<>(this.tabbedEntries.size());
-        if (this.tabbedEntries.containsKey(null)) {
-            List<IImageEntry<?>> nullEntries = this.tabbedEntries.get(null);
-            JButton layerButton = createImageLayerButton(null, nullEntries);
-            layerButtons.add(layerButton);
-            layerButton.setEnabled(!nullEntries.contains(current));
+        List<MoveImgDisplayPanel<?>> alreadyAdded = this.getImageTabs().collect(Collectors.toList());
+        List<Layer> availableLayers = MainApplication.getLayerManager().getLayers();
+        for (IGeoImageLayer layer : availableLayers.stream()
+                .sorted(Comparator.comparingInt(entry -> /*reverse*/-availableLayers.indexOf(entry)))
+                .filter(IGeoImageLayer.class::isInstance).map(IGeoImageLayer.class::cast).collect(Collectors.toList())) {
+            final int index = availableLayers.size() - availableLayers.indexOf((Layer) layer);
+            final String label = (ExpertToggleAction.isExpert() ? "[" + index + "] " : "") + ((Layer) layer).getLabel();
+            final Optional<MoveImgDisplayPanel<?>> originalPanel = alreadyAdded.stream()
+                    .filter(m -> Objects.equals(m.layer, layer)).findFirst();
+            if (originalPanel.isPresent()) {
+                int componentIndex = this.layers.indexOfComponent(originalPanel.get());
+                this.layers.setTitleAt(componentIndex, label);
+            } else {
+                this.layers.addTab(label, new MoveImgDisplayPanel<>(this.imgDisplay, (Layer & IGeoImageLayer) layer));
+            }
         }
-        for (Map.Entry<Layer, List<IImageEntry<?>>> entry :
-                this.tabbedEntries.entrySet().stream().filter(entry -> entry.getKey() != null)
-                        .sorted(Comparator.comparing(entry -> entry.getKey().getName())).collect(Collectors.toList())) {
-            JButton layerButton = createImageLayerButton(entry.getKey(), entry.getValue());
-            layerButtons.add(layerButton);
-            layerButton.setEnabled(!entry.getValue().contains(current));
+        this.getImageTabs().map(p -> p.layer).filter(layer -> !availableLayers.contains(layer))
+                // We have to collect to a list prior to removal -- if we don't, then the stream may get a layer at index 0,
+                // remove that layer, and then get a layer at index 1, which was previously at index 2.
+                .collect(Collectors.toList()).forEach(this::removeImageTab);
+
+        // This is need to avoid the first button becoming visible, and then recalling this method.
+        this.getImageTabs().forEach(m -> m.finishedAddingButtons = true);
+        // After that, trigger the visibility set code
+        this.getImageTabs().forEach(m -> m.setVisible(m.isVisible()));
+    }
+
+    /**
+     * Remove a tab for a layer from the {@link #layers} tab pane
+     * @param layer The layer to remove
+     */
+    private void removeImageTab(Layer layer) {
+        // This must be reversed to avoid removing the wrong tab
+        for (int i = this.layers.getTabCount() - 1; i >= 0; i--) {
+            Component component = this.layers.getComponentAt(i);
+            if (component instanceof MoveImgDisplayPanel) {
+                MoveImgDisplayPanel<?> moveImgDisplayPanel = (MoveImgDisplayPanel<?>) component;
+                if (Objects.equals(layer, moveImgDisplayPanel.layer)) {
+                    this.layers.removeTabAt(i);
+                    this.layers.remove(moveImgDisplayPanel);
+                }
+            }
         }
-        layerButtons.forEach(this.layers::add);
     }
 
     /**
-     * Create a button for a specific layer and its entries
-     *
-     * @param layer     The layer to switch to
-     * @param entries   The entries to display
-     * @return The button to use to switch to the specified layer
+     * Get the {@link MoveImgDisplayPanel} objects in {@link #layers}.
+     * @return The individual panels
      */
-    private static JButton createImageLayerButton(Layer layer, List<IImageEntry<?>> entries) {
-        final JButton layerButton = new JButton();
-        layerButton.addActionListener(new ImageActionListener(layer, entries));
-        layerButton.setText(layer != null ? layer.getLabel() : tr("Default"));
-        return layerButton;
+    private Stream<MoveImgDisplayPanel<?>> getImageTabs() {
+        return IntStream.range(0, this.layers.getTabCount())
+                .mapToObj(this.layers::getComponentAt)
+                .filter(MoveImgDisplayPanel.class::isInstance)
+                .map(m -> (MoveImgDisplayPanel<?>) m);
     }
 
     @Override
@@ -309 +359 @@
         imageZoomAction.destroy();
         cancelLoadingImage();
         super.destroy();
-        dialog = null;
+        destroyInstance();
     }
 
     /**
@@ -433 +483 @@
         }
     }
 
-    /**
-     * A listener that is called to change the viewing layer
-     */
-    private static class ImageActionListener implements ActionListener {
-
-        private final Layer layer;
-        private final List<IImageEntry<?>> entries;
-
-        ImageActionListener(Layer layer, List<IImageEntry<?>> entries) {
-            this.layer = layer;
-            this.entries = entries;
-        }
-
-        @Override
-        public void actionPerformed(ActionEvent e) {
-            ImageViewerDialog.getInstance().displayImages(this.layer, this.entries);
-        }
-    }
-
     private class ImageFirstAction extends ImageRememberAction {
         ImageFirstAction() {
             super(null, new ImageProvider(DIALOG_FOLDER, "first"), tr("First"), Shortcut.registerShortcut(
@@ -478 +509 @@
         public void actionPerformed(ActionEvent e) {
             final JToggleButton button = (JToggleButton) e.getSource();
             centerView = button.isEnabled() && button.isSelected();
+            Config.getPref().putBoolean("geoimage.viewer.centre.on.image", centerView);
             if (centerView && currentEntry != null && currentEntry.getPos() != null) {
                 MainApplication.getMap().mapView.zoomTo(currentEntry.getPos());
             }
@@ -618 +650 @@
         }
     }
 
+    /**
+     * A JPanel whose entire purpose is to display an image by (a) moving the imgDisplay arround and (b) setting the imgDisplay as a child
+     * for this panel.
+     */
+    private static class MoveImgDisplayPanel<T extends Layer & IGeoImageLayer> extends JPanel {
+        private final T layer;
+        private final ImageDisplay imgDisplay;
+
+        /**
+         * The purpose of this field is to avoid having the same tab added to the dialog multiple times. This is only a problem when the dialog
+         * has multiple tabs on initialization (like from a session).
+         */
+        boolean finishedAddingButtons;
+        MoveImgDisplayPanel(ImageDisplay imgDisplay, T layer) {
+            super(new BorderLayout());
+            this.layer = layer;
+            this.imgDisplay = imgDisplay;
+        }
+
+        @Override
+        public void setVisible(boolean visible) {
+            super.setVisible(visible);
+            if (visible && this.finishedAddingButtons) {
+                if (!this.layer.getSelection().isEmpty() && !this.layer.getSelection().contains(ImageViewerDialog.getCurrentImage())) {
+                    ImageViewerDialog.getInstance().displayImages(this.layer.getSelection());
+                    this.layer.invalidate();
+                }
+                if (this.imgDisplay.getParent() != this) {
+                    this.add(this.imgDisplay, BorderLayout.CENTER);
+                    this.imgDisplay.invalidate();
+                    this.revalidate();
+                }
+            }
+        }
+    }
+
     /**
      * Enables (or disables) the "Previous" button.
      * @param value {@code true} to enable the button, {@code false} otherwise
@@ -651 +719 @@
         return wasEnabled;
     }
 
-    /** Used for tabbed panes */
-    private final transient Map<Layer, List<IImageEntry<?>>> tabbedEntries = new HashMap<>();
     private transient IImageEntry<? extends IImageEntry<?>> currentEntry;
 
     /**
@@ -679 +745 @@
      * @param entries image entries
      * @since 18246
      */
-    public void displayImages(List<IImageEntry<?>> entries) {
-        this.displayImages((Layer) null, entries);
-    }
-
-    /**
-     * Displays images for the given layer.
-     * @param layer The layer to use for the tab ui
-     * @param entries image entries
-     * @since 18591
-     */
-    public void displayImages(Layer layer, List<IImageEntry<?>> entries) {
+    public void displayImages(List<? extends IImageEntry<?>> entries) {
         boolean imageChanged;
         IImageEntry<?> entry = entries != null && entries.size() == 1 ? entries.get(0) : null;
 
@@ -710 +766 @@
             }
         }
 
-        if (entries == null || entries.isEmpty() || entries.stream().allMatch(Objects::isNull)) {
-            this.tabbedEntries.remove(layer);
+
+        final boolean updateRequired;
+        final List<IGeoImageLayer> imageLayers = MainApplication.getLayerManager().getLayers().stream()
+                    .filter(IGeoImageLayer.class::isInstance).map(IGeoImageLayer.class::cast).collect(Collectors.toList());
+        if (!Config.getPref().getBoolean("geoimage.viewer.show.tabs", true)) {
+            updateRequired = true;
+            // Clear the selected images in other geoimage layers
+            this.getImageTabs().map(m -> m.layer).filter(IGeoImageLayer.class::isInstance).map(IGeoImageLayer.class::cast)
+                    .filter(l -> !Objects.equals(entries, l.getSelection()))
+                    .forEach(IGeoImageLayer::clearSelection);
         } else {
-            this.tabbedEntries.put(layer, entries);
+            updateRequired = imageLayers.stream().anyMatch(l -> this.getImageTabs().map(m -> m.layer).noneMatch(l::equals));
         }
-        this.updateLayers();
+        this.updateLayers(updateRequired);
         if (entry != null) {
             this.updateButtonsNonNullEntry(entry, imageChanged);
-        } else if (this.tabbedEntries.isEmpty()) {
+        } else if (imageLayers.isEmpty()) {
             this.updateButtonsNullEntry(entries);
             return;
         } else {
-            Map.Entry<Layer, List<IImageEntry<?>>> realEntry =
-                    this.tabbedEntries.entrySet().stream().filter(mapEntry -> mapEntry.getValue().size() == 1).findFirst().orElse(null);
-            if (realEntry == null) {
+            IGeoImageLayer layer = imageLayers.stream().filter(l -> l.getSelection().size() == 1).findFirst().orElse(null);
+            if (layer == null) {
                 this.updateButtonsNullEntry(entries);
             } else {
-                this.displayImages(realEntry.getKey(), realEntry.getValue());
+                this.displayImages(layer.getSelection());
             }
             return;
         }
@@ -744 +807 @@
      * Update buttons for null entry
      * @param entries {@code true} if multiple images are selected
      */
-    private void updateButtonsNullEntry(List<IImageEntry<?>> entries) {
+    private void updateButtonsNullEntry(List<? extends IImageEntry<?>> entries) {
         boolean hasMultipleImages = entries != null && entries.size() > 1;
         // if this method is called to reinitialize dialog content with a blank image,
         // do not actually show the dialog again with a blank image if currently hidden (fix #10672)
-        setTitle(tr("Geotagged Images"));
+        this.updateTitle();
         imgDisplay.setImage(null);
         imgDisplay.setOsdText("");
         setNextEnabled(false);
@@ -787 +850 @@
         btnCopyPath.setEnabled(true);
         btnOpenExternal.setEnabled(true);
 
-        setTitle(tr("Geotagged Images") + (!entry.getDisplayName().isEmpty() ? " - " + entry.getDisplayName() : ""));
+        this.updateTitle();
         StringBuilder osd = new StringBuilder(entry.getDisplayName());
         if (entry.getElevation() != null) {
             osd.append(tr("\nAltitude: {0} m", Math.round(entry.getElevation())));
@@ -818 +881 @@
         imgDisplay.setOsdText(osd.toString());
     }
 
-    /**
-     * Displays images for the given layer.
-     * @param ignoredData the image data (unused, may be {@code null})
-     * @param entries image entries
-     * @since 18246 (signature)
-     * @deprecated Use {@link #displayImages(List)} (The data param is no longer used)
-     */
-    @Deprecated
-    public void displayImages(ImageData ignoredData, List<IImageEntry<?>> entries) {
-        this.displayImages(entries);
+    private void updateTitle() {
+        final IImageEntry<?> entry;
+        synchronized (this) {
+            entry = this.currentEntry;
+        }
+        String baseTitle = Optional.of(this.layers.getSelectedComponent())
+                .filter(MoveImgDisplayPanel.class::isInstance).map(MoveImgDisplayPanel.class::cast)
+                .map(m -> m.layer).map(Layer::getLabel).orElse(tr("Geotagged Images"));
+        if (entry == null) {
+            this.setTitle(baseTitle);
+        } else {
+            this.setTitle(baseTitle + (!entry.getDisplayName().isEmpty() ? " - " + entry.getDisplayName() : ""));
+        }
     }
 
-    private static boolean isLastImageSelected(List<IImageEntry<?>> data) {
+    private static boolean isLastImageSelected(List<? extends IImageEntry<?>> data) {
         return data.stream().anyMatch(image -> data.contains(image.getLastImage()));
     }
 
-    private static boolean isFirstImageSelected(List<IImageEntry<?>> data) {
+    private static boolean isFirstImageSelected(List<? extends IImageEntry<?>> data) {
         return data.stream().anyMatch(image -> data.contains(image.getFirstImage()));
     }
 
@@ -857 +923 @@
         if (btnCollapse != null) {
             btnCollapse.setVisible(!isDocked);
         }
-        this.updateLayers();
+        this.updateLayers(true);
     }
 
     /**
@@ -904 +970 @@
 
     @Override
     public void layerRemoving(LayerRemoveEvent e) {
-        if (e.getRemovedLayer() instanceof GeoImageLayer && this.currentEntry instanceof ImageEntry) {
-            ImageData removedData = ((GeoImageLayer) e.getRemovedLayer()).getImageData();
-            if (removedData == ((ImageEntry) this.currentEntry).getDataSet()) {
-                displayImages(e.getRemovedLayer(), null);
-            }
-            removedData.removeImageDataUpdateListener(this);
+        if (e.getRemovedLayer() instanceof IGeoImageLayer && ((IGeoImageLayer) e.getRemovedLayer()).containsImage(this.currentEntry)) {
+            displayImages(null);
         }
-        // Unfortunately, there will be no way to remove the default null layer. This will be fixed as plugins update.
-        this.tabbedEntries.remove(e.getRemovedLayer());
+        this.updateLayers(true);
     }
 
     @Override
     public void layerOrderChanged(LayerOrderChangeEvent e) {
-        // ignored
+        this.updateLayers(true);
     }
 
     @Override
@@ -941 +1002 @@
     }
 
     private void registerOnLayer(Layer layer) {
-        if (layer instanceof GeoImageLayer) {
-            ((GeoImageLayer) layer).getImageData().addImageDataUpdateListener(this);
+        if (layer instanceof IGeoImageLayer) {
+            layer.addPropertyChangeListener(l -> {
+                final List<?> currentTabLayers = this.getImageTabs().map(m -> m.layer).collect(Collectors.toList());
+                if (Layer.NAME_PROP.equals(l.getPropertyName()) && currentTabLayers.contains(layer)) {
+                    this.updateLayers(true);
+                        if (((IGeoImageLayer) layer).containsImage(this.currentEntry)) {
+                            this.updateTitle();
+                        }
+                } else if (Layer.VISIBLE_PROP.equals(l.getPropertyName()) && currentTabLayers.contains(layer)) {
+                    this.getImageTabs().filter(m -> Objects.equals(m.layer, layer)).mapToInt(this.layers::indexOfComponent)
+                            .filter(i -> i >= 0).forEach(i -> this.layers.setEnabledAt(i, layer.isVisible()));
+                }
+            });
         }
     }
 
@@ -959 +1031 @@
             imgLoadingFuture = null;
         }
     }
-
-    @Override
-    public void selectedImageChanged(ImageData data) {
-        if (this.currentEntry != data.getSelectedImage() && this.currentEntry instanceof ImageEntry &&
-                !data.getSelectedImages().contains(this.currentEntry)) {
-            displayImages(data.getLayer(), new ArrayList<>(data.getSelectedImages()));
-        }
-    }
-
-    @Override
-    public void imageDataUpdated(ImageData data) {
-        displayImages(data.getLayer(), new ArrayList<>(data.getSelectedImages()));
-    }
 }

src/org/openstreetmap/josm/gui/layer/geoimage/RemoteEntry.java

@@ -19 +19 @@
 import org.openstreetmap.josm.data.coor.ILatLon;
 import org.openstreetmap.josm.data.imagery.street_level.IImageEntry;
 import org.openstreetmap.josm.data.imagery.street_level.Projections;
-import org.openstreetmap.josm.gui.layer.Layer;
 import org.openstreetmap.josm.tools.HttpClient;
 import org.openstreetmap.josm.tools.JosmRuntimeException;
 
@@ -33 +32 @@
     private final Supplier<RemoteEntry> nextImage;
     private final Supplier<RemoteEntry> previousImage;
     private final Supplier<RemoteEntry> lastImage;
-    private final Layer layer;
     private int width;
     private int height;
     private ILatLon pos;
@@ -54 +52 @@
 
     /**
      * Create a new remote entry
-     * @param layer The originating layer, used for tabs in the image viewer
      * @param uri The URI to use
      * @param firstImage first image supplier
      * @param nextImage next image supplier
      * @param lastImage last image supplier
      * @param previousImage previous image supplier
      */
-    public RemoteEntry(Layer layer, URI uri, Supplier<RemoteEntry> firstImage, Supplier<RemoteEntry> previousImage,
+    public RemoteEntry(URI uri, Supplier<RemoteEntry> firstImage, Supplier<RemoteEntry> previousImage,
                        Supplier<RemoteEntry> nextImage, Supplier<RemoteEntry> lastImage) {
         Objects.requireNonNull(uri);
         Objects.requireNonNull(firstImage);
@@ -73 +70 @@
         this.previousImage = previousImage;
         this.nextImage = nextImage;
         this.lastImage = lastImage;
-        this.layer = layer;
     }
 
     @Override
@@ -319 +315 @@
 
     @Override
     public void selectImage(ImageViewerDialog imageViewerDialog, IImageEntry<?> entry) {
-        imageViewerDialog.displayImages(this.layer, Collections.singletonList(entry));
+        imageViewerDialog.displayImages(Collections.singletonList(entry));
     }
 
     @Override

src/org/openstreetmap/josm/gui/layer/markerlayer/ImageMarker.java

@@ -4 +4 @@
 import static org.openstreetmap.josm.tools.I18n.tr;
 
 import java.awt.event.ActionEvent;
+import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
 import java.time.Instant;
@@ -16 +17 @@
 import org.openstreetmap.josm.data.gpx.GpxConstants;
 import org.openstreetmap.josm.data.gpx.GpxLink;
 import org.openstreetmap.josm.data.gpx.WayPoint;
+import org.openstreetmap.josm.data.imagery.street_level.IImageEntry;
 import org.openstreetmap.josm.gui.Notification;
 import org.openstreetmap.josm.gui.layer.geoimage.ImageViewerDialog;
 import org.openstreetmap.josm.gui.layer.geoimage.RemoteEntry;
@@ -40 +42 @@
 
     @Override
     public void actionPerformed(ActionEvent ev) {
-        ImageViewerDialog.getInstance().displayImages(this.parentLayer, Collections.singletonList(getRemoteEntry()));
+        this.parentLayer.setCurrentMarker(this);
+        ImageViewerDialog.getInstance().displayImages(Collections.singletonList(getRemoteEntry()));
     }
 
-    private RemoteEntry getRemoteEntry() {
+    RemoteEntry getRemoteEntry() {
         try {
-            final RemoteEntry remoteEntry = new RemoteEntry(this.parentLayer, imageUrl.toURI(), getFirstImage(), getPreviousImage(),
+            final RemoteEntry remoteEntry = new MarkerRemoteEntry(imageUrl.toURI(), getFirstImage(), getPreviousImage(),
                     getNextImage(), getLastImage());
             // First, extract EXIF data
             remoteEntry.extractExif();
@@ -128 +131 @@
         wpt.put(GpxConstants.META_LINKS, Collections.singleton(link));
         return wpt;
     }
+
+    private class MarkerRemoteEntry extends RemoteEntry {
+        /**
+         * Create a new remote entry
+         *
+         * @param uri           The URI to use
+         * @param firstImage    first image supplier
+         * @param previousImage previous image supplier
+         * @param nextImage     next image supplier
+         * @param lastImage     last image supplier
+         */
+        MarkerRemoteEntry(URI uri, Supplier<RemoteEntry> firstImage, Supplier<RemoteEntry> previousImage,
+                                 Supplier<RemoteEntry> nextImage, Supplier<RemoteEntry> lastImage) {
+            super(uri, firstImage, previousImage, nextImage, lastImage);
+        }
+
+        @Override
+        public void selectImage(ImageViewerDialog imageViewerDialog, IImageEntry<?> entry) {
+            ImageMarker.this.parentLayer.setCurrentMarker(ImageMarker.this);
+            super.selectImage(imageViewerDialog, entry);
+        }
+    }
 }

src/org/openstreetmap/josm/gui/layer/markerlayer/Marker.java

@@ -20 +20 @@
 
 import javax.swing.ImageIcon;
 
+import org.openstreetmap.josm.data.IQuadBucketType;
 import org.openstreetmap.josm.data.Preferences;
 import org.openstreetmap.josm.data.coor.CachedLatLon;
 import org.openstreetmap.josm.data.coor.EastNorth;
@@ -27 +28 @@
 import org.openstreetmap.josm.data.coor.LatLon;
 import org.openstreetmap.josm.data.gpx.GpxConstants;
 import org.openstreetmap.josm.data.gpx.WayPoint;
+import org.openstreetmap.josm.data.osm.BBox;
 import org.openstreetmap.josm.data.osm.search.SearchCompiler.Match;
 import org.openstreetmap.josm.gui.MapView;
 import org.openstreetmap.josm.gui.layer.GpxLayer;
@@ -75 +77 @@
  *
  * @author Frederik Ramm
  */
-public class Marker implements TemplateEngineDataProvider, ILatLon, Destroyable {
+public class Marker implements TemplateEngineDataProvider, ILatLon, Destroyable, IQuadBucketType {
 
     /**
      * Plugins can add their Marker creation stuff at the bottom or top of this list
@@ -447 +449 @@
     private String getPreferenceKey() {
         return "draw.rawgps." + getTextTemplateKey();
     }
+
+    @Override
+    public BBox getBBox() {
+        return new BBox(this);
+    }
 }

src/org/openstreetmap/josm/gui/layer/markerlayer/MarkerLayer.java

@@ -19 +19 @@
 import java.net.URISyntaxException;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.List;
+import java.util.ListIterator;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 
 import javax.swing.AbstractAction;
@@ -41 +44 @@
 import org.openstreetmap.josm.data.gpx.GpxLink;
 import org.openstreetmap.josm.data.gpx.IGpxLayerPrefs;
 import org.openstreetmap.josm.data.gpx.WayPoint;
+import org.openstreetmap.josm.data.imagery.street_level.IImageEntry;
+import org.openstreetmap.josm.data.osm.BBox;
+import org.openstreetmap.josm.data.osm.QuadBuckets;
 import org.openstreetmap.josm.data.osm.visitor.BoundingXYVisitor;
 import org.openstreetmap.josm.data.preferences.IntegerProperty;
 import org.openstreetmap.josm.data.preferences.NamedColorProperty;
@@ -55 +61 @@
 import org.openstreetmap.josm.gui.layer.JumpToMarkerActions.JumpToNextMarker;
 import org.openstreetmap.josm.gui.layer.JumpToMarkerActions.JumpToPreviousMarker;
 import org.openstreetmap.josm.gui.layer.Layer;
+import org.openstreetmap.josm.gui.layer.geoimage.IGeoImageLayer;
+import org.openstreetmap.josm.gui.layer.geoimage.RemoteEntry;
 import org.openstreetmap.josm.gui.layer.gpx.ConvertFromMarkerLayerAction;
 import org.openstreetmap.josm.gui.preferences.display.GPXSettingsPanel;
 import org.openstreetmap.josm.io.audio.AudioPlayer;
 import org.openstreetmap.josm.spi.preferences.Config;
 import org.openstreetmap.josm.tools.ColorHelper;
 import org.openstreetmap.josm.tools.ImageProvider;
+import org.openstreetmap.josm.tools.ListenerList;
 import org.openstreetmap.josm.tools.Logging;
 import org.openstreetmap.josm.tools.Utils;
 
@@ -75 +84 @@
  *
  * The data is read only.
  */
-public class MarkerLayer extends Layer implements JumpToMarkerLayer {
+public class MarkerLayer extends Layer implements JumpToMarkerLayer, IGeoImageLayer {
 
     /**
      * A list of markers.
@@ -89 +98 @@
     final int markerSize = new IntegerProperty("draw.rawgps.markers.size", 4).get();
     final BasicStroke markerStroke = new StrokeProperty("draw.rawgps.markers.stroke", "1").get();
 
+    private final ListenerList<IGeoImageLayer.ImageChangeListener> imageChangeListenerListenerList = ListenerList.create();
+
     /**
      * The default color that is used for drawing markers.
      */
@@ -401 +412 @@
         MainApplication.getMap().mapView.zoomTo(currentMarker);
     }
 
+    /**
+     * Set the current marker
+     * @param newMarker The marker to set
+     */
+    void setCurrentMarker(Marker newMarker) {
+        this.currentMarker = newMarker;
+    }
+
     public static void playAudio() {
         playAdjacentMarker(null, true);
     }
@@ -495 +514 @@
         this.realcolor = Optional.ofNullable(color).orElse(DEFAULT_COLOR_PROPERTY.get());
     }
 
+    @Override
+    public void clearSelection() {
+        this.currentMarker = null;
+    }
+
+    @Override
+    public List<? extends IImageEntry<?>> getSelection() {
+        if (this.currentMarker instanceof ImageMarker) {
+            return Collections.singletonList(((ImageMarker) this.currentMarker).getRemoteEntry());
+        }
+        return Collections.emptyList();
+    }
+
+    @Override
+    public boolean containsImage(IImageEntry<?> imageEntry) {
+        if (imageEntry instanceof RemoteEntry) {
+            RemoteEntry entry = (RemoteEntry) imageEntry;
+            if (entry.getPos() != null && entry.getPos().isLatLonKnown()) {
+                List<Marker> markers = this.data.search(new BBox(entry.getPos()));
+                return checkIfListContainsEntry(markers, entry);
+            } else if (entry.getExifCoor() != null && entry.getExifCoor().isLatLonKnown()) {
+                List<Marker> markers = this.data.search(new BBox(entry.getExifCoor()));
+                return checkIfListContainsEntry(markers, entry);
+            } else {
+                return checkIfListContainsEntry(this.data, entry);
+            }
+        }
+        return false;
+    }
+
+    /**
+     * Check if a list contains an entry
+     * @param markerList The list to look through
+     * @param imageEntry The image entry to check
+     * @return {@code true} if the entry is in the list
+     */
+    private static boolean checkIfListContainsEntry(List<Marker> markerList, RemoteEntry imageEntry) {
+        for (Marker marker : markerList) {
+            if (marker instanceof ImageMarker) {
+                ImageMarker imageMarker = (ImageMarker) marker;
+                try {
+                    if (Objects.equals(imageMarker.imageUrl.toURI(), imageEntry.getImageURI())) {
+                        return true;
+                    }
+                } catch (URISyntaxException e) {
+                    Logging.trace(e);
+                }
+            }
+        }
+        return false;
+    }
+
+    @Override
+    public void addImageChangeListener(ImageChangeListener listener) {
+        this.imageChangeListenerListenerList.addListener(listener);
+    }
+
+    @Override
+    public void removeImageChangeListener(ImageChangeListener listener) {
+        this.imageChangeListenerListenerList.removeListener(listener);
+    }
+
     private final class MarkerMouseAdapter extends MouseAdapter {
         @Override
         public void mousePressed(MouseEvent e) {
@@ -627 +708 @@
      * the data of a MarkerLayer
      * @since 18287
      */
-    public class MarkerData extends ArrayList<Marker> implements IGpxLayerPrefs {
+    public class MarkerData extends QuadBuckets<Marker> implements List<Marker>, IGpxLayerPrefs {
 
         private Map<String, String> ownLayerPrefs;
+        private final List<Marker> markerList = new ArrayList<>();
 
         @Override
         public Map<String, String> getLayerPrefs() {
@@ -658 +740 @@
                 fromLayer.data.setModified(value);
             }
         }
+
+        @Override
+        public boolean addAll(int index, Collection<? extends Marker> c) {
+            c.forEach(this::add);
+            return this.markerList.addAll(index, c);
+        }
+
+        @Override
+        public boolean addAll(Collection<? extends Marker> objects) {
+            return this.markerList.addAll(objects) && super.addAll(objects);
+        }
+
+        @Override
+        public Marker get(int index) {
+            return this.markerList.get(index);
+        }
+
+        @Override
+        public Marker set(int index, Marker element) {
+            Marker original = this.markerList.set(index, element);
+            this.remove(original);
+            return original;
+        }
+
+        @Override
+        public void add(int index, Marker element) {
+            this.add(element);
+            this.markerList.add(index, element);
+        }
+
+        @Override
+        public Marker remove(int index) {
+            Marker toRemove = this.markerList.remove(index);
+            this.remove(toRemove);
+            return toRemove;
+        }
+
+        @Override
+        public int indexOf(Object o) {
+            return this.markerList.indexOf(o);
+        }
+
+        @Override
+        public int lastIndexOf(Object o) {
+            return this.markerList.lastIndexOf(o);
+        }
+
+        @Override
+        public ListIterator<Marker> listIterator() {
+            return this.markerList.listIterator();
+        }
+
+        @Override
+        public ListIterator<Marker> listIterator(int index) {
+            return this.markerList.listIterator(index);
+        }
+
+        @Override
+        public List<Marker> subList(int fromIndex, int toIndex) {
+            return this.markerList.subList(fromIndex, toIndex);
+        }
+
+        @Override
+        public boolean retainAll(Collection<?> objects) {
+            return this.markerList.retainAll(objects) && super.retainAll(objects);
+        }
+
+        @Override
+        public boolean contains(Object o) {
+            return this.markerList.contains(o) && super.contains(o);
+        }
     }
 }