diff --git a/src/main/java/com/conveyal/analysis/controllers/DataSourceController.java b/src/main/java/com/conveyal/analysis/controllers/DataSourceController.java index 8fbcbe2da..c61f6a687 100644 --- a/src/main/java/com/conveyal/analysis/controllers/DataSourceController.java +++ b/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; /** diff --git a/src/main/java/com/conveyal/analysis/controllers/OpportunityDatasetController.java b/src/main/java/com/conveyal/analysis/controllers/OpportunityDatasetController.java index cd2932c03..b1afcfc05 100644 --- a/src/main/java/com/conveyal/analysis/controllers/OpportunityDatasetController.java +++ b/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; /** diff --git a/src/main/java/com/conveyal/analysis/datasource/derivation/AggregationAreaDerivation.java b/src/main/java/com/conveyal/analysis/datasource/derivation/AggregationAreaDerivation.java index dd16c11a1..07d162c10 100644 --- a/src/main/java/com/conveyal/analysis/datasource/derivation/AggregationAreaDerivation.java +++ b/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; diff --git a/src/main/java/com/conveyal/r5/analyst/GridTransformWrapper.java b/src/main/java/com/conveyal/r5/analyst/GridTransformWrapper.java index ddc5fcec5..1bc265c0e 100644 --- a/src/main/java/com/conveyal/r5/analyst/GridTransformWrapper.java +++ b/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; diff --git a/src/main/java/com/conveyal/r5/analyst/LinkageCache.java b/src/main/java/com/conveyal/r5/analyst/LinkageCache.java index 8ded0a681..0bc7b7738 100644 --- a/src/main/java/com/conveyal/r5/analyst/LinkageCache.java +++ b/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); } diff --git a/src/main/java/com/conveyal/r5/analyst/WebMercatorExtents.java b/src/main/java/com/conveyal/r5/analyst/WebMercatorExtents.java index d332d32f6..2be5d515a 100644 --- a/src/main/java/com/conveyal/r5/analyst/WebMercatorExtents.java +++ b/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. diff --git a/src/main/java/com/conveyal/r5/analyst/WebMercatorGridPointSet.java b/src/main/java/com/conveyal/r5/analyst/WebMercatorGridPointSet.java index bc5475c2c..6df8f731d 100644 --- a/src/main/java/com/conveyal/r5/analyst/WebMercatorGridPointSet.java +++ b/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; } } diff --git a/src/main/java/com/conveyal/r5/streets/EgressCostTable.java b/src/main/java/com/conveyal/r5/streets/EgressCostTable.java index 60bf5e336..fb97f4cff 100644 --- a/src/main/java/com/conveyal/r5/streets/EgressCostTable.java +++ b/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 } diff --git a/src/main/java/com/conveyal/r5/streets/LinkedPointSet.java b/src/main/java/com/conveyal/r5/streets/LinkedPointSet.java index 01a51a43d..36c1d3628 100644 --- a/src/main/java/com/conveyal/r5/streets/LinkedPointSet.java +++ b/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]; diff --git a/src/test/java/com/conveyal/r5/analyst/GridTest.java b/src/test/java/com/conveyal/r5/analyst/GridTest.java index 7e5bf62a4..c7f8833e0 100644 --- a/src/test/java/com/conveyal/r5/analyst/GridTest.java +++ b/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); } diff --git a/src/test/java/com/conveyal/r5/analyst/network/GridLayout.java b/src/test/java/com/conveyal/r5/analyst/network/GridLayout.java index 67afe013a..fccf12e15 100644 --- a/src/test/java/com/conveyal/r5/analyst/network/GridLayout.java +++ b/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 diff --git a/src/test/java/com/conveyal/r5/analyst/scenario/ModifyStreetsTest.java b/src/test/java/com/conveyal/r5/analyst/scenario/ModifyStreetsTest.java index 29b85c54c..533e08606 100644 --- a/src/test/java/com/conveyal/r5/analyst/scenario/ModifyStreetsTest.java +++ b/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;