Ticket #11710: 11710-V2.patch

src/org/openstreetmap/josm/data/gpx/GpxImageEntry.java

@@ -571 +571 @@
             elevation = tmp.elevation;
             gpsTime = tmp.gpsTime;
             exifImgDir = tmp.exifImgDir;
-            isNewGpsData = tmp.isNewGpsData;
+            isNewGpsData = isNewGpsData || tmp.isNewGpsData;
             tmp = null;
         }
         tmpUpdated();

src/org/openstreetmap/josm/data/gpx/GpxImageCorrelation.java

@@ -229 +229 @@
                     break;
                 }
                 long tagms = TimeUnit.MINUTES.toMillis(tagTime);
-                if (curTmp.getPos() == null &&
+                if (!curTmp.hasNewGpsData() &&
                         (Math.abs(time - curWpTime) <= tagms
                         || Math.abs(prevWpTime - time) <= tagms)) {
                     if (prevWp != null && time < curWpTime - half) {
@@ -255 +255 @@
                 if (imgTime < prevWpTime) {
                     break;
                 }
-                if (curTmp.getPos() == null) {
+                if (!curTmp.hasNewGpsData()) {
                     // The values of timeDiff are between 0 and 1, it is not seconds but a dimensionless variable
                     double timeDiff = (double) (imgTime - prevWpTime) / Math.abs(curWpTime - prevWpTime);
                     curTmp.setPos(prevWp.getCoor().interpolate(curWp.getCoor(), timeDiff));

src/org/openstreetmap/josm/gui/layer/geoimage/CorrelateGpxWithImages.java

@@ -1090 +1090 @@
             // Construct a list of images that have a date, and sort them on the date.
             List<ImageEntry> dateImgLst = getSortedImgList();
             // Create a temporary copy for each image
-            for (ImageEntry ie : dateImgLst) {
-                ie.createTmp();
-                ie.getTmp().setPos(null);
-            }
+            dateImgLst.forEach(i -> {
+                i.createTmp();
+                i.getTmp().unflagNewGpsData();
+            });
 
             GpxDataWrapper selGpx = selectedGPX(false);
             if (selGpx == null)

test/unit/org/openstreetmap/josm/data/gpx/GpxImageCorrelationTest.java

@@ -64 +64 @@
         i0.createTmp();
         final GpxImageEntry i1 = new GpxImageEntry();
         i1.setExifTime(DateUtils.parseInstant("2016:01:03 12:04:01"));
+        i1.setPos(new CachedLatLon(2, 3)); //existing position inside the track, should always be overridden
         i1.createTmp();
         final GpxImageEntry i2 = new GpxImageEntry();
         i2.setExifTime(DateUtils.parseInstant("2016:01:03 12:04:57"));
@@ -80 +81 @@
         final GpxImageEntry i6 = new GpxImageEntry(); //between two tracks, closer to second (more than 1 minute from any track)
         i6.setExifTime(DateUtils.parseInstant("2016:01:03 12:07:45"));
         i6.createTmp();
+        final GpxImageEntry i7 = new GpxImageEntry();
+        i7.setExifTime(DateUtils.parseInstant("2021:01:01 00:00:00"));
+        i7.setPos(new CachedLatLon(1, 2)); //existing position outside the track, should never be null (either overridden or keeping old position, see #11710)
+        i7.createTmp();
 
-        List<GpxImageEntry> images = Arrays.asList(ib, i0, i1, i2, i3, i4, i5, i6);
+        List<GpxImageEntry> images = Arrays.asList(ib, i0, i1, i2, i3, i4, i5, i6, i7);
 
         // TEST #1: default settings
         // tag images within 2 minutes to tracks/segments, interpolate between segments only
@@ -98 +103 @@
                 i5.getPos()); // tagged to last WP of first track, because closer and within 2 min (default setting)
         assertEquals(new CachedLatLon(47.20138901844621, 8.774476982653141),
                 i6.getPos()); // tagged to first WP of second track, because closer and within 2 min (default setting)
-        assertFalse(ib.hasNewGpsData());
+        assertEquals(new CachedLatLon(1, 2), i7.getPos()); //existing EXIF data is kept
+        assertFalse(ib.hasNewGpsData() || i7.hasNewGpsData());
         assertTrue(i0.hasNewGpsData() && i1.hasNewGpsData() && i2.hasNewGpsData() && i3.hasNewGpsData()
                 && i4.hasNewGpsData() && i5.hasNewGpsData() && i6.hasNewGpsData());
         // First waypoint has no speed in matchGpxTrack(). Speed is calculated
@@ -147 +153 @@
 
         // TEST #3: Disable all interpolation and allow tagging within 1 minute of a track. i0-i5 are tagged.
         // i6 will not be tagged, because it's 68 seconds away from the next waypoint in either direction
+        // i7 will keep the old position
 
         s.putBoolean("geoimage.trk.tag", true);
         s.putBoolean("geoimage.trk.tag.time", true);
@@ -165 +172 @@
         assertEquals(new CachedLatLon(47.197568312311816, 8.790292849679897), i4.getPos());
         assertEquals(new CachedLatLon(47.19819249585271, 8.78536943346262), i5.getPos());
         assertEquals(null, i6.getPos());
+        assertEquals(new CachedLatLon(1, 2), i7.getPos());
 
         clearTmp(images);
 
         // TEST #4: Force tagging (parameter forceTags=true, no change of configuration). All images will be tagged
         // i5-i6 will now be interpolated, therefore it will have an elevation and different coordinates than in tests above
+        // i7 will be at the end of the track
 
-        assertEquals(8, GpxImageCorrelation.matchGpxTrack(images, gpx, 0, true));
+        assertEquals(9, GpxImageCorrelation.matchGpxTrack(images, gpx, 0, true));
         assertEquals(new CachedLatLon(47.19286847859621, 8.79732714034617), ib.getPos());
         assertEquals(new CachedLatLon(47.19286847859621, 8.79732714034617), i0.getPos());
         assertEquals(new CachedLatLon(47.196979885920882, 8.79541271366179), i1.getPos());
@@ -184 +193 @@
         assertEquals(Double.valueOf(447.894014085), i5.getElevation(), 0.000001);
         assertEquals(Double.valueOf(437.395070423), i6.getElevation(), 0.000001);
 
+        assertEquals(new CachedLatLon(47.20126815140247, 8.77192972227931), i7.getPos());
+
         clearTmp(images);
 
         // TEST #5: Force tagging (parameter forceTags=false, but configuration changed).
@@ -201 +212 @@
         s.putBoolean("geoimage.seg.int.time", false);
         s.putBoolean("geoimage.seg.int.dist", false);
 
-        assertEquals(8, GpxImageCorrelation.matchGpxTrack(images, gpx, 0, false));
+        assertEquals(9, GpxImageCorrelation.matchGpxTrack(images, gpx, 0, false));
         assertEquals(new CachedLatLon(47.19286847859621, 8.79732714034617), ib.getPos());
         assertEquals(new CachedLatLon(47.19286847859621, 8.79732714034617), i0.getPos());
         assertEquals(new CachedLatLon(47.196979885920882, 8.79541271366179), i1.getPos());
@@ -214 +225 @@
         assertEquals(Double.valueOf(447.894014085), i5.getElevation(), 0.000001);
         assertEquals(Double.valueOf(437.395070423), i6.getElevation(), 0.000001);
 
+        assertEquals(new CachedLatLon(47.20126815140247, 8.77192972227931), i7.getPos());
+
         clearTmp(images);
 
         // TEST #6: Disable tagging but allow interpolation when tracks are less than 500m apart. i0-i4 are tagged.