Index: /trunk/src/org/openstreetmap/josm/data/Bounds.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/Bounds.java (revision 11271) +++ /trunk/src/org/openstreetmap/josm/data/Bounds.java (revision 11272) @@ -27,4 +27,8 @@ private double minLat, minLon, maxLat, maxLon; + /** + * Gets the point that has both the minimal lat and lon coordinate + * @return The point + */ public LatLon getMin() { return new LatLon(minLat, minLon); @@ -51,4 +55,8 @@ } + /** + * Gets the point that has both the maximum lat and lon coordinate + * @return The point + */ public LatLon getMax() { return new LatLon(maxLat, maxLon); Index: /trunk/src/org/openstreetmap/josm/data/osm/BBox.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/osm/BBox.java (revision 11271) +++ /trunk/src/org/openstreetmap/josm/data/osm/BBox.java (revision 11272) @@ -6,8 +6,14 @@ import java.util.Objects; +import org.openstreetmap.josm.data.Bounds; import org.openstreetmap.josm.data.coor.LatLon; import org.openstreetmap.josm.data.coor.QuadTiling; import org.openstreetmap.josm.tools.Utils; +/** + * A BBox represents an area in lat/lon space. It is used for the quad tree. + * + * In contrast to a {@link Bounds} object, a BBox can represent an invalid (empty) area. + */ public class BBox { @@ -30,10 +36,5 @@ */ public BBox(final double x, final double y) { - if (!Double.isNaN(x) && !Double.isNaN(y)) { - xmin = x; - ymin = y; - xmax = x; - ymax = y; - } + add(x, y); } @@ -69,23 +70,9 @@ */ public BBox(double ax, double ay, double bx, double by) { - if (Double.isNaN(ax) || Double.isNaN(ay) || Double.isNaN(bx) || Double.isNaN(by)) { - return; // use default which is an invalid BBox - } - - if (ax > bx) { - xmax = ax; - xmin = bx; - } else { - xmax = bx; - xmin = ax; - } - - if (ay > by) { - ymax = ay; - ymin = by; - } else { - ymax = by; - ymin = ay; - } + if (!(Double.isNaN(ax) || Double.isNaN(ay) || Double.isNaN(bx) || Double.isNaN(by))) { + add(ax, ay); + add(bx, by); + } + // otherwise use default which is an invalid BBox } @@ -96,5 +83,5 @@ */ public BBox(Way w) { - w.getNodes().forEach((n) -> add(n.getCoor())); + w.getNodes().forEach(n -> add(n.getCoor())); } @@ -104,6 +91,7 @@ */ public BBox(Node n) { - if (n.isLatLonKnown()) + if (n.isLatLonKnown()) { add(n.getCoor()); + } } @@ -114,6 +102,7 @@ */ public final void add(LatLon c) { - if (c != null && c.isValid()) + if (c != null && c.isValid()) { add(c.lon(), c.lat()); + } } @@ -124,10 +113,10 @@ */ public final void add(double x, double y) { - if (Double.isNaN(x) || Double.isNaN(y)) - return; - xmin = Math.min(xmin, x); - xmax = Math.max(xmax, x); - ymin = Math.min(ymin, y); - ymax = Math.max(ymax, y); + if (!Double.isNaN(x) && !Double.isNaN(y)) { + xmin = Math.min(xmin, x); + xmax = Math.max(xmax, x); + ymin = Math.min(ymin, y); + ymax = Math.max(ymax, y); + } } @@ -156,10 +145,26 @@ } + /** + * Gets the height of the bbox. + * @return The difference between ymax and ymin. 0 for invalid bboxes. + */ public double height() { - return ymax-ymin; - } - + if (isValid()) { + return ymax - ymin; + } else { + return 0; + } + } + + /** + * Gets the width of the bbox. + * @return The difference between xmax and xmin. 0 for invalid bboxes. + */ public double width() { - return xmax-xmin; + if (isValid()) { + return xmax - xmin; + } else { + return 0; + } } @@ -191,13 +196,6 @@ */ public boolean intersects(BBox b) { - if (xmin > b.xmax) - return false; - if (xmax < b.xmin) - return false; - if (ymin > b.ymax) - return false; - if (ymax < b.ymin) - return false; - return true; + return xmin <= b.xmax && xmax >= b.xmin + && ymin <= b.ymax && ymax >= b.ymin; } @@ -254,4 +252,8 @@ } + /** + * Gets the center of this BBox. + * @return The center. + */ public LatLon getCenter() { return new LatLon(ymin + (ymax-ymin)/2.0, xmin + (xmax-xmin)/2.0); @@ -298,14 +300,16 @@ * @return true if the bbox covers a part of the planets surface * Height and width must be non-negative, but may (both) be 0. + * @since 11269 */ public boolean isValid() { - return (xmin <= xmax && ymin <= ymax); - } - - /** - * @return true if the bbox covers a part of the planets surface + return xmin <= xmax && ymin <= ymax; + } + + /** + * @return true if the bbox is avalid and covers a part of the planets surface + * @since 11269 */ public boolean isInWorld() { - return !(xmin < -180.0 || xmax > 180.0 || ymin < -90.0 || ymax > 90.0); + return isValid() && xmin >= -180.0 && xmax <= 180.0 && ymin >= -90.0 && ymax <= 90.0; } Index: /trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitive.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitive.java (revision 11271) +++ /trunk/src/org/openstreetmap/josm/data/osm/OsmPrimitive.java (revision 11272) @@ -1422,4 +1422,5 @@ * @param box a bbox instance * @param visited a set of visited members or null + * @since 11269 */ protected abstract void addToBBox(BBox box, Set visited); Index: /trunk/src/org/openstreetmap/josm/data/osm/QuadBuckets.java =================================================================== --- /trunk/src/org/openstreetmap/josm/data/osm/QuadBuckets.java (revision 11271) +++ /trunk/src/org/openstreetmap/josm/data/osm/QuadBuckets.java (revision 11272) @@ -314,6 +314,7 @@ QBLevel[] children = getChildren(); for (int i = 0; i < QuadTiling.TILES_PER_LEVEL; i++) { - if (children[i] == findThis) + if (children[i] == findThis) { return i; + } } return -1; @@ -344,9 +345,5 @@ boolean canRemove() { - if (content != null && !content.isEmpty()) - return false; - if (this.hasChildren()) - return false; - return true; + return (content == null || content.isEmpty()) && !this.hasChildren(); } } @@ -374,8 +371,9 @@ @Override public boolean add(T n) { - if (n.getBBox().isValid()) + if (n.getBBox().isValid()) { root.add(n); - else + } else { invalidBBoxPrimitives.add(n); + } size++; return true; @@ -388,6 +386,7 @@ continue; } - if (!this.remove(o)) + if (!this.remove(o)) { return false; + } } return true; @@ -415,6 +414,7 @@ public boolean containsAll(Collection objects) { for (Object o : objects) { - if (!this.contains(o)) + if (!this.contains(o)) { return false; + } } return true; @@ -428,8 +428,10 @@ QBLevel bucket = root.findBucket(t.getBBox()); boolean removed = bucket.removeContent(t); - if (!removed) + if (!removed) { removed = invalidBBoxPrimitives.remove(o); - if (removed) + } + if (removed) { size--; + } return removed; } @@ -439,6 +441,7 @@ @SuppressWarnings("unchecked") T t = (T) o; - if (!t.getBBox().isValid()) + if (!t.getBBox().isValid()) { return invalidBBoxPrimitives.contains(o); + } QBLevel bucket = root.findBucket(t.getBBox()); return bucket != null && bucket.content != null && bucket.content.contains(t); @@ -521,6 +524,7 @@ @Override public T next() { - if (fromInvalidBBoxPrimitives) + if (fromInvalidBBoxPrimitives) { return invalidBBoxIterator.next(); + } T ret = peek(); if (ret == null) Index: /trunk/test/unit/org/openstreetmap/josm/data/osm/BBoxTest.java =================================================================== --- /trunk/test/unit/org/openstreetmap/josm/data/osm/BBoxTest.java (revision 11271) +++ /trunk/test/unit/org/openstreetmap/josm/data/osm/BBoxTest.java (revision 11272) @@ -52,10 +52,10 @@ assertTrue(b1.bounds(b2)); - // invalid latlon values - LatLon invalid1 = new LatLon(-190, 340); - BBox b3 = new BBox(invalid1, latLon1); - BBox b4 = new BBox(latLon1, invalid1); - BBox b5 = new BBox(invalid1, invalid1); - // what should be the result? + // outside of world latlon values + LatLon outOfWorld = new LatLon(-190, 340); + BBox b3 = new BBox(outOfWorld, latLon1); + BBox b4 = new BBox(latLon1, outOfWorld); + BBox b5 = new BBox(outOfWorld, outOfWorld); + assertTrue(b3.isValid()); assertTrue(b4.isValid()); @@ -63,4 +63,6 @@ assertTrue(b4.bounds(latLon1)); assertTrue(b5.isValid()); + assertFalse(b3.isInWorld()); + assertFalse(b4.isInWorld()); assertFalse(b5.isInWorld()); }