diff --git a/src/org/openstreetmap/josm/data/ImageData.java b/src/org/openstreetmap/josm/data/ImageData.java new file mode 100644 index 000000000..0d01699db --- /dev/null +++ b/src/org/openstreetmap/josm/data/ImageData.java @@ -0,0 +1,218 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.data; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import org.openstreetmap.josm.gui.layer.geoimage.ImageEntry; +import org.openstreetmap.josm.tools.ListenerList; + +/** + * Class to hold {@link ImageEntry} and the current selection + * @since xxx + */ +public class ImageData { + /** + * A listener that is informed when the current selection change + */ + public interface ImageDataUpdateListener { + /** + * Called when the selection change + * @param data the image data + */ + void selectedImageChanged(ImageData data); + } + + private final List data; + + private int selectedImageIndex = -1; + + private final ListenerList listeners = ListenerList.create(); + + /** + * Construct a new image container without images + */ + public ImageData() { + this(null); + } + + /** + * Construct a new image container with a list of images + * @param data the list of {@link ImageEntry} + */ + public ImageData(List data) { + if (data != null) { + Collections.sort(data); + this.data = data; + } else { + this.data = new ArrayList<>(); + } + } + + /** + * Returns the images + * @return the images + */ + public List getImages() { + return this.data; + } + + /** + * Determines if one image has modified GPS data. + * @return {@code true} if data has been modified; {@code false}, otherwise + */ + public boolean isModified() { + for (ImageEntry e : data) { + if (e.hasNewGpsData()) { + return true; + } + } + return false; + } + + /** + * Merge 2 ImageData + * @param data {@link ImageData} + */ + public void mergeFrom(ImageData data) { + this.data.addAll(data.getImages()); + Collections.sort(this.data); + + final ImageEntry selected = data.getSelectedImage(); + + // Suppress the double photos. + if (this.data.size() > 1) { + ImageEntry cur; + ImageEntry prev = this.data.get(this.data.size() - 1); + for (int i = this.data.size() - 2; i >= 0; i--) { + cur = this.data.get(i); + if (cur.getFile().equals(prev.getFile())) { + this.data.remove(i); + } else { + prev = cur; + } + } + } + if (selected != null) { + this.setSelectedImageIndex(this.data.indexOf(selected)); + } + } + + /** + * Return the current selected image + * @return the selected image as {@link ImageEntry} or null + */ + public ImageEntry getSelectedImage() { + if (this.selectedImageIndex > -1) { + return data.get(this.selectedImageIndex); + } + return null; + } + + /** + * Select the first image of the sequence + */ + public void selectFirstImage() { + if (!data.isEmpty()) { + this.setSelectedImageIndex(0); + } + } + + /** + * Select the last image of the sequence + */ + public void selectLastImage() { + this.setSelectedImageIndex(data.size() - 1); + } + + /** + * Check if there is a next image in the sequence + * @return {@code true} is there is a next image, {@code false} otherwise + */ + public boolean hasNextImage() { + return (this.selectedImageIndex != data.size() - 1); + } + + /** + * Select the next image of the sequence + */ + public void selectNextImage() { + if (this.hasNextImage()) { + this.setSelectedImageIndex(this.selectedImageIndex + 1); + } + } + + /** + * Check if there is a previous image in the sequence + * @return {@code true} is there is a previous image, {@code false} otherwise + */ + public boolean hasPreviousImage() { + return this.selectedImageIndex - 1 > -1; + } + + /** + * Select the previous image of the sequence + */ + public void selectPreviousImage() { + if (data.size() == 0) { + return; + } + this.setSelectedImageIndex(Integer.max(0, this.selectedImageIndex - 1)); + } + + /** + * Select as the selected the given image + * @param image + */ + public void setSelectedImage(ImageEntry image) { + this.setSelectedImageIndex(this.data.indexOf(image)); + } + + /** + * Clear the selected image + */ + public void clearSelectedImage() { + this.setSelectedImageIndex(-1); + } + + private void setSelectedImageIndex(int index) { + this.setSelectedImageIndex(index, false); + } + + private void setSelectedImageIndex(int index, boolean forceTrigger) { + if (index == this.selectedImageIndex && !forceTrigger) { + return; + } + this.selectedImageIndex = index; + listeners.fireEvent(l -> l.selectedImageChanged(this)); + } + + /** + * Remove the current selected image from the list + */ + public void removeSelectedImage() { + data.remove(this.getSelectedImage()); + if (this.selectedImageIndex == data.size()) { + this.setSelectedImageIndex(data.size() - 1); + } else { + this.setSelectedImageIndex(this.selectedImageIndex, true); + } + } + + /** + * Add a listener that listens to image data changes + * @param listener + */ + public void addImageDataUpdateListener(ImageDataUpdateListener listener) { + listeners.addListener(listener); + } + + /** + * Removes a listener that listens to image data changes + * @param listener The listener + */ + public void removeImageDataUpdateListener(ImageDataUpdateListener listener) { + listeners.removeListener(listener); + } +} diff --git a/src/org/openstreetmap/josm/gui/layer/geoimage/CorrelateGpxWithImages.java b/src/org/openstreetmap/josm/gui/layer/geoimage/CorrelateGpxWithImages.java index 9badaffd1..2488f409c 100644 --- a/src/org/openstreetmap/josm/gui/layer/geoimage/CorrelateGpxWithImages.java +++ b/src/org/openstreetmap/josm/gui/layer/geoimage/CorrelateGpxWithImages.java @@ -178,11 +178,10 @@ public class CorrelateGpxWithImages extends AbstractAction { break; case CANCEL: if (yLayer != null) { - if (yLayer.data != null) { - for (ImageEntry ie : yLayer.data) { - ie.discardTmp(); - } + for (ImageEntry ie : yLayer.getImageData().getImages()) { + ie.discardTmp(); } + yLayer.updateBufferAndRepaint(); } break; @@ -216,12 +215,12 @@ public class CorrelateGpxWithImages extends AbstractAction { MainApplication.getMap().mapView.zoomTo(bbox); } - if (yLayer.data != null) { - for (ImageEntry ie : yLayer.data) { - ie.applyTmp(); - } + + for (ImageEntry ie : yLayer.getImageData().getImages()) { + ie.applyTmp(); } + yLayer.updateBufferAndRepaint(); break; @@ -645,19 +644,20 @@ public class CorrelateGpxWithImages extends AbstractAction { JList imgList = new JList<>(new AbstractListModel() { @Override public String getElementAt(int i) { - return yLayer.data.get(i).getFile().getName(); + return yLayer.getImageData().getImages().get(i).getFile().getName(); } @Override public int getSize() { - return yLayer.data != null ? yLayer.data.size() : 0; + return yLayer.getImageData().getImages().size(); } }); imgList.getSelectionModel().setSelectionMode(ListSelectionModel.SINGLE_SELECTION); imgList.getSelectionModel().addListSelectionListener(evt -> { int index = imgList.getSelectedIndex(); - imgDisp.setImage(yLayer.data.get(index)); - Date date = yLayer.data.get(index).getExifTime(); + ImageEntry img = yLayer.getImageData().getImages().get(index); + imgDisp.setImage(img); + Date date = img.getExifTime(); if (date != null) { DateFormat df = DateUtils.getDateTimeFormat(DateFormat.SHORT, DateFormat.MEDIUM); lbExifTime.setText(df.format(date)); @@ -1041,10 +1041,8 @@ public class CorrelateGpxWithImages extends AbstractAction { // The selection of images we are about to correlate may have changed. // So reset all images. - if (yLayer.data != null) { - for (ImageEntry ie: yLayer.data) { - ie.discardTmp(); - } + for (ImageEntry ie: yLayer.getImageData().getImages()) { + ie.discardTmp(); } // Construct a list of images that have a date, and sort them on the date. @@ -1300,11 +1298,8 @@ public class CorrelateGpxWithImages extends AbstractAction { * @return matching images */ private List getSortedImgList(boolean exif, boolean tagged) { - if (yLayer.data == null) { - return Collections.emptyList(); - } - List dateImgLst = new ArrayList<>(yLayer.data.size()); - for (ImageEntry e : yLayer.data) { + List dateImgLst = new ArrayList<>(yLayer.getImageData().getImages().size()); + for (ImageEntry e : yLayer.getImageData().getImages()) { if (!e.hasExifTime()) { continue; } diff --git a/src/org/openstreetmap/josm/gui/layer/geoimage/GeoImageLayer.java b/src/org/openstreetmap/josm/gui/layer/geoimage/GeoImageLayer.java index efbdac4fd..faa65406b 100644 --- a/src/org/openstreetmap/josm/gui/layer/geoimage/GeoImageLayer.java +++ b/src/org/openstreetmap/josm/gui/layer/geoimage/GeoImageLayer.java @@ -23,7 +23,6 @@ import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; import java.util.HashSet; import java.util.LinkedHashSet; import java.util.LinkedList; @@ -34,24 +33,22 @@ import java.util.concurrent.Executors; import javax.swing.Action; import javax.swing.Icon; -import javax.swing.JLabel; import javax.swing.JOptionPane; -import javax.swing.SwingConstants; import org.openstreetmap.josm.actions.LassoModeAction; import org.openstreetmap.josm.actions.RenameLayerAction; import org.openstreetmap.josm.actions.mapmode.MapMode; import org.openstreetmap.josm.actions.mapmode.SelectAction; import org.openstreetmap.josm.data.Bounds; +import org.openstreetmap.josm.data.ImageData; +import org.openstreetmap.josm.data.ImageData.ImageDataUpdateListener; import org.openstreetmap.josm.data.osm.visitor.BoundingXYVisitor; -import org.openstreetmap.josm.gui.ExtendedDialog; import org.openstreetmap.josm.gui.MainApplication; import org.openstreetmap.josm.gui.MapFrame; import org.openstreetmap.josm.gui.MapFrame.MapModeChangeListener; import org.openstreetmap.josm.gui.MapView; import org.openstreetmap.josm.gui.NavigatableComponent; import org.openstreetmap.josm.gui.PleaseWaitRunnable; -import org.openstreetmap.josm.gui.datatransfer.ClipboardUtils; import org.openstreetmap.josm.gui.dialogs.LayerListDialog; import org.openstreetmap.josm.gui.dialogs.LayerListPopup; import org.openstreetmap.josm.gui.io.importexport.JpgImporter; @@ -62,7 +59,6 @@ import org.openstreetmap.josm.gui.layer.JumpToMarkerActions.JumpToNextMarker; import org.openstreetmap.josm.gui.layer.JumpToMarkerActions.JumpToPreviousMarker; import org.openstreetmap.josm.gui.layer.Layer; import org.openstreetmap.josm.gui.layer.MainLayerManager.ActiveLayerChangeListener; -import org.openstreetmap.josm.gui.util.GuiHelper; import org.openstreetmap.josm.tools.ImageProvider; import org.openstreetmap.josm.tools.Logging; import org.openstreetmap.josm.tools.Utils; @@ -71,20 +67,18 @@ import org.openstreetmap.josm.tools.Utils; * Layer displaying geottaged pictures. */ public class GeoImageLayer extends AbstractModifiableLayer implements - JumpToMarkerLayer, NavigatableComponent.ZoomChangeListener { + JumpToMarkerLayer, NavigatableComponent.ZoomChangeListener, ImageDataUpdateListener { private static List menuAdditions = new LinkedList<>(); private static volatile List supportedMapModes; - List data; + private final ImageData data; GpxLayer gpxLayer; private final Icon icon = ImageProvider.get("dialogs/geoimage/photo-marker"); private final Icon selectedIcon = ImageProvider.get("dialogs/geoimage/photo-marker-selected"); - private int currentPhoto = -1; - boolean useThumbs; private final ExecutorService thumbsLoaderExecutor = Executors.newSingleThreadExecutor(Utils.newThreadFactory("thumbnail-loader-%d", Thread.MIN_PRIORITY)); @@ -151,12 +145,10 @@ public class GeoImageLayer extends AbstractModifiableLayer implements */ public GeoImageLayer(final List data, GpxLayer gpxLayer, final String name, boolean useThumbs) { super(name != null ? name : tr("Geotagged Images")); - if (data != null) { - Collections.sort(data); - } - this.data = data; + this.data = new ImageData(data); this.gpxLayer = gpxLayer; this.useThumbs = useThumbs; + this.data.addImageDataUpdateListener(this); } /** @@ -218,6 +210,7 @@ public class GeoImageLayer extends AbstractModifiableLayer implements e.extractExif(); entries.add(e); } + layer = new GeoImageLayer(entries, gpxLayer); files.clear(); } @@ -292,9 +285,9 @@ public class GeoImageLayer extends AbstractModifiableLayer implements if (layer != null) { MainApplication.getLayerManager().addLayer(layer); - if (!canceled && layer.data != null && !layer.data.isEmpty()) { + if (!canceled && !layer.getImageData().getImages().isEmpty()) { boolean noGeotagFound = true; - for (ImageEntry e : layer.data) { + for (ImageEntry e : layer.getImageData().getImages()) { if (e.getPos() != null) { noGeotagFound = false; } @@ -356,16 +349,13 @@ public class GeoImageLayer extends AbstractModifiableLayer implements private String infoText() { int tagged = 0; int newdata = 0; - int n = 0; - if (data != null) { - n = data.size(); - for (ImageEntry e : data) { - if (e.getPos() != null) { - tagged++; - } - if (e.hasNewGpsData()) { - newdata++; - } + int n = data.getImages().size(); + for (ImageEntry e : data.getImages()) { + if (e.getPos() != null) { + tagged++; + } + if (e.hasNewGpsData()) { + newdata++; } } return "" @@ -391,14 +381,7 @@ public class GeoImageLayer extends AbstractModifiableLayer implements */ @Override public boolean isModified() { - if (data != null) { - for (ImageEntry e : data) { - if (e.hasNewGpsData()) { - return true; - } - } - } - return false; + return this.data.isModified(); } @Override @@ -417,38 +400,7 @@ public class GeoImageLayer extends AbstractModifiableLayer implements stopLoadThumbs(); l.stopLoadThumbs(); - final ImageEntry selected = l.data != null && l.currentPhoto >= 0 ? l.data.get(l.currentPhoto) : null; - - if (l.data != null) { - data.addAll(l.data); - } - Collections.sort(data); - - // Suppress the double photos. - if (data.size() > 1) { - ImageEntry cur; - ImageEntry prev = data.get(data.size() - 1); - for (int i = data.size() - 2; i >= 0; i--) { - cur = data.get(i); - if (cur.getFile().equals(prev.getFile())) { - data.remove(i); - } else { - prev = cur; - } - } - } - - if (selected != null && !data.isEmpty()) { - GuiHelper.runInEDTAndWait(() -> { - for (int i = 0; i < data.size(); i++) { - if (selected.equals(data.get(i))) { - currentPhoto = i; - ImageViewerDialog.showImage(this, data.get(i)); - break; - } - } - }); - } + this.data.mergeFrom(l.getImageData()); setName(l.getName()); thumbsLoaded &= l.thumbsLoaded; @@ -530,20 +482,18 @@ public class GeoImageLayer extends AbstractModifiableLayer implements tempG.fillRect(0, 0, width, height); tempG.setComposite(saveComp); - if (data != null) { - for (ImageEntry e : data) { - paintImage(e, mv, clip, tempG); - } - if (currentPhoto >= 0 && currentPhoto < data.size()) { - // Make sure the selected image is on top in case multiple images overlap. - paintImage(data.get(currentPhoto), mv, clip, tempG); - } + for (ImageEntry e : this.data.getImages()) { + paintImage(e, mv, clip, tempG); + } + if (this.data.getSelectedImage() != null) { + // Make sure the selected image is on top in case multiple images overlap. + paintImage(this.data.getSelectedImage(), mv, clip, tempG); } updateOffscreenBuffer = false; } g.drawImage(offscreenBuffer, 0, 0, null); - } else if (data != null) { - for (ImageEntry e : data) { + } else { + for (ImageEntry e : data.getImages()) { if (e.getPos() == null) { continue; } @@ -554,9 +504,8 @@ public class GeoImageLayer extends AbstractModifiableLayer implements } } - if (currentPhoto >= 0 && currentPhoto < data.size()) { - ImageEntry e = data.get(currentPhoto); - + ImageEntry e = data.getSelectedImage(); + if (e != null) { if (e.getPos() != null) { Point p = mv.getPoint(e.getPos()); @@ -621,7 +570,7 @@ public class GeoImageLayer extends AbstractModifiableLayer implements @Override public void visitBoundingBox(BoundingXYVisitor v) { - for (ImageEntry e : data) { + for (ImageEntry e : data.getImages()) { v.visit(e.getPos()); } } @@ -630,11 +579,8 @@ public class GeoImageLayer extends AbstractModifiableLayer implements * Show current photo on map and in image viewer. */ public void showCurrentPhoto() { - clearOtherCurrentPhotos(); - if (currentPhoto >= 0) { - ImageViewerDialog.showImage(this, data.get(currentPhoto)); - } else { - ImageViewerDialog.showImage(this, null); + if (data.getSelectedImage() != null) { + clearOtherCurrentPhotos(); } updateBufferAndRepaint(); } @@ -643,157 +589,58 @@ public class GeoImageLayer extends AbstractModifiableLayer implements * Shows next photo. */ public void showNextPhoto() { - if (data != null && !data.isEmpty()) { - currentPhoto++; - if (currentPhoto >= data.size()) { - currentPhoto = data.size() - 1; - } - } else { - currentPhoto = -1; - } - showCurrentPhoto(); + this.data.selectNextImage(); } /** * Shows previous photo. */ public void showPreviousPhoto() { - if (data != null && !data.isEmpty()) { - currentPhoto--; - if (currentPhoto < 0) { - currentPhoto = 0; - } - } else { - currentPhoto = -1; - } - showCurrentPhoto(); + this.data.selectPreviousImage(); } /** * Shows first photo. */ public void showFirstPhoto() { - if (data != null && !data.isEmpty()) { - currentPhoto = 0; - } else { - currentPhoto = -1; - } - showCurrentPhoto(); + this.data.selectFirstImage(); } /** * Shows last photo. */ public void showLastPhoto() { - if (data != null && !data.isEmpty()) { - currentPhoto = data.size() - 1; - } else { - currentPhoto = -1; - } - showCurrentPhoto(); - } - - public void checkPreviousNextButtons() { - ImageViewerDialog.setNextEnabled(data != null && currentPhoto < data.size() - 1); - ImageViewerDialog.setPreviousEnabled(currentPhoto > 0); + this.data.selectLastImage(); } - public void removeCurrentPhoto() { - if (data != null && !data.isEmpty() && currentPhoto >= 0 && currentPhoto < data.size()) { - data.remove(currentPhoto); - if (currentPhoto >= data.size()) { - currentPhoto = data.size() - 1; - } - showCurrentPhoto(); - } - } - - public void removeCurrentPhotoFromDisk() { - ImageEntry toDelete; - if (data != null && !data.isEmpty() && currentPhoto >= 0 && currentPhoto < data.size()) { - toDelete = data.get(currentPhoto); - - int result = new ExtendedDialog( - MainApplication.getMainFrame(), - tr("Delete image file from disk"), - tr("Cancel"), tr("Delete")) - .setButtonIcons("cancel", "dialogs/delete") - .setContent(new JLabel(tr("

Delete the file {0} from disk?

The image file will be permanently lost!

", - toDelete.getFile().getName()), ImageProvider.get("dialogs/geoimage/deletefromdisk"), SwingConstants.LEFT)) - .toggleEnable("geoimage.deleteimagefromdisk") - .setCancelButton(1) - .setDefaultButton(2) - .showDialog() - .getValue(); - - if (result == 2) { - data.remove(currentPhoto); - if (currentPhoto >= data.size()) { - currentPhoto = data.size() - 1; - } - - if (Utils.deleteFile(toDelete.getFile())) { - Logging.info("File "+toDelete.getFile()+" deleted. "); - } else { - JOptionPane.showMessageDialog( - MainApplication.getMainFrame(), - tr("Image file could not be deleted."), - tr("Error"), - JOptionPane.ERROR_MESSAGE - ); - } - - showCurrentPhoto(); - } - } - } - - public void copyCurrentPhotoPath() { - if (data != null && !data.isEmpty() && currentPhoto >= 0 && currentPhoto < data.size()) { - ClipboardUtils.copyString(data.get(currentPhoto).getFile().toString()); - } - } - - /** - * Removes a photo from the list of images by index. - * @param idx Image index - * @since 6392 - */ - public void removePhotoByIdx(int idx) { - if (idx >= 0 && data != null && idx < data.size()) { - data.remove(idx); - } - } /** * Check if the position of the mouse event is within the rectangle of the photo icon or thumbnail. - * @param idx Image index, range 0 .. size-1 + * @param idx the image index * @param evt Mouse event * @return {@code true} if the photo matches the mouse position, {@code false} otherwise */ private boolean isPhotoIdxUnderMouse(int idx, MouseEvent evt) { - if (idx >= 0 && data != null && idx < data.size()) { - ImageEntry img = data.get(idx); - if (img.getPos() != null) { - Point imgCenter = MainApplication.getMap().mapView.getPoint(img.getPos()); - Rectangle imgRect; - if (useThumbs && img.hasThumbnail()) { - Dimension imgDim = scaledDimension(img.getThumbnail()); - if (imgDim != null) { - imgRect = new Rectangle(imgCenter.x - imgDim.width / 2, - imgCenter.y - imgDim.height / 2, - imgDim.width, imgDim.height); - } else { - imgRect = null; - } + ImageEntry img = this.data.getImages().get(idx); + if (img.getPos() != null) { + Point imgCenter = MainApplication.getMap().mapView.getPoint(img.getPos()); + Rectangle imgRect; + if (useThumbs && img.hasThumbnail()) { + Dimension imgDim = scaledDimension(img.getThumbnail()); + if (imgDim != null) { + imgRect = new Rectangle(imgCenter.x - imgDim.width / 2, + imgCenter.y - imgDim.height / 2, + imgDim.width, imgDim.height); } else { - imgRect = new Rectangle(imgCenter.x - icon.getIconWidth() / 2, - imgCenter.y - icon.getIconHeight() / 2, - icon.getIconWidth(), icon.getIconHeight()); - } - if (imgRect != null && imgRect.contains(evt.getPoint())) { - return true; + imgRect = null; } + } else { + imgRect = new Rectangle(imgCenter.x - icon.getIconWidth() / 2, + imgCenter.y - icon.getIconHeight() / 2, + icon.getIconWidth(), icon.getIconHeight()); + } + if (imgRect != null && imgRect.contains(evt.getPoint())) { + return true; } } return false; @@ -809,31 +656,32 @@ public class GeoImageLayer extends AbstractModifiableLayer implements * or {@code -1} if there is no image at the mouse position */ private int getPhotoIdxUnderMouse(MouseEvent evt, boolean cycle) { - if (data != null) { - if (cycle && currentPhoto >= 0) { - // Cycle loop is forward as that is the natural order. - // Loop 1: One after current photo up to last one. - for (int idx = currentPhoto + 1; idx < data.size(); ++idx) { - if (isPhotoIdxUnderMouse(idx, evt)) { - return idx; - } - } - // Loop 2: First photo up to current one. - for (int idx = 0; idx <= currentPhoto; ++idx) { - if (isPhotoIdxUnderMouse(idx, evt)) { - return idx; - } + ImageEntry selectedImage = this.data.getSelectedImage(); + int selectedIndex = this.data.getImages().indexOf(selectedImage); + + if (cycle && selectedImage != null) { + // Cycle loop is forward as that is the natural order. + // Loop 1: One after current photo up to last one. + for (int idx = selectedIndex + 1; idx < this.data.getImages().size(); ++idx) { + if (isPhotoIdxUnderMouse(idx, evt)) { + return idx; } - } else { - // Check for current photo first, i.e. keep it selected if it is under the mouse. - if (currentPhoto >= 0 && isPhotoIdxUnderMouse(currentPhoto, evt)) { - return currentPhoto; + } + // Loop 2: First photo up to current one. + for (int idx = 0; idx <= selectedIndex; ++idx) { + if (isPhotoIdxUnderMouse(idx, evt)) { + return idx; } - // Loop from last to first to prefer topmost image. - for (int idx = data.size() - 1; idx >= 0; --idx) { - if (isPhotoIdxUnderMouse(idx, evt)) { - return idx; - } + } + } else { + // Check for current photo first, i.e. keep it selected if it is under the mouse. + if (selectedImage != null && isPhotoIdxUnderMouse(selectedIndex, evt)) { + return selectedIndex; + } + // Loop from last to first to prefer topmost image. + for (int idx = this.data.getImages().size() - 1; idx >= 0; --idx) { + if (isPhotoIdxUnderMouse(idx, evt)) { + return idx; } } } @@ -861,24 +709,12 @@ public class GeoImageLayer extends AbstractModifiableLayer implements public ImageEntry getPhotoUnderMouse(MouseEvent evt) { int idx = getPhotoIdxUnderMouse(evt); if (idx >= 0) { - return data.get(idx); + return this.data.getImages().get(idx); } else { return null; } } - /** - * Clears the currentPhoto, i.e. remove select marker, and optionally repaint. - * @param repaint Repaint flag - * @since 6392 - */ - public void clearCurrentPhoto(boolean repaint) { - currentPhoto = -1; - if (repaint) { - updateBufferAndRepaint(); - } - } - /** * Clears the currentPhoto of the other GeoImageLayer's. Otherwise there could be multiple selected photos. */ @@ -886,7 +722,7 @@ public class GeoImageLayer extends AbstractModifiableLayer implements for (GeoImageLayer layer: MainApplication.getLayerManager().getLayersOfType(GeoImageLayer.class)) { if (layer != this) { - layer.clearCurrentPhoto(false); + layer.getImageData().clearSelectedImage(); } } } @@ -947,7 +783,7 @@ public class GeoImageLayer extends AbstractModifiableLayer implements public void mouseReleased(MouseEvent ev) { if (ev.getButton() != MouseEvent.BUTTON1) return; - if (data == null || !isVisible() || !isMapModeOk()) + if (!isVisible() || !isMapModeOk()) return; Point mousePos = ev.getPoint(); @@ -956,8 +792,7 @@ public class GeoImageLayer extends AbstractModifiableLayer implements if (idx >= 0) { lastSelPos = mousePos; cycleModeArmed = false; - currentPhoto = idx; - showCurrentPhoto(); + data.setSelectedImage(data.getImages().get(idx)); } } }; @@ -1012,11 +847,6 @@ public class GeoImageLayer extends AbstractModifiableLayer implements mapView.removeMouseMotionListener(mouseMotionAdapter); MapFrame.removeMapModeChangeListener(mapModeListener); MainApplication.getLayerManager().removeActiveLayerChangeListener(activeLayerChangeListener); - currentPhoto = -1; - if (data != null) { - data.clear(); - } - data = null; } @Override @@ -1082,7 +912,16 @@ public class GeoImageLayer extends AbstractModifiableLayer implements * @return List of images in layer */ public List getImages() { - return data == null ? Collections.emptyList() : new ArrayList<>(data); + return new ArrayList<>(this.data.getImages()); + } + + /** + * Returns the image data store being used by this layer + * @return imageData + * @since xxx + */ + public ImageData getImageData() { + return data; } /** @@ -1127,4 +966,9 @@ public class GeoImageLayer extends AbstractModifiableLayer implements } invalidate(); } + + @Override + public void selectedImageChanged(ImageData data) { + this.showCurrentPhoto(); + } } diff --git a/src/org/openstreetmap/josm/gui/layer/geoimage/ImageViewerDialog.java b/src/org/openstreetmap/josm/gui/layer/geoimage/ImageViewerDialog.java index 53cf7ee8a..39d8da647 100644 --- a/src/org/openstreetmap/josm/gui/layer/geoimage/ImageViewerDialog.java +++ b/src/org/openstreetmap/josm/gui/layer/geoimage/ImageViewerDialog.java @@ -13,14 +13,22 @@ import java.awt.event.KeyEvent; import java.awt.event.WindowEvent; import java.text.DateFormat; import java.text.SimpleDateFormat; +import java.util.Objects; import javax.swing.Box; import javax.swing.JButton; +import javax.swing.JLabel; +import javax.swing.JOptionPane; import javax.swing.JPanel; import javax.swing.JToggleButton; +import javax.swing.SwingConstants; import org.openstreetmap.josm.actions.JosmAction; +import org.openstreetmap.josm.data.ImageData; +import org.openstreetmap.josm.data.ImageData.ImageDataUpdateListener; +import org.openstreetmap.josm.gui.ExtendedDialog; import org.openstreetmap.josm.gui.MainApplication; +import org.openstreetmap.josm.gui.datatransfer.ClipboardUtils; import org.openstreetmap.josm.gui.dialogs.DialogsPanel.Action; import org.openstreetmap.josm.gui.dialogs.ToggleDialog; import org.openstreetmap.josm.gui.layer.Layer; @@ -31,13 +39,15 @@ import org.openstreetmap.josm.gui.layer.LayerManager.LayerRemoveEvent; import org.openstreetmap.josm.gui.layer.MainLayerManager.ActiveLayerChangeEvent; import org.openstreetmap.josm.gui.layer.MainLayerManager.ActiveLayerChangeListener; import org.openstreetmap.josm.tools.ImageProvider; +import org.openstreetmap.josm.tools.Logging; import org.openstreetmap.josm.tools.Shortcut; +import org.openstreetmap.josm.tools.Utils; import org.openstreetmap.josm.tools.date.DateUtils; /** * Dialog to view and manipulate geo-tagged images from a {@link GeoImageLayer}. */ -public final class ImageViewerDialog extends ToggleDialog implements LayerChangeListener, ActiveLayerChangeListener { +public final class ImageViewerDialog extends ToggleDialog implements LayerChangeListener, ActiveLayerChangeListener, ImageDataUpdateListener { private final ImageZoomAction imageZoomAction = new ImageZoomAction(); private final ImageCenterViewAction imageCenterViewAction = new ImageCenterViewAction(); @@ -79,6 +89,9 @@ public final class ImageViewerDialog extends ToggleDialog implements LayerChange private JButton btnPrevious; private JButton btnFirst; private JButton btnCollapse; + private JButton btnDelete; + private JButton btnCopyPath; + private JButton btnDeleteFromDisk; private JToggleButton tbCentre; private ImageViewerDialog() { @@ -87,6 +100,9 @@ public final class ImageViewerDialog extends ToggleDialog implements LayerChange build(); MainApplication.getLayerManager().addActiveLayerChangeListener(this); MainApplication.getLayerManager().addLayerChangeListener(this); + for (Layer l: MainApplication.getLayerManager().getLayers()) { + this.registerOnLayer(l); + } } private static JButton createNavigationButton(JosmAction action, Dimension buttonDim) { @@ -106,13 +122,13 @@ public final class ImageViewerDialog extends ToggleDialog implements LayerChange btnFirst = createNavigationButton(imageFirstAction, buttonDim); btnPrevious = createNavigationButton(imagePreviousAction, buttonDim); - JButton btnDelete = new JButton(imageRemoveAction); + btnDelete = new JButton(imageRemoveAction); btnDelete.setPreferredSize(buttonDim); - JButton btnDeleteFromDisk = new JButton(imageRemoveFromDiskAction); + btnDeleteFromDisk = new JButton(imageRemoveFromDiskAction); btnDeleteFromDisk.setPreferredSize(buttonDim); - JButton btnCopyPath = new JButton(imageCopyPathAction); + btnCopyPath = new JButton(imageCopyPathAction); btnCopyPath.setPreferredSize(buttonDim); btnNext = createNavigationButton(imageNextAction, buttonDim); @@ -189,8 +205,8 @@ public final class ImageViewerDialog extends ToggleDialog implements LayerChange @Override public void actionPerformed(ActionEvent e) { - if (currentLayer != null) { - currentLayer.showNextPhoto(); + if (currentData != null) { + currentData.selectNextImage(); } } } @@ -204,8 +220,8 @@ public final class ImageViewerDialog extends ToggleDialog implements LayerChange @Override public void actionPerformed(ActionEvent e) { - if (currentLayer != null) { - currentLayer.showPreviousPhoto(); + if (currentData != null) { + currentData.selectPreviousImage(); } } } @@ -219,8 +235,8 @@ public final class ImageViewerDialog extends ToggleDialog implements LayerChange @Override public void actionPerformed(ActionEvent e) { - if (currentLayer != null) { - currentLayer.showFirstPhoto(); + if (currentData != null) { + currentData.selectFirstImage(); } } } @@ -234,8 +250,8 @@ public final class ImageViewerDialog extends ToggleDialog implements LayerChange @Override public void actionPerformed(ActionEvent e) { - if (currentLayer != null) { - currentLayer.showLastPhoto(); + if (currentData != null) { + currentData.selectLastImage(); } } } @@ -277,8 +293,8 @@ public final class ImageViewerDialog extends ToggleDialog implements LayerChange @Override public void actionPerformed(ActionEvent e) { - if (currentLayer != null) { - currentLayer.removeCurrentPhoto(); + if (currentData != null) { + currentData.removeSelectedImage(); } } } @@ -293,8 +309,36 @@ public final class ImageViewerDialog extends ToggleDialog implements LayerChange @Override public void actionPerformed(ActionEvent e) { - if (currentLayer != null) { - currentLayer.removeCurrentPhotoFromDisk(); + if (currentData != null && currentData.getSelectedImage() != null) { + ImageEntry toDelete = currentData.getSelectedImage(); + + int result = new ExtendedDialog( + MainApplication.getMainFrame(), + tr("Delete image file from disk"), + tr("Cancel"), tr("Delete")) + .setButtonIcons("cancel", "dialogs/delete") + .setContent(new JLabel(tr("

Delete the file {0} from disk?

The image file will be permanently lost!

", + toDelete.getFile().getName()), ImageProvider.get("dialogs/geoimage/deletefromdisk"), SwingConstants.LEFT)) + .toggleEnable("geoimage.deleteimagefromdisk") + .setCancelButton(1) + .setDefaultButton(2) + .showDialog() + .getValue(); + + if (result == 2) { + currentData.removeSelectedImage(); + + if (Utils.deleteFile(toDelete.getFile())) { + Logging.info("File "+toDelete.getFile()+" deleted. "); + } else { + JOptionPane.showMessageDialog( + MainApplication.getMainFrame(), + tr("Image file could not be deleted."), + tr("Error"), + JOptionPane.ERROR_MESSAGE + ); + } + } } } } @@ -308,8 +352,8 @@ public final class ImageViewerDialog extends ToggleDialog implements LayerChange @Override public void actionPerformed(ActionEvent e) { - if (currentLayer != null) { - currentLayer.copyCurrentPhotoPath(); + if (currentData != null) { + ClipboardUtils.copyString(currentData.getSelectedImage().getFile().toString()); } } } @@ -329,35 +373,29 @@ public final class ImageViewerDialog extends ToggleDialog implements LayerChange /** * Displays image for the given layer. - * @param layer geo image layer + * @param data geo image layer * @param entry image entry */ - public static void showImage(GeoImageLayer layer, ImageEntry entry) { - getInstance().displayImage(layer, entry); - if (layer != null) { - layer.checkPreviousNextButtons(); - } else { - setPreviousEnabled(false); - setNextEnabled(false); - } + public static void showImage(ImageData data, ImageEntry entry) { + getInstance().displayImage(data, entry); } /** * Enables (or disables) the "Previous" button. * @param value {@code true} to enable the button, {@code false} otherwise */ - public static void setPreviousEnabled(boolean value) { - getInstance().btnFirst.setEnabled(value); - getInstance().btnPrevious.setEnabled(value); + public void setPreviousEnabled(boolean value) { + this.btnFirst.setEnabled(value); + this.btnPrevious.setEnabled(value); } /** * Enables (or disables) the "Next" button. * @param value {@code true} to enable the button, {@code false} otherwise */ - public static void setNextEnabled(boolean value) { - getInstance().btnNext.setEnabled(value); - getInstance().btnLast.setEnabled(value); + public void setNextEnabled(boolean value) { + this.btnNext.setEnabled(value); + this.btnLast.setEnabled(value); } /** @@ -373,15 +411,15 @@ public final class ImageViewerDialog extends ToggleDialog implements LayerChange return wasEnabled; } - private transient GeoImageLayer currentLayer; + private transient ImageData currentData; private transient ImageEntry currentEntry; /** * Displays image for the given layer. - * @param layer geo image layer + * @param data the image data * @param entry image entry */ - public void displayImage(GeoImageLayer layer, ImageEntry entry) { + public void displayImage(ImageData data, ImageEntry entry) { boolean imageChanged; synchronized (this) { @@ -393,11 +431,19 @@ public final class ImageViewerDialog extends ToggleDialog implements LayerChange MainApplication.getMap().mapView.zoomTo(entry.getPos()); } - currentLayer = layer; + currentData = data; currentEntry = entry; } + if (entry != null) { + Objects.requireNonNull(data, "data cannot be null!"); + this.setNextEnabled(data.hasNextImage()); + this.setPreviousEnabled(data.hasPreviousImage()); + btnDelete.setEnabled(true); + btnDeleteFromDisk.setEnabled(true); + btnCopyPath.setEnabled(true); + if (imageChanged) { // Set only if the image is new to preserve zoom and position if the same image is redisplayed // (e.g. to update the OSD). @@ -436,6 +482,11 @@ public final class ImageViewerDialog extends ToggleDialog implements LayerChange setTitle(tr("Geotagged Images")); imgDisplay.setImage(null); imgDisplay.setOsdText(""); + this.setNextEnabled(false); + this.setPreviousEnabled(false); + btnDelete.setEnabled(false); + btnDeleteFromDisk.setEnabled(false); + btnCopyPath.setEnabled(false); return; } if (!isDialogShowing()) { @@ -493,7 +544,7 @@ public final class ImageViewerDialog extends ToggleDialog implements LayerChange * @since 6392 */ public static GeoImageLayer getCurrentLayer() { - return getInstance().currentLayer; + return null; } /** @@ -507,18 +558,17 @@ public final class ImageViewerDialog extends ToggleDialog implements LayerChange @Override public void layerAdded(LayerAddEvent e) { + this.registerOnLayer(e.getAddedLayer()); showLayer(e.getAddedLayer()); } @Override public void layerRemoving(LayerRemoveEvent e) { - // Clear current image and layer if current layer is deleted - if (currentLayer != null && currentLayer.equals(e.getRemovedLayer())) { - showImage(null, null); - } - // Check buttons state in case of layer merging - if (currentLayer != null && e.getRemovedLayer() instanceof GeoImageLayer) { - currentLayer.checkPreviousNextButtons(); + if (e.getRemovedLayer() instanceof GeoImageLayer) { + if (((GeoImageLayer) e.getRemovedLayer()).getImageData() == currentData) { + displayImage(null, null); + } + ((GeoImageLayer) e.getRemovedLayer()).getImageData().removeImageDataUpdateListener(this); } } @@ -532,9 +582,20 @@ public final class ImageViewerDialog extends ToggleDialog implements LayerChange showLayer(e.getSource().getActiveLayer()); } + private void registerOnLayer(Layer layer) { + if (layer instanceof GeoImageLayer) { + ((GeoImageLayer) layer).getImageData().addImageDataUpdateListener(this); + } + } + private void showLayer(Layer newLayer) { - if (currentLayer == null && newLayer instanceof GeoImageLayer) { - ((GeoImageLayer) newLayer).showFirstPhoto(); + if (currentData == null && newLayer instanceof GeoImageLayer) { + ((GeoImageLayer) newLayer).getImageData().selectFirstImage(); } } + + @Override + public void selectedImageChanged(ImageData data) { + showImage(data, data.getSelectedImage()); + } } diff --git a/src/org/openstreetmap/josm/gui/layer/geoimage/ShowThumbnailAction.java b/src/org/openstreetmap/josm/gui/layer/geoimage/ShowThumbnailAction.java index 7a7b62791..5f75cad5f 100644 --- a/src/org/openstreetmap/josm/gui/layer/geoimage/ShowThumbnailAction.java +++ b/src/org/openstreetmap/josm/gui/layer/geoimage/ShowThumbnailAction.java @@ -49,7 +49,7 @@ public class ShowThumbnailAction extends AbstractAction implements LayerAction { * {@code false} otherwise */ private static boolean enabled(GeoImageLayer layer) { - return layer.data != null && !layer.data.isEmpty(); + return !layer.getImageData().getImages().isEmpty(); } /** Create actual menu entry and define if it is enabled or not. */ diff --git a/src/org/openstreetmap/josm/gui/layer/geoimage/ThumbsLoader.java b/src/org/openstreetmap/josm/gui/layer/geoimage/ThumbsLoader.java index a49bbc8a3..0ead613c5 100644 --- a/src/org/openstreetmap/josm/gui/layer/geoimage/ThumbsLoader.java +++ b/src/org/openstreetmap/josm/gui/layer/geoimage/ThumbsLoader.java @@ -51,7 +51,7 @@ public class ThumbsLoader implements Runnable { * @param layer geoimage layer */ public ThumbsLoader(GeoImageLayer layer) { - this(new ArrayList<>(layer.data), layer); + this(new ArrayList<>(layer.getImageData().getImages()), layer); } /** diff --git a/test/unit/org/openstreetmap/josm/data/ImageDataTest.java b/test/unit/org/openstreetmap/josm/data/ImageDataTest.java new file mode 100644 index 000000000..8ce314ff0 --- /dev/null +++ b/test/unit/org/openstreetmap/josm/data/ImageDataTest.java @@ -0,0 +1,232 @@ +// License: GPL. For details, see LICENSE file. +package org.openstreetmap.josm.data; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import java.io.File; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +import org.junit.Test; +import org.openstreetmap.josm.data.ImageData.ImageDataUpdateListener; +import org.openstreetmap.josm.gui.layer.geoimage.ImageEntry; + +import mockit.Expectations; +import mockit.Mock; +import mockit.MockUp; + +/** + * Unit tests for class {@link ImageData}. + */ +public class ImageDataTest { + + private List getOneImage() { + ArrayList list = new ArrayList<>(); + list.add(new ImageEntry(new File("test"))); + return list; + } + + @Test + public void testWithullData() { + ImageData data = new ImageData(); + assertEquals(0, data.getImages().size()); + assertNull(data.getSelectedImage()); + data.selectFirstImage(); + assertNull(data.getSelectedImage()); + data.selectLastImage(); + assertNull(data.getSelectedImage()); + data.selectFirstImage(); + assertNull(data.getSelectedImage()); + data.selectPreviousImage(); + assertNull(data.getSelectedImage()); + assertFalse(data.hasNextImage()); + assertFalse(data.hasPreviousImage()); + data.removeSelectedImage(); + } + + @Test + public void testmageEntryWithImages() { + assertEquals(1, new ImageData(this.getOneImage()).getImages().size()); + } + + @Test + public void testSortData() { + List list = this.getOneImage(); + + new Expectations(Collections.class) {{ + Collections.sort(list); + }}; + + new ImageData(list); + } + + @Test + public void testIsModifiedFalse() { + assertFalse(new ImageData(this.getOneImage()).isModified()); + } + + @Test + public void testIsModifiedTrue() { + List list = this.getOneImage(); + + new Expectations(list.get(0)) {{ + list.get(0).hasNewGpsData(); result = true; + }}; + + assertTrue(new ImageData(list).isModified()); + } + + @Test + public void testSelectFirstImage() { + List list = this.getOneImage(); + + ImageData data = new ImageData(list); + data.selectFirstImage(); + assertEquals(list.get(0), data.getSelectedImage()); + } + + @Test + public void testSelectLastImage() { + List list = this.getOneImage(); + list.add(new ImageEntry()); + + ImageData data = new ImageData(list); + data.selectLastImage(); + assertEquals(list.get(1), data.getSelectedImage()); + } + + @Test + public void testSelectNextImage() { + List list = this.getOneImage(); + + ImageData data = new ImageData(list); + assertTrue(data.hasNextImage()); + data.selectNextImage(); + assertEquals(list.get(0), data.getSelectedImage()); + assertFalse(data.hasNextImage()); + data.selectNextImage(); + assertEquals(list.get(0), data.getSelectedImage()); + } + + @Test + public void testSelectPreviousImage() { + List list = this.getOneImage(); + list.add(new ImageEntry()); + + ImageData data = new ImageData(list); + assertFalse(data.hasPreviousImage()); + data.selectLastImage(); + assertTrue(data.hasPreviousImage()); + data.selectPreviousImage(); + assertEquals(list.get(0), data.getSelectedImage()); + data.selectPreviousImage(); + assertEquals(list.get(0), data.getSelectedImage()); + } + + @Test + public void testSetSelectedImage() { + List list = this.getOneImage(); + + ImageData data = new ImageData(list); + data.setSelectedImage(list.get(0)); + assertEquals(list.get(0), data.getSelectedImage()); + } + + @Test + public void testClearSelectedImage() { + List list = this.getOneImage(); + + ImageData data = new ImageData(list); + data.setSelectedImage(list.get(0)); + data.clearSelectedImage(); + assertNull(data.getSelectedImage()); + } + + @Test + public void testSelectionListener() { + List list = this.getOneImage(); + ImageData data = new ImageData(list); + ImageDataUpdateListener listener = new ImageDataUpdateListener() { + @Override + public void selectedImageChanged(ImageData data) {} + }; + new Expectations(listener) {{ + listener.selectedImageChanged(data); times = 1; + }}; + data.addImageDataUpdateListener(listener); + data.selectFirstImage(); + data.selectFirstImage(); + } + + @Test + public void testRemoveSelectedImage() { + List list = this.getOneImage(); + ImageData data = new ImageData(list); + data.selectFirstImage(); + data.removeSelectedImage(); + assertEquals(0, data.getImages().size()); + assertNull(data.getSelectedImage()); + } + + @Test + public void testRemoveSelectedWithImageTriggerListener() { + List list = this.getOneImage(); + list.add(new ImageEntry()); + ImageData data = new ImageData(list); + ImageDataUpdateListener listener = new ImageDataUpdateListener() { + @Override + public void selectedImageChanged(ImageData data) {} + }; + new Expectations(listener) {{ + listener.selectedImageChanged(data); times = 2; + }}; + data.addImageDataUpdateListener(listener); + data.selectFirstImage(); + data.removeSelectedImage(); + } + + @Test + public void testMergeFrom() { + ImageEntry image = new ImageEntry(new File("test2")); + List list1 = this.getOneImage(); + list1.add(image); + List list2 = this.getOneImage(); + list2.add(new ImageEntry(new File("test3"))); + + ImageData data = new ImageData(list1); + data.setSelectedImage(list1.get(0)); + ImageData data2 = new ImageData(list2); + + new MockUp() { + @Mock + public void sort(List o) { + list1.remove(image); + list1.add(image); + } + }; + + data.mergeFrom(data2); + assertEquals(3, data.getImages().size()); + assertEquals(list1.get(0), data.getSelectedImage()); + } + + @Test + public void testMergeFromSelectedImage() { + ImageEntry image = new ImageEntry(new File("test2")); + List list1 = this.getOneImage(); + list1.add(image); + List list2 = this.getOneImage(); + + ImageData data = new ImageData(list1); + ImageData data2 = new ImageData(list2); + data2.setSelectedImage(list2.get(0)); + + data.mergeFrom(data2); + assertEquals(3, data.getImages().size()); + assertEquals(list2.get(0), data.getSelectedImage()); + } +}