Changeset 11269 in josm for trunk/src/org


Ignore:
Timestamp:
2016-11-17T19:58:39+01:00 (3 years ago)
Author:
michael2402
Message:

Fix #13361: Use a more consistent invalid bbox for primitives.

Patch by Gerd Petermann

Location:
trunk/src/org/openstreetmap/josm/data/osm
Files:
7 edited

Legend:

Unmodified
Added
Removed
  • trunk/src/org/openstreetmap/josm/data/osm/BBox.java

    r11237 r11269  
    2020     * Constructs a new (invalid) BBox
    2121     */
    22     protected BBox() { }
    23 
     22    public BBox() { }
    2423
    2524    /**
     
    3130     */
    3231    public BBox(final double x, final double y) {
    33         xmax = xmin = x;
    34         ymax = ymin = y;
    35         sanity();
     32        if (!Double.isNaN(x) && !Double.isNaN(y)) {
     33            xmin = x;
     34            ymin = y;
     35            xmax = x;
     36            ymax = y;
     37        }
    3638    }
    3739
    3840    /**
    3941     * Constructs a new {@code BBox} defined by points <code>a</code> and <code>b</code>.
    40      * Result is minimal BBox containing both points.
     42     * Result is minimal BBox containing both points if they are both valid, else undefined
    4143     *
    4244     * @param a first point
     
    5961    }
    6062
     63    /**
     64     * Create minimal  BBox so that {@code this.bounds(ax,ay)} and {@code this.bounds(bx,by)} will both return true
     65     * @param ax left or right X value (-180 .. 180)
     66     * @param ay top or bottom Y value (-90 .. 90)
     67     * @param bx left or right X value (-180 .. 180)
     68     * @param by top or bottom Y value (-90 .. 90)
     69     */
    6170    public BBox(double ax, double ay, double bx, double by) {
     71        if (Double.isNaN(ax) || Double.isNaN(ay) || Double.isNaN(bx) || Double.isNaN(by)) {
     72            return; // use default which is an invalid BBox
     73        }
    6274
    6375        if (ax > bx) {
     
    7688            ymin = ay;
    7789        }
    78 
    79         sanity();
    80     }
    81 
     90    }
     91
     92    /**
     93     * Create BBox for all nodes of the way with known coordinates.
     94     * If no node has a known coordinate, an invalid BBox is returned.
     95     * @param w the way
     96     */
    8297    public BBox(Way w) {
    83         for (Node n : w.getNodes()) {
    84             LatLon coor = n.getCoor();
    85             if (coor == null) {
    86                 continue;
    87             }
    88             add(coor);
    89         }
    90     }
    91 
     98        w.getNodes().forEach((n) -> add(n.getCoor()));
     99    }
     100
     101    /**
     102     * Create BBox for a node. An invalid BBox is returned if the coordinates are not known.
     103     * @param n the node
     104     */
    92105    public BBox(Node n) {
    93         LatLon coor = n.getCoor();
    94         if (coor == null) {
    95             xmin = xmax = ymin = ymax = 0;
    96         } else {
    97             xmin = xmax = coor.lon();
    98             ymin = ymax = coor.lat();
    99         }
    100     }
    101 
    102     private void sanity() {
    103         if (xmin < -180.0) {
    104             xmin = -180.0;
    105         }
    106         if (xmax > 180.0) {
    107             xmax = 180.0;
    108         }
    109         if (ymin < -90.0) {
    110             ymin = -90.0;
    111         }
    112         if (ymax > 90.0) {
    113             ymax = 90.0;
    114         }
    115     }
    116 
     106        if (n.isLatLonKnown())
     107            add(n.getCoor());
     108    }
     109
     110    /**
     111     * Add a point to an existing BBox. Extends this bbox if necessary so that this.bounds(c) will return true
     112     * if c is a valid LatLon instance.
     113     * @param c a LatLon point
     114     */
    117115    public final void add(LatLon c) {
    118         add(c.lon(), c.lat());
     116        if (c != null && c.isValid())
     117            add(c.lon(), c.lat());
    119118    }
    120119
     
    125124     */
    126125    public final void add(double x, double y) {
     126        if (Double.isNaN(x) || Double.isNaN(y))
     127            return;
    127128        xmin = Math.min(xmin, x);
    128129        xmax = Math.max(xmax, x);
    129130        ymin = Math.min(ymin, y);
    130131        ymax = Math.max(ymax, y);
    131         sanity();
    132     }
    133 
    134     public final void add(BBox box) {
    135         xmin = Math.min(xmin, box.xmin);
    136         xmax = Math.max(xmax, box.xmax);
    137         ymin = Math.min(ymin, box.ymin);
    138         ymax = Math.max(ymax, box.ymax);
    139         sanity();
    140     }
    141 
     132    }
     133
     134    /**
     135     * Extends this bbox to include the bbox other. Does nothing if other is not valid.
     136     * @param other a bbox
     137     */
     138    public final void add(BBox other) {
     139        if (other.isValid()) {
     140            xmin = Math.min(xmin, other.xmin);
     141            xmax = Math.max(xmax, other.xmax);
     142            ymin = Math.min(ymin, other.ymin);
     143            ymax = Math.max(ymax, other.ymax);
     144        }
     145    }
     146
     147    /**
     148     * Extends this bbox to include the bbox of the primitive extended by extraSpace.
     149     * @param primitive an OSM primitive
     150     * @param extraSpace the value to extend the primitives bbox. Unit is in LatLon degrees.
     151     */
    142152    public void addPrimitive(OsmPrimitive primitive, double extraSpace) {
    143153        BBox primBbox = primitive.getBBox();
     
    285295    }
    286296
     297    /**
     298     * @return true if the bbox covers a part of the planets surface
     299     * Height and width must be non-negative, but may (both) be 0.
     300     */
     301    public boolean isValid() {
     302        return (xmin <= xmax && ymin <= ymax);
     303    }
     304
     305    /**
     306     * @return true if the bbox covers a part of the planets surface
     307     */
     308    public boolean isInWorld() {
     309        return !(xmin < -180.0 || xmax > 180.0 || ymin < -90.0 || ymax > 90.0);
     310    }
     311
    287312    @Override
    288313    public String toString() {
  • trunk/src/org/openstreetmap/josm/data/osm/DataSet.java

    r11115 r11269  
    504504                        tr("Unable to add primitive {0} to the dataset because it is already included", primitive.toString()));
    505505
    506             primitive.updatePosition(); // Set cached bbox for way and relation (required for reindexWay and reinexRelation to work properly)
     506            allPrimitives.add(primitive);
     507            primitive.setDataset(this);
     508            primitive.updatePosition(); // Set cached bbox for way and relation (required for reindexWay and reindexRelation to work properly)
    507509            boolean success = false;
    508510            if (primitive instanceof Node) {
     
    515517            if (!success)
    516518                throw new RuntimeException("failed to add primitive: "+primitive);
    517             allPrimitives.add(primitive);
    518             primitive.setDataset(this);
    519519            firePrimitivesAdded(Collections.singletonList(primitive), false);
    520520        } finally {
  • trunk/src/org/openstreetmap/josm/data/osm/Node.java

    r10919 r11269  
    330330    @Override
    331331    public BBox getBBox() {
    332         return new BBox(this);
     332        return new BBox(lon, lat);
     333    }
     334
     335    @Override
     336    protected void addToBBox(BBox box, Set<PrimitiveId> visited) {
     337        box.add(lon, lat);
    333338    }
    334339
  • trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitive.java

    r11079 r11269  
    14171417        return false;
    14181418    }
     1419
     1420    /**
     1421     * If necessary, extend the bbox to contain this primitive
     1422     * @param box a bbox instance
     1423     * @param visited a set of visited members  or null
     1424     */
     1425    protected abstract void addToBBox(BBox box, Set<PrimitiveId> visited);
    14191426}
  • trunk/src/org/openstreetmap/josm/data/osm/QuadBuckets.java

    r11237 r11269  
    66import java.util.Collection;
    77import java.util.Iterator;
     8import java.util.LinkedHashSet;
    89import java.util.List;
    910import java.util.NoSuchElementException;
     
    167168
    168169        boolean matches(final T o, final BBox searchBbox) {
    169             if (o instanceof Node) {
    170                 final LatLon latLon = ((Node) o).getCoor();
    171                 // node without coords -> bbox[0,0,0,0]
    172                 return searchBbox.bounds(latLon != null ? latLon : LatLon.ZERO);
    173             }
    174170            return o.getBBox().intersects(searchBbox);
    175171        }
     
    359355    private QBLevel<T> searchCache;
    360356    private int size;
     357    private Collection<T> invalidBBoxPrimitives;
    361358
    362359    /**
     
    370367    public final void clear() {
    371368        root = new QBLevel<>();
     369        invalidBBoxPrimitives = new LinkedHashSet<>();
    372370        searchCache = null;
    373371        size = 0;
     
    376374    @Override
    377375    public boolean add(T n) {
    378         root.add(n);
     376        if (n.getBBox().isValid())
     377            root.add(n);
     378        else
     379            invalidBBoxPrimitives.add(n);
    379380        size++;
    380381        return true;
     
    426427        searchCache = null; // Search cache might point to one of removed buckets
    427428        QBLevel<T> bucket = root.findBucket(t.getBBox());
    428         if (bucket.removeContent(t)) {
     429        boolean removed = bucket.removeContent(t);
     430        if (!removed)
     431            removed = invalidBBoxPrimitives.remove(o);
     432        if (removed)
    429433            size--;
    430             return true;
    431         } else
    432             return false;
     434        return removed;
    433435    }
    434436
     
    437439        @SuppressWarnings("unchecked")
    438440        T t = (T) o;
     441        if (!t.getBBox().isValid())
     442            return invalidBBoxPrimitives.contains(o);
    439443        QBLevel<T> bucket = root.findBucket(t.getBBox());
    440444        return bucket != null && bucket.content != null && bucket.content.contains(t);
     
    466470        private QBLevel<T> currentNode;
    467471        private int contentIndex;
     472        private Iterator<T> invalidBBoxIterator = invalidBBoxPrimitives.iterator();
     473        boolean fromInvalidBBoxPrimitives;
    468474        QuadBuckets<T> qb;
    469475
     
    491497        @Override
    492498        public boolean hasNext() {
    493             if (this.peek() == null)
    494                 return false;
     499            if (this.peek() == null) {
     500                fromInvalidBBoxPrimitives = true;
     501                return invalidBBoxIterator.hasNext();
     502            }
    495503            return true;
    496504        }
     
    513521        @Override
    514522        public T next() {
     523            if (fromInvalidBBoxPrimitives)
     524                return invalidBBoxIterator.next();
    515525            T ret = peek();
    516526            if (ret == null)
     
    522532        @Override
    523533        public void remove() {
    524             // two uses
    525             // 1. Back up to the thing we just returned
    526             // 2. move the index back since we removed
    527             //    an element
    528             contentIndex--;
    529             T object = peek();
    530             if (currentNode.removeContent(object))
     534            if (fromInvalidBBoxPrimitives) {
     535                invalidBBoxIterator.remove();
    531536                qb.size--;
     537            } else {
     538                // two uses
     539                // 1. Back up to the thing we just returned
     540                // 2. move the index back since we removed
     541                //    an element
     542                contentIndex--;
     543                T object = peek();
     544                if (currentNode.removeContent(object))
     545                    qb.size--;
     546
     547            }
    532548        }
    533549    }
     
    555571    public List<T> search(BBox searchBbox) {
    556572        List<T> ret = new ArrayList<>();
     573        if (!searchBbox.isValid()) {
     574            return ret;
     575        }
     576
    557577        // Doing this cuts down search cost on a real-life data set by about 25%
    558578        if (searchCache == null) {
  • trunk/src/org/openstreetmap/josm/data/osm/Relation.java

    r11038 r11269  
    447447    @Override
    448448    public BBox getBBox() {
    449         RelationMember[] members = this.members;
    450 
    451         if (members.length == 0)
    452             return new BBox(0, 0, 0, 0);
    453         if (getDataSet() == null)
    454             return calculateBBox(new HashSet<PrimitiveId>());
    455         else {
    456             if (bbox == null) {
    457                 bbox = calculateBBox(new HashSet<PrimitiveId>());
    458             }
    459             if (bbox == null)
    460                 return new BBox(0, 0, 0, 0); // No real members
    461             else
    462                 return new BBox(bbox);
    463         }
    464     }
    465 
    466     private BBox calculateBBox(Set<PrimitiveId> visitedRelations) {
    467         if (visitedRelations.contains(this))
    468             return null;
    469         visitedRelations.add(this);
    470 
    471         RelationMember[] members = this.members;
    472         if (members.length == 0)
    473             return null;
    474         else {
    475             BBox result = null;
    476             for (RelationMember rm:members) {
    477                 BBox box = rm.isRelation() ? rm.getRelation().calculateBBox(visitedRelations) : rm.getMember().getBBox();
    478                 if (box != null) {
    479                     if (result == null) {
    480                         result = box;
    481                     } else {
    482                         result.add(box);
    483                     }
    484                 }
    485             }
    486             return result;
     449        if (getDataSet() != null && bbox != null)
     450            return new BBox(bbox); // use cached value
     451
     452        BBox box = new BBox();
     453        addToBBox(box, new HashSet<PrimitiveId>());
     454        if (getDataSet() != null)
     455            bbox = box; // set cache
     456        return new BBox(box);
     457    }
     458
     459    @Override
     460    protected void addToBBox(BBox box, Set<PrimitiveId> visited) {
     461        for (RelationMember rm : members) {
     462            if (visited.add(rm.getMember()))
     463                rm.getMember().addToBBox(box, visited);
    487464        }
    488465    }
     
    490467    @Override
    491468    public void updatePosition() {
    492         bbox = calculateBBox(new HashSet<PrimitiveId>());
     469        bbox = null; // make sure that it is recalculated
     470        bbox = getBBox();
    493471    }
    494472
  • trunk/src/org/openstreetmap/josm/data/osm/Way.java

    r10662 r11269  
    631631        }
    632632        return new BBox(bbox);
     633    }
     634
     635    @Override
     636    protected void addToBBox(BBox box, Set<PrimitiveId> visited) {
     637        box.add(getBBox());
    633638    }
    634639
Note: See TracChangeset for help on using the changeset viewer.