Changeset 18613 in josm for trunk/src

Timestamp:

2022-12-12T22:38:24+01:00 (17 months ago)

Author:

taylor.smock

Message:

Fix #21605: Add tabs to ImageViewerDialog for use with different image layers

This allows users to have multiple geotagged image layers, and
quickly switch between them.

This is a complete rework of the functionality introduced in r18591 after user feedback.

Changes:

• Tabs are now scrollable
• Tabs now have a close button
• Removes the functions where plugins could send a layer in (plugins should use IGeoImageLayer instead)
• Tabs are sorted (by layer order)
• GeoImageLayers will use a darker red for selected but not viewed images
• Tabs are only opened when the user clicks on a geoimage from a different layer
• GpxMarkers now implement IQuadBucketType. This was to speed up the containsImage method in MarkerLayer.

Location:

trunk/src/org/openstreetmap/josm/gui/layer

Files:

• geoimage/GeoImageLayer.java (modified) (12 diffs)
• geoimage/IGeoImageLayer.java (added)
• geoimage/ImageEntry.java (modified) (1 diff)
• geoimage/ImageViewerDialog.java (modified) (28 diffs)
• geoimage/RemoteEntry.java (modified) (6 diffs)
• markerlayer/ImageMarker.java (modified) (4 diffs)
• markerlayer/Marker.java (modified) (4 diffs)
• markerlayer/MarkerLayer.java (modified) (10 diffs)

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

@@ -34 +34 @@
 import javax.swing.Action;
 import javax.swing.Icon;
+import javax.swing.ImageIcon;
 
 import org.openstreetmap.josm.actions.AutoScaleAction;
@@ -48 +49 @@
 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;
@@ -63 +66 @@
 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;
 
@@ -72 +77 @@
  */
 public class GeoImageLayer extends AbstractModifiableLayer implements
-        JumpToMarkerLayer, NavigatableComponent.ZoomChangeListener, ImageDataUpdateListener {
+        JumpToMarkerLayer, NavigatableComponent.ZoomChangeListener, ImageDataUpdateListener,
+        IGeoImageLayer {
 
     private static final List<Action> menuAdditions = new LinkedList<>();
@@ -79 +85 @@
 
     private final ImageData data;
+    private final ListenerList<IGeoImageLayer.ImageChangeListener> imageChangeListeners = ListenerList.create();
     GpxData gpxData;
     GpxLayer gpxFauxLayer;
@@ -87 +94 @@
     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(50, 0, 0)).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;
@@ -173 +193 @@
         this.data.addImageDataUpdateListener(this);
         this.data.setLayer(this);
+        synchronized (ImageViewerDialog.class) {
+            if (!ImageViewerDialog.hasInstance()) {
+                GuiHelper.runInEDTAndWait(ImageViewerDialog::createInstance);
+            }
+        }
+        if (getInvalidGeoImages().size() == data.size()) {
+            ImageViewerDialog.getInstance().displayImages(Collections.singletonList(this.data.getFirstImage()));
+        }
     }
 
@@ -233 +261 @@
                 } else {
                     data.setSelectedImage(img);
-                    ImageViewerDialog.getInstance().displayImages(GeoImageLayer.this, Collections.singletonList(img));
-                }
+                    ImageViewerDialog.getInstance().displayImages(Collections.singletonList(img));
+                }
+                GeoImageLayer.this.invalidate(); // Needed to update which image is being shown in the image viewer in the mapview
             }
         }
@@ -249 +278 @@
 
     @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);
     }
 
@@ -452 +515 @@
         }
 
+        final IImageEntry<?> currentImage = ImageViewerDialog.getCurrentImage();
         for (ImageEntry e: data.getSelectedImages()) {
             if (e != null && e.getPos() != null) {
@@ -466 +530 @@
                     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);
@@ -885 +953 @@
     public void selectedImageChanged(ImageData data) {
         showCurrentPhoto();
+        this.imageChangeListeners.fireEvent(e -> e.imageChanged(this, null, data.getSelectedImages()));
     }
 

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

@@ -148 +148 @@
             this.dataSet.setSelectedImage((ImageEntry) entry);
         }
-        imageViewerDialog.displayImages(this.dataSet.getLayer(), Collections.singletonList(entry));
+        imageViewerDialog.displayImages(Collections.singletonList(entry));
     }
 

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

@@ -15 +15 @@
 import java.awt.event.KeyEvent;
 import java.awt.event.WindowEvent;
+import java.beans.PropertyChangeEvent;
+import java.beans.PropertyChangeListener;
 import java.io.IOException;
 import java.io.Serializable;
@@ -24 +26 @@
 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;
@@ -32 +32 @@
 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.BorderFactory;
 import javax.swing.Box;
 import javax.swing.JButton;
@@ -39 +43 @@
 import javax.swing.JOptionPane;
 import javax.swing.JPanel;
+import javax.swing.JTabbedPane;
 import javax.swing.JToggleButton;
 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;
@@ -65 +70 @@
 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;
@@ -74 +81 @@
  * 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";
@@ -125 +132 @@
     }
 
+    /**
+     * Check if there is an instance for the {@link ImageViewerDialog}
+     * @return {@code true} if there is a static singleton instance of {@link ImageViewerDialog}
+     * @since 18613
+     */
+    public static boolean hasInstance() {
+        return dialog != null;
+    }
+
+    /**
+     * Destroy the current dialog
+     */
+    private static void destroyInstance() {
+        dialog = null;
+    }
+
     private JButton btnLast;
     private JButton btnNext;
@@ -136 +159 @@
     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() {
@@ -169 +192 @@
     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);
@@ -188 +208 @@
 
         tbCentre = new JToggleButton(imageCenterViewAction);
+        tbCentre.setSelected(Config.getPref().getBoolean("geoimage.viewer.centre.on.image", false));
         tbCentre.setPreferredSize(buttonDim);
 
@@ -197 +218 @@
 
         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());
@@ -232 +243 @@
     }
 
-    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();
     }
@@ -257 +291 @@
      */
     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));
-        }
-        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));
-        }
-        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
-     */
-    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;
+        List<MoveImgDisplayPanel<?>> alreadyAdded = this.getImageTabs().collect(Collectors.toList());
+        // Avoid the setVisible call recursively calling this method and adding duplicates
+        alreadyAdded.forEach(m -> m.finishedAddingButtons = false);
+        List<Layer> availableLayers = MainApplication.getLayerManager().getLayers();
+        List<IGeoImageLayer> geoImageLayers = availableLayers.stream()
+                .sorted(Comparator.comparingInt(entry -> /*reverse*/-availableLayers.indexOf(entry)))
+                .filter(IGeoImageLayer.class::isInstance).map(IGeoImageLayer.class::cast).collect(Collectors.toList());
+        List<IGeoImageLayer> tabLayers = geoImageLayers.stream()
+                .filter(l -> alreadyAdded.stream().anyMatch(m -> Objects.equals(l, m.layer)) || l.containsImage(this.currentEntry))
+                .collect(Collectors.toList());
+        for (IGeoImageLayer layer : tabLayers) {
+            final MoveImgDisplayPanel<?> panel = alreadyAdded.stream()
+                    .filter(m -> Objects.equals(m.layer, layer)).findFirst()
+                    .orElseGet(() -> new MoveImgDisplayPanel<>(this.imgDisplay, (Layer & IGeoImageLayer) layer));
+            int componentIndex = this.layers.indexOfComponent(panel);
+            if (componentIndex == geoImageLayers.indexOf(layer)) {
+                this.layers.setTitleAt(componentIndex, panel.getLabel(availableLayers));
+            } else {
+                this.removeImageTab((Layer) layer);
+                this.layers.insertTab(panel.getLabel(availableLayers), null, panel, null, tabLayers.indexOf(layer));
+                int idx = this.layers.indexOfComponent(panel);
+                CloseableTab closeableTab = new CloseableTab(this.layers, l -> {
+                    Component source = (Component) l.getSource();
+                    do {
+                        int index = layers.indexOfTabComponent(source);
+                        if (index >= 0) {
+                            getImageTabs().forEach(m -> m.finishedAddingButtons = false);
+                            removeImageTab(((MoveImgDisplayPanel<?>) layers.getComponentAt(index)).layer);
+                            getImageTabs().forEach(m -> m.finishedAddingButtons = true);
+                            getImageTabs().forEach(m -> m.setVisible(m.isVisible()));
+                            return;
+                        }
+                        source = source.getParent();
+                    } while (source != null);
+                });
+                this.layers.setTabComponentAt(idx, closeableTab);
+            }
+            if (layer.containsImage(this.currentEntry)) {
+                this.layers.setSelectedComponent(panel);
+            }
+        }
+        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);
+                }
+            }
+        }
+    }
+
+    /**
+     * Get the {@link MoveImgDisplayPanel} objects in {@link #layers}.
+     * @return The individual panels
+     */
+    private Stream<MoveImgDisplayPanel<?>> getImageTabs() {
+        return IntStream.range(0, this.layers.getTabCount())
+                .mapToObj(this.layers::getComponentAt)
+                .filter(MoveImgDisplayPanel.class::isInstance)
+                .map(m -> (MoveImgDisplayPanel<?>) m);
     }
 
@@ -310 +390 @@
         cancelLoadingImage();
         super.destroy();
-        dialog = null;
+        destroyInstance();
     }
 
@@ -434 +514 @@
     }
 
-    /**
-     * 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() {
@@ -479 +540 @@
             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());
@@ -620 +682 @@
 
     /**
+     * A tab title renderer for {@link HideableTabbedPane} that allows us to close tabs.
+     */
+    private static class CloseableTab extends JPanel implements PropertyChangeListener {
+        private final JLabel title;
+        private final JButton close;
+
+        /**
+         * Create a new {@link CloseableTab}.
+         * @param parent The parent to add property change listeners to. It should be a {@link HideableTabbedPane} in most cases.
+         * @param closeAction The action to run to close the tab. You probably want to call {@link JTabbedPane#removeTabAt(int)}
+         *                    at the very least.
+         */
+        CloseableTab(Component parent, ActionListener closeAction) {
+            this.title = new JLabel();
+            this.add(this.title);
+            close = new JButton(ImageProvider.get("misc", "close"));
+            close.setBorder(BorderFactory.createEmptyBorder());
+            this.add(close);
+            close.addActionListener(closeAction);
+            close.addActionListener(l -> parent.removePropertyChangeListener("indexForTitle", this));
+            parent.addPropertyChangeListener("indexForTitle", this);
+        }
+
+        @Override
+        public void propertyChange(PropertyChangeEvent evt) {
+            if (evt.getSource() instanceof JTabbedPane) {
+                JTabbedPane source = (JTabbedPane) evt.getSource();
+                if (this.getParent() == null) {
+                    source.removePropertyChangeListener(evt.getPropertyName(), this);
+                }
+                if ("indexForTitle".equals(evt.getPropertyName())) {
+                    int idx = source.indexOfTabComponent(this);
+                    if (idx >= 0) {
+                        this.title.setText(source.getTitleAt(idx));
+                    }
+                }
+                // Used to hack around UI staying visible. This assumes that the parent component is a HideableTabbedPane.
+                this.title.setVisible(source.getTabCount() != 1);
+                this.close.setVisible(source.getTabCount() != 1);
+            }
+        }
+    }
+
+    /**
+     * A JPanel whose entire purpose is to display an image by (a) moving the imgDisplay around 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);
+            JTabbedPane layers = ImageViewerDialog.getInstance().layers;
+            int index = layers.indexOfComponent(this);
+            if (visible && this.finishedAddingButtons) {
+                if (!this.layer.getSelection().isEmpty() && !this.layer.getSelection().contains(ImageViewerDialog.getCurrentImage())) {
+                    ImageViewerDialog.getInstance().displayImages(this.layer.getSelection());
+                    this.layer.invalidate(); // This will force the geoimage layers to update properly.
+                }
+                if (this.imgDisplay.getParent() != this) {
+                    this.add(this.imgDisplay, BorderLayout.CENTER);
+                    this.imgDisplay.invalidate();
+                    this.revalidate();
+                }
+                if (index >= 0) {
+                    layers.setTitleAt(index, "* " + getLabel(MainApplication.getLayerManager().getLayers()));
+                }
+            } else if (index >= 0) {
+                layers.setTitleAt(index, getLabel(MainApplication.getLayerManager().getLayers()));
+            }
+        }
+
+        /**
+         * Get the label for this panel
+         * @param availableLayers The layers to use to get the index
+         * @return The label for this layer
+         */
+        String getLabel(List<Layer> availableLayers) {
+            final int index = availableLayers.size() - availableLayers.indexOf(layer);
+            return (ExpertToggleAction.isExpert() ? "[" + index + "] " : "") + layer.getLabel();
+        }
+    }
+
+    /**
      * Enables (or disables) the "Previous" button.
      * @param value {@code true} to enable the button, {@code false} otherwise
@@ -652 +811 @@
     }
 
-    /** Used for tabbed panes */
-    private final transient Map<Layer, List<IImageEntry<?>>> tabbedEntries = new HashMap<>();
     private transient IImageEntry<? extends IImageEntry<?>> currentEntry;
 
@@ -680 +837 @@
      * @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;
@@ -711 +858 @@
         }
 
-        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);
-        }
-        this.updateLayers();
+            updateRequired = imageLayers.stream().anyMatch(l -> this.getImageTabs().map(m -> m.layer).noneMatch(l::equals));
+        }
+        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 = this.getImageTabs().map(m -> m.layer).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;
@@ -745 +899 @@
      * @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("");
@@ -788 +942 @@
         btnOpenExternal.setEnabled(true);
 
-        setTitle(tr("Geotagged Images") + (!entry.getDisplayName().isEmpty() ? " - " + entry.getDisplayName() : ""));
+        this.updateTitle();
         StringBuilder osd = new StringBuilder(entry.getDisplayName());
         if (entry.getElevation() != null) {
@@ -819 +973 @@
     }
 
-    /**
-     * 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 static boolean isLastImageSelected(List<IImageEntry<?>> data) {
+    private void updateTitle() {
+        final IImageEntry<?> entry;
+        synchronized (this) {
+            entry = this.currentEntry;
+        }
+        String baseTitle = Optional.ofNullable(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<? 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()));
     }
@@ -858 +1015 @@
             btnCollapse.setVisible(!isDocked);
         }
-        this.updateLayers();
+        this.updateLayers(true);
     }
 
@@ -905 +1062 @@
     @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);
-        }
-        // Unfortunately, there will be no way to remove the default null layer. This will be fixed as plugins update.
-        this.tabbedEntries.remove(e.getRemovedLayer());
+        if (e.getRemovedLayer() instanceof IGeoImageLayer && ((IGeoImageLayer) e.getRemovedLayer()).containsImage(this.currentEntry)) {
+            displayImages(null);
+        }
+        this.updateLayers(true);
     }
 
     @Override
     public void layerOrderChanged(LayerOrderChangeEvent e) {
-        // ignored
+        this.updateLayers(true);
     }
 
@@ -942 +1094 @@
 
     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();
+                        }
+                } // Use Layer.VISIBLE_PROP here if we decide to do something when layer visibility changes
+            });
         }
     }
@@ -952 +1112 @@
             imageData.setSelectedImage(imageData.getFirstImage());
         }
+        if (newLayer instanceof IGeoImageLayer) {
+            this.updateLayers(true);
+        }
     }
 
@@ -960 +1123 @@
         }
     }
-
-    @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()));
-    }
 }

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

@@ -20 +20 @@
 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;
@@ -34 +33 @@
     private final Supplier<RemoteEntry> previousImage;
     private final Supplier<RemoteEntry> lastImage;
-    private final Layer layer;
     private int width;
     private int height;
@@ -55 +53 @@
     /**
      * 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
@@ -62 +59 @@
      * @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);
@@ -74 +71 @@
         this.nextImage = nextImage;
         this.lastImage = lastImage;
-        this.layer = layer;
     }
 
@@ -320 +316 @@
     @Override
     public void selectImage(ImageViewerDialog imageViewerDialog, IImageEntry<?> entry) {
-        imageViewerDialog.displayImages(this.layer, Collections.singletonList(entry));
+        imageViewerDialog.displayImages(Collections.singletonList(entry));
     }
 

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

@@ -5 +5 @@
 
 import java.awt.event.ActionEvent;
+import java.net.URI;
 import java.net.URISyntaxException;
 import java.net.URL;
@@ -17 +18 @@
 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;
@@ -41 +43 @@
     @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
@@ -129 +132 @@
         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);
+        }
+    }
 }

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

@@ -21 +21 @@
 import javax.swing.ImageIcon;
 
+import org.openstreetmap.josm.data.IQuadBucketType;
 import org.openstreetmap.josm.data.Preferences;
 import org.openstreetmap.josm.data.coor.CachedLatLon;
@@ -28 +29 @@
 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;
@@ -76 +78 @@
  * @author Frederik Ramm
  */
-public class Marker implements TemplateEngineDataProvider, ILatLon, Destroyable {
+public class Marker implements TemplateEngineDataProvider, ILatLon, Destroyable, IQuadBucketType {
 
     /**
@@ -448 +450 @@
         return "draw.rawgps." + getTextTemplateKey();
     }
+
+    @Override
+    public BBox getBBox() {
+        return new BBox(this);
+    }
 }

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

@@ -20 +20 @@
 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;
 
@@ -42 +45 @@
 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;
@@ -56 +62 @@
 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;
@@ -62 +70 @@
 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;
@@ -76 +85 @@
  * The data is read only.
  */
-public class MarkerLayer extends Layer implements JumpToMarkerLayer {
+public class MarkerLayer extends Layer implements JumpToMarkerLayer, IGeoImageLayer {
 
     /**
@@ -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();
 
     /**
@@ -400 +411 @@
         }
         MainApplication.getMap().mapView.zoomTo(currentMarker);
+    }
+
+    /**
+     * Set the current marker
+     * @param newMarker The marker to set
+     */
+    void setCurrentMarker(Marker newMarker) {
+        this.currentMarker = newMarker;
     }
 
@@ -496 +515 @@
     }
 
+    @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
@@ -628 +709 @@
      * @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
@@ -659 +741 @@
             }
         }
+
+        @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);
+        }
     }
 }