Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: JOSM/Mapillary
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: v1769
Choose a base ref
...
head repository: JOSM/Mapillary
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: v1770
Choose a head ref
  • 1 commit
  • 4 files changed
  • 1 contributor

Commits on Aug 8, 2024

  1. Don't update node positions and nodes in ways in vector dataset

    This can cause issues where a visible node cannot be clicked since it
    does not exist in the dataset.
    
    In addition, some lint issues have been fixed.
    
    Signed-off-by: Taylor Smock <tsmock@meta.com>
    tsmock committed Aug 8, 2024

    Verified

    This commit was signed with the committer’s verified signature.
    tsmock Taylor Smock
    Copy the full SHA
    772dd5e View commit details
35 changes: 35 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
@@ -71,6 +71,41 @@
</archive>
</configuration>
</plugin>
<plugin>
<groupId>com.diffplug.spotless</groupId>
<artifactId>spotless-maven-plugin</artifactId>
<version>2.43.0</version>
<configuration>
<java>
<!-- Avoid large formatting commits. -->
<ratchetFrom>origin/master</ratchetFrom>
<eclipse>
<version>4.21.0</version>
<file>${project.basedir}/config/format/code_format.xml</file>
</eclipse>
<removeUnusedImports/>
<endWithNewline/>
<indent>
<spaces>true</spaces>
<spacesPerTab>4</spacesPerTab>
</indent>
<importOrder>
<order>java,javax,</order>
</importOrder>
<trimTrailingWhitespace/>
<licenseHeader>
<content>// License: GPL. For details, see LICENSE file.</content>
</licenseHeader>
</java>
</configuration>
<executions>
<execution>
<goals>
<goal>check</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>
</project>
Original file line number Diff line number Diff line change
@@ -11,6 +11,7 @@
import java.util.Optional;
import java.util.concurrent.locks.Lock;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import java.util.stream.Stream;

import javax.swing.event.MouseInputAdapter;
@@ -85,14 +86,14 @@ private static void mouseClickedInner(final MouseEvent e, final MVTLayer layer,
LatLon click = MainApplication.getMap().mapView.getLatLon(e.getX(), e.getY());
Optional<INode> nodes = searchNodes(layer, searchBBox).stream().distinct().filter(AbstractPrimitive::isDrawable)
.filter(INode.class::isInstance).map(INode.class::cast).filter(i -> i.getUniqueId() > 0)
.sorted(Comparator.comparingDouble(i -> i.distanceSq(click))).findFirst();
.min(Comparator.comparingDouble(i -> i.distanceSq(click)));
if (nodes.isPresent()) {
// This is needed since Mapillary ids are only unique within a tile.
layer.getData().setSelected(Collections.singletonList(nodes.get()));
} else if (layer instanceof MapillaryLayer) {
} else if (layer instanceof MapillaryLayer mapillaryLayer) {
if (e.getClickCount() >= MapillaryProperties.DESELECT_CLICK_COUNT.get()) {
layer.getData().clearSelection();
((MapillaryLayer) layer).setCurrentImage(null);
mapillaryLayer.setCurrentImage(null);
}
} else {
Collection<VectorWay> ways = layer.getData().searchWays(searchBBox);
@@ -115,64 +116,64 @@ public void mouseMoved(MouseEvent e) {
final BBox searchBBox = getSmallBBox(e.getPoint());
for (MVTLayer layer : MainApplication.getLayerManager().getLayersOfType(MVTLayer.class)) {
if (layer instanceof MapillaryLayer || layer instanceof PointObjectLayer) {
final Lock readLock = layer.getData().getReadLock();
if (readLock.tryLock()) {
layer.getData().getPrimitivesById(layer.getData().getHighlighted().toArray(new PrimitiveId[0]))
.forEach(vectorPrimitive -> vectorPrimitive.setHighlighted(false));
try {
if (searchBBox == null) {
layer.getData().setHighlighted(Collections.emptyList());
continue;
}
if (layer instanceof MapillaryLayer) {
MapillaryNode node = ((MapillaryLayer) layer).getImage();
if (node != null && node.getSequence() != null) {
node.getSequence().getNodes().forEach(n -> n.setHighlighted(false));
}
}
List<? extends AbstractPrimitive> nodes = searchNodes(layer, searchBBox);
if (!nodes.isEmpty()) {
layer.getData().setHighlighted(nodes.stream().filter(IPrimitive::isDrawable)
.map(IPrimitive::getPrimitiveId).collect(Collectors.toSet()));
nodes.forEach(node -> node.setHighlighted(true));
layer.invalidate();
continue;
}
Collection<VectorWay> ways = SubclassFilteredCollection.filter(
layer.getData().searchWays(searchBBox),
way -> convertToWaySegmentBBox(way).anyMatch(searchBBox::intersects));
if (!ways.isEmpty()) {
layer.getData().setHighlighted(
ways.stream().map(IPrimitive::getPrimitiveId).collect(Collectors.toSet()));
layer.invalidate();
} else if (!layer.getData().getHighlighted().isEmpty()) {
layer.getData().setHighlighted(Collections.emptyList());
}
} finally {
readLock.unlock();
performHighlight(layer, searchBBox);
}
}
}

private static void performHighlight(MVTLayer layer, BBox searchBBox) {
final Lock readLock = layer.getData().getReadLock();
if (readLock.tryLock()) {
layer.getData().getPrimitivesById(layer.getData().getHighlighted().toArray(new PrimitiveId[0]))
.forEach(vectorPrimitive -> vectorPrimitive.setHighlighted(false));
try {
if (searchBBox == null) {
layer.getData().setHighlighted(Collections.emptyList());
return;
}
if (layer instanceof MapillaryLayer mapillaryLayer) {
MapillaryNode node = mapillaryLayer.getImage();
if (node != null && node.getSequence() != null) {
node.getSequence().getNodes().forEach(n -> n.setHighlighted(false));
}
}
List<? extends AbstractPrimitive> nodes = searchNodes(layer, searchBBox);
if (!nodes.isEmpty()) {
layer.getData().setHighlighted(nodes.stream().filter(IPrimitive::isDrawable)
.map(IPrimitive::getPrimitiveId).collect(Collectors.toSet()));
nodes.forEach(node -> node.setHighlighted(true));
layer.invalidate();
return;
}
Collection<VectorWay> ways = SubclassFilteredCollection.filter(layer.getData().searchWays(searchBBox),
way -> convertToWaySegmentBBox(way).anyMatch(searchBBox::intersects));
if (!ways.isEmpty()) {
layer.getData()
.setHighlighted(ways.stream().map(IPrimitive::getPrimitiveId).collect(Collectors.toSet()));
layer.invalidate();
} else if (!layer.getData().getHighlighted().isEmpty()) {
layer.getData().setHighlighted(Collections.emptyList());
}
} finally {
readLock.unlock();
}
}
}

private static <N extends INode, W extends IWay<N>> Stream<BBox> convertToWaySegmentBBox(W way) {
// TODO: Stream.iterate would be good here (Java 9)
Stream.Builder<BBox> builder = Stream.builder();
for (int i = 0; i < way.getNodesCount() - 1; i++) {
return IntStream.iterate(0, i -> i < way.getNodesCount() - 1, i -> i + 1).mapToObj(i -> {
BBox bbox = new BBox(way.getNode(i));
bbox.add(way.getNode(i + 1));
builder.add(bbox);
}
return builder.build();
return bbox;
});
}

private static List<? extends AbstractPrimitive> searchNodes(MVTLayer layer, BBox searchBBox) {
if (searchBBox == null) {
return Collections.emptyList();
}
if (layer instanceof MapillaryLayer) {
final MapillaryNode image = ((MapillaryLayer) layer).getImage();
if (layer instanceof MapillaryLayer mapillaryLayer) {
final MapillaryNode image = mapillaryLayer.getImage();
if (image != null) {
final List<AbstractPrimitive> nodes;
if (image.getSequence() != null) {
Original file line number Diff line number Diff line change
@@ -351,15 +351,15 @@ private void paintWithLock(final Graphics2D g, final MapView mv, final Bounds bo
final double distPer100Pixel = mv.getDist100Pixel();
final String sequenceKey = MapillaryImageUtils.getSequenceKey(selectedImage);
for (IWay<?> seq : getData().searchWays(box.toBBox()).stream()
.sorted(Comparator.comparingInt(IWay::getRawTimestamp)).distinct().collect(Collectors.toList())) {
.sorted(Comparator.comparingInt(IWay::getRawTimestamp)).distinct().toList()) {
if (!Objects.equals(sequenceKey, MapillarySequenceUtils.getKey(seq))) {
drawSequence(g, mv, seq, selectedImage, originalTransform, distPer100Pixel);
}
}

// Paint single images (see GH #219)
for (INode node : getData().searchNodes(box.toBBox()).stream().filter(node -> !node.isReferredByWays(1))
.sorted(Comparator.comparingInt(INode::getRawTimestamp)).distinct().collect(Collectors.toList())) {
.sorted(Comparator.comparingInt(INode::getRawTimestamp)).distinct().toList()) {
drawImageMarker(originalTransform, selectedImage, g, node, distPer100Pixel, false, null);
}

@@ -435,9 +435,7 @@ private Color getColor(final INode node) {
MapillaryImageUtils.getDate(node);
}
return this.dateScale.getColor(node.getRawTimestamp());
case VELOCITY_FOOT:
case VELOCITY_BIKE:
case VELOCITY_CAR:
case VELOCITY_FOOT, VELOCITY_BIKE, VELOCITY_CAR:
final double normalized = calculateVelocity(node);
if (Double.isNaN(normalized)) {
return this.velocityScale.getNoDataColor();
@@ -545,9 +543,9 @@ private void drawImageMarker(final AffineTransform originalTransform, final INod

if (offset && selectedImg == img) {
final ILatLon drawnCoordinates = OffsetUtils.getOffsetLocation(img);
final Point offsetP = drawnCoordinates instanceof INode
final Point offsetP = drawnCoordinates instanceof INode node
// INode implementations may optimize getEastNorth, so prefer that where possible.
? mv.getPoint(((INode) drawnCoordinates).getEastNorth())
? mv.getPoint(node.getEastNorth())
: mv.getPoint(drawnCoordinates);
paintDirectionIndicator(g, directionC, img, originalTransform, offsetP, i);
g.setTransform(originalTransform);
@@ -689,11 +687,6 @@ public String getChangesetSourceTag() {
return isApplicable() ? "Mapillary" : null;
}

@Override
public boolean isMergable(Layer other) {
return false;
}

@Override
public void mergeFrom(Layer from) {
throw new UnsupportedOperationException("This layer does not support merging yet");
@@ -750,7 +743,7 @@ public void layerAdded(LayerAddEvent e) {
@Override
public void layerRemoving(LayerRemoveEvent e) {
List<DataSet> currentDataSets = MainApplication.getLayerManager().getLayersOfType(OsmDataLayer.class).stream()
.map(OsmDataLayer::getDataSet).collect(Collectors.toList());
.map(OsmDataLayer::getDataSet).toList();
for (Map.Entry<DataSet, Set<INode>> entry : imageViewedMap.entrySet()) {
if (!currentDataSets.contains(entry.getKey())) {
imageViewedMap.remove(entry.getKey());
@@ -852,7 +845,7 @@ public void selectionChanged(
}
}

new MapillaryNodeDownloader(node, MapillaryLayer.getInstance()::setCurrentImage).execute();
new MapillaryNodeDownloader(node, this::setCurrentImage).execute();
}
}

@@ -899,15 +892,14 @@ public List<? extends IImageEntry<?>> getSelection() {

@Override
public boolean containsImage(IImageEntry<?> imageEntry) {
if (!(imageEntry instanceof MapillaryImageEntry)) {
if (!(imageEntry instanceof MapillaryImageEntry entry)) {
return false;
}
MapillaryImageEntry entry = (MapillaryImageEntry) imageEntry;
if (Objects.equals(entry.getImage(), this.image)) {
return true;
}
if (entry.getImage() instanceof VectorNode) {
return this.getData().containsNode((VectorNode) entry.getImage());
if (entry.getImage()instanceof VectorNode vectorNode) {
return this.getData().containsNode(vectorNode);
}
return this.getData().getPrimitiveById(entry.getImage().getPrimitiveId()) != null;
}
Original file line number Diff line number Diff line change
@@ -9,15 +9,11 @@
import java.util.Objects;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.locks.Lock;
import java.util.function.Consumer;

import org.openstreetmap.josm.data.vector.VectorDataSet;
import org.openstreetmap.josm.data.vector.VectorNode;
import org.openstreetmap.josm.plugins.mapillary.data.mapillary.MapillaryDownloader;
import org.openstreetmap.josm.plugins.mapillary.data.mapillary.MapillaryNode;
import org.openstreetmap.josm.plugins.mapillary.data.mapillary.MapillarySequence;
import org.openstreetmap.josm.plugins.mapillary.gui.layer.MapillaryLayer;
import org.openstreetmap.josm.plugins.mapillary.utils.MapillaryImageUtils;
import org.openstreetmap.josm.spi.preferences.Config;
import org.openstreetmap.josm.tools.JosmRuntimeException;
@@ -181,24 +177,6 @@ protected void done() {
@Override
protected void process(List<MapillarySequence> chunks) {
super.process(chunks);
VectorDataSet ds = MapillaryLayer.getInstance().getData();
// Technically a writeLock would be better, but we cannot get that with the current VectorDataSet
// implementation.
final Lock dsLock = ds.getReadLock();
try {
dsLock.lock();
for (MapillarySequence seq : chunks) {
for (MapillaryNode oldNode : seq.getNodes()) {
VectorNode oldPrimitive = (VectorNode) ds.getPrimitiveById(oldNode);
if (oldPrimitive != null) {
oldPrimitive.putAll(oldNode.getKeys());
oldPrimitive.setCoor(oldNode.getCoor());
}
}
}
} finally {
dsLock.unlock();
}
// The counter just avoids many resets of the imagery window in short order
if (!chunks.isEmpty() && counter.getAndAdd(chunks.size()) < 3) {
this.updater.accept(chunks.get(chunks.size() - 1));