Merge pull request #13103 from tschaub/limited-precision-rounding

Assume limited precision when rounding

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));
+}

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 x = floor((coordinate[0] - extent[0]) / resolution, DECIMALS);
-    const y = Math.floor((extent[3] - coordinate[1]) / resolution);
+    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 viewWidth = ceil(getWidth(extent) / imageResolution, DECIMALS);
-    const viewHeight = Math.ceil(getHeight(extent) / imageResolution);
+    const viewHeight = ceil(getHeight(extent) / imageResolution, DECIMALS);
     const viewExtent = getForViewAndSize(center, imageResolution, 0, [
       viewWidth,
       viewHeight,
     ]);
-    const requestWidth = Math.ceil(
+    const requestWidth = ceil(
-      (this.ratio_ * getWidth(extent)) / imageResolution
+      (this.ratio_ * getWidth(extent)) / imageResolution,
+      DECIMALS
     );
-    const requestHeight = Math.ceil(
+    const requestHeight = ceil(
-      (this.ratio_ * getHeight(extent)) / imageResolution
+      (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_[0] = round(
-    this.imageSize_[1] = Math.round(getHeight(requestExtent) / imageResolution);
+      getWidth(requestExtent) / imageResolution,
+      DECIMALS
+    );
+    this.imageSize_[1] = round(
+      getHeight(requestExtent) / imageResolution,
+      DECIMALS
+    );
 
     const url = this.getRequestUrl_(
       requestExtent,

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);

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]);
+      });
+    }
+  });
 });