Changeset 14300 in josm

Timestamp:

2018-10-06T22:01:03+02:00 (6 years ago)

Author:

Don-vip

Message:

fix #16809 - SlippyMapBBoxChooser: include tile sources from current layers (patch by ris, modified)

Location:

trunk

Files:

• src/org/openstreetmap/josm/gui/bbox/SlippyMapBBoxChooser.java (modified) (11 diffs)
• src/org/openstreetmap/josm/gui/bbox/SourceButton.java (modified) (1 diff)
• src/org/openstreetmap/josm/gui/dialogs/MinimapDialog.java (modified) (1 diff)
• test/unit/org/openstreetmap/josm/gui/dialogs/MinimapDialogTest.java (modified) (6 diffs)

trunk/src/org/openstreetmap/josm/gui/bbox/SlippyMapBBoxChooser.java

@@ -16 +16 @@
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
 
 import javax.swing.ButtonModel;
@@ -53 +53 @@
 import org.openstreetmap.josm.gui.MainApplication;
 import org.openstreetmap.josm.gui.layer.AbstractCachedTileSourceLayer;
+import org.openstreetmap.josm.gui.layer.ImageryLayer;
 import org.openstreetmap.josm.gui.layer.MainLayerManager;
 import org.openstreetmap.josm.gui.layer.TMSLayer;
@@ -61 +62 @@
  * This panel displays a map and lets the user chose a {@link BBox}.
  */
-public class SlippyMapBBoxChooser extends JMapViewer implements BBoxChooser, ChangeListener, MainLayerManager.ActiveLayerChangeListener {
-
+public class SlippyMapBBoxChooser extends JMapViewer implements BBoxChooser, ChangeListener,
+    MainLayerManager.ActiveLayerChangeListener, MainLayerManager.LayerChangeListener {
     /**
      * A list of tile sources that can be used for displaying the map.
@@ -76 +77 @@
 
     /**
-     * TMS TileSource provider for the slippymap chooser
-     */
-    public static class TMSTileSourceProvider implements TileSourceProvider {
-        private static final Set<String> existingSlippyMapUrls = new HashSet<>();
-        static {
-            // Urls that already exist in the slippymap chooser and shouldn't be copied from TMS layer list
-            existingSlippyMapUrls.add("https://{switch:a,b,c}.tile.openstreetmap.org/{zoom}/{x}/{y}.png");      // Mapnik
-        }
+     * TileSource provider for the slippymap chooser.
+     * @since 14300
+     */
+    public abstract static class AbstractImageryInfoBasedTileSourceProvider implements TileSourceProvider {
+        /**
+         * Returns the list of imagery infos backing tile sources.
+         * @return the list of imagery infos backing tile sources
+         */
+        public abstract List<ImageryInfo> getImageryInfos();
 
         @Override
@@ -89 +91 @@
             if (!TMSLayer.PROP_ADD_TO_SLIPPYMAP_CHOOSER.get()) return Collections.<TileSource>emptyList();
             List<TileSource> sources = new ArrayList<>();
-            for (ImageryInfo info : ImageryLayerInfo.instance.getLayers()) {
-                if (existingSlippyMapUrls.contains(info.getUrl())) {
-                    continue;
-                }
+            for (ImageryInfo info : this.getImageryInfos()) {
                 try {
                     TileSource source = TMSLayer.getTileSourceStatic(info);
@@ -112 +111 @@
 
     /**
+     * TileSource provider for the slippymap chooser - providing sources from imagery sources menu
+     * @since 14300
+     */
+    public static class TMSTileSourceProvider extends AbstractImageryInfoBasedTileSourceProvider {
+        @Override
+        public List<ImageryInfo> getImageryInfos() {
+            return ImageryLayerInfo.instance.getLayers();
+        }
+    }
+
+    /**
+     * TileSource provider for the slippymap chooser - providing sources from current layers
+     * @since 14300
+     */
+    public static class CurrentLayersTileSourceProvider extends AbstractImageryInfoBasedTileSourceProvider {
+        @Override
+        public List<ImageryInfo> getImageryInfos() {
+            return MainApplication.getLayerManager().getLayers().stream().filter(
+                layer -> layer instanceof ImageryLayer
+            ).map(
+                layer -> ((ImageryLayer) layer).getInfo()
+            ).collect(Collectors.toList());
+        }
+    }
+
+    /**
      * Plugins that wish to add custom tile sources to slippy map choose should call this method
      * @param tileSourceProvider new tile source provider
@@ -123 +148 @@
         addTileSourceProvider(() -> Arrays.<TileSource>asList(new OsmTileSource.Mapnik()));
         addTileSourceProvider(new TMSTileSourceProvider());
+        addTileSourceProvider(new CurrentLayersTileSourceProvider());
     }
 
@@ -178 +204 @@
         setMaxTilesInMemory(Config.getPref().getInt("slippy_map_chooser.max_tiles", 1000));
 
-        List<TileSource> tileSources = getAllTileSources();
+        List<TileSource> tileSources = new ArrayList<>(getAllTileSources().values());
 
         this.showDownloadAreaButtonModel = new JToggleButton.ToggleButtonModel();
@@ -211 +237 @@
     }
 
-    private static List<TileSource> getAllTileSources() {
-        List<TileSource> tileSources = new ArrayList<>();
-        for (TileSourceProvider provider: providers) {
-            tileSources.addAll(provider.getTileSources());
-        }
-        return tileSources;
+    private static LinkedHashMap<String, TileSource> getAllTileSources() {
+        // using a LinkedHashMap of <id, TileSource> to retain ordering but provide deduplication
+        return providers.stream().flatMap(
+            provider -> provider.getTileSources().stream()
+        ).collect(Collectors.toMap(
+            TileSource::getId,
+            ts -> ts,
+            (oldTs, newTs) -> oldTs,
+            LinkedHashMap::new
+        ));
     }
 
@@ -359 +389 @@
         this.setTileSource(tileSource);
         PROP_MAPSTYLE.put(tileSource.getName()); // TODO Is name really unique?
-        if (this.iSourceButton.getCurrentSource() != tileSource) { // prevent infinite recursion
-            this.iSourceButton.setCurrentMap(tileSource);
-        }
+
+        // we need to refresh the tile sources in case the deselected source should no longer be present
+        // (and only remained there because its removal was deferred while the source was still the
+        // selected one). this should also have the effect of propagating the new selection to the
+        // iSourceButton & menu: it attempts to re-select the current source when rebuilding its menu.
+        this.refreshTileSources();
     }
 
@@ -417 +450 @@
      */
     public final void refreshTileSources() {
-        iSourceButton.setSources(getAllTileSources());
-    }
+        final LinkedHashMap<String, TileSource> newTileSources = getAllTileSources();
+        final TileSource currentTileSource = this.getTileController().getTileSource();
+
+        // re-add the currently active TileSource to prevent inconsistent display of menu
+        newTileSources.putIfAbsent(currentTileSource.getId(), currentTileSource);
+
+        this.iSourceButton.setSources(new ArrayList<>(newTileSources.values()));
+    }
+
+    @Override
+    public void layerAdded(MainLayerManager.LayerAddEvent e) {
+        if (e.getAddedLayer() instanceof ImageryLayer) {
+            this.refreshTileSources();
+        }
+    }
+
+    @Override
+    public void layerRemoving(MainLayerManager.LayerRemoveEvent e) {
+        if (e.getRemovedLayer() instanceof ImageryLayer) {
+            this.refreshTileSources();
+        }
+    }
+
+    @Override
+    public void layerOrderChanged(MainLayerManager.LayerOrderChangeEvent e) {}
 }

trunk/src/org/openstreetmap/josm/gui/bbox/SourceButton.java

@@ -82 +82 @@
 
             // attempt to initialize button group matching current state of slippyMapBBoxChooser
-            buttonModel.setSelected(this.slippyMapBBoxChooser.getTileController().getTileSource() == ts);
+            buttonModel.setSelected(this.slippyMapBBoxChooser.getTileController().getTileSource().getId().equals(ts.getId()));
         }
 

trunk/src/org/openstreetmap/josm/gui/dialogs/MinimapDialog.java

@@ -38 +38 @@
         slippyMap.setSizeButtonVisible(false);
         slippyMap.addPropertyChangeListener(BBoxChooser.BBOX_PROP, this);
+        MainApplication.getLayerManager().addLayerChangeListener(slippyMap);
     }
 

trunk/test/unit/org/openstreetmap/josm/gui/dialogs/MinimapDialogTest.java

@@ -3 +3 @@
 
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -14 +15 @@
 import java.awt.event.ComponentEvent;
 import java.awt.image.BufferedImage;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Map;
@@ -31 +33 @@
 import org.openstreetmap.josm.data.DataSource;
 import org.openstreetmap.josm.data.osm.DataSet;
+import org.openstreetmap.josm.data.imagery.ImageryInfo;
+import org.openstreetmap.josm.data.imagery.ImageryLayerInfo;
 import org.openstreetmap.josm.data.projection.ProjectionRegistry;
 import org.openstreetmap.josm.data.projection.Projections;
@@ -37 +41 @@
 import org.openstreetmap.josm.gui.bbox.SlippyMapBBoxChooser;
 import org.openstreetmap.josm.gui.bbox.SourceButton;
+import org.openstreetmap.josm.gui.layer.ImageryLayer;
 import org.openstreetmap.josm.gui.layer.LayerManagerTest.TestLayer;
 import org.openstreetmap.josm.gui.layer.OsmDataLayer;
@@ -128 +133 @@
     }
 
+    protected void assertSourceLabelsVisible(final String... labels) {
+        GuiHelper.runInEDTAndWaitWithException(() -> {
+            final ArrayList<String> menuLabels = new ArrayList<>();
+            final JPopupMenu menu = this.sourceButton.getPopupMenu();
+            for (Component c: menu.getComponents()) {
+                if (c instanceof JPopupMenu.Separator) {
+                    break;
+                }
+                menuLabels.add(((JMenuItem) c).getText());
+            }
+
+            assertArrayEquals(
+                labels,
+                menuLabels.toArray()
+            );
+        });
+    }
+
     private MinimapDialog minimap;
     private SlippyMapBBoxChooser slippyMap;
@@ -218 +241 @@
 
         assertEquals("Green Tiles", Config.getPref().get("slippy_map_chooser.mapstyle", "Fail"));
+    }
+
+    /**
+     * Tests that the apparently-selected TileSource survives the tile sources being refreshed.
+     * @throws Exception if any error occurs
+     */
+    @Test
+    public void testRefreshSourcesRetainsSelection() throws Exception {
+        // relevant prefs starting out empty, should choose the first source and have shown download area enabled
+        // (not that there's a data layer for it to use)
+
+        this.setUpMiniMap();
+
+        this.clickSourceMenuItemByLabel("Magenta Tiles");
+        this.assertSingleSelectedSourceLabel("Magenta Tiles");
+
+        // call paint to trigger new tile fetch
+        this.paintSlippyMap();
+
+        Awaitility.await().atMost(1000, MILLISECONDS).until(this.slippyMapTasksFinished);
+
+        this.paintSlippyMap();
+
+        assertEquals(0xffff00ff, paintedSlippyMap.getRGB(0, 0));
+
+        this.slippyMap.refreshTileSources();
+
+        this.assertSingleSelectedSourceLabel("Magenta Tiles");
+
+        // call paint to trigger new tile fetch
+        this.paintSlippyMap();
+
+        Awaitility.await().atMost(1000, MILLISECONDS).until(this.slippyMapTasksFinished);
+
+        this.paintSlippyMap();
+
+        assertEquals(0xffff00ff, paintedSlippyMap.getRGB(0, 0));
+    }
+
+    /**
+     * Tests that the currently selected source being removed from ImageryLayerInfo will remain present and
+     * selected in the source menu even after the tile sources have been refreshed.
+     * @throws Exception if any error occurs
+     */
+    @Test
+    public void testRemovedSourceStillSelected() throws Exception {
+        // relevant prefs starting out empty, should choose the first source and have shown download area enabled
+        // (not that there's a data layer for it to use)
+
+        this.setUpMiniMap();
+
+        this.clickSourceMenuItemByLabel("Green Tiles");
+
+        ImageryLayerInfo.instance.remove(
+            ImageryLayerInfo.instance.getLayers().stream().filter(i -> i.getName().equals("Green Tiles")).findAny().get()
+        );
+
+        this.assertSingleSelectedSourceLabel("Green Tiles");
+
+        this.slippyMap.refreshTileSources();
+
+        this.assertSingleSelectedSourceLabel("Green Tiles");
+
+        // call paint to trigger new tile fetch
+        this.paintSlippyMap();
+
+        Awaitility.await().atMost(1000, MILLISECONDS).until(this.slippyMapTasksFinished);
+
+        this.paintSlippyMap();
+
+        assertEquals(0xff00ff00, paintedSlippyMap.getRGB(0, 0));
+    }
+
+    /**
+     * Tests the tile source list includes sources only present in the LayerManager
+     * @throws Exception if any error occurs
+     */
+    @Test
+    public void testTileSourcesFromCurrentLayers() throws Exception {
+        // relevant prefs starting out empty, should choose the first (ImageryLayerInfo) source and have shown download area enabled
+        // (not that there's a data layer for it to use)
+
+        final ImageryInfo magentaTilesInfo = ImageryLayerInfo.instance.getLayers().stream().filter(
+            i -> i.getName().equals("Magenta Tiles")
+        ).findAny().get();
+        final ImageryInfo blackTilesInfo = ImageryLayerInfo.instance.getLayers().stream().filter(
+            i -> i.getName().equals("Black Tiles")
+        ).findAny().get();
+
+        // first we will remove "Magenta Tiles" from ImageryLayerInfo
+        ImageryLayerInfo.instance.remove(magentaTilesInfo);
+
+        this.setUpMiniMap();
+
+        assertSourceLabelsVisible(
+            "White Tiles",
+            "Black Tiles",
+            "Green Tiles"
+        );
+
+        final ImageryLayer magentaTilesLayer = ImageryLayer.create(magentaTilesInfo);
+        GuiHelper.runInEDT(() -> MainApplication.getLayerManager().addLayer(magentaTilesLayer));
+
+        assertSourceLabelsVisible(
+            "White Tiles",
+            "Black Tiles",
+            "Green Tiles",
+            "Magenta Tiles"
+        );
+
+        this.clickSourceMenuItemByLabel("Magenta Tiles");
+        this.assertSingleSelectedSourceLabel("Magenta Tiles");
+
+        // call paint to trigger new tile fetch
+        this.paintSlippyMap();
+
+        Awaitility.await().atMost(1000, MILLISECONDS).until(this.slippyMapTasksFinished);
+
+        this.paintSlippyMap();
+
+        assertEquals(0xffff00ff, paintedSlippyMap.getRGB(0, 0));
+
+        final ImageryLayer blackTilesLayer = ImageryLayer.create(blackTilesInfo);
+        GuiHelper.runInEDT(() -> MainApplication.getLayerManager().addLayer(blackTilesLayer));
+
+        assertSourceLabelsVisible(
+            "White Tiles",
+            "Black Tiles",
+            "Green Tiles",
+            "Magenta Tiles"
+        );
+
+        this.clickSourceMenuItemByLabel("Black Tiles");
+        this.assertSingleSelectedSourceLabel("Black Tiles");
+
+        // call paint to trigger new tile fetch
+        this.paintSlippyMap();
+
+        Awaitility.await().atMost(1000, MILLISECONDS).until(this.slippyMapTasksFinished);
+
+        this.paintSlippyMap();
+
+        assertEquals(0xff000000, paintedSlippyMap.getRGB(0, 0));
+
+        // removing magentaTilesLayer while it is *not* the selected TileSource should make it disappear
+        // immediately
+        GuiHelper.runInEDT(() -> MainApplication.getLayerManager().removeLayer(magentaTilesLayer));
+
+        assertSourceLabelsVisible(
+            "White Tiles",
+            "Black Tiles",
+            "Green Tiles"
+        );
+        this.assertSingleSelectedSourceLabel("Black Tiles");
+
+        final ImageryLayer magentaTilesLayer2 = ImageryLayer.create(magentaTilesInfo);
+        GuiHelper.runInEDT(() -> MainApplication.getLayerManager().addLayer(magentaTilesLayer2));
+
+        assertSourceLabelsVisible(
+            "White Tiles",
+            "Black Tiles",
+            "Green Tiles",
+            "Magenta Tiles"
+        );
+
+        this.clickSourceMenuItemByLabel("Magenta Tiles");
+        this.assertSingleSelectedSourceLabel("Magenta Tiles");
+
+        // call paint to trigger new tile fetch
+        this.paintSlippyMap();
+
+        Awaitility.await().atMost(1000, MILLISECONDS).until(this.slippyMapTasksFinished);
+
+        this.paintSlippyMap();
+
+        assertEquals(0xffff00ff, paintedSlippyMap.getRGB(0, 0));
+
+        // removing magentaTilesLayer while it *is* the selected TileSource...
+        GuiHelper.runInEDT(() -> MainApplication.getLayerManager().removeLayer(magentaTilesLayer2));
+
+        assertSourceLabelsVisible(
+            "White Tiles",
+            "Black Tiles",
+            "Green Tiles",
+            "Magenta Tiles"
+        );
+        this.assertSingleSelectedSourceLabel("Magenta Tiles");
+
+        this.clickSourceMenuItemByLabel("Green Tiles");
+        this.assertSingleSelectedSourceLabel("Green Tiles");
+        assertSourceLabelsVisible(
+            "White Tiles",
+            "Black Tiles",
+            "Green Tiles"
+        );
+
+        // removing blackTilesLayer shouldn't remove it from the menu as it is already in ImageryLayerInfo
+        GuiHelper.runInEDT(() -> MainApplication.getLayerManager().removeLayer(blackTilesLayer));
+
+        this.assertSingleSelectedSourceLabel("Green Tiles");
+        assertSourceLabelsVisible(
+            "White Tiles",
+            "Black Tiles",
+            "Green Tiles"
+        );
     }