Skip to content

Commit ecc859d

Browse files
tsmocktsmock
committed
Fix wildly wrong computed locations (lat, lon, alt)
This adds a new property, "mapillary.assumed_hdop", which is the HDOP assumed for most cameras (in meters). VDOP is assumed to be 3x the HDOP. The default HDOP is 6m. Signed-off-by: Taylor Smock <[email protected]>
1 parent 6a749c2 commit ecc859d

File tree

4 files changed

+70
-13
lines changed

4 files changed

+70
-13
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
## What's Changed
22
### [Unreleased](https://github.com/JOSM/Mapillary/compare/v2.0.0-beta.2...master)
33
* Lint fixes (PMD)
4+
* JOSM [#21871](https://josm.openstreetmap.de/ticket/21871): Computed altitude and location can be wildly wrong
45

56
### [v2.0.0-beta.2](https://github.com/JOSM/Mapillary/compare/v2.0.0-beta.1...v2.0.0-beta.2)
67
* JOSM [#20274](https://josm.openstreetmap.de/ticket/20274): Cannot zoom in to detail.

‎src/main/java/org/openstreetmap/josm/plugins/mapillary/gui/layer/geoimage/MapillaryImageEntry.java

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import java.time.Instant;
1919
import java.time.ZoneOffset;
2020
import java.util.ArrayList;
21-
import java.util.Arrays;
2221
import java.util.Collection;
2322
import java.util.Date;
2423
import java.util.List;
@@ -37,6 +36,7 @@
3736
import javax.swing.SwingUtilities;
3837

3938
import org.apache.commons.jcs3.access.CacheAccess;
39+
import org.openstreetmap.josm.actions.ExpertToggleAction;
4040
import org.openstreetmap.josm.data.cache.BufferedImageCacheEntry;
4141
import org.openstreetmap.josm.data.cache.JCSCacheManager;
4242
import org.openstreetmap.josm.data.coor.ILatLon;
@@ -66,6 +66,7 @@
6666
import org.openstreetmap.josm.plugins.mapillary.utils.VectorDataSetUtils;
6767
import org.openstreetmap.josm.tools.JosmRuntimeException;
6868
import org.openstreetmap.josm.tools.Logging;
69+
import org.openstreetmap.josm.tools.Utils;
6970
import org.openstreetmap.josm.tools.date.DateUtils;
7071

7172
public class MapillaryImageEntry
@@ -380,15 +381,29 @@ public Double getSpeed() {
380381
@Override
381382
@Nullable
382383
public Double getElevation() {
383-
for (MapillaryImageUtils.ImageProperties property : Arrays.asList(
384-
MapillaryImageUtils.ImageProperties.COMPUTED_ALTITUDE, MapillaryImageUtils.ImageProperties.ALTITUDE)) {
385-
if (this.image.hasKey(property.toString())) {
386-
try {
387-
return Double.parseDouble(this.image.get(property.toString()));
388-
} catch (NumberFormatException e) {
389-
Logging.trace(e);
384+
// Sometimes the computed altitude is very wrong. See JOSM #21871
385+
try {
386+
final Double computedAltitude = Optional
387+
.ofNullable(this.image.get(MapillaryImageUtils.ImageProperties.COMPUTED_ALTITUDE.toString()))
388+
.map(Double::parseDouble).orElse(null);
389+
final Double originalAltitude = Optional
390+
.ofNullable(this.image.get(MapillaryImageUtils.ImageProperties.ALTITUDE.toString()))
391+
.map(Double::parseDouble).orElse(null);
392+
// Assume that the max hdop is 3m, and vdop is 3x that.
393+
if (computedAltitude != null && originalAltitude != null) {
394+
if (Math.abs(computedAltitude - originalAltitude) < 3 * MapillaryProperties.ASSUMED_HDOP.get()) {
395+
return computedAltitude;
390396
}
397+
return originalAltitude;
391398
}
399+
// Fall back to whichever is not null, preferring computedAltitude
400+
return Utils.firstNonNull(computedAltitude, originalAltitude);
401+
} catch (NumberFormatException e) {
402+
// Experts hopefully know how to file a bug report.
403+
if (ExpertToggleAction.isExpert()) {
404+
throw e;
405+
}
406+
Logging.debug(e);
392407
}
393408
return null;
394409
}
@@ -476,6 +491,13 @@ public Projections getProjectionType() {
476491
return IImageEntry.super.getProjectionType();
477492
}
478493

494+
/**
495+
* Refresh the image in the image viewer
496+
*/
497+
public void reload() {
498+
this.updateImageEntry();
499+
}
500+
479501
private void updateImageEntry() {
480502
// Clone this entry. Needed to ensure that the image display refreshes.
481503
final MapillaryImageEntry temporaryImageEntry = new MapillaryImageEntry(this);

‎src/main/java/org/openstreetmap/josm/plugins/mapillary/utils/MapillaryProperties.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ public final class MapillaryProperties {
4343
public static final StringProperty START_DIR = new StringProperty("mapillary.start-directory",
4444
System.getProperty("user.home"));
4545

46+
/** The assumed HDOP for Mapillary images. VDOP is assumed to be 3x this. Meters. */
47+
public static final IntegerProperty ASSUMED_HDOP = new IntegerProperty("mapillary.assumed_hdop", 6);
48+
4649
/**
4750
* The number of times the help popup for the {@link ImageInfoPanel} will be displayed.
4851
* But regardless of this number, the popup will only show up at most once between two (re)starts of JOSM.

‎src/main/java/org/openstreetmap/josm/plugins/mapillary/utils/api/JsonImageDetailsDecoder.java

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import java.util.Collections;
77
import java.util.Map;
88
import java.util.Objects;
9+
import java.util.Optional;
910
import java.util.stream.Collectors;
1011

1112
import javax.annotation.Nonnull;
@@ -22,7 +23,10 @@
2223
import org.openstreetmap.josm.data.osm.TagMap;
2324
import org.openstreetmap.josm.data.vector.VectorDataSet;
2425
import org.openstreetmap.josm.data.vector.VectorNode;
26+
import org.openstreetmap.josm.gui.MainApplication;
27+
import org.openstreetmap.josm.gui.layer.geoimage.ImageViewerDialog;
2528
import org.openstreetmap.josm.plugins.mapillary.gui.layer.MapillaryLayer;
29+
import org.openstreetmap.josm.plugins.mapillary.gui.layer.geoimage.MapillaryImageEntry;
2630
import org.openstreetmap.josm.plugins.mapillary.model.ImageDetection;
2731
import org.openstreetmap.josm.plugins.mapillary.utils.MapillaryImageUtils;
2832
import org.openstreetmap.josm.plugins.mapillary.utils.MapillaryKeys;
@@ -94,11 +98,7 @@ private static Pair<String, VectorNode> decodeImageInfo(@Nullable final JsonObje
9498
return null;
9599
}
96100
final long id = Long.parseLong(key);
97-
final LatLon coordinates = JsonDecoder.decodeLatLon(json.getJsonObject(
98-
useComputedData && json.containsKey(MapillaryImageUtils.ImageProperties.COMPUTED_GEOMETRY.toString())
99-
? MapillaryImageUtils.ImageProperties.COMPUTED_GEOMETRY.toString()
100-
: MapillaryImageUtils.ImageProperties.GEOMETRY.toString())
101-
.getJsonArray("coordinates"));
101+
final LatLon coordinates = getCoordinates(json, useComputedData);
102102
final BBox searchBBox = new BBox(coordinates);
103103
searchBBox.addLatLon(coordinates, 0.001);
104104
VectorNode image = VectorDataSetUtils.tryRead(data, () -> data.getPrimitiveById(id, OsmPrimitiveType.NODE))
@@ -110,6 +110,21 @@ private static Pair<String, VectorNode> decodeImageInfo(@Nullable final JsonObje
110110

111111
if (coordinates != null) {
112112
image.setCoor(coordinates);
113+
// Force the dataset to recache the location
114+
VectorDataSetUtils.tryWrite(data, () -> {
115+
if (data.containsNode(image)) {
116+
data.removePrimitive(image);
117+
}
118+
data.addPrimitive(image);
119+
if (Optional.ofNullable(MainApplication.getMap())
120+
.map(map -> map.getToggleDialog(ImageViewerDialog.class)).isPresent()
121+
&& ImageViewerDialog.getCurrentImage() instanceof MapillaryImageEntry) {
122+
MapillaryImageEntry entry = (MapillaryImageEntry) ImageViewerDialog.getCurrentImage();
123+
if (coordinates.equals(entry.getPos())) {
124+
entry.reload();
125+
}
126+
}
127+
});
113128
}
114129
for (Map.Entry<String, String> entry : JsonTagMapDecoder.getTagMap(json).entrySet()) {
115130
image.put(entry.getKey(), entry.getValue());
@@ -139,6 +154,22 @@ private static Pair<String, VectorNode> decodeImageInfo(@Nullable final JsonObje
139154
return null;
140155
}
141156

157+
private static LatLon getCoordinates(JsonObject json, boolean useComputedData) {
158+
final LatLon originalCoordinates = JsonDecoder.decodeLatLon(
159+
json.getJsonObject(MapillaryImageUtils.ImageProperties.GEOMETRY.toString()).getJsonArray("coordinates"));
160+
if (useComputedData && json.containsKey(MapillaryImageUtils.ImageProperties.COMPUTED_GEOMETRY.toString())) {
161+
final LatLon computedCoordinates = JsonDecoder
162+
.decodeLatLon(json.getJsonObject(MapillaryImageUtils.ImageProperties.COMPUTED_GEOMETRY.toString())
163+
.getJsonArray("coordinates"));
164+
if (computedCoordinates != null && originalCoordinates != null && computedCoordinates
165+
.greatCircleDistance(originalCoordinates) < MapillaryProperties.ASSUMED_HDOP.get()) {
166+
return computedCoordinates;
167+
}
168+
return Utils.firstNonNull(originalCoordinates, computedCoordinates);
169+
}
170+
return originalCoordinates;
171+
}
172+
142173
/**
143174
* Create a new image from a json
144175
*

0 commit comments

Comments
 (0)