From 0095e3d6486475d17a26e88deb97ddeaea72578d Mon Sep 17 00:00:00 2001 From: David Beaumont Date: Mon, 31 Oct 2011 18:43:31 -0400 Subject: A more complex CL to avoid exposing the 'bounds' arrays from several of the geometry class (paving the way for them to be made immutable later). ------------- Created by MOE: http://code.google.com/p/moe-java MOE_MIGRATED_REVID=24952612 --- src/com/google/common/geometry/R1Interval.java | 33 +++++-------- src/com/google/common/geometry/R2Vector.java | 17 +++---- src/com/google/common/geometry/S1Interval.java | 56 ++++++++-------------- src/com/google/common/geometry/S2Cell.java | 19 ++++---- src/com/google/common/geometry/S2CellId.java | 12 ++--- src/com/google/common/geometry/S2LatLngRect.java | 18 +++++-- src/com/google/common/geometry/S2Projections.java | 18 +++---- .../com/google/common/geometry/R1IntervalTest.java | 4 +- .../com/google/common/geometry/S1IntervalTest.java | 11 ++--- tests/com/google/common/geometry/S2CellIdTest.java | 5 +- 10 files changed, 87 insertions(+), 106 deletions(-) diff --git a/src/com/google/common/geometry/R1Interval.java b/src/com/google/common/geometry/R1Interval.java index e8edbe4..5b912ec 100644 --- a/src/com/google/common/geometry/R1Interval.java +++ b/src/com/google/common/geometry/R1Interval.java @@ -22,13 +22,16 @@ package com.google.common.geometry; * */ -public strictfp class R1Interval { - private final double[] bounds = new double[2]; +public final strictfp class R1Interval { + + // TODO(dbeaumont): Make these final and make this class fully immutable. + private double lo; + private double hi; /** Interval constructor. If lo > hi, the interval is empty. */ public R1Interval(double lo, double hi) { - bounds[0] = lo; - bounds[1] = hi; + this.lo = lo; + this.hi = hi; } /** @@ -60,27 +63,19 @@ public strictfp class R1Interval { } public double lo() { - return bounds[0]; + return lo; } public double hi() { - return bounds[1]; - } - - public double bound(int i) { - return bounds[i]; - } - - public double[] bounds() { - return bounds; + return hi; } public void setLo(double p) { - bounds[0] = p; + this.lo = p; } public void setHi(double p) { - bounds[1] = p; + this.hi = p; } /** @@ -98,7 +93,6 @@ public strictfp class R1Interval { return 0.5 * (lo() + hi()); } - /** * Return the length of the interval. The length of an empty interval is * negative. @@ -107,7 +101,6 @@ public strictfp class R1Interval { return hi() - lo(); } - public boolean contains(double p) { return p >= lo() && p <= hi(); } @@ -221,8 +214,8 @@ public strictfp class R1Interval { } long value = 17; - value = 37 * value + Double.doubleToLongBits(bounds[0]); - value = 37 * value + Double.doubleToLongBits(bounds[1]); + value = 37 * value + Double.doubleToLongBits(lo); + value = 37 * value + Double.doubleToLongBits(hi); return (int) (value ^ (value >>> 32)); } diff --git a/src/com/google/common/geometry/R2Vector.java b/src/com/google/common/geometry/R2Vector.java index 70ca580..2198d1f 100644 --- a/src/com/google/common/geometry/R2Vector.java +++ b/src/com/google/common/geometry/R2Vector.java @@ -21,9 +21,13 @@ package com.google.common.geometry; * norm, comparison etc. * */ -public strictfp class R2Vector { - double x; - double y; +public final strictfp class R2Vector { + final double x; + final double y; + + public R2Vector() { + this(0, 0); + } public R2Vector(double x, double y) { this.x = x; @@ -38,9 +42,6 @@ public strictfp class R2Vector { y = coord[1]; } - public R2Vector() { - } - public double get(int index) { if (index > 1) { throw new ArrayIndexOutOfBoundsException(index); @@ -57,11 +58,11 @@ public strictfp class R2Vector { } public double norm2() { - return x * x + y * y; + return (x * x) + (y * y); } public static double dotProd(final R2Vector p1, final R2Vector p2) { - return p1.x * p2.x + p1.y * p2.y; + return (p1.x * p2.x) + (p1.y * p2.y); } public double dotProd(R2Vector that) { diff --git a/src/com/google/common/geometry/S1Interval.java b/src/com/google/common/geometry/S1Interval.java index fa6f1ad..eafca63 100644 --- a/src/com/google/common/geometry/S1Interval.java +++ b/src/com/google/common/geometry/S1Interval.java @@ -36,9 +36,11 @@ package com.google.common.geometry; * */ -public strictfp class S1Interval implements Cloneable { +public final strictfp class S1Interval implements Cloneable { - private final double[] bounds = new double[2]; + // TODO(dbeaumont): Make this class immutable and fix callers. + private double lo; + private double hi; /** * Both endpoints must be in the range -Pi to Pi inclusive. The value -Pi is @@ -50,10 +52,12 @@ public strictfp class S1Interval implements Cloneable { /** * Copy constructor. Assumes that the given interval is valid. + * + * TODO(dbeaumont): Make this class immutable and remove this method. */ public S1Interval(S1Interval interval) { - bounds[0] = interval.bounds[0]; - bounds[1] = interval.bounds[1]; + this.lo = interval.lo; + this.hi = interval.hi; } /** @@ -61,21 +65,20 @@ public strictfp class S1Interval implements Cloneable { * range, i.e. normalization from -Pi to Pi is already done. */ private S1Interval(double lo, double hi, boolean checked) { - bounds[0] = lo; - bounds[1] = hi; - + double newLo = lo; + double newHi = hi; if (!checked) { if (lo == -S2.M_PI && hi != S2.M_PI) { - setLo(S2.M_PI); + newLo = S2.M_PI; } if (hi == -S2.M_PI && lo != S2.M_PI) { - setHi(S2.M_PI); + newHi = S2.M_PI; } } - // assert (isValid()); + this.lo = newLo; + this.hi = newHi; } - public static S1Interval empty() { return new S1Interval(S2.M_PI, -S2.M_PI, true); } @@ -112,30 +115,21 @@ public strictfp class S1Interval implements Cloneable { } } - public double lo() { - return bounds[0]; + return lo; } public double hi() { - return bounds[1]; + return hi; } - public double bound(int i) { - return bounds[i]; - } - - public double[] bounds() { - return bounds; - } - - public void setLo(double p) { - bounds[0] = p; + public void setLo(double lo) { + this.lo = lo; // assert (isValid()); } - public void setHi(double p) { - bounds[1] = p; + public void setHi(double hi) { + this.hi = hi; // assert (isValid()); } @@ -148,7 +142,6 @@ public strictfp class S1Interval implements Cloneable { && !(lo() == -S2.M_PI && hi() != S2.M_PI) && !(hi() == -S2.M_PI && lo() != S2.M_PI)); } - /** Return true if the interval contains all points on the unit circle. */ public boolean isFull() { return hi() - lo() == 2 * S2.M_PI; @@ -166,7 +159,6 @@ public strictfp class S1Interval implements Cloneable { return lo() > hi(); } - /** * Return the midpoint of the interval. For full and empty intervals, the * result is arbitrary. @@ -194,7 +186,6 @@ public strictfp class S1Interval implements Cloneable { return (length > 0) ? length : -1; } - /** * Return the complement of the interior of the interval. An interval and its * complement have the same boundary but do not share any interior values. The @@ -221,7 +212,6 @@ public strictfp class S1Interval implements Cloneable { return fastContains(p); } - /** * Return true if the interval (which is closed) contains the point 'p'. Skips * the normalization of 'p' from -Pi to Pi. @@ -235,7 +225,6 @@ public strictfp class S1Interval implements Cloneable { } } - /** Return true if the interior of the interval contains the point 'p'. */ public boolean interiorContains(double p) { // Works for empty, full, and singleton intervals. @@ -251,7 +240,6 @@ public strictfp class S1Interval implements Cloneable { } } - /** * Return true if the interval contains the given interval 'y'. Works for * empty, full, and singleton intervals. @@ -332,7 +320,6 @@ public strictfp class S1Interval implements Cloneable { } } - /** * Expand the interval by the minimum amount necessary so that it contains the * given point "p" (an angle in the range [-Pi, Pi]). @@ -388,7 +375,6 @@ public strictfp class S1Interval implements Cloneable { return result; } - /** * Return the smallest interval that contains this interval and the given * interval "y". @@ -432,7 +418,6 @@ public strictfp class S1Interval implements Cloneable { } } - /** * Return the smallest interval that contains the intersection of this * interval with "y". Note that the region of intersection may consist of two @@ -472,7 +457,6 @@ public strictfp class S1Interval implements Cloneable { return empty(); } - /** * Return true if the length of the symmetric difference between the two * intervals is at most the given tolerance. diff --git a/src/com/google/common/geometry/S2Cell.java b/src/com/google/common/geometry/S2Cell.java index 35f70ea..26c7af1 100644 --- a/src/com/google/common/geometry/S2Cell.java +++ b/src/com/google/common/geometry/S2Cell.java @@ -23,11 +23,10 @@ package com.google.common.geometry; * */ -public strictfp class S2Cell implements S2Region { +public final strictfp class S2Cell implements S2Region { private static final int MAX_CELL_SIZE = 1 << S2CellId.MAX_LEVEL; - // This structure occupies 44 bytes plus one pointer for the vtable. byte face; byte level; byte orientation; @@ -182,17 +181,17 @@ public strictfp class S2Cell implements S2Region { public R2Vector getCenterUV() { MutableInteger i = new MutableInteger(0); MutableInteger j = new MutableInteger(0); - R2Vector centerUv = new R2Vector(); cellId.toFaceIJOrientation(i, j, null); int cellSize = 1 << (S2CellId.MAX_LEVEL - level); - int sij = (i.intValue() & -cellSize) * 2 + cellSize - MAX_CELL_SIZE; - centerUv.x = S2Projections.stToUV((1.0 / MAX_CELL_SIZE) * sij); + // TODO(dbeaumont): Figure out a better naming of the variables here (and elsewhere). + int si = (i.intValue() & -cellSize) * 2 + cellSize - MAX_CELL_SIZE; + double x = S2Projections.stToUV((1.0 / MAX_CELL_SIZE) * si); - sij = (j.intValue() & -cellSize) * 2 + cellSize - MAX_CELL_SIZE; - centerUv.y = S2Projections.stToUV((1.0 / MAX_CELL_SIZE) * sij); + int sj = (j.intValue() & -cellSize) * 2 + cellSize - MAX_CELL_SIZE; + double y = S2Projections.stToUV((1.0 / MAX_CELL_SIZE) * sj); - return centerUv; + return new R2Vector(x, y); } /** @@ -365,8 +364,8 @@ public strictfp class S2Cell implements S2Region { // We can't just call XYZtoFaceUV, because for points that lie on the // boundary between two faces (i.e. u or v is +1/-1) we need to return // true for both adjacent cells. - R2Vector uvPoint = new R2Vector(); - if (!S2Projections.faceXyzToUv(face, p, uvPoint)) { + R2Vector uvPoint = S2Projections.faceXyzToUv(face, p); + if (uvPoint == null) { return false; } return (uvPoint.x >= uv[0][0] && uvPoint.x <= uv[0][1] && uvPoint.y >= uv[1][0] diff --git a/src/com/google/common/geometry/S2CellId.java b/src/com/google/common/geometry/S2CellId.java index 4e5f1a3..4de0a59 100644 --- a/src/com/google/common/geometry/S2CellId.java +++ b/src/com/google/common/geometry/S2CellId.java @@ -48,7 +48,7 @@ import java.util.Locale; * * */ -public strictfp class S2CellId implements Comparable { +public final strictfp class S2CellId implements Comparable { // Although only 60 bits are needed to represent the index of a leaf // cell, we need an extra bit in order to represent the position of @@ -146,8 +146,8 @@ public strictfp class S2CellId implements Comparable { * necessarily unit length). */ public static S2CellId fromPoint(S2Point p) { - R2Vector uv = new R2Vector(); - int face = S2Projections.xyzToFaceUV(p, uv); + int face = S2Projections.xyzToFace(p); + R2Vector uv = S2Projections.validFaceXyzToUv(face, p); int i = stToIJ(S2Projections.uvToST(uv.x)); int j = stToIJ(S2Projections.uvToST(uv.y)); return fromFaceIJ(face, i, j); @@ -863,8 +863,9 @@ public strictfp class S2CellId implements Comparable { // Find the leaf cell coordinates on the adjacent face, and convert // them to a cell id at the appropriate level. - R2Vector st = new R2Vector(); - face = S2Projections.xyzToFaceUV(S2Projections.faceUvToXyz(face, s, t), st); + S2Point p = S2Projections.faceUvToXyz(face, s, t); + face = S2Projections.xyzToFace(p); + R2Vector st = S2Projections.validFaceXyzToUv(face, p); return fromFaceIJ(face, stToIJ(st.x), stToIJ(st.y)); } @@ -960,5 +961,4 @@ public strictfp class S2CellId implements Comparable { return unsignedLongLessThan(this.id, that.id) ? -1 : unsignedLongGreaterThan(this.id, that.id) ? 1 : 0; } - } diff --git a/src/com/google/common/geometry/S2LatLngRect.java b/src/com/google/common/geometry/S2LatLngRect.java index cb9ad0c..fae637b 100644 --- a/src/com/google/common/geometry/S2LatLngRect.java +++ b/src/com/google/common/geometry/S2LatLngRect.java @@ -23,7 +23,7 @@ import com.google.common.base.Preconditions; * */ -public strictfp class S2LatLngRect implements S2Region { +public final strictfp class S2LatLngRect implements S2Region { private final R1Interval lat; private final S1Interval lng; @@ -196,9 +196,19 @@ public strictfp class S2LatLngRect implements S2Region { /** Return the k-th vertex of the rectangle (k = 0,1,2,3) in CCW order. */ public S2LatLng getVertex(int k) { - // Twiddle bits to return the points in CCW order (SW, SE, NE, NW). - return S2LatLng.fromRadians(lat.bound(k >> 1), lng.bound((k >> 1) - ^ (k & 1))); + // Return the points in CCW order (SW, SE, NE, NW). + switch (k) { + case 0: + return S2LatLng.fromRadians(lat.lo(), lng.lo()); + case 1: + return S2LatLng.fromRadians(lat.lo(), lng.hi()); + case 2: + return S2LatLng.fromRadians(lat.hi(), lng.hi()); + case 3: + return S2LatLng.fromRadians(lat.hi(), lng.lo()); + default: + throw new IllegalArgumentException("Invalid vertex index."); + } } /** diff --git a/src/com/google/common/geometry/S2Projections.java b/src/com/google/common/geometry/S2Projections.java index 7032799..0f366b1 100644 --- a/src/com/google/common/geometry/S2Projections.java +++ b/src/com/google/common/geometry/S2Projections.java @@ -69,7 +69,7 @@ import com.google.common.geometry.S2.Metric; * */ -public strictfp class S2Projections { +public final strictfp class S2Projections { public enum Projections { S2_LINEAR_PROJECTION, S2_TAN_PROJECTION, S2_QUADRATIC_PROJECTION } @@ -333,31 +333,25 @@ public strictfp class S2Projections { return new R2Vector(pu, pv); } - public static int xyzToFaceUV(S2Point p, R2Vector uv) { + public static int xyzToFace(S2Point p) { int face = p.largestAbsComponent(); if (p.get(face) < 0) { face += 3; } - R2Vector point = validFaceXyzToUv(face, p); - uv.x = point.x; - uv.y = point.y; return face; } - public static boolean faceXyzToUv(int face, S2Point p, R2Vector uv) { + public static R2Vector faceXyzToUv(int face, S2Point p) { if (face < 3) { if (p.get(face) <= 0) { - return false; + return null; } } else { if (p.get(face - 3) >= 0) { - return false; + return null; } } - R2Vector point = validFaceXyzToUv(face, p); - uv.x = point.x; - uv.y = point.y; - return true; + return validFaceXyzToUv(face, p); } public static S2Point getUNorm(int face, double u) { diff --git a/tests/com/google/common/geometry/R1IntervalTest.java b/tests/com/google/common/geometry/R1IntervalTest.java index 0fe87ea..b4c2bba 100644 --- a/tests/com/google/common/geometry/R1IntervalTest.java +++ b/tests/com/google/common/geometry/R1IntervalTest.java @@ -40,8 +40,8 @@ public strictfp class R1IntervalTest extends GeometryTestCase { R1Interval negunit = new R1Interval(-1, 0); assertEquals(unit.lo(), 0.0); assertEquals(unit.hi(), 1.0); - assertEquals(negunit.bound(0), -1.0); - assertEquals(negunit.bound(1), 0.0); + assertEquals(negunit.lo(), -1.0); + assertEquals(negunit.hi(), 0.0); R1Interval ten = new R1Interval(0, 0); ten.setHi(10); assertEquals(ten.hi(), 10.0); diff --git a/tests/com/google/common/geometry/S1IntervalTest.java b/tests/com/google/common/geometry/S1IntervalTest.java index 146ec74..2215104 100644 --- a/tests/com/google/common/geometry/S1IntervalTest.java +++ b/tests/com/google/common/geometry/S1IntervalTest.java @@ -15,7 +15,6 @@ */ package com.google.common.geometry; -import java.util.Arrays; public strictfp class S1IntervalTest extends GeometryTestCase { @@ -33,15 +32,15 @@ public strictfp class S1IntervalTest extends GeometryTestCase { // bounds() returns a const reference to a member variable, so we need to // make a copy when invoking it on a temporary object. - assertTrue(Arrays.equals(x.union(y).bounds(), expectedUnion.bounds())); - assertTrue(Arrays.equals(x.intersection(y).bounds(), expectedIntersection.bounds())); + assertEquals(expectedUnion, x.union(y)); + assertEquals(expectedIntersection, x.intersection(y)); assertEquals(x.contains(y), x.union(y) == x); assertEquals(x.intersects(y), !x.intersection(y).isEmpty()); if (y.lo() == y.hi()) { S1Interval r = x.addPoint(y.lo()); - assertTrue(Arrays.equals(r.bounds(), expectedUnion.bounds())); + assertEquals(expectedUnion, r); } } @@ -57,8 +56,8 @@ public strictfp class S1IntervalTest extends GeometryTestCase { assertEquals(quad12.lo(), 0.0); assertEquals(quad12.hi(), S2.M_PI); S1Interval quad34 = new S1Interval(-S2.M_PI, 0); - assertEquals(quad34.bound(0), S2.M_PI); - assertEquals(quad34.bound(1), 0.0); + assertEquals(quad34.lo(), S2.M_PI); + assertEquals(quad34.hi(), 0.0); S1Interval pi = new S1Interval(S2.M_PI, S2.M_PI); assertEquals(pi.lo(), S2.M_PI); assertEquals(pi.hi(), S2.M_PI); diff --git a/tests/com/google/common/geometry/S2CellIdTest.java b/tests/com/google/common/geometry/S2CellIdTest.java index ce089db..456c542 100644 --- a/tests/com/google/common/geometry/S2CellIdTest.java +++ b/tests/com/google/common/geometry/S2CellIdTest.java @@ -193,8 +193,9 @@ public strictfp class S2CellIdTest extends GeometryTestCase { // Check that the ToPointRaw() returns the center of each cell // in (s,t) coordinates. - R2Vector uv = new R2Vector(); - S2Projections.xyzToFaceUV(id.toPointRaw(), uv); + S2Point p = id.toPointRaw(); + int face = S2Projections.xyzToFace(p); + R2Vector uv = S2Projections.validFaceXyzToUv(face, p); assertDoubleNear( Math.IEEEremainder(S2Projections.uvToST(uv.x), 1.0 / (1 << MAX_WALK_LEVEL)), 0); assertDoubleNear( -- cgit v1.2.3