Merge pull request #10463 from pjsg/fix_triangulation

Fix issue with reprojection and double drawing pixels.

src/ol/reproj.js

@@ -2,11 +2,73 @@
  * @module ol/reproj
  */
 import {createCanvasContext2D} from './dom.js';
-import {containsCoordinate, createEmpty, extend, getHeight, getTopLeft, getWidth} from './extent.js';
+import {containsCoordinate, createEmpty, extend, forEachCorner, getCenter, getHeight, getTopLeft, getWidth} from './extent.js';
 import {solveLinearSystem} from './math.js';
 import {getPointResolution, transform} from './proj.js';
 import {assign} from './obj.js';
 
+let brokenDiagonalRendering_;
+
+/**
+ * This draws a small triangle into a canvas by setting the triangle as the clip region
+ * and then drawing a (too large) rectangle
+ *
+ * @param {CanvasRenderingContext2D} ctx The context in which to draw the triangle
+ * @param {number} u1 The x-coordinate of the second point. The first point is 0,0.
+ * @param {number} v1 The y-coordinate of the second point.
+ * @param {number} u2 The x-coordinate of the third point.
+ * @param {number} v2 The y-coordinate of the third point.
+ */
+function drawTestTriangle(ctx, u1, v1, u2, v2) {
+  ctx.beginPath();
+  ctx.moveTo(0, 0);
+  ctx.lineTo(u1, v1);
+  ctx.lineTo(u2, v2);
+  ctx.closePath();
+  ctx.save();
+  ctx.clip();
+  ctx.fillRect(0, 0, Math.max(u1, u2) + 1, Math.max(v1, v2));
+  ctx.restore();
+}
+
+/**
+ * Given the data from getImageData, see if the right values appear at the provided offset.
+ * Returns true if either the color or transparency is off
+ *
+ * @param {Uint8ClampedArray} data The data returned from getImageData
+ * @param {number} offset The pixel offset from the start of data.
+ * @return {boolean} true if the diagonal rendering is broken
+ */
+function verifyBrokenDiagonalRendering(data, offset) {
+  // the values ought to be close to the rgba(210, 0, 0, 0.75)
+  return Math.abs(data[offset * 4] - 210) > 2 || Math.abs(data[offset * 4 + 3] - 0.75 * 255) > 2;
+}
+
+/**
+ * Determines if the current browser configuration can render triangular clip regions correctly.
+ * This value is cached so the function is only expensive the first time called.
+ * Firefox on Windows (as of now) does not if HWA is enabled. See https://bugzilla.mozilla.org/show_bug.cgi?id=1606976
+ * IE also doesn't. Chrome works, and everything seems to work on OSX and Android. This function caches the
+ * result. I suppose that it is conceivably possible that a browser might flip modes while the app is
+ * running, but lets hope not.
+ *
+ * @return {boolean} true if the Diagonal Rendering is broken.
+ */
+function isBrokenDiagonalRendering() {
+  if (brokenDiagonalRendering_ === undefined) {
+    const ctx = document.createElement('canvas').getContext('2d');
+    ctx.globalCompositeOperation = 'lighter';
+    ctx.fillStyle = 'rgba(210, 0, 0, 0.75)';
+    drawTestTriangle(ctx, 4, 5, 4, 0);
+    drawTestTriangle(ctx, 4, 5, 0, 5);
+    const data = ctx.getImageData(0, 0, 3, 3).data;
+    brokenDiagonalRendering_ = verifyBrokenDiagonalRendering(data, 0) ||
+        verifyBrokenDiagonalRendering(data, 4) ||
+        verifyBrokenDiagonalRendering(data, 8);
+  }
+
+  return brokenDiagonalRendering_;
+}
 
 /**
  * Calculates ideal resolution to use from the source in order to achieve
@@ -55,20 +117,31 @@ export function calculateSourceResolution(sourceProj, targetProj,
 
 
 /**
- * Enlarge the clipping triangle point by 1 pixel to ensure the edges overlap
+ * Calculates ideal resolution to use from the source in order to achieve
- * in order to mask gaps caused by antialiasing.
+ * pixel mapping as close as possible to 1:1 during reprojection.
+ * The resolution is calculated regardless of what resolutions
+ * are actually available in the dataset (TileGrid, Image, ...).
  *
- * @param {number} centroidX Centroid of the triangle (x coordinate in pixels).
+ * @param {import("./proj/Projection.js").default} sourceProj Source projection.
- * @param {number} centroidY Centroid of the triangle (y coordinate in pixels).
+ * @param {import("./proj/Projection.js").default} targetProj Target projection.
- * @param {number} x X coordinate of the point (in pixels).
+ * @param {import("./extent.js").Extent} targetExtent Target extent
- * @param {number} y Y coordinate of the point (in pixels).
+ * @param {number} targetResolution Target resolution.
- * @return {import("./coordinate.js").Coordinate} New point 1 px farther from the centroid.
+ * @return {number} The best resolution to use. Can be +-Infinity, NaN or 0.
  */
-function enlargeClipPoint(centroidX, centroidY, x, y) {
+export function calculateSourceExtentResolution(sourceProj, targetProj,
-  const dX = x - centroidX;
+  targetExtent, targetResolution) {
-  const dY = y - centroidY;
+
-  const distance = Math.sqrt(dX * dX + dY * dY);
+  const targetCenter = getCenter(targetExtent);
-  return [Math.round(x + dX / distance), Math.round(y + dY / distance)];
+  let sourceResolution = calculateSourceResolution(sourceProj, targetProj, targetCenter, targetResolution);
+
+  if (!isFinite(sourceResolution) || sourceResolution <= 0) {
+    forEachCorner(targetExtent, function(corner) {
+      sourceResolution = calculateSourceResolution(sourceProj, targetProj, corner, targetResolution);
+      return isFinite(sourceResolution) && sourceResolution > 0;
+    });
+  }
+
+  return sourceResolution;
 }
 
 
@@ -106,6 +179,12 @@ export function render(width, height, pixelRatio,
 
   context.scale(pixelRatio, pixelRatio);
 
+  function pixelRound(value) {
+    return Math.round(value * pixelRatio) / pixelRatio;
+  }
+
+  context.globalCompositeOperation = 'lighter';
+
   const sourceDataExtent = createEmpty();
   sources.forEach(function(src, i, arr) {
     extend(sourceDataExtent, src.extent);
@@ -126,12 +205,15 @@ export function render(width, height, pixelRatio,
     const srcWidth = getWidth(src.extent);
     const srcHeight = getHeight(src.extent);
 
-    stitchContext.drawImage(
+    // This test should never fail -- but it does. Need to find a fix the upstream condition
-      src.image,
+    if (src.image.width > 0 && src.image.height > 0) {
-      gutter, gutter,
+      stitchContext.drawImage(
-      src.image.width - 2 * gutter, src.image.height - 2 * gutter,
+        src.image,
-      xPos * stitchScale, yPos * stitchScale,
+        gutter, gutter,
-      srcWidth * stitchScale, srcHeight * stitchScale);
+        src.image.width - 2 * gutter, src.image.height - 2 * gutter,
+        xPos * stitchScale, yPos * stitchScale,
+        srcWidth * stitchScale, srcHeight * stitchScale);
+    }
   });
 
   const targetTopLeft = getTopLeft(targetExtent);
@@ -194,15 +276,37 @@ export function render(width, height, pixelRatio,
 
     context.save();
     context.beginPath();
-    const centroidX = (u0 + u1 + u2) / 3;
-    const centroidY = (v0 + v1 + v2) / 3;
-    const p0 = enlargeClipPoint(centroidX, centroidY, u0, v0);
-    const p1 = enlargeClipPoint(centroidX, centroidY, u1, v1);
-    const p2 = enlargeClipPoint(centroidX, centroidY, u2, v2);
 
-    context.moveTo(p1[0], p1[1]);
+    if (isBrokenDiagonalRendering()) {
-    context.lineTo(p0[0], p0[1]);
+      // Make sure that everything is on pixel boundaries
-    context.lineTo(p2[0], p2[1]);
+      const u0r = pixelRound(u0);
+      const v0r = pixelRound(v0);
+      const u1r = pixelRound(u1);
+      const v1r = pixelRound(v1);
+      const u2r = pixelRound(u2);
+      const v2r = pixelRound(v2);
+      // Make sure that all lines are horizontal or vertical
+      context.moveTo(u1r, v1r);
+      // This is the diagonal line. Do it in 4 steps
+      const steps = 4;
+      const ud = u0r - u1r;
+      const vd = v0r - v1r;
+      for (let step = 0; step < steps; step++) {
+        // Go horizontally
+        context.lineTo(u1r + pixelRound((step + 1) * ud / steps), v1r + pixelRound(step * vd / (steps - 1)));
+        // Go vertically
+        if (step != (steps - 1)) {
+          context.lineTo(u1r + pixelRound((step + 1) * ud / steps), v1r + pixelRound((step + 1) * vd / (steps - 1)));
+        }
+      }
+      // We are almost at u0r, v0r
+      context.lineTo(u2r, v2r);
+    } else {
+      context.moveTo(u1, v1);
+      context.lineTo(u0, v0);
+      context.lineTo(u2, v2);
+    }
+
     context.clip();
 
     context.transform(

src/ol/reproj/Tile.js

@@ -7,9 +7,9 @@ import Tile from '../Tile.js';
 import TileState from '../TileState.js';
 import {listen, unlistenByKey} from '../events.js';
 import EventType from '../events/EventType.js';
-import {getArea, getCenter, getIntersection} from '../extent.js';
+import {getArea, getIntersection} from '../extent.js';
 import {clamp} from '../math.js';
-import {calculateSourceResolution, render as renderReprojected} from '../reproj.js';
+import {calculateSourceExtentResolution, render as renderReprojected} from '../reproj.js';
 import Triangulation from './Triangulation.js';
 
 
@@ -148,9 +148,8 @@ class ReprojTile extends Tile {
     const targetResolution = targetTileGrid.getResolution(
       this.wrappedTileCoord_[0]);
 
-    const targetCenter = getCenter(limitedTargetExtent);
+    const sourceResolution = calculateSourceExtentResolution(
-    const sourceResolution = calculateSourceResolution(
+      sourceProj, targetProj, limitedTargetExtent, targetResolution);
-      sourceProj, targetProj, targetCenter, targetResolution);
 
     if (!isFinite(sourceResolution) || sourceResolution <= 0) {
       // invalid sourceResolution -> EMPTY

src/ol/reproj/Triangulation.js

@@ -263,12 +263,19 @@ class Triangulation {
     }
 
     if (!needsSubdivision && this.maxSourceExtent_) {
-      if (!intersects(sourceQuadExtent, this.maxSourceExtent_)) {
+      if (isFinite(sourceQuadExtent[0]) &&
-        // whole quad outside source projection extent -> ignore
+          isFinite(sourceQuadExtent[1]) &&
-        return;
+          isFinite(sourceQuadExtent[2]) &&
+          isFinite(sourceQuadExtent[3])) {
+        if (!intersects(sourceQuadExtent, this.maxSourceExtent_)) {
+          // whole quad outside source projection extent -> ignore
+          return;
+        }
       }
     }
 
+    let isNotFinite = 0;
+
     if (!needsSubdivision) {
       if (!isFinite(aSrc[0]) || !isFinite(aSrc[1]) ||
           !isFinite(bSrc[0]) || !isFinite(bSrc[1]) ||
@@ -277,7 +284,16 @@ class Triangulation {
         if (maxSubdivision > 0) {
           needsSubdivision = true;
         } else {
-          return;
+          // It might be the case that only 1 of the points is infinite. In this case
+          // we can draw a single triangle with the other three points
+          isNotFinite =
+              ((!isFinite(aSrc[0]) || !isFinite(aSrc[1])) ? 8 : 0) +
+              ((!isFinite(bSrc[0]) || !isFinite(bSrc[1])) ? 4 : 0) +
+              ((!isFinite(cSrc[0]) || !isFinite(cSrc[1])) ? 2 : 0) +
+              ((!isFinite(dSrc[0]) || !isFinite(dSrc[1])) ? 1 : 0);
+          if (isNotFinite != 1 && isNotFinite != 2 && isNotFinite != 4 && isNotFinite != 8) {
+            return;
+          }
         }
       }
     }
@@ -336,8 +352,25 @@ class Triangulation {
       this.wrapsXInSource_ = true;
     }
 
-    this.addTriangle_(a, c, d, aSrc, cSrc, dSrc);
+    // Exactly zero or one of *Src is not finite
-    this.addTriangle_(a, b, c, aSrc, bSrc, cSrc);
+    // The triangles must have the diagonal line as the first side
+    // This is to allow easy code in reproj.s to make it straight for broken
+    // browsers that can't handle diagonal clipping
+    if ((isNotFinite & 0xb) == 0) {
+      this.addTriangle_(a, c, d, aSrc, cSrc, dSrc);
+    }
+    if ((isNotFinite & 0xe) == 0) {
+      this.addTriangle_(a, c, b, aSrc, cSrc, bSrc);
+    }
+    if (isNotFinite) {
+      // Try the other two triangles
+      if ((isNotFinite & 0xd) == 0) {
+        this.addTriangle_(b, d, a, bSrc, dSrc, aSrc);
+      }
+      if ((isNotFinite & 0x7) == 0) {
+        this.addTriangle_(b, d, c, bSrc, dSrc, cSrc);
+      }
+    }
   }
 
   /**

test/spec/ol/reproj/image.test.js

@@ -19,6 +19,20 @@ describe('ol.reproj.Image', function() {
       });
   }
 
+  function createTranslucentImage(pixelRatio) {
+    return new ReprojImage(
+      getProjection('EPSG:3857'), getProjection('EPSG:4326'),
+      [-180, -85, 180, 85], 10, pixelRatio,
+      function(extent, resolution, pixelRatio) {
+        return new ImageWrapper(extent, resolution, pixelRatio,
+          'data:image/png;base64,' +
+              'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8depePQAIiwMjFXlnJQAAAABJRU5ErkJggg==', null,
+          function(image, src) {
+            image.getImage().src = src;
+          });
+      });
+  }
+
   it('changes state as expected', function(done) {
     const image = createImage(1);
     expect(image.getState()).to.be(0); // IDLE
@@ -55,4 +69,26 @@ describe('ol.reproj.Image', function() {
     });
     image.load();
   });
+
+  it('has uniform color', function(done) {
+    const image = createTranslucentImage(1);
+    listen(image, 'change', function() {
+      if (image.getState() == 2) { // LOADED
+        const canvas = image.getImage();
+        expect(canvas.width).to.be(36);
+        expect(canvas.height).to.be(17);
+        const pixels = canvas.getContext('2d').getImageData(0, 0, canvas.width, canvas.height).data;
+
+        for (let i = 0; i < canvas.width * canvas.height * 4; i += 4) {
+          expect(pixels[i + 0]).to.be.within(pixels[0] - 2, pixels[0] + 2);
+          expect(pixels[i + 1]).to.be.within(pixels[1] - 2, pixels[1] + 2);
+          expect(pixels[i + 2]).to.be.within(pixels[2] - 2, pixels[2] + 2);
+          expect(pixels[i + 3]).to.be.within(pixels[3] - 2, pixels[3] + 2);
+        }
+        done();
+      }
+    });
+    image.load();
+  });
+
 });