Index: src/org/openstreetmap/josm/gui/layer/geoimage/GeoImageLayer.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/src/org/openstreetmap/josm/gui/layer/geoimage/GeoImageLayer.java b/src/org/openstreetmap/josm/gui/layer/geoimage/GeoImageLayer.java --- a/src/org/openstreetmap/josm/gui/layer/geoimage/GeoImageLayer.java (revision 18606) +++ b/src/org/openstreetmap/josm/gui/layer/geoimage/GeoImageLayer.java (date 1669746302817) @@ -62,6 +62,7 @@ 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.Utils; @@ -71,7 +72,8 @@ * @since 99 */ public class GeoImageLayer extends AbstractModifiableLayer implements - JumpToMarkerLayer, NavigatableComponent.ZoomChangeListener, ImageDataUpdateListener { + JumpToMarkerLayer, NavigatableComponent.ZoomChangeListener, ImageDataUpdateListener, + IGeoImageLayer { private static final List menuAdditions = new LinkedList<>(); @@ -172,6 +174,16 @@ 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(this, Collections.singletonList(this.data.getSelectedImage())))); + } } private final class ImageMouseListener extends MouseAdapter { @@ -247,11 +259,21 @@ MainApplication.worker.execute(new ImagesLoader(files, gpxLayer)); } + @Override + public void clearSelection() { + this.getImageData().clearSelectedImage(); + } + @Override public Icon getIcon() { return ImageProvider.get("dialogs/geoimage", ImageProvider.ImageSizes.LAYER); } + @Override + public List getSelection() { + return this.getImageData().getSelectedImages(); + } + /** * Register actions on the layer * @param addition the action to be added Index: src/org/openstreetmap/josm/gui/layer/geoimage/IGeoImageLayer.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/src/org/openstreetmap/josm/gui/layer/geoimage/IGeoImageLayer.java b/src/org/openstreetmap/josm/gui/layer/geoimage/IGeoImageLayer.java new file mode 100644 --- /dev/null (date 1669744670612) +++ b/src/org/openstreetmap/josm/gui/layer/geoimage/IGeoImageLayer.java (date 1669744670612) @@ -0,0 +1,23 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.gui.layer.geoimage; + +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> getSelection(); +} Index: src/org/openstreetmap/josm/gui/layer/geoimage/ImageViewerDialog.java IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/src/org/openstreetmap/josm/gui/layer/geoimage/ImageViewerDialog.java b/src/org/openstreetmap/josm/gui/layer/geoimage/ImageViewerDialog.java --- a/src/org/openstreetmap/josm/gui/layer/geoimage/ImageViewerDialog.java (revision 18606) +++ b/src/org/openstreetmap/josm/gui/layer/geoimage/ImageViewerDialog.java (date 1669750042605) @@ -11,7 +11,6 @@ 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; @@ -31,8 +30,13 @@ 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.annotation.Nonnull; +import javax.annotation.Nullable; import javax.swing.AbstractAction; +import javax.swing.AbstractButton; import javax.swing.Box; import javax.swing.JButton; import javax.swing.JLabel; @@ -42,6 +46,7 @@ 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; @@ -64,6 +69,8 @@ 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; @@ -124,6 +131,22 @@ 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,7 +158,7 @@ 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 HideableTabbedPane layers; private ImageViewerDialog() { super(tr("Geotagged Images"), "geoimage", tr("Display geotagged images"), Shortcut.registerShortcut("tools:geotagged", @@ -168,10 +191,8 @@ 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); + this.layers = new HideableTabbedPane(); + content.add(layers, BorderLayout.CENTER); Dimension buttonDim = new Dimension(26, 26); @@ -187,6 +208,7 @@ 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,21 +218,11 @@ 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,24 +243,54 @@ 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) { + if (this.tabbedEntries.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 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); + if (this.tabbedEntries.containsKey(null) && !this.tabbedEntries.getOrDefault(null, Collections.emptyList()).isEmpty()) { + 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.entries.contains(this.currentEntry)) { + selected.setVisible(selected.isVisible()); + } else if (selected != null && !selected.entries.contains(this.currentEntry)) { + this.getImageTabs().filter(m -> m.entries.contains(this.currentEntry)).mapToInt(this.layers::indexOfComponent).findFirst() + .ifPresent(this.layers::setSelectedIndex); + } this.layers.invalidate(); } + this.layers.getParent().invalidate(); this.revalidate(); } @@ -256,39 +298,65 @@ * Add the buttons for image layers */ private void addButtonsForImageLayers() { - final IImageEntry current; - synchronized (this) { - current = this.currentEntry; - } - List layerButtons = new ArrayList<>(this.tabbedEntries.size()); + List alreadyAdded = this.getImageTabs().collect(Collectors.toList()); if (this.tabbedEntries.containsKey(null)) { List> nullEntries = this.tabbedEntries.get(null); - JButton layerButton = createImageLayerButton(null, nullEntries); - layerButtons.add(layerButton); - layerButton.setEnabled(!nullEntries.contains(current)); + if (alreadyAdded.stream().noneMatch(m -> Objects.isNull(m.layer) && Objects.equals(nullEntries, m.entries))) { + this.layers.addTab(tr("Default"), new MoveImgDisplayPanel(this.imgDisplay, null, nullEntries)); + } + } else { + this.removeImageTab(null); } + List availableLayers = MainApplication.getLayerManager().getLayers(); for (Map.Entry>> 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)); + .sorted(Comparator.comparingInt(entry -> /*reverse*/-availableLayers.indexOf(entry.getKey()))) + .collect(Collectors.toList())) { + final Layer layer = entry.getKey(); + final int index = availableLayers.size() - availableLayers.indexOf(layer); + final String label = (ExpertToggleAction.isExpert() ? "[" + index + "] " : "") + layer.getLabel(); + final Optional originalPanel = alreadyAdded.stream() + .filter(m -> Objects.equals(m.layer, entry.getKey())).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, entry.getKey(), entry.getValue())); + } } - layerButtons.forEach(this.layers::add); + this.getImageTabs().map(p -> p.layer).filter(layer -> !this.tabbedEntries.containsKey(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); } + + /** + * 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); + } + } + } + } /** - * 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> entries) { - final JButton layerButton = new JButton(); - layerButton.addActionListener(new ImageActionListener(layer, entries)); - layerButton.setText(layer != null ? layer.getLabel() : tr("Default")); - return layerButton; + private Stream getImageTabs() { + return IntStream.range(0, this.layers.getTabCount()) + .mapToObj(this.layers::getComponentAt) + .filter(MoveImgDisplayPanel.class::isInstance) + .map(MoveImgDisplayPanel.class::cast); } @Override @@ -309,7 +377,7 @@ imageZoomAction.destroy(); cancelLoadingImage(); super.destroy(); - dialog = null; + destroyInstance(); } /** @@ -433,25 +501,6 @@ } } - /** - * A listener that is called to change the viewing layer - */ - private static class ImageActionListener implements ActionListener { - - private final Layer layer; - private final List> entries; - - ImageActionListener(Layer layer, List> 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,6 +527,7 @@ 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,6 +668,49 @@ } } + /** + * 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 extends JPanel { + private final Layer layer; + private final List> entries; + private final ImageDisplay imgDisplay; + MoveImgDisplayPanel(ImageDisplay imgDisplay, Layer layer, List> entries) { + super(new BorderLayout()); + this.layer = layer; + this.entries = entries; + this.imgDisplay = imgDisplay; + } + + @Override + public void setVisible(boolean visible) { + super.setVisible(visible); + if (visible) { + if (!ImageViewerDialog.getInstance().getDisplayedImages(this.layer).isEmpty() + && !this.entries.contains(ImageViewerDialog.getCurrentImage())) { + ImageViewerDialog.getInstance().displayImages(this.layer, this.entries); + } + if (this.imgDisplay.getParent() != this) { + this.add(this.imgDisplay, BorderLayout.CENTER); + this.imgDisplay.invalidate(); + this.revalidate(); + } + } + } + } + + /** + * Get the images displayed for a layer + * @param layer The layer to get the displayed images for + * @return The images for the layer that are displayed (assuming the tab for the layer is selected) + * @since xxx + */ + @Nonnull + public List> getDisplayedImages(Layer layer) { + return this.tabbedEntries.getOrDefault(layer, Collections.emptyList()); + } + /** * Enables (or disables) the "Previous" button. * @param value {@code true} to enable the button, {@code false} otherwise @@ -680,7 +773,7 @@ * @since 18246 */ public void displayImages(List> entries) { - this.displayImages((Layer) null, entries); + this.displayImages(null, entries); } /** @@ -689,7 +782,7 @@ * @param entries image entries * @since 18591 */ - public void displayImages(Layer layer, List> entries) { + public void displayImages(@Nullable Layer layer, @Nullable List> entries) { boolean imageChanged; IImageEntry entry = entries != null && entries.size() == 1 ? entries.get(0) : null; @@ -710,12 +803,24 @@ } } - if (entries == null || entries.isEmpty() || entries.stream().allMatch(Objects::isNull)) { + + final boolean updateRequired; + 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); + this.tabbedEntries.clear(); + this.tabbedEntries.put(layer, entries); + } else if (entries == null || entries.isEmpty() || entries.stream().allMatch(Objects::isNull)) { + updateRequired = this.tabbedEntries.containsKey(layer); this.tabbedEntries.remove(layer); } else { + updateRequired = !this.tabbedEntries.containsKey(layer); this.tabbedEntries.put(layer, entries); } - this.updateLayers(); + this.updateLayers(updateRequired); if (entry != null) { this.updateButtonsNonNullEntry(entry, imageChanged); } else if (this.tabbedEntries.isEmpty()) { @@ -818,18 +923,6 @@ 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> entries) { - this.displayImages(entries); - } - private static boolean isLastImageSelected(List> data) { return data.stream().anyMatch(image -> data.contains(image.getLastImage())); } @@ -857,7 +950,7 @@ if (btnCollapse != null) { btnCollapse.setVisible(!isDocked); } - this.updateLayers(); + this.updateLayers(true); } /** @@ -912,12 +1005,12 @@ 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()); + this.updateLayers(true); } @Override public void layerOrderChanged(LayerOrderChangeEvent e) { - // ignored + this.updateLayers(true); } @Override @@ -944,6 +1037,17 @@ if (layer instanceof GeoImageLayer) { ((GeoImageLayer) layer).getImageData().addImageDataUpdateListener(this); } + layer.addPropertyChangeListener(l -> { + if (Layer.NAME_PROP.equals(l.getPropertyName()) && this.tabbedEntries.containsKey(layer)) { + this.updateLayers(true); + if (this.tabbedEntries.get(layer).contains(this.currentEntry)) { + this.setTitle(layer.getLabel()); + } + } else if (Layer.VISIBLE_PROP.equals(l.getPropertyName()) && this.tabbedEntries.containsKey(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())); + } + }); } private void showLayer(Layer newLayer) {