Ticket #13361: 13361_v2.patch

File 13361_v2.patch, 22.3 KB (added by GerdP, 9 years ago)

  • src/org/openstreetmap/josm/data/osm/BBox.java

     
    99import org.openstreetmap.josm.data.coor.QuadTiling;
    1010import org.openstreetmap.josm.tools.Utils;
    1111
     12/**
     13 * A bounding box with latitude  / longitude coordinates
     14 */
    1215public class BBox {
     16    private double xmin;
     17    private double xmax;
     18    private double ymin;
     19    private double ymax;
    1320
    14     private double xmin = Double.POSITIVE_INFINITY;
    15     private double xmax = Double.NEGATIVE_INFINITY;
    16     private double ymin = Double.POSITIVE_INFINITY;
    17     private double ymax = Double.NEGATIVE_INFINITY;
     21    /**
     22     * Constructs a new (invalid) BBox
     23     */
     24    public BBox() {
     25        setInvalid();
     26    }
    1827
    1928    /**
    2029     * Constructs a new {@code BBox} defined by a single point.
    … …  
    2433     * @since 6203
    2534     */
    2635    public BBox(final double x, final double y) {
     36        if (Double.isNaN(x) || Double.isNaN(y))
     37            return; // use default which is an invalid BBox
    2738        xmax = xmin = x;
    2839        ymax = ymin = y;
    2940        sanity();
    … …  
    5263        this.ymax = copy.ymax;
    5364    }
    5465
     66    /**
     67     * Create BBox so that {@code this.bounds(ax,ay)} and {@code this.bounds(bx,by)} will both return true
     68     * @param ax left or right X value
     69     * @param ay top or bottom Y value
     70     * @param bx left or right X value
     71     * @param by top or bottom Y value
     72     */
    5573    public BBox(double ax, double ay, double bx, double by) {
     74        if (Double.isNaN(ax) || Double.isNaN(ay) || Double.isNaN(bx) || Double.isNaN(by))
     75            return; // use default which is an invalid BBox
    5676
    5777        if (ax > bx) {
    5878            xmax = ax;
    … …  
    7393        sanity();
    7494    }
    7595
     96    /**
     97     * Create BBox for all nodes of the way with known coordinates.
     98     * If no node has a known coordinate, an invalid BBox is returned.
     99     * @param w the way
     100     */
    76101    public BBox(Way w) {
    77         for (Node n : w.getNodes()) {
    78             LatLon coor = n.getCoor();
    79             if (coor == null) {
    80                 continue;
    81             }
    82             add(coor);
    83         }
     102        setInvalid();
     103        w.getNodes().stream().forEach((n) -> add(n.getCoor()));
    84104    }
    85105
     106    /**
     107     * Create BBox for a node. An invalid BBox is returned if the coordinates are not known.
     108     * @param n the node
     109     */
    86110    public BBox(Node n) {
    87         LatLon coor = n.getCoor();
    88         if (coor == null) {
    89             xmin = xmax = ymin = ymax = 0;
    90         } else {
    91             xmin = xmax = coor.lon();
    92             ymin = ymax = coor.lat();
    93         }
     111        setInvalid();
     112        add(n.getCoor());
    94113    }
    95114
     115    private void setInvalid() {
     116        xmin = ymin = Double.POSITIVE_INFINITY;
     117        xmax = ymax = Double.NEGATIVE_INFINITY;
     118    }
     119
    96120    private void sanity() {
    97121        if (xmin < -180.0) {
    98122            xmin = -180.0;
    … …  
    108132        }
    109133    }
    110134
     135    /**
     136     * Add a point to an existing BBox. Extends this bbox if necessary so that this.bounds(c) will return true
     137     * if c is a valid LatLon instance.
     138     * @param c a LatLon point
     139     */
    111140    public final void add(LatLon c) {
    112         add(c.lon(), c.lat());
     141        if (c != null && c.isValid())
     142            add(c.lon(), c.lat());
    113143    }
    114144
    115145    /**
    … …  
    118148     * @param y Y coordinate
    119149     */
    120150    public final void add(double x, double y) {
     151        if (Double.isNaN(x) || Double.isNaN(y))
     152            return;
    121153        xmin = Math.min(xmin, x);
    122154        xmax = Math.max(xmax, x);
    123155        ymin = Math.min(ymin, y);
    … …  
    125157        sanity();
    126158    }
    127159
    128     public final void add(BBox box) {
    129         xmin = Math.min(xmin, box.xmin);
    130         xmax = Math.max(xmax, box.xmax);
    131         ymin = Math.min(ymin, box.ymin);
    132         ymax = Math.max(ymax, box.ymax);
    133         sanity();
     160    /**
     161     * Extends this bbox to include the bbox other. Does nothing if other is not valid.
     162     * @param other a bbox
     163     */
     164    public final void add(BBox other) {
     165        if (other.isValid()) {
     166            xmin = Math.min(xmin, other.xmin);
     167            xmax = Math.max(xmax, other.xmax);
     168            ymin = Math.min(ymin, other.ymin);
     169            ymax = Math.max(ymax, other.ymax);
     170            sanity();
     171        }
    134172    }
    135173
     174    /**
     175     * Extends this bbox to include the bbox of the primitive extended by extraSpace.
     176     * @param primitive an OSM primitive
     177     * @param extraSpace the value to extend the primitives bbox. Unit is in LatLon degrees.
     178     */
    136179    public void addPrimitive(OsmPrimitive primitive, double extraSpace) {
    137180        BBox primBbox = primitive.getBBox();
    138181        add(primBbox.xmin - extraSpace, primBbox.ymin - extraSpace);
    … …  
    278321            && Double.compare(b.xmin, xmin) == 0 && Double.compare(b.ymin, ymin) == 0;
    279322    }
    280323
     324    /**
     325     * @return true if the bbox covers a part of the planets surface
     326     * Height and width must be non-negative, but may (both) be 0.
     327     */
     328    public boolean isValid() {
     329        if (xmin > xmax || ymin > ymax)
     330            return false;
     331        return true;
     332    }
     333
     334    /**
     335     * @return true if the bbox covers a part of the planets surface
     336     */
     337    public boolean isInWorld() {
     338        if (xmin < -180.0 || xmax > 180.0 || ymin < -90.0 || ymax > 90.0)
     339            return false;
     340        return true;
     341    }
     342
    281343    @Override
    282344    public String toString() {
    283345        return "[ x: " + xmin + " -> " + xmax + ", y: " + ymin + " -> " + ymax + " ]";

  • src/org/openstreetmap/josm/data/osm/DataSet.java

     
    508508                throw new DataIntegrityProblemException(
    509509                        tr("Unable to add primitive {0} to the dataset because it is already included", primitive.toString()));
    510510
    511             primitive.updatePosition(); // Set cached bbox for way and relation (required for reindexWay and reinexRelation to work properly)
     511            allPrimitives.add(primitive);
     512            primitive.setDataset(this);
     513            primitive.updatePosition(); // Set cached bbox for way and relation (required for reindexWay and reindexRelation to work properly)
    512514            boolean success = false;
    513515            if (primitive instanceof Node) {
    514516                success = nodes.add((Node) primitive);
    … …  
    519521            }
    520522            if (!success)
    521523                throw new RuntimeException("failed to add primitive: "+primitive);
    522             allPrimitives.add(primitive);
    523             primitive.setDataset(this);
    524524            firePrimitivesAdded(Collections.singletonList(primitive), false);
    525525        } finally {
    526526            endUpdate();

  • src/org/openstreetmap/josm/data/osm/QuadBuckets.java

     
    44import java.util.ArrayList;
    55import java.util.Arrays;
    66import java.util.Collection;
     7import java.util.HashSet;
    78import java.util.Iterator;
    89import java.util.List;
    910import java.util.NoSuchElementException;
    … …  
    3132        throw new AssertionError(s);
    3233    }
    3334
    34     public static final int MAX_OBJECTS_PER_LEVEL = 16;
     35    private static final int MAX_OBJECTS_PER_LEVEL = 16;
    3536
    3637    static class QBLevel<T extends OsmPrimitive> {
    3738        private final int level;
    … …  
    181182        }
    182183
    183184        boolean matches(final T o, final BBox searchBbox) {
    184             if (o instanceof Node) {
    185                 final LatLon latLon = ((Node) o).getCoor();
    186                 // node without coords -> bbox[0,0,0,0]
    187                 return searchBbox.bounds(latLon != null ? latLon : LatLon.ZERO);
    188             }
    189185            return o.getBBox().intersects(searchBbox);
    190186        }
    191187
    … …  
    393389    private QBLevel<T> root;
    394390    private QBLevel<T> searchCache;
    395391    private int size;
     392    private Collection<T> invalidBBoxPrimitives;
    396393
    397394    /**
    398395     * Constructs a new {@code QuadBuckets}.
    … …  
    404401    @Override
    405402    public final void clear() {
    406403        root = new QBLevel<>(this);
     404        invalidBBoxPrimitives = new HashSet<>();
    407405        searchCache = null;
    408406        size = 0;
    409407    }
    … …  
    410408
    411409    @Override
    412410    public boolean add(T n) {
    413         root.add(n);
     411        if (n.getBBox().isValid())
     412            root.add(n);
     413        else
     414            invalidBBoxPrimitives.add(n);
    414415        size++;
    415416        return true;
    416417    }
    … …  
    460461        T t = (T) o;
    461462        searchCache = null; // Search cache might point to one of removed buckets
    462463        QBLevel<T> bucket = root.findBucket(t.getBBox());
    463         if (bucket.removeContent(t)) {
     464        boolean removed = bucket.removeContent(t);
     465        if (!removed)
     466            removed = invalidBBoxPrimitives.remove(o);
     467        if (removed)
    464468            size--;
    465             return true;
    466         } else
    467             return false;
     469        return removed;
    468470    }
    469471
    470472    @Override
    … …  
    471473    public boolean contains(Object o) {
    472474        @SuppressWarnings("unchecked")
    473475        T t = (T) o;
     476        if (!t.getBBox().isValid())
     477            return invalidBBoxPrimitives.contains(o);
    474478        QBLevel<T> bucket = root.findBucket(t.getBBox());
    475479        return bucket != null && bucket.content != null && bucket.content.contains(t);
    476480    }
    … …  
    501505        private QBLevel<T> currentNode;
    502506        private int contentIndex;
    503507        private int iteratedOver;
     508        private Iterator<T> invalidBBoxIterator = invalidBBoxPrimitives.iterator();
     509        boolean fromInvalidBBoxPrimitives;
    504510        QuadBuckets<T> qb;
    505511
    506512        final QBLevel<T> nextContentNode(QBLevel<T> q) {
    … …  
    527533
    528534        @Override
    529535        public boolean hasNext() {
    530             if (this.peek() == null)
    531                 return false;
     536            if (this.peek() == null) {
     537                fromInvalidBBoxPrimitives = true;
     538                return invalidBBoxIterator.hasNext();
     539            }
    532540            return true;
    533541        }
    534542
    … …  
    549557
    550558        @Override
    551559        public T next() {
     560            if (fromInvalidBBoxPrimitives)
     561                return invalidBBoxIterator.next();
    552562            T ret = peek();
    553563            if (ret == null)
    554564                throw new NoSuchElementException();
    … …  
    559569
    560570        @Override
    561571        public void remove() {
    562             // two uses
    563             // 1. Back up to the thing we just returned
    564             // 2. move the index back since we removed
    565             //    an element
    566             contentIndex--;
    567             T object = peek();
    568             if (currentNode.removeContent(object))
     572            if (fromInvalidBBoxPrimitives) {
     573                invalidBBoxIterator.remove();
    569574                qb.size--;
     575            } else {
     576                // two uses
     577                // 1. Back up to the thing we just returned
     578                // 2. move the index back since we removed
     579                //    an element
     580                contentIndex--;
     581                T object = peek();
     582                if (currentNode.removeContent(object))
     583                    qb.size--;
     584
     585            }
    570586        }
    571587    }
    572588
    … …  
    585601        return size == 0;
    586602    }
    587603
     604    /**
     605     * Search for elements in the given bbox.
     606     * @param searchBbox the bbox
     607     * @return a list of elements, maybe empty but never null
     608     */
    588609    public List<T> search(BBox searchBbox) {
    589610        List<T> ret = new ArrayList<>();
     611        if (!searchBbox.isValid()) {
     612            return ret;
     613        }
    590614        // Doing this cuts down search cost on a real-life data set by about 25%
    591615        if (searchCache == null) {
    592616            searchCache = root;

  • src/org/openstreetmap/josm/data/osm/Relation.java

     
    430430        RelationMember[] members = this.members;
    431431
    432432        if (members.length == 0)
    433             return new BBox(0, 0, 0, 0);
     433            return new BBox();
    434434        if (getDataSet() == null)
    435435            return calculateBBox(new HashSet<PrimitiveId>());
    436436        else {
    … …  
    438438                bbox = calculateBBox(new HashSet<PrimitiveId>());
    439439            }
    440440            if (bbox == null)
    441                 return new BBox(0, 0, 0, 0); // No real members
     441                return new BBox(); // No real members
    442442            else
    443443                return new BBox(bbox);
    444444        }
    … …  
    455455        else {
    456456            BBox result = null;
    457457            for (RelationMember rm:members) {
    458                 BBox box = rm.isRelation() ? rm.getRelation().calculateBBox(visitedRelations) : rm.getMember().getBBox();
     458                BBox box = null;
     459                if (rm.isRelation())
     460                    box = rm.getRelation().calculateBBox(visitedRelations);
     461                else if (rm.isWay()) {
     462                    box = rm.getWay().getBBox();
     463                    if (!box.isValid())
     464                        box = null;
     465                } else if (rm.isNode()) {
     466                    Node n = rm.getNode();
     467                    if (n.getCoor() != null)
     468                        box = n.getBBox();
     469                }
    459470                if (box != null) {
    460471                    if (result == null) {
    461472                        result = box;

  • test/unit/org/openstreetmap/josm/data/osm/NodeTest.java

     
    11// License: GPL. For details, see LICENSE file.
    22package org.openstreetmap.josm.data.osm;
    33
     4import static org.junit.Assert.assertEquals;
    45import static org.junit.Assert.assertFalse;
    56import static org.junit.Assert.assertNotNull;
    67import static org.junit.Assert.assertNull;
     8import static org.junit.Assert.assertTrue;
    79
    810import org.junit.BeforeClass;
    911import org.junit.Test;
    … …  
    4143        assertNull(n.getCoor());
    4244        assertFalse(n.isOutsideDownloadArea());
    4345    }
     46
     47    /**
     48     * Test BBox calculation with Node
     49     */
     50    @Test
     51    public void testBBox() {
     52        DataSet ds = new DataSet();
     53        Node n1 = new Node(1);
     54        Node n2 = new Node(2);
     55        Node n3 = new Node(3);
     56        Node n4 = new Node(4);
     57        n1.setIncomplete(true);
     58        n2.setCoor(new LatLon(10, 10));
     59        n3.setCoor(new LatLon(20, 20));
     60        n4.setCoor(new LatLon(90, 180));
     61        ds.addPrimitive(n1);
     62        ds.addPrimitive(n2);
     63        ds.addPrimitive(n3);
     64        ds.addPrimitive(n4);
     65
     66        assertFalse(n1.getBBox().isValid());
     67        assertTrue(n2.getBBox().isValid());
     68        assertTrue(n3.getBBox().isValid());
     69        assertTrue(n4.getBBox().isValid());
     70        BBox box1 = n1.getBBox();
     71        box1.add(n2.getCoor());
     72        assertTrue(box1.isValid());
     73        BBox box2 = n2.getBBox();
     74        box2.add(n1.getCoor());
     75        assertTrue(box2.isValid());
     76        assertEquals(box1, box2);
     77        box1.add(n3.getCoor());
     78        assertTrue(box1.isValid());
     79        assertEquals(box1.getCenter(), new LatLon(15, 15));
     80    }
    4481}

  • test/unit/org/openstreetmap/josm/data/osm/QuadBucketsTest.java

     
    88import java.util.Collection;
    99import java.util.Iterator;
    1010import java.util.List;
     11import java.util.Random;
    1112
    1213import org.fest.reflect.core.Reflection;
    1314import org.fest.reflect.reference.TypeRef;
    … …  
    173174            Assert.assertEquals(count, qbWays.size());
    174175        }
    175176        Assert.assertEquals(0, qbWays.size());
     177
    176178    }
     179
     180    /**
     181     *  Add more data so that quad buckets tree has a few leaves
     182     */
     183    @Test
     184    public void testSplitsWithIncompleteData() {
     185        DataSet ds = new DataSet();
     186        long nodeId = 1;
     187        long wayId = 1;
     188        final int NUM_COMPLETE_WAYS = 300;
     189        final int NUM_INCOMPLETE_WAYS = 10;
     190        final int NUM_NODES_PER_WAY = 20;
     191        final int NUM_INCOMPLETE_NODES = 10;
     192
     193        // force splits in quad buckets
     194        Random random = new Random(31);
     195        for (int i = 0; i < NUM_COMPLETE_WAYS; i++) {
     196            Way w = new Way(wayId++);
     197            List<Node> nodes = new ArrayList<>();
     198            double center = random.nextDouble() * 10;
     199            for (int j = 0; j < NUM_NODES_PER_WAY; j++) {
     200                Node n = new Node(nodeId++);
     201                double lat = random.nextDouble() * 0.001;
     202                double lon = random.nextDouble() * 0.001;
     203                n.setCoor(new LatLon(center + lat, center + lon));
     204                nodes.add(n);
     205                ds.addPrimitive(n);
     206            }
     207            w.setNodes(nodes);
     208            ds.addPrimitive(w);
     209        }
     210        Assert.assertEquals(NUM_COMPLETE_WAYS, ds.getWays().size());
     211        Assert.assertEquals(NUM_COMPLETE_WAYS * NUM_NODES_PER_WAY, ds.getNodes().size());
     212
     213        // add some incomplete nodes
     214        List<Node> incompleteNodes = new ArrayList<>();
     215        for (int i = 0; i < NUM_INCOMPLETE_NODES; i++) {
     216            Node n = new Node(nodeId++);
     217            incompleteNodes.add(n);
     218            n.setIncomplete(true);
     219            ds.addPrimitive(n);
     220        }
     221        Assert.assertEquals(NUM_COMPLETE_WAYS * NUM_NODES_PER_WAY + NUM_INCOMPLETE_NODES, ds.getNodes().size());
     222        // add some incomplete ways
     223        List<Way> incompleteWays = new ArrayList<>();
     224        for (int i = 0; i < NUM_INCOMPLETE_WAYS; i++) {
     225            Way w = new Way(wayId++);
     226            incompleteWays.add(w);
     227            w.setIncomplete(true);
     228            ds.addPrimitive(w);
     229        }
     230        Assert.assertEquals(NUM_COMPLETE_WAYS + NUM_INCOMPLETE_WAYS, ds.getWays().size());
     231
     232        BBox planet = new BBox(-180, -90, 190, 90);
     233        // incomplete ways should not be found with search
     234        Assert.assertEquals(NUM_COMPLETE_WAYS, ds.searchWays(planet).size());
     235        // incomplete ways are only retrieved via iterator or object reference
     236        for (Way w : incompleteWays) {
     237            Assert.assertTrue(ds.getWays().contains(w));
     238        }
     239
     240        QuadBuckets<Way> qb = new QuadBuckets<>();
     241        qb.addAll(ds.getWays());
     242        int count = qb.size();
     243        Assert.assertEquals(count, ds.getWays().size());
     244        Iterator<Way> iter = qb.iterator();
     245        while (iter.hasNext()) {
     246            iter.next();
     247            iter.remove();
     248            count--;
     249            Assert.assertEquals(count, qb.size());
     250        }
     251        Assert.assertEquals(0, qb.size());
     252    }
    177253}

  • test/unit/org/openstreetmap/josm/data/osm/RelationTest.java

     
    7474        w1.addNode(n3);
    7575        Assert.assertEquals(w1.getBBox(), r1.getBBox());
    7676        Assert.assertEquals(w1.getBBox(), r2.getBBox());
     77
     78        // create incomplete node and add it to the relation, this must not change the bbox
     79        BBox oldBBox = r2.getBBox();
     80        Node n4 = new Node();
     81        n4.setIncomplete(true);
     82        ds.addPrimitive(n4);
     83        r2.addMember(new RelationMember("", n4));
     84
     85        Assert.assertEquals(oldBBox, r2.getBBox());
    7786    }
    7887
    7988    @Test

  • test/unit/org/openstreetmap/josm/data/osm/WayTest.java

     
     1// License: GPL. For details, see LICENSE file.
     2package org.openstreetmap.josm.data.osm;
     3
     4import static org.junit.Assert.assertEquals;
     5import static org.junit.Assert.assertFalse;
     6import static org.junit.Assert.assertTrue;
     7
     8import java.util.ArrayList;
     9import java.util.Arrays;
     10import java.util.List;
     11
     12import org.junit.BeforeClass;
     13import org.junit.Test;
     14import org.openstreetmap.josm.JOSMFixture;
     15import org.openstreetmap.josm.data.coor.LatLon;
     16
     17/**
     18 * Unit tests of the {@code Node} class.
     19 */
     20public class WayTest {
     21
     22    /**
     23     * Setup test.
     24     */
     25    @BeforeClass
     26    public static void setUpBeforeClass() {
     27        JOSMFixture.createUnitTestFixture().init();
     28    }
     29
     30    /**
     31     * Test BBox calculation with Way
     32     */
     33    @Test
     34    public void testBBox() {
     35        DataSet ds = new DataSet();
     36        Node n1 = new Node(1);
     37        Node n2 = new Node(2);
     38        Node n3 = new Node(3);
     39        Node n4 = new Node(4);
     40        n1.setIncomplete(true);
     41        n2.setCoor(new LatLon(10, 10));
     42        n3.setCoor(new LatLon(20, 20));
     43        n4.setCoor(new LatLon(90, 180));
     44        ds.addPrimitive(n1);
     45        ds.addPrimitive(n2);
     46        ds.addPrimitive(n3);
     47        ds.addPrimitive(n4);
     48        Way way = new Way(1);
     49        assertFalse(way.getBBox().isValid());
     50        way.setNodes(Arrays.asList(n1));
     51        assertFalse(way.getBBox().isValid());
     52        way.setNodes(Arrays.asList(n2));
     53        assertTrue(way.getBBox().isValid());
     54        way.setNodes(Arrays.asList(n1, n2));
     55        assertTrue(way.getBBox().isValid());
     56        assertEquals(way.getBBox(), new BBox(10, 10));
     57    }
     58
     59    @Test
     60    public void testBBoxSpeed1() {
     61        DataSet ds = new DataSet();
     62        Way way = new Way(1);
     63
     64        // speed test
     65        List<Node> nodes = new ArrayList<>();
     66        for (int i = 0; i < 1000; i++) {
     67            Node n = new Node(100 + i);
     68            n.setCoor(new LatLon(10 + (double) i / 1000, 10 + (double) i / 1000));
     69            ds.addPrimitive(n);
     70            nodes.add(n);
     71        }
     72        way.setNodes(nodes);
     73        ds.addPrimitive(way);
     74        for (int i = 0; i < 100000; i++) {
     75            way.updatePosition();
     76        }
     77    }
     78}