From bef4d8a4943cdb31a0f7c77033a71b13ae842947 Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Fri, 26 Jun 2020 00:16:52 +0200 Subject: [PATCH 1/7] Make proj4 transforms behave like built-in transforms --- src/ol/coordinate.js | 27 ++++++++++++-------- src/ol/proj.js | 54 +++++++++++++++++++++++++++++++++++++-- src/ol/proj/proj4.js | 15 +++++++++-- test/spec/ol/proj.test.js | 19 ++++++++++++++ 4 files changed, 100 insertions(+), 15 deletions(-) diff --git a/src/ol/coordinate.js b/src/ol/coordinate.js index ec9d2a101b..29048f173a 100644 --- a/src/ol/coordinate.js +++ b/src/ol/coordinate.js @@ -410,17 +410,22 @@ export function toStringXY(coordinate, opt_fractionDigits) { * @return {Coordinate} The coordinate within the real world extent. */ export function wrapX(coordinate, projection) { - const projectionExtent = projection.getExtent(); - if ( - projection.canWrapX() && - (coordinate[0] < projectionExtent[0] || - coordinate[0] >= projectionExtent[2]) - ) { - const worldWidth = getWidth(projectionExtent); - const worldsAway = Math.floor( - (coordinate[0] - projectionExtent[0]) / worldWidth - ); - coordinate[0] -= worldsAway * worldWidth; + const worldsAway = getWorldsAway(coordinate, projection); + if (worldsAway) { + coordinate[0] -= worldsAway * getWidth(projection.getExtent()); } return coordinate; } + +export function getWorldsAway(coordinate, projection) { + const projectionExtent = projection.getExtent(); + let worldsAway = 0; + if ( + projection.canWrapX() && + (coordinate[0] < projectionExtent[0] || coordinate[0] > projectionExtent[2]) + ) { + const worldWidth = getWidth(projectionExtent); + worldsAway = Math.floor((coordinate[0] - projectionExtent[0]) / worldWidth); + } + return worldsAway; +} diff --git a/src/ol/proj.js b/src/ol/proj.js index ed4e1e00b4..29f51cd817 100644 --- a/src/ol/proj.js +++ b/src/ol/proj.js @@ -71,9 +71,10 @@ import { clear as clearTransformFuncs, get as getTransformFunc, } from './proj/transforms.js'; -import {applyTransform} from './extent.js'; +import {applyTransform, getWidth} from './extent.js'; +import {clamp, modulo} from './math.js'; import {getDistance} from './sphere.js'; -import {modulo} from './math.js'; +import {getWorldsAway} from './coordinate.js'; /** * A projection as {@link module:ol/proj/Projection}, SRS identifier @@ -625,6 +626,55 @@ export function fromUserExtent(extent, destProjection) { return transformExtent(extent, userProjection, destProjection); } +/** + * Creates a safe coordinate transform function from a coordinate transform function. + * "Safe" means that it can handle wrapping of x-coordinates for global projections, + * and that coordinates exceeding the projection validity extent's range will be + * clamped to the validity range. + * @param {Projection} sourceProj Source projection. + * @param {Projection} destProj Destination projection. + * @param {function(import("./coordinate.js").Coordinate): import("./coordinate.js").Coordinate} forward Transform function (source to destiation). + * @param {function(import("./coordinate.js").Coordinate): import("./coordinate.js").Coordinate} inverse Transform function (destiation to source). + * @return {function(import("./coordinate.js").Coordinate): import("./coordinate.js").Coordinate} Safe transform function (source to destiation). + */ +export function createSafeCoordinateTransform( + sourceProj, + destProj, + forward, + inverse +) { + return function (coord) { + const worldsAway = getWorldsAway(coord, sourceProj); + const sourceExtent = sourceProj.getExtent(); + const destExtent = destProj.getExtent(); + let clampBottom, clampTop; + if (destExtent) { + const clampBottomLeft = inverse(destExtent.slice(0, 2)); + const clampTopRight = inverse(destExtent.slice(2, 4)); + clampBottom = isNaN(clampBottomLeft[1]) + ? sourceExtent + ? sourceExtent[1] + : undefined + : clampBottomLeft[1]; + clampTop = isNaN(clampTopRight[1]) + ? sourceExtent + ? sourceExtent[3] + : undefined + : clampTopRight[1]; + } + const transformed = sourceExtent + ? forward([ + coord[0] - worldsAway * getWidth(sourceExtent), + clampTop ? clamp(coord[1], clampBottom, clampTop) : coord[1], + ]) + : forward(coord); + if (worldsAway && destProj.canWrapX()) { + transformed[0] += worldsAway * getWidth(destProj.getExtent()); + } + return transformed; + }; +} + /** * Add transforms to and from EPSG:4326 and EPSG:3857. This function is called * by when this module is executed and should only need to be called again after diff --git a/src/ol/proj/proj4.js b/src/ol/proj/proj4.js index ca7c17cc35..f4fb255074 100644 --- a/src/ol/proj/proj4.js +++ b/src/ol/proj/proj4.js @@ -6,6 +6,7 @@ import { addCoordinateTransforms, addEquivalentProjections, addProjection, + createSafeCoordinateTransform, get, } from '../proj.js'; import {assign} from '../obj.js'; @@ -60,8 +61,18 @@ export function register(proj4) { addCoordinateTransforms( proj1, proj2, - transform.forward, - transform.inverse + createSafeCoordinateTransform( + proj1, + proj2, + transform.forward, + transform.inverse + ), + createSafeCoordinateTransform( + proj2, + proj1, + transform.inverse, + transform.forward + ) ); } } diff --git a/test/spec/ol/proj.test.js b/test/spec/ol/proj.test.js index 4d9592267e..bc5990256b 100644 --- a/test/spec/ol/proj.test.js +++ b/test/spec/ol/proj.test.js @@ -560,6 +560,25 @@ describe('ol.proj', function () { }); expect(getProjection('EPSG:4326')).to.equal(epsg4326); }); + + it('uses safe transform functions', function () { + register(proj4); + const wgs84 = getProjection('WGS84'); + const epsg4326 = getProjection('EPSG:4326'); + wgs84.setExtent(epsg4326.getExtent()); + wgs84.setGlobal(true); + const google = getProjection('GOOGLE'); + const epsg3857 = getProjection('EPSG:3857'); + google.setExtent(epsg3857.getExtent()); + google.setGlobal(true); + const coord = [-190, 90]; + const transformed = transform(coord, wgs84, google); + expect(transformed).to.eql(transform(coord, epsg4326, epsg3857)); + const got = transform(transformed, google, wgs84); + const expected = transform(transformed, epsg3857, epsg4326); + expect(got[0]).to.roughlyEqual(expected[0], 1e-9); + expect(got[1]).to.roughlyEqual(expected[1], 1e-9); + }); }); describe('ol.proj.getTransformFromProjections()', function () { From 39f4627b8c93152bdee456b4318284e027cc736f Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Fri, 26 Jun 2020 10:14:22 +0200 Subject: [PATCH 2/7] Improve readability and efficiency --- src/ol/proj.js | 45 +++++++++++++++++++++------------------------ 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/src/ol/proj.js b/src/ol/proj.js index 29f51cd817..81f9f05c92 100644 --- a/src/ol/proj.js +++ b/src/ol/proj.js @@ -644,32 +644,29 @@ export function createSafeCoordinateTransform( inverse ) { return function (coord) { - const worldsAway = getWorldsAway(coord, sourceProj); - const sourceExtent = sourceProj.getExtent(); - const destExtent = destProj.getExtent(); - let clampBottom, clampTop; - if (destExtent) { - const clampBottomLeft = inverse(destExtent.slice(0, 2)); - const clampTopRight = inverse(destExtent.slice(2, 4)); - clampBottom = isNaN(clampBottomLeft[1]) - ? sourceExtent - ? sourceExtent[1] - : undefined - : clampBottomLeft[1]; - clampTop = isNaN(clampTopRight[1]) - ? sourceExtent - ? sourceExtent[3] - : undefined - : clampTopRight[1]; + let x, y, worldsAway; + if (sourceProj.canWrapX()) { + worldsAway = getWorldsAway(coord, sourceProj); + const sourceExtent = sourceProj.getExtent(); + if (worldsAway && sourceExtent) { + x = coord[0] - worldsAway * getWidth(sourceExtent); + } } - const transformed = sourceExtent - ? forward([ - coord[0] - worldsAway * getWidth(sourceExtent), - clampTop ? clamp(coord[1], clampBottom, clampTop) : coord[1], - ]) - : forward(coord); + const destExtent = destProj.getExtent(); + if (destExtent) { + const clampMin = inverse(destExtent.slice(0, 2)); + const clampMax = inverse(destExtent.slice(2, 4)); + if (!isNaN(clampMin[1]) && !isNaN(clampMax[1])) { + y = clamp(coord[1], clampMin[1], clampMax[1]); + } + } + const transformed = forward( + x === undefined && y === undefined + ? coord + : [x === undefined ? coord[0] : x, y === undefined ? coord[1] : y] + ); if (worldsAway && destProj.canWrapX()) { - transformed[0] += worldsAway * getWidth(destProj.getExtent()); + transformed[0] += worldsAway * getWidth(destExtent); } return transformed; }; From fef349120bc762076fbf5e99483d10cbbb46cffa Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Fri, 26 Jun 2020 17:26:41 +0200 Subject: [PATCH 3/7] Avoid extra extent width calculation --- src/ol/coordinate.js | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/ol/coordinate.js b/src/ol/coordinate.js index 29048f173a..85b31f325c 100644 --- a/src/ol/coordinate.js +++ b/src/ol/coordinate.js @@ -406,26 +406,35 @@ export function toStringXY(coordinate, opt_fractionDigits) { * exclusive. * * @param {Coordinate} coordinate Coordinate. - * @param {import("./proj/Projection.js").default} projection Projection + * @param {import("./proj/Projection.js").default} projection Projection. * @return {Coordinate} The coordinate within the real world extent. */ export function wrapX(coordinate, projection) { - const worldsAway = getWorldsAway(coordinate, projection); + const worldWidth = getWidth(projection.getExtent()); + const worldsAway = getWorldsAway(coordinate, projection, worldWidth); if (worldsAway) { - coordinate[0] -= worldsAway * getWidth(projection.getExtent()); + coordinate[0] -= worldsAway * worldWidth; } return coordinate; } - -export function getWorldsAway(coordinate, projection) { +/** + * @param {Coordinate} coordinate Coordinate. + * @param {import("./proj/Projection.js").default} projection Projection. + * @param {number=} opt_sourceExtentWidth Width of the source extent. + * @return {number} Offset in world widths. + */ +export function getWorldsAway(coordinate, projection, opt_sourceExtentWidth) { const projectionExtent = projection.getExtent(); let worldsAway = 0; if ( projection.canWrapX() && (coordinate[0] < projectionExtent[0] || coordinate[0] > projectionExtent[2]) ) { - const worldWidth = getWidth(projectionExtent); - worldsAway = Math.floor((coordinate[0] - projectionExtent[0]) / worldWidth); + const sourceExtentWidth = + opt_sourceExtentWidth || getWidth(projectionExtent); + worldsAway = Math.floor( + (coordinate[0] - projectionExtent[0]) / sourceExtentWidth + ); } return worldsAway; } From 7b4b77433bffcd14c9af615b9cc50232c993e90d Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Fri, 26 Jun 2020 17:27:51 +0200 Subject: [PATCH 4/7] Only clamp to dest extent when transform failed --- src/ol/proj.js | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/ol/proj.js b/src/ol/proj.js index 81f9f05c92..f928da8d4b 100644 --- a/src/ol/proj.js +++ b/src/ol/proj.js @@ -646,26 +646,29 @@ export function createSafeCoordinateTransform( return function (coord) { let x, y, worldsAway; if (sourceProj.canWrapX()) { - worldsAway = getWorldsAway(coord, sourceProj); const sourceExtent = sourceProj.getExtent(); + const sourceExtentWidth = getWidth(sourceExtent); + worldsAway = getWorldsAway(coord, sourceProj, sourceExtentWidth); if (worldsAway && sourceExtent) { - x = coord[0] - worldsAway * getWidth(sourceExtent); + // Move x to the real world + x = coord[0] - worldsAway * sourceExtentWidth; } } + let transformed = forward(x === undefined ? coord : [x, coord[1]]); const destExtent = destProj.getExtent(); - if (destExtent) { - const clampMin = inverse(destExtent.slice(0, 2)); - const clampMax = inverse(destExtent.slice(2, 4)); - if (!isNaN(clampMin[1]) && !isNaN(clampMax[1])) { - y = clamp(coord[1], clampMin[1], clampMax[1]); + if (!isFinite(transformed[0]) || !isFinite(transformed[1])) { + // Try to recover from out-of-bounds transform + if (destExtent) { + const y1 = inverse(destExtent.slice(0, 2))[1]; + const y2 = inverse(destExtent.slice(2, 4))[1]; + if (isFinite(y1) && isFinite(y2)) { + y = clamp(coord[1], Math.min(y1, y2), Math.max(y1, y2)); + transformed = forward([x === undefined ? coord[0] : x, y]); + } } } - const transformed = forward( - x === undefined && y === undefined - ? coord - : [x === undefined ? coord[0] : x, y === undefined ? coord[1] : y] - ); if (worldsAway && destProj.canWrapX()) { + // Move transformed coordinate back to the offset world transformed[0] += worldsAway * getWidth(destExtent); } return transformed; From f41776cbb7b7c7c3a8b3de55238df26889724881 Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Fri, 26 Jun 2020 17:35:32 +0200 Subject: [PATCH 5/7] Also clamp x when transform failed --- src/ol/proj.js | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/ol/proj.js b/src/ol/proj.js index f928da8d4b..03368f8d54 100644 --- a/src/ol/proj.js +++ b/src/ol/proj.js @@ -659,12 +659,23 @@ export function createSafeCoordinateTransform( if (!isFinite(transformed[0]) || !isFinite(transformed[1])) { // Try to recover from out-of-bounds transform if (destExtent) { - const y1 = inverse(destExtent.slice(0, 2))[1]; - const y2 = inverse(destExtent.slice(2, 4))[1]; + const corner1 = inverse(destExtent.slice(0, 2)); + const corner2 = inverse(destExtent.slice(2, 4)); + const x1 = corner1[0]; + const x2 = corner2[0]; + const y1 = corner1[1]; + const y2 = corner2[1]; + if (isFinite(x1) && isFinite(x2)) { + x = clamp( + x == undefined ? coord[1] : x, + Math.min(x1, x2), + Math.max(x1, x2) + ); + } if (isFinite(y1) && isFinite(y2)) { y = clamp(coord[1], Math.min(y1, y2), Math.max(y1, y2)); - transformed = forward([x === undefined ? coord[0] : x, y]); } + transformed = forward([x, y]); } } if (worldsAway && destProj.canWrapX()) { From 36a57ce6cc6d0d0be4fc34fe6eeba478b2c99ccb Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Fri, 26 Jun 2020 19:29:36 +0200 Subject: [PATCH 6/7] Remove redundant check --- src/ol/proj.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ol/proj.js b/src/ol/proj.js index 03368f8d54..327d016a0d 100644 --- a/src/ol/proj.js +++ b/src/ol/proj.js @@ -649,7 +649,7 @@ export function createSafeCoordinateTransform( const sourceExtent = sourceProj.getExtent(); const sourceExtentWidth = getWidth(sourceExtent); worldsAway = getWorldsAway(coord, sourceProj, sourceExtentWidth); - if (worldsAway && sourceExtent) { + if (worldsAway) { // Move x to the real world x = coord[0] - worldsAway * sourceExtentWidth; } From 04598d06418bb2b57835b61ca382a14d0a741998 Mon Sep 17 00:00:00 2001 From: Andreas Hocevar Date: Fri, 26 Jun 2020 19:29:51 +0200 Subject: [PATCH 7/7] Consider all corners for min/max x/y check --- src/ol/proj.js | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/src/ol/proj.js b/src/ol/proj.js index 327d016a0d..347d4fa017 100644 --- a/src/ol/proj.js +++ b/src/ol/proj.js @@ -71,7 +71,14 @@ import { clear as clearTransformFuncs, get as getTransformFunc, } from './proj/transforms.js'; -import {applyTransform, getWidth} from './extent.js'; +import { + applyTransform, + getBottomLeft, + getBottomRight, + getTopLeft, + getTopRight, + getWidth, +} from './extent.js'; import {clamp, modulo} from './math.js'; import {getDistance} from './sphere.js'; import {getWorldsAway} from './coordinate.js'; @@ -659,21 +666,19 @@ export function createSafeCoordinateTransform( if (!isFinite(transformed[0]) || !isFinite(transformed[1])) { // Try to recover from out-of-bounds transform if (destExtent) { - const corner1 = inverse(destExtent.slice(0, 2)); - const corner2 = inverse(destExtent.slice(2, 4)); - const x1 = corner1[0]; - const x2 = corner2[0]; - const y1 = corner1[1]; - const y2 = corner2[1]; - if (isFinite(x1) && isFinite(x2)) { - x = clamp( - x == undefined ? coord[1] : x, - Math.min(x1, x2), - Math.max(x1, x2) - ); + const corner1 = inverse(getBottomLeft(destExtent)); + const corner2 = inverse(getBottomRight(destExtent)); + const corner3 = inverse(getTopLeft(destExtent)); + const corner4 = inverse(getTopRight(destExtent)); + const minX = Math.min(corner1[0], corner2[0], corner3[0], corner4[0]); + const maxX = Math.max(corner1[0], corner2[0], corner3[0], corner4[0]); + const minY = Math.min(corner1[1], corner2[1], corner3[1], corner4[1]); + const maxY = Math.max(corner1[1], corner2[1], corner3[1], corner4[1]); + if (isFinite(minX) && isFinite(maxX)) { + x = clamp(x == undefined ? coord[1] : x, minX, maxX); } - if (isFinite(y1) && isFinite(y2)) { - y = clamp(coord[1], Math.min(y1, y2), Math.max(y1, y2)); + if (isFinite(minY) && isFinite(maxY)) { + y = clamp(coord[1], minY, maxY); } transformed = forward([x, y]); }