From 78bf0a36790f399e447fced00cd71f47d39ed569 Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Thu, 9 Dec 2021 13:22:29 -0500 Subject: [PATCH] Assume limited precision when rounding --- src/ol/math.js | 44 ++++++++++ src/ol/source/ImageWMS.js | 35 +++++--- .../{imagewms.test.js => ImageWMS.test.js} | 30 ++++++- test/node/ol/math.test.js | 82 +++++++++++++++++++ 4 files changed, 179 insertions(+), 12 deletions(-) rename test/browser/spec/ol/source/{imagewms.test.js => ImageWMS.test.js} (94%) diff --git a/src/ol/math.js b/src/ol/math.js index 8de0517fe6..96087f0d03 100644 --- a/src/ol/math.js +++ b/src/ol/math.js @@ -204,3 +204,47 @@ export function modulo(a, b) { export function lerp(a, b, x) { return a + x * (b - a); } + +/** + * Returns a number with a limited number of decimal digits. + * @param {number} n The input number. + * @param {number} decimals The maximum number of decimal digits. + * @return {number} The input number with a limited number of decimal digits. + */ +export function toFixed(n, decimals) { + const factor = Math.pow(10, decimals); + return Math.round(n * factor) / factor; +} + +/** + * Rounds a number to the nearest integer value considering only the given number + * of decimal digits (with rounding on the final digit). + * @param {number} n The input number. + * @param {number} decimals The maximum number of decimal digits. + * @return {number} The nearest integer. + */ +export function round(n, decimals) { + return Math.round(toFixed(n, decimals)); +} + +/** + * Rounds a number to the next smaller integer considering only the given number + * of decimal digits (with rounding on the final digit). + * @param {number} n The input number. + * @param {number} decimals The maximum number of decimal digits. + * @return {number} The next smaller integer. + */ +export function floor(n, decimals) { + return Math.floor(toFixed(n, decimals)); +} + +/** + * Rounds a number to the next bigger integer considering only the given number + * of decimal digits (with rounding on the final digit). + * @param {number} n The input number. + * @param {number} decimals The maximum number of decimal digits. + * @return {number} The next bigger integer. + */ +export function ceil(n, decimals) { + return Math.ceil(toFixed(n, decimals)); +} diff --git a/src/ol/source/ImageWMS.js b/src/ol/source/ImageWMS.js index ad815637de..4d0aaef839 100644 --- a/src/ol/source/ImageWMS.js +++ b/src/ol/source/ImageWMS.js @@ -12,6 +12,7 @@ import {appendParams} from '../uri.js'; import {assert} from '../asserts.js'; import {assign} from '../obj.js'; import {calculateSourceResolution} from '../reproj.js'; +import {ceil, floor, round} from '../math.js'; import {compareVersions} from '../string.js'; import { containsExtent, @@ -22,6 +23,12 @@ import { } from '../extent.js'; import {get as getProjection, transform} from '../proj.js'; +/** + * Number of decimal digits to consider in integer values when rounding. + * @type {number} + */ +const DECIMALS = 4; + /** * @const * @type {import("../size.js").Size} @@ -197,8 +204,8 @@ class ImageWMS extends ImageSource { }; assign(baseParams, this.params_, params); - const x = Math.floor((coordinate[0] - extent[0]) / resolution); - const y = Math.floor((extent[3] - coordinate[1]) / resolution); + const x = floor((coordinate[0] - extent[0]) / resolution, DECIMALS); + const y = floor((extent[3] - coordinate[1]) / resolution, DECIMALS); baseParams[this.v13_ ? 'I' : 'X'] = x; baseParams[this.v13_ ? 'J' : 'Y'] = y; @@ -290,17 +297,19 @@ class ImageWMS extends ImageSource { const imageResolution = resolution / pixelRatio; const center = getCenter(extent); - const viewWidth = Math.ceil(getWidth(extent) / imageResolution); - const viewHeight = Math.ceil(getHeight(extent) / imageResolution); + const viewWidth = ceil(getWidth(extent) / imageResolution, DECIMALS); + const viewHeight = ceil(getHeight(extent) / imageResolution, DECIMALS); const viewExtent = getForViewAndSize(center, imageResolution, 0, [ viewWidth, viewHeight, ]); - const requestWidth = Math.ceil( - (this.ratio_ * getWidth(extent)) / imageResolution + const requestWidth = ceil( + (this.ratio_ * getWidth(extent)) / imageResolution, + DECIMALS ); - const requestHeight = Math.ceil( - (this.ratio_ * getHeight(extent)) / imageResolution + const requestHeight = ceil( + (this.ratio_ * getHeight(extent)) / imageResolution, + DECIMALS ); const requestExtent = getForViewAndSize(center, imageResolution, 0, [ requestWidth, @@ -327,8 +336,14 @@ class ImageWMS extends ImageSource { }; assign(params, this.params_); - this.imageSize_[0] = Math.round(getWidth(requestExtent) / imageResolution); - this.imageSize_[1] = Math.round(getHeight(requestExtent) / imageResolution); + this.imageSize_[0] = round( + getWidth(requestExtent) / imageResolution, + DECIMALS + ); + this.imageSize_[1] = round( + getHeight(requestExtent) / imageResolution, + DECIMALS + ); const url = this.getRequestUrl_( requestExtent, diff --git a/test/browser/spec/ol/source/imagewms.test.js b/test/browser/spec/ol/source/ImageWMS.test.js similarity index 94% rename from test/browser/spec/ol/source/imagewms.test.js rename to test/browser/spec/ol/source/ImageWMS.test.js index a74478b520..ef2c15991f 100644 --- a/test/browser/spec/ol/source/imagewms.test.js +++ b/test/browser/spec/ol/source/ImageWMS.test.js @@ -3,10 +3,14 @@ import ImageState from '../../../../../src/ol/ImageState.js'; import ImageWMS from '../../../../../src/ol/source/ImageWMS.js'; import Map from '../../../../../src/ol/Map.js'; import View from '../../../../../src/ol/View.js'; -import {getHeight, getWidth} from '../../../../../src/ol/extent.js'; +import { + getForViewAndSize, + getHeight, + getWidth, +} from '../../../../../src/ol/extent.js'; import {get as getProjection} from '../../../../../src/ol/proj.js'; -describe('ol.source.ImageWMS', function () { +describe('ol/source/ImageWMS', function () { let extent, pixelRatio, options, optionsReproj, projection, resolution; beforeEach(function () { extent = [10, 20, 30, 40]; @@ -87,6 +91,28 @@ describe('ol.source.ImageWMS', function () { expect(height).to.be(Math.round(height)); }); + it('does not request extra pixels due to floating point issues', function () { + const source = new ImageWMS({ + params: {LAYERS: 'layer'}, + url: 'http://example.com/wms', + ratio: 1, + }); + + const mapSize = [1110, 670]; + const rotation = 0; + const resolution = 354.64216525539024; + const center = [1224885.7248147277, 6681822.177576577]; + const extent = getForViewAndSize(center, resolution, rotation, mapSize); + const projection = getProjection('EPSG:3857'); + const image = source.getImage(extent, resolution, 1, projection); + const params = new URL(image.src_).searchParams; + + const imageWidth = Number(params.get('WIDTH')); + const imageHeight = Number(params.get('HEIGHT')); + expect(imageWidth).to.be(mapSize[0]); + expect(imageHeight).to.be(mapSize[1]); + }); + it('sets WIDTH and HEIGHT to match the aspect ratio of BBOX', function () { const source = new ImageWMS(options); const image = source.getImage(extent, resolution, pixelRatio, projection); diff --git a/test/node/ol/math.test.js b/test/node/ol/math.test.js index 0bf3838f9b..ab7fc0da1e 100644 --- a/test/node/ol/math.test.js +++ b/test/node/ol/math.test.js @@ -1,12 +1,16 @@ import expect from '../expect.js'; import { + ceil, clamp, cosh, + floor, lerp, log2, modulo, + round, solveLinearSystem, toDegrees, + toFixed, toRadians, } from '../../../src/ol/math.js'; @@ -162,4 +166,82 @@ describe('ol/math.js', () => { expect(lerp(0.25, 0.75, 0.5)).to.be(0.5); }); }); + + describe('toFixed', () => { + it('returns a number with a limited number of decimals', () => { + expect(toFixed(0.123456789, 3)).to.be(0.123); + }); + + it('rounds up', () => { + expect(toFixed(0.123456789, 4)).to.be(0.1235); + }); + + const cases = [ + [1.23456789, 0], + [1.23456789, 1], + [1.23456789, 2], + [1.23456789, 3], + [1.23456789, 4], + [1.23456789, 5], + [1.23456789, 6], + [1.23456789, 7], + [1.23456789, 8], + [1.23456789, 9], + [1.23456789, 10], + ]; + for (const c of cases) { + it(`provides numeric equivalent of (${c[0]}).toFixed(${c[1]})`, () => { + const string = c[0].toFixed(c[1]); + const expected = parseFloat(string); + const actual = toFixed(c[0], c[1]); + expect(actual).to.be(expected); + }); + } + }); + + describe('round', () => { + const cases = [ + [1.23, 2, 1], + [3.45, 1, 4], + [3.45, 2, 3], + [-3.45, 1, -3], + [-3.45, 2, -3], + ]; + + for (const c of cases) { + it(`works for round(${c[0]}, ${c[1]})`, () => { + expect(round(c[0], c[1])).to.be(c[2]); + }); + } + }); + + describe('floor', () => { + const cases = [ + [3.999, 4, 3], + [3.999, 2, 4], + [-3.01, 2, -4], + [-3.01, 1, -3], + ]; + + for (const c of cases) { + it(`works for floor(${c[0]}, ${c[1]})`, () => { + expect(floor(c[0], c[1])).to.be(c[2]); + }); + } + }); + + describe('ceil', () => { + const cases = [ + [4.001, 4, 5], + [4.001, 2, 4], + [-3.99, 3, -3], + [-3.99, 1, -4], + ]; + + for (const c of cases) { + it(`works for ceil(${c[0]}, ${c[1]})`, () => { + expect(ceil(c[0], c[1])).to.be(c[2]); + }); + } + }); });