Compose with WebMercatorExtents to reduce duplication

Height and width in WebMercatorGridPointSet constructor were often one
unit too narrow (not consistent with WebMercatorExtents).

src/main/java/com/conveyal/analysis/controllers/DataSourceController.java

@@ -35,7 +35,7 @@
 import static com.conveyal.analysis.util.JsonUtil.toJson;
 import static com.conveyal.file.FileCategory.DATASOURCES;
 import static com.conveyal.file.FileStorageFormat.SHP;
-import static com.conveyal.r5.analyst.WebMercatorGridPointSet.parseZoom;
+import static com.conveyal.r5.analyst.WebMercatorExtents.parseZoom;
 import static com.mongodb.client.model.Filters.eq;
 
 /**

src/main/java/com/conveyal/analysis/controllers/OpportunityDatasetController.java

@@ -59,7 +59,7 @@
 import static com.conveyal.analysis.datasource.DataSourceUtil.detectUploadFormatAndValidate;
 import static com.conveyal.analysis.util.JsonUtil.toJson;
 import static com.conveyal.file.FileCategory.GRIDS;
-import static com.conveyal.r5.analyst.WebMercatorGridPointSet.parseZoom;
+import static com.conveyal.r5.analyst.WebMercatorExtents.parseZoom;
 import static com.conveyal.r5.analyst.progress.WorkProductType.OPPORTUNITY_DATASET;
 
 /**

src/main/java/com/conveyal/analysis/datasource/derivation/AggregationAreaDerivation.java

@@ -36,7 +36,7 @@
 import java.util.zip.GZIPOutputStream;
 
 import static com.conveyal.file.FileStorageFormat.SHP;
-import static com.conveyal.r5.analyst.WebMercatorGridPointSet.parseZoom;
+import static com.conveyal.r5.analyst.WebMercatorExtents.parseZoom;
 import static com.conveyal.r5.analyst.progress.WorkProductType.AGGREGATION_AREA;
 import static com.conveyal.r5.util.ShapefileReader.GeometryType.POLYGON;
 import static com.google.common.base.Preconditions.checkArgument;

src/main/java/com/conveyal/r5/analyst/GridTransformWrapper.java

@@ -37,8 +37,8 @@ public GridTransformWrapper (WebMercatorExtents targetGridExtents, Grid sourceGr
     // This could certainly be made more efficient (but complex) by forcing sequential iteration over opportunity counts
     // and disallowing random access, using a new PointSetIterator class that allows reading lat, lon, and counts.
     private int transformIndex (int i) {
-        final int x = (i % targetGrid.width) + targetGrid.west - sourceGrid.extents.west;
-        final int y = (i / targetGrid.width) + targetGrid.north - sourceGrid.extents.north;
+        final int x = (i % targetGrid.extents.width) + targetGrid.extents.west - sourceGrid.extents.west;
+        final int y = (i / targetGrid.extents.width) + targetGrid.extents.north - sourceGrid.extents.north;
         if (x < 0 || x >= sourceGrid.extents.width || y < 0 || y >= sourceGrid.extents.height) {
             // Point in target grid lies outside source grid, there is no valid index. Return special value.
             return -1;

src/main/java/com/conveyal/r5/analyst/LinkageCache.java

@@ -95,7 +95,7 @@ public LinkedPointSet load(Key key) {
                             key.streetLayer,
                             key.streetMode
                     );
-                    if (basePointSetLinkage != null && basePointSet.zoom == keyPointSet.zoom) {
+                    if (basePointSetLinkage != null && basePointSet.extents.zoom == keyPointSet.extents.zoom) {
                         LOG.info("Cutting linkage for {} out of existing linkage for {}.", keyPointSet, basePointSet);
                         return new LinkedPointSet(basePointSetLinkage, keyPointSet);
                     }

src/main/java/com/conveyal/r5/analyst/WebMercatorExtents.java

@@ -8,6 +8,7 @@
 import org.locationtech.jts.geom.Envelope;
 import org.opengis.referencing.crs.CoordinateReferenceSystem;
 
+import java.io.Serializable;
 import java.util.Arrays;
 
 import static com.conveyal.r5.analyst.Grid.latToFractionalPixel;
@@ -29,10 +30,15 @@
  *      and OpportunityGrid (AKA Grid) which adds opportunity counts. These can compose, not necessarily subclass.
  *      Of course they could all be one class, with the opportunity grid nulled out when there is no density.
  */
-public class WebMercatorExtents {
+public class WebMercatorExtents implements Serializable {
 
-    private static final int MIN_ZOOM = 9;
-    private static final int MAX_ZOOM = 12;
+    /**
+     * Default Web Mercator zoom level for grids (origin/destination layers, aggregation area masks, etc.).
+     * Level 10 is probably ideal but will quadruple calculation relative to 9.
+     */
+    public static final int DEFAULT_ZOOM = 9;
+    public static final int MIN_ZOOM = 9;
+    public static final int MAX_ZOOM = 12;
     private static final int MAX_GRID_CELLS = 5_000_000;
 
     /** The pixel number of the westernmost pixel (smallest x value). */
@@ -98,6 +104,12 @@ public static WebMercatorExtents forPointsets (PointSet[] pointSets) {
         }
     }
 
+    public static int parseZoom(String zoomString) {
+        int zoom = (zoomString == null) ? DEFAULT_ZOOM : Integer.parseInt(zoomString);
+        checkArgument(zoom >= MIN_ZOOM && zoom <= MAX_ZOOM);
+        return zoom;
+    }
+
     /**
      * Create the minimum-size immutable WebMercatorExtents containing both this one and the other one.
      * Note that WebMercatorExtents fields are immutable, and this method does not modify the instance in place.

src/main/java/com/conveyal/r5/analyst/WebMercatorGridPointSet.java

@@ -27,46 +27,17 @@ public class WebMercatorGridPointSet extends PointSet implements Serializable {
 
     public static final Logger LOG = LoggerFactory.getLogger(WebMercatorGridPointSet.class);
 
-    /**
-     * Default Web Mercator zoom level for grids (origin/destination layers, aggregation area masks, etc.).
-     * Level 10 is probably ideal but will quadruple calculation relative to 9.
-     */
-    public static final int DEFAULT_ZOOM = 9;
-
-    public static final int MIN_ZOOM = 9;
-
-    public static final int MAX_ZOOM = 12;
-
-    /** web mercator zoom level */
-    public final int zoom;
-
-    /** westernmost pixel  */
-    public final int west;
-
-    /** northernmost pixel */
-    public final int north;
-
-    /** The number of pixels across this grid in the east-west direction. */
-    public final int width;
-
-    /** The number of pixels from top to bottom of this grid in the north-south direction. */
-    public final int height;
-
+    public final WebMercatorExtents extents;
     /** Base pointset; linkages will be shared with this pointset */
     public final WebMercatorGridPointSet basePointSet;
 
     /**
      * Create a new WebMercatorGridPointSet.
-     *
      * @oaram basePointSet the super-grid pointset from which linkages will be copied or shared, or null if no
      *        such grid exists.
      */
     public WebMercatorGridPointSet(int zoom, int west, int north, int width, int height, WebMercatorGridPointSet basePointSet) {
-        this.zoom = zoom;
-        this.west = west;
-        this.north = north;
-        this.width = width;
-        this.height = height;
+        this.extents = new WebMercatorExtents(west, north, width, height, zoom);
         this.basePointSet = basePointSet;
     }
 
@@ -85,15 +56,8 @@ public WebMercatorGridPointSet (TransportNetwork transportNetwork) {
     public WebMercatorGridPointSet (Envelope wgsEnvelope) {
         LOG.info("Creating WebMercatorGridPointSet with WGS84 extents {}", wgsEnvelope);
         checkWgsEnvelopeSize(wgsEnvelope, "grid point set");
-        this.zoom = DEFAULT_ZOOM;
-        int west = lonToPixel(wgsEnvelope.getMinX(), zoom);
-        int east = lonToPixel(wgsEnvelope.getMaxX(), zoom);
-        int north = latToPixel(wgsEnvelope.getMaxY(), zoom);
-        int south = latToPixel(wgsEnvelope.getMinY(), zoom);
-        this.west = west;
-        this.north = north;
-        this.height = south - north;
-        this.width = east - west;
+        // NOTE the width and height calculations were wrong before, use standard ones in WebMercatorExtents
+        this.extents = WebMercatorExtents.forWgsEnvelope(wgsEnvelope, WebMercatorExtents.DEFAULT_ZOOM); // USE TRIMMED?
         this.basePointSet = null;
     }
 
@@ -104,7 +68,7 @@ public WebMercatorGridPointSet (WebMercatorExtents extents) {
 
     @Override
     public int featureCount() {
-        return height * width;
+        return extents.height * extents.width;
     }
 
     @Override
@@ -115,38 +79,40 @@ public double sumTotalOpportunities () {
 
     @Override
     public double getLat(int i) {
-        long y = i / this.width + this.north;
-        return pixelToLat(y, zoom);
+        long y = i / extents.width + extents.north;
+        return pixelToLat(y, extents.zoom);
     }
 
     @Override
     public double getLon(int i) {
-        long x = i % this.width + this.west;
-        return pixelToLon(x, zoom);
+        long x = i % extents.width + extents.west;
+        return pixelToLon(x, extents.zoom);
     }
 
     @Override
     public TIntList getPointsInEnvelope (Envelope envelopeFixedDegrees) {
         // Convert fixed-degree envelope to floating, then to world-scale web Mercator pixels at this grid's zoom level.
         // This is not very DRY since we do something very similar in the constructor and elsewhere.
-        int west = lonToPixel(fixedDegreesToFloating(envelopeFixedDegrees.getMinX()), zoom);
-        int east = lonToPixel(fixedDegreesToFloating(envelopeFixedDegrees.getMaxX()), zoom);
-        int north = latToPixel(fixedDegreesToFloating(envelopeFixedDegrees.getMaxY()), zoom);
-        int south = latToPixel(fixedDegreesToFloating(envelopeFixedDegrees.getMinY()), zoom);
+        // These are the integer pixel numbers containing the envelope, so iteration below must be inclusive of the max.
+        final int z = extents.zoom;
+        int west = lonToPixel(fixedDegreesToFloating(envelopeFixedDegrees.getMinX()), z);
+        int east = lonToPixel(fixedDegreesToFloating(envelopeFixedDegrees.getMaxX()), z);
+        int north = latToPixel(fixedDegreesToFloating(envelopeFixedDegrees.getMaxY()), z);
+        int south = latToPixel(fixedDegreesToFloating(envelopeFixedDegrees.getMinY()), z);
         // Make the envelope's pixel values relative to the edges of this WebMercatorGridPointSet, rather than
         // absolute world-scale coordinates at this zoom level.
-        west -= this.west;
-        east -= this.west;
-        north -= this.north;
-        south -= this.north;
+        west -= extents.west;
+        east -= extents.west;
+        north -= extents.north;
+        south -= extents.north;
         TIntList pointsInEnvelope = new TIntArrayList();
         // Pixels are truncated toward zero, and coords increase toward East and South in web Mercator, so <= south/east.
         for (int y = north; y <= south; y++) {
-            if (y < 0 || y >= this.height) continue;
+            if (y < 0 || y >= extents.height) continue;
             for (int x = west; x <= east; x++) {
-                if (x < 0 || x >= this.width) continue;
+                if (x < 0 || x >= extents.width) continue;
                 // Calculate the 1D (flattened) index into this pointset for the grid cell at (x,y).
-                int pointIndex = y * this.width + x;
+                int pointIndex = y * extents.width + x;
                 pointsInEnvelope.add(pointIndex);
             }
         }
@@ -162,8 +128,8 @@ public double getOpportunityCount (int i) {
     //@Override
     // TODO add this to the PointSet interface
     public String getPointId (int i) {
-        int y = i / this.width;
-        int x = i % this.width;
+        int y = i / extents.width;
+        int x = i % extents.width;
         return x + "," + y;
     }
 
@@ -172,14 +138,17 @@ public int getPointIndexContaining (Coordinate latLon) {
     }
 
     public int getPointIndexContaining (double lon, double lat) {
-        int x = lonToPixel(lon, zoom) - west;
-        int y = latToPixel(lat, zoom) - north;
-        return y * width + x;
+        int x = lonToPixel(lon, extents.zoom) - extents.west;
+        int y = latToPixel(lat, extents.zoom) - extents.north;
+        return y * extents.width + x;
     }
 
     @Override
-    public String toString () {
-        return "WebMercatorGridPointSet{" + "zoom=" + zoom + ", west=" + west + ", north=" + north + ", width=" + width + ", height=" + height + ", basePointSet=" + basePointSet + '}';
+    public String toString() {
+        return "WebMercatorGridPointSet{" +
+                "extents=" + extents +
+                ", basePointSet=" + basePointSet +
+                '}';
     }
 
     @Override
@@ -189,13 +158,8 @@ public Envelope getWgsEnvelope () {
 
     @Override
     public WebMercatorExtents getWebMercatorExtents () {
-        return new WebMercatorExtents(west, north, width, height, zoom);
-    }
-
-    public static int parseZoom(String zoomString) {
-        int zoom = (zoomString == null) ? DEFAULT_ZOOM : Integer.parseInt(zoomString);
-        checkArgument(zoom >= MIN_ZOOM && zoom <= MAX_ZOOM);
-        return zoom;
+        // WebMercatorExtents are immutable so we can return this without making a protective copy.
+        return extents;
     }
 
 }

src/main/java/com/conveyal/r5/streets/EgressCostTable.java

@@ -377,15 +377,15 @@ public static EgressCostTable geographicallyCroppedCopy (LinkedPointSet subLinka
                         int targetInSuperLinkage = distanceTable[i];
                         int distance = distanceTable[i + 1];
 
-                        int superX = targetInSuperLinkage % superGrid.width;
-                        int superY = targetInSuperLinkage / superGrid.width;
+                        int superX = targetInSuperLinkage % superGrid.extents.width;
+                        int superY = targetInSuperLinkage / superGrid.extents.width;
 
-                        int subX = superX + superGrid.west - subGrid.west;
-                        int subY = superY + superGrid.north - subGrid.north;
+                        int subX = superX + superGrid.extents.west - subGrid.extents.west;
+                        int subY = superY + superGrid.extents.north - subGrid.extents.north;
 
                         // Only retain distance information for points that fall within this sub-grid.
-                        if (subX >= 0 && subX < subGrid.width && subY >= 0 && subY < subGrid.height) {
-                            int targetInSubLinkage = subY * subGrid.width + subX;
+                        if (subX >= 0 && subX < subGrid.extents.width && subY >= 0 && subY < subGrid.extents.height) {
+                            int targetInSubLinkage = subY * subGrid.extents.width + subX;
                             newDistanceTable.add(targetInSubLinkage);
                             newDistanceTable.add(distance); // distance to target does not change when we crop the pointset
                         }

src/main/java/com/conveyal/r5/streets/LinkedPointSet.java

@@ -201,13 +201,13 @@ public LinkedPointSet (LinkedPointSet sourceLinkage, WebMercatorGridPointSet sub
             throw new IllegalArgumentException("Source linkage must be for a gridded point set.");
         }
         WebMercatorGridPointSet superGrid = (WebMercatorGridPointSet) sourceLinkage.pointSet;
-        if (superGrid.zoom != subGrid.zoom) {
+        if (superGrid.extents.zoom != subGrid.extents.zoom) {
             throw new IllegalArgumentException("Source and sub-grid zoom level do not match.");
         }
-        if (subGrid.west + subGrid.width < superGrid.west //sub-grid is entirely west of super-grid
-                || superGrid.west + superGrid.width < subGrid.west // super-grid is entirely west of sub-grid
-                || subGrid.north + subGrid.height < superGrid.north //sub-grid is entirely north of super-grid (note Web Mercator conventions)
-                || superGrid.north + superGrid.height < subGrid.north) { //super-grid is entirely north of sub-grid
+        if (subGrid.extents.west + subGrid.extents.width < superGrid.extents.west //sub-grid is entirely west of super-grid
+                || superGrid.extents.west + superGrid.extents.width < subGrid.extents.west // super-grid is entirely west of sub-grid
+                || subGrid.extents.north + subGrid.extents.height < superGrid.extents.north //sub-grid is entirely north of super-grid (note Web Mercator conventions)
+                || superGrid.extents.north + superGrid.extents.height < subGrid.extents.north) { //super-grid is entirely north of sub-grid
             LOG.warn("Sub-grid is entirely outside the super-grid.  Points will not be linked to any street edges.");
         }
 
@@ -219,7 +219,7 @@ public LinkedPointSet (LinkedPointSet sourceLinkage, WebMercatorGridPointSet sub
         this.baseLinkage = sourceLinkage;
         this.cropped = true; // This allows calling the correct cost table builder function later, see Javadoc on field.
 
-        int nCells = subGrid.width * subGrid.height;
+        int nCells = subGrid.extents.width * subGrid.extents.height;
         edges = new int[nCells];
         distancesToEdge_mm = new int[nCells];
         distances0_mm = new int[nCells];
@@ -231,16 +231,16 @@ public LinkedPointSet (LinkedPointSet sourceLinkage, WebMercatorGridPointSet sub
         // Copy a subset of linkage information (edges and distances for each cell) over from the source linkage to
         // the new sub-linkage. This basically crops a smaller rectangle out of the larger one (or copies it if
         // dimensions are the same). Variables x, y, and pixel are relative to the new linkage, not the source one.
-        for (int y = 0, pixel = 0; y < subGrid.height; y++) {
-            for (int x = 0; x < subGrid.width; x++, pixel++) {
-                int sourceColumn = subGrid.west + x - superGrid.west;
-                int sourceRow = subGrid.north + y - superGrid.north;
-                if (sourceColumn < 0 || sourceColumn >= superGrid.width || sourceRow < 0 || sourceRow >= superGrid.height) { //point is outside super-grid
+        for (int y = 0, pixel = 0; y < subGrid.extents.height; y++) {
+            for (int x = 0; x < subGrid.extents.width; x++, pixel++) {
+                int sourceColumn = subGrid.extents.west + x - superGrid.extents.west;
+                int sourceRow = subGrid.extents.north + y - superGrid.extents.north;
+                if (sourceColumn < 0 || sourceColumn >= superGrid.extents.width || sourceRow < 0 || sourceRow >= superGrid.extents.height) { //point is outside super-grid
                     // Set the edge value to -1 to indicate no linkage.
                     // Distances should never be read downstream, so they don't need to be set here.
                     edges[pixel] = -1;
                 } else { //point is inside super-grid
-                    int sourcePixel = sourceRow * superGrid.width + sourceColumn;
+                    int sourcePixel = sourceRow * superGrid.extents.width + sourceColumn;
                     edges[pixel] = sourceLinkage.edges[sourcePixel];
                     distancesToEdge_mm[pixel] = sourceLinkage.distancesToEdge_mm[sourcePixel];
                     distances0_mm[pixel] = sourceLinkage.distances0_mm[sourcePixel];

src/test/java/com/conveyal/r5/analyst/GridTest.java

@@ -181,7 +181,7 @@ private static void assertArrayEquals (
     /** Test that latitude/longitude to pixel conversions are correct */
     @Test
     public void testLatLonPixelConversions () {
-        final int ZOOM = WebMercatorGridPointSet.DEFAULT_ZOOM;
+        final int ZOOM = WebMercatorExtents.DEFAULT_ZOOM;
         for (double lat : new double [] { -75, -25, 0, 25, 75 }) {
             assertEquals(lat, Grid.pixelToLat(Grid.latToPixel(lat, ZOOM), ZOOM), 1e-2);
         }

src/test/java/com/conveyal/r5/analyst/network/GridLayout.java

@@ -18,7 +18,7 @@
 import java.util.UUID;
 import java.util.stream.Stream;
 
-import static com.conveyal.r5.analyst.WebMercatorGridPointSet.DEFAULT_ZOOM;
+import static com.conveyal.r5.analyst.WebMercatorExtents.DEFAULT_ZOOM;
 
 /**
  * This is used in testing, to represent and create gridded transport systems with very regular spacing of roads and

src/test/java/com/conveyal/r5/analyst/scenario/ModifyStreetsTest.java

@@ -83,12 +83,12 @@ private static TIntIntMap getReachedVertices(TransportNetwork network) {
         task.fromLat = FROM_LAT;
         task.fromLon = FROM_LON;
 
-        WebMercatorGridPointSet gps = new WebMercatorGridPointSet(network);
-        task.north = gps.north;
-        task.west = gps.west;
-        task.height = gps.height;
-        task.width = gps.width;
-        task.zoom = gps.zoom;
+        WebMercatorGridPointSet grid = new WebMercatorGridPointSet(network);
+        task.north = grid.extents.north;
+        task.west = grid.extents.west;
+        task.height = grid.extents.height;
+        task.width = grid.extents.width;
+        task.zoom = grid.extents.zoom;
 
         var streetRouter = new StreetRouter(network.streetLayer);
         streetRouter.profileRequest = task;