Fix some issues with lineWidth < 1 after transform (bug 1753075)

 - it aims to fix https://bugzilla.mozilla.org/show_bug.cgi?id=1753075 and issue #13211;
 - previously we were trying to adjust lineWidth to have something correct after the current transform is applied but this approach was not correct because finally the pixel is rescaled with the same factors in both directions.
  And sometimes those factors must be different (see bug 1753075).
 - So the idea of this patch is to apply a scale matrix to the current transform just before setting lineWidth and stroking. This scale matrix is computed in order to ensure that after transform, a pixel will have its two thickness greater than 1.

src/display/canvas.js

@@ -1110,6 +1110,7 @@ class CanvasGraphics {
       // the transformation must already be set in canvasCtx._transformMatrix.
       addContextCurrentTransform(canvasCtx);
     }
+    this._cachedScaleForStroking = null;
     this._cachedGetSinglePixelWidth = null;
   }
 
@@ -1159,10 +1160,6 @@ class CanvasGraphics {
     this.viewportScale = viewport.scale;
 
     this.baseTransform = this.ctx.mozCurrentTransform.slice();
-    this._combinedScaleFactor = Math.hypot(
-      this.baseTransform[0],
-      this.baseTransform[2]
-    );
 
     if (this.imageLayer) {
       this.imageLayer.beginLayout();
@@ -1419,6 +1416,9 @@ class CanvasGraphics {
 
   // Graphics state
   setLineWidth(width) {
+    if (width !== this.current.lineWidth) {
+      this._cachedScaleForStroking = null;
+    }
     this.current.lineWidth = width;
     this.ctx.lineWidth = width;
   }
@@ -1608,13 +1608,15 @@ class CanvasGraphics {
       // Ensure that the clipping path is reset (fixes issue6413.pdf).
       this.pendingClip = null;
 
+      this._cachedScaleForStroking = null;
       this._cachedGetSinglePixelWidth = null;
     }
   }
 
   transform(a, b, c, d, e, f) {
     this.ctx.transform(a, b, c, d, e, f);
 
+    this._cachedScaleForStroking = null;
     this._cachedGetSinglePixelWidth = null;
   }
 
@@ -1751,33 +1753,17 @@ class CanvasGraphics {
     ctx.globalAlpha = this.current.strokeAlpha;
     if (this.contentVisible) {
       if (typeof strokeColor === "object" && strokeColor?.getPattern) {
-        const lineWidth = this.getSinglePixelWidth();
         ctx.save();
         ctx.strokeStyle = strokeColor.getPattern(
           ctx,
           this,
           ctx.mozCurrentTransformInverse,
           PathType.STROKE
         );
-        // Prevent drawing too thin lines by enforcing a minimum line width.
-        ctx.lineWidth = Math.max(lineWidth, this.current.lineWidth);
-        ctx.stroke();
+        this.rescaleAndStrokeNoSave();
         ctx.restore();
       } else {
-        const lineWidth = this.getSinglePixelWidth();
-        if (lineWidth < 0 && -lineWidth >= this.current.lineWidth) {
-          // The current transform will transform a square pixel into a
-          // parallelogram where both heights are lower than 1 and not equal.
-          ctx.save();
-          ctx.resetTransform();
-          ctx.lineWidth = Math.floor(this._combinedScaleFactor);
-          ctx.stroke();
-          ctx.restore();
-        } else {
-          // Prevent drawing too thin lines by enforcing a minimum line width.
-          ctx.lineWidth = Math.max(lineWidth, this.current.lineWidth);
-          ctx.stroke();
-        }
+        this.rescaleAndStroke();
       }
     }
     if (consumePath) {
@@ -2002,7 +1988,7 @@ class CanvasGraphics {
     this.moveText(0, this.current.leading);
   }
 
-  paintChar(character, x, y, patternTransform, resetLineWidthToOne) {
+  paintChar(character, x, y, patternTransform) {
     const ctx = this.ctx;
     const current = this.current;
     const font = current.font;
@@ -2038,10 +2024,6 @@ class CanvasGraphics {
         fillStrokeMode === TextRenderingMode.STROKE ||
         fillStrokeMode === TextRenderingMode.FILL_STROKE
       ) {
-        if (resetLineWidthToOne) {
-          ctx.resetTransform();
-          ctx.lineWidth = Math.floor(this._combinedScaleFactor);
-        }
         ctx.stroke();
       }
       ctx.restore();
@@ -2056,16 +2038,7 @@ class CanvasGraphics {
         fillStrokeMode === TextRenderingMode.STROKE ||
         fillStrokeMode === TextRenderingMode.FILL_STROKE
       ) {
-        if (resetLineWidthToOne) {
-          ctx.save();
-          ctx.moveTo(x, y);
-          ctx.resetTransform();
-          ctx.lineWidth = Math.floor(this._combinedScaleFactor);
-          ctx.strokeText(character, 0, 0);
-          ctx.restore();
-        } else {
-          ctx.strokeText(character, x, y);
-        }
+        ctx.strokeText(character, x, y);
       }
     }
 
@@ -2156,7 +2129,6 @@ class CanvasGraphics {
     }
 
     let lineWidth = current.lineWidth;
-    let resetLineWidthToOne = false;
     const scale = current.textMatrixScale;
     if (scale === 0 || lineWidth === 0) {
       const fillStrokeMode =
@@ -2165,9 +2137,7 @@ class CanvasGraphics {
         fillStrokeMode === TextRenderingMode.STROKE ||
         fillStrokeMode === TextRenderingMode.FILL_STROKE
       ) {
-        this._cachedGetSinglePixelWidth = null;
         lineWidth = this.getSinglePixelWidth();
-        resetLineWidthToOne = lineWidth < 0;
       }
     } else {
       lineWidth /= scale;
@@ -2235,13 +2205,7 @@ class CanvasGraphics {
           // common case
           ctx.fillText(character, scaledX, scaledY);
         } else {
-          this.paintChar(
-            character,
-            scaledX,
-            scaledY,
-            patternTransform,
-            resetLineWidthToOne
-          );
+          this.paintChar(character, scaledX, scaledY, patternTransform);
           if (accent) {
             const scaledAccentX =
               scaledX + (fontSize * accent.offset.x) / fontSizeScale;
@@ -2251,8 +2215,7 @@ class CanvasGraphics {
               accent.fontChar,
               scaledAccentX,
               scaledAccentY,
-              patternTransform,
-              resetLineWidthToOne
+              patternTransform
             );
           }
         }
@@ -2277,6 +2240,7 @@ class CanvasGraphics {
     }
     ctx.restore();
     this.compose();
+
     return undefined;
   }
 
@@ -2300,6 +2264,7 @@ class CanvasGraphics {
     if (isTextInvisible || fontSize === 0) {
       return;
     }
+    this._cachedScaleForStroking = null;
     this._cachedGetSinglePixelWidth = null;
 
     ctx.save();
@@ -3104,46 +3069,140 @@ class CanvasGraphics {
   }
 
   getSinglePixelWidth() {
-    if (this._cachedGetSinglePixelWidth === null) {
-      // If transform is [a b] then a pixel (square) is transformed
-      //                 [c d]
-      // into a parallelogram: its area is the abs value of the determinant.
-      // This parallelogram has 2 heights:
-      //  - Area / |col_1|;
-      //  - Area / |col_2|.
-      // so in order to get a height of at least 1, pixel height
-      // must be computed as followed:
-      //  h = max(sqrt(a² + c²) / |det(M)|, sqrt(b² + d²) / |det(M)|).
-      // This is equivalent to:
-      //  h = max(|line_1_inv(M)|, |line_2_inv(M)|)
+    if (!this._cachedGetSinglePixelWidth) {
       const m = this.ctx.mozCurrentTransform;
-
-      const absDet = Math.abs(m[0] * m[3] - m[2] * m[1]);
-      const sqNorm1 = m[0] ** 2 + m[2] ** 2;
-      const sqNorm2 = m[1] ** 2 + m[3] ** 2;
-      const pixelHeight = Math.sqrt(Math.max(sqNorm1, sqNorm2)) / absDet;
-      if (sqNorm1 !== sqNorm2 && this._combinedScaleFactor * pixelHeight > 1) {
-        // The parallelogram isn't a square and at least one height
-        // is lower than 1 so the resulting line width must be 1
-        // but it cannot be achieved with one scale: when scaling a pixel
-        // we'll get a rectangle (see issue #12295).
-        // For example with matrix [0.001 0, 0, 100], a pixel is transformed
-        // in a rectangle 0.001x100. If we just scale by 1000 (to have a 1)
-        // then we'll get a rectangle 1x1e5 which is wrong.
-        // In this case, we must reset the transform, set linewidth to 1
-        // and then stroke.
-        this._cachedGetSinglePixelWidth = -(
-          this._combinedScaleFactor * pixelHeight
+      if (m[1] === 0 && m[2] === 0) {
+        // Fast path
+        this._cachedGetSinglePixelWidth = 1;
+      } else {
+        const absDet = Math.abs(m[0] * m[3] - m[2] * m[1]);
+        const normX = Math.hypot(m[0], m[1]);
+        const normY = Math.hypot(m[2], m[3]);
+        this._cachedGetSinglePixelWidth = Math.max(
+          1,
+          Math.floor(Math.max(normX, normY) / absDet)
         );
-      } else if (absDet > Number.EPSILON) {
-        this._cachedGetSinglePixelWidth = pixelHeight;
+      }
+    }
+    return this._cachedGetSinglePixelWidth;
+  }
+
+  getScaleForStroking() {
+    // A pixel has thicknessX = thicknessY = 1;
+    // A transformed pixel is a parallelogram and the thicknesses
+    // corresponds to the heights.
+    // The goal of this function is to rescale before setting the
+    // lineWidth in order to have both thicknesses greater or equal
+    // to 1 after transform.
+    if (!this._cachedScaleForStroking) {
+      const { lineWidth } = this.current;
+      const m = this.ctx.mozCurrentTransform;
+      let scaleX, scaleY;
+
+      // We must take into account that outputScale is a part of the
+      // current transform.
+      if (m[1] === 0 && m[2] === 0) {
+        // Fast path
+        const normX = Math.abs(m[0]);
+        const normY = Math.abs(m[3]);
+        if (lineWidth === 0) {
+          scaleX = 1 / normX;
+          scaleY = 1 / normY;
+        } else {
+          const scaledXLineWidth = normX * lineWidth;
+          const scaledYLineWidth = normY * lineWidth;
+          scaleX =
+            scaledXLineWidth < this.outputScaleX
+              ? this.outputScaleX / scaledXLineWidth
+              : 1;
+          scaleY =
+            scaledYLineWidth < this.outputScaleY
+              ? this.outputScaleY / scaledYLineWidth
+              : 1;
+        }
       } else {
-        // Matrix is non-invertible.
-        this._cachedGetSinglePixelWidth = 1;
+        // A pixel (base (x, y)) is transformed by M into a parallelogram:
+        //  - its area is |det(M)|;
+        //  - heightY (orthogonal to Mx) has a length: |det(M)| / norm(Mx);
+        //  - heightX (orthogonal to My) has a length: |det(M)| / norm(My).
+        // heightX and heightY are the thicknesses of the transformed pixel
+        // and they must be both greater or equal to 1.
+        const absDet = Math.abs(m[0] * m[3] - m[2] * m[1]);
+        const normX = Math.hypot(m[0], m[1]);
+        const normY = Math.hypot(m[2], m[3]);
+        if (lineWidth === 0) {
+          scaleX = normY / absDet;
+          scaleY = normX / absDet;
+        } else {
+          const baseArea = lineWidth * absDet;
+          const scaledNormY = normY * this.outputScaleX;
+          const scaledNormX = normX * this.outputScaleY;
+          scaleX = scaledNormY > baseArea ? scaledNormY / baseArea : 1;
+          scaleY = scaledNormX > baseArea ? scaledNormX / baseArea : 1;
+        }
       }
+      this._cachedScaleForStroking = [scaleX, scaleY];
     }
+    return this._cachedScaleForStroking;
+  }
 
-    return this._cachedGetSinglePixelWidth;
+  // Rescale before stroking in order to have a final lineWidth
+  // with both thicknesses greater or equal to 1.
+  rescaleAndStroke() {
+    const { ctx } = this;
+    const { lineWidth } = this.current;
+    const [scaleX, scaleY] = this.getScaleForStroking();
+    if (scaleX === 1 && scaleY === 1) {
+      ctx.lineWidth = lineWidth || 1;
+      ctx.stroke();
+      return;
+    }
+
+    const savedMatrix = ctx.mozCurrentTransform.slice();
+    const savedDashes = ctx.getLineDash().slice();
+    const savedDashOffset = ctx.lineDashOffset;
+
+    ctx.scale(scaleX, scaleY);
+    ctx.lineWidth = lineWidth || 1;
+
+    // How the dashed line is rendered depends on the current transform...
+    // so we added a rescale to handle too thin lines and consequently
+    // the way the line is dashed will be modified.
+    // If scaleX === scaleY, the dashed lines will be rendered correctly
+    // else we'll have some bugs (but only with too thin lines).
+    // Here we take the max... why not taking the min... or something else.
+    // Anyway, as said it's buggy when scaleX !== scaleY.
+    const scale = Math.max(scaleX, scaleY);
+    ctx.setLineDash(savedDashes.map(x => x / scale));
+    ctx.lineDashOffset /= scale;
+
+    ctx.stroke();
+    ctx.setTransform(...savedMatrix);
+    ctx.setLineDash(savedDashes);
+    ctx.lineDashOffset = savedDashOffset;
+  }
+
+  // Same as above, but it's expected to be called between a ctx.save()
+  // and a ctx.restore() (so no need save/restore the currentTransform).
+  rescaleAndStrokeNoSave() {
+    const { ctx } = this;
+    const { lineWidth } = this.current;
+    const [scaleX, scaleY] = this.getScaleForStroking();
+
+    ctx.lineWidth = lineWidth || 1;
+
+    if (scaleX === 1 && scaleY === 1) {
+      ctx.stroke();
+      return;
+    }
+
+    ctx.scale(scaleX, scaleY);
+
+    const scale = Math.max(scaleX, scaleY);
+    ctx.setLineDash(ctx.getLineDash().map(x => x / scale));
+    ctx.lineDashOffset /= scale;
+
+    ctx.stroke();
   }
 
   getCanvasPosition(x, y) {

test/pdfs/.gitignore

@@ -510,3 +510,4 @@
 !issue14307.pdf
 !issue14497.pdf
 !issue14502.pdf
+!issue13211.pdf

test/pdfs/bug1753075.pdf.link

@@ -0,0 +1 @@
+https://bugzilla.mozilla.org/attachment.cgi?id=9262522

test/test_manifest.json

@@ -6278,5 +6278,19 @@
       "rounds": 1,
       "link": true,
       "type": "other"
+   },
+   {  "id": "bug1753075",
+      "file": "pdfs/bug1753075.pdf",
+      "md5": "12716fa2dc3e0b3a61d88fef10abc7cf",
+      "rounds": 1,
+      "link": true,
+      "lastPage": 1,
+      "type": "eq"
+   },
+   {  "id": "issue13211",
+      "file": "pdfs/issue13211.pdf",
+      "md5": "d193853e8a123dc50eeea593a4150b60",
+      "rounds": 1,
+      "type": "eq"
    }
 ]