Merge pull request #18798 from calixteman/pattern_negative_bbox

Fix the rendering of tiling pattern when the steps are lower than the tile dimensions (bug 1837738)

src/display/pattern_helper.js

@@ -471,14 +471,17 @@ class TilingPattern {
   }
 
   createPatternCanvas(owner) {
-    const operatorList = this.operatorList;
-    const bbox = this.bbox;
-    const xstep = this.xstep;
-    const ystep = this.ystep;
-    const paintType = this.paintType;
-    const tilingType = this.tilingType;
-    const color = this.color;
-    const canvasGraphicsFactory = this.canvasGraphicsFactory;
+    const {
+      bbox,
+      operatorList,
+      paintType,
+      tilingType,
+      color,
+      canvasGraphicsFactory,
+    } = this;
+    let { xstep, ystep } = this;
+    xstep = Math.abs(xstep);
+    ystep = Math.abs(ystep);
 
     info("TilingType: " + tilingType);
 
@@ -499,36 +502,55 @@ class TilingPattern {
     //   bbox boundary will be missing. This is INCORRECT behavior.
     //   "Figures on adjacent tiles should not overlap" (PDF spec 8.7.3.1),
     //   but overlapping cells without common pixels are still valid.
-    //   TODO: Fix the implementation, to allow this scenario to be painted
-    //   correctly.
 
     const x0 = bbox[0],
       y0 = bbox[1],
       x1 = bbox[2],
       y1 = bbox[3];
+    const width = x1 - x0;
+    const height = y1 - y0;
 
     // Obtain scale from matrix and current transformation matrix.
     const matrixScale = Util.singularValueDecompose2dScale(this.matrix);
     const curMatrixScale = Util.singularValueDecompose2dScale(
       this.baseTransform
     );
-    const combinedScale = [
-      matrixScale[0] * curMatrixScale[0],
-      matrixScale[1] * curMatrixScale[1],
-    ];
+    const combinedScaleX = matrixScale[0] * curMatrixScale[0];
+    const combinedScaleY = matrixScale[1] * curMatrixScale[1];
+
+    let canvasWidth = width,
+      canvasHeight = height,
+      redrawHorizontally = false,
+      redrawVertically = false;
+
+    const xScaledStep = Math.ceil(xstep * combinedScaleX);
+    const yScaledStep = Math.ceil(ystep * combinedScaleY);
+    const xScaledWidth = Math.ceil(width * combinedScaleX);
+    const yScaledHeight = Math.ceil(height * combinedScaleY);
+
+    if (xScaledStep >= xScaledWidth) {
+      canvasWidth = xstep;
+    } else {
+      redrawHorizontally = true;
+    }
+    if (yScaledStep >= yScaledHeight) {
+      canvasHeight = ystep;
+    } else {
+      redrawVertically = true;
+    }
 
     // Use width and height values that are as close as possible to the end
     // result when the pattern is used. Too low value makes the pattern look
     // blurry. Too large value makes it look too crispy.
     const dimx = this.getSizeAndScale(
-      xstep,
+      canvasWidth,
       this.ctx.canvas.width,
-      combinedScale[0]
+      combinedScaleX
     );
     const dimy = this.getSizeAndScale(
-      ystep,
+      canvasHeight,
       this.ctx.canvas.height,
-      combinedScale[1]
+      combinedScaleY
     );
 
     const tmpCanvas = owner.cachedCanvases.getCanvas(
@@ -543,48 +565,97 @@ class TilingPattern {
 
     this.setFillAndStrokeStyleToContext(graphics, paintType, color);
 
-    let adjustedX0 = x0;
-    let adjustedY0 = y0;
-    let adjustedX1 = x1;
-    let adjustedY1 = y1;
-    // Some bounding boxes have negative x0/y0 coordinates which will cause the
-    // some of the drawing to be off of the canvas. To avoid this shift the
-    // bounding box over.
-    if (x0 < 0) {
-      adjustedX0 = 0;
-      adjustedX1 += Math.abs(x0);
-    }
-    if (y0 < 0) {
-      adjustedY0 = 0;
-      adjustedY1 += Math.abs(y0);
-    }
-    tmpCtx.translate(-(dimx.scale * adjustedX0), -(dimy.scale * adjustedY0));
+    tmpCtx.translate(-dimx.scale * x0, -dimy.scale * y0);
     graphics.transform(dimx.scale, 0, 0, dimy.scale, 0, 0);
 
     // To match CanvasGraphics beginDrawing we must save the context here or
     // else we end up with unbalanced save/restores.
     tmpCtx.save();
 
-    this.clipBbox(graphics, adjustedX0, adjustedY0, adjustedX1, adjustedY1);
+    this.clipBbox(graphics, x0, y0, x1, y1);
 
     graphics.baseTransform = getCurrentTransform(graphics.ctx);
 
     graphics.executeOperatorList(operatorList);
 
     graphics.endDrawing();
 
+    tmpCtx.restore();
+
+    if (redrawHorizontally || redrawVertically) {
+      // The tile is overlapping itself, so we create a new tile with
+      // dimensions xstep * ystep.
+      // Then we draw the overlapping parts of the original tile on the new
+      // tile.
+      // Just as a side note, the code here works correctly even if we don't
+      // have to redraw the tile horizontally or vertically. In that case, the
+      // original tile is drawn on the new tile only once, but it's useless.
+      const image = tmpCanvas.canvas;
+      if (redrawHorizontally) {
+        canvasWidth = xstep;
+      }
+      if (redrawVertically) {
+        canvasHeight = ystep;
+      }
+
+      const dimx2 = this.getSizeAndScale(
+        canvasWidth,
+        this.ctx.canvas.width,
+        combinedScaleX
+      );
+      const dimy2 = this.getSizeAndScale(
+        canvasHeight,
+        this.ctx.canvas.height,
+        combinedScaleY
+      );
+
+      const xSize = dimx2.size;
+      const ySize = dimy2.size;
+      const tmpCanvas2 = owner.cachedCanvases.getCanvas(
+        "pattern-workaround",
+        xSize,
+        ySize,
+        true
+      );
+      const tmpCtx2 = tmpCanvas2.context;
+      const ii = redrawHorizontally ? Math.floor(width / xstep) : 0;
+      const jj = redrawVertically ? Math.floor(height / ystep) : 0;
+
+      // Draw the overlapping parts of the original tile on the new tile.
+      for (let i = 0; i <= ii; i++) {
+        for (let j = 0; j <= jj; j++) {
+          tmpCtx2.drawImage(
+            image,
+            xSize * i,
+            ySize * j,
+            xSize,
+            ySize,
+            0,
+            0,
+            xSize,
+            ySize
+          );
+        }
+      }
+      return {
+        canvas: tmpCanvas2.canvas,
+        scaleX: dimx2.scale,
+        scaleY: dimy2.scale,
+        offsetX: x0,
+        offsetY: y0,
+      };
+    }
+
     return {
       canvas: tmpCanvas.canvas,
       scaleX: dimx.scale,
       scaleY: dimy.scale,
-      offsetX: adjustedX0,
-      offsetY: adjustedY0,
+      offsetX: x0,
+      offsetY: y0,
     };
   }
 
   getSizeAndScale(step, realOutputSize, scale) {
-    // xstep / ystep may be negative -- normalize.
-    step = Math.abs(step);
     // MAX_PATTERN_SIZE is used to avoid OOM situation.
     // Use the destination canvas's size if it is bigger than the hard-coded
     // limit of MAX_PATTERN_SIZE to avoid clipping patterns that cover the

test/pdfs/.gitignore

@@ -669,3 +669,4 @@
 !issue18693.pdf
 !bug1918115.pdf
 !bug1919513.pdf
+!issue16038.pdf

test/pdfs/issue16444.pdf.link

@@ -0,0 +1 @@
+https://github.com/mozilla/pdf.js/files/11514576/imagesNotRenderingRegression.pdf

test/test_manifest.json

@@ -10500,5 +10500,20 @@
     "rounds": 1,
     "talos": false,
     "type": "eq"
+  },
+  {
+    "id": "issue16444",
+    "file": "pdfs/issue16444.pdf",
+    "md5": "b3f594ad122e281c615bb2d62c5ccd4d",
+    "rounds": 1,
+    "link": true,
+    "type": "eq"
+  },
+  {
+    "id": "issue16038",
+    "file": "pdfs/issue16038.pdf",
+    "md5": "47262993e04689c327b7ce85396bce99",
+    "rounds": 1,
+    "type": "eq"
   }
 ]