Index: /trunk/src/org/openstreetmap/josm/gui/bbox/SlippyMapBBoxChooser.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/bbox/SlippyMapBBoxChooser.java (revision 14299) +++ /trunk/src/org/openstreetmap/josm/gui/bbox/SlippyMapBBoxChooser.java (revision 14300) @@ -16,10 +16,10 @@ 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,4 +53,5 @@ 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,6 +62,6 @@ * 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,12 +77,13 @@ /** - * TMS TileSource provider for the slippymap chooser - */ - public static class TMSTileSourceProvider implements TileSourceProvider { - private static final Set 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 getImageryInfos(); @Override @@ -89,8 +91,5 @@ if (!TMSLayer.PROP_ADD_TO_SLIPPYMAP_CHOOSER.get()) return Collections.emptyList(); List 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,4 +111,30 @@ /** + * TileSource provider for the slippymap chooser - providing sources from imagery sources menu + * @since 14300 + */ + public static class TMSTileSourceProvider extends AbstractImageryInfoBasedTileSourceProvider { + @Override + public List 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 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,4 +148,5 @@ addTileSourceProvider(() -> Arrays.asList(new OsmTileSource.Mapnik())); addTileSourceProvider(new TMSTileSourceProvider()); + addTileSourceProvider(new CurrentLayersTileSourceProvider()); } @@ -178,5 +204,5 @@ setMaxTilesInMemory(Config.getPref().getInt("slippy_map_chooser.max_tiles", 1000)); - List tileSources = getAllTileSources(); + List tileSources = new ArrayList<>(getAllTileSources().values()); this.showDownloadAreaButtonModel = new JToggleButton.ToggleButtonModel(); @@ -211,10 +237,14 @@ } - private static List getAllTileSources() { - List tileSources = new ArrayList<>(); - for (TileSourceProvider provider: providers) { - tileSources.addAll(provider.getTileSources()); - } - return tileSources; + private static LinkedHashMap getAllTileSources() { + // using a LinkedHashMap of 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,7 +389,10 @@ 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,5 +450,28 @@ */ public final void refreshTileSources() { - iSourceButton.setSources(getAllTileSources()); - } + final LinkedHashMap 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) {} } Index: /trunk/src/org/openstreetmap/josm/gui/bbox/SourceButton.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/bbox/SourceButton.java (revision 14299) +++ /trunk/src/org/openstreetmap/josm/gui/bbox/SourceButton.java (revision 14300) @@ -82,5 +82,5 @@ // 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())); } Index: /trunk/src/org/openstreetmap/josm/gui/dialogs/MinimapDialog.java =================================================================== --- /trunk/src/org/openstreetmap/josm/gui/dialogs/MinimapDialog.java (revision 14299) +++ /trunk/src/org/openstreetmap/josm/gui/dialogs/MinimapDialog.java (revision 14300) @@ -38,4 +38,5 @@ slippyMap.setSizeButtonVisible(false); slippyMap.addPropertyChangeListener(BBoxChooser.BBOX_PROP, this); + MainApplication.getLayerManager().addLayerChangeListener(slippyMap); } Index: /trunk/test/unit/org/openstreetmap/josm/gui/dialogs/MinimapDialogTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/gui/dialogs/MinimapDialogTest.java (revision 14299) +++ /trunk/test/unit/org/openstreetmap/josm/gui/dialogs/MinimapDialogTest.java (revision 14300) @@ -3,4 +3,5 @@ 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,4 +15,5 @@ import java.awt.event.ComponentEvent; import java.awt.image.BufferedImage; +import java.util.ArrayList; import java.util.Arrays; import java.util.Map; @@ -31,4 +33,6 @@ 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,4 +41,5 @@ 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,4 +133,22 @@ } + protected void assertSourceLabelsVisible(final String... labels) { + GuiHelper.runInEDTAndWaitWithException(() -> { + final ArrayList 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,4 +241,209 @@ 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" + ); }