Skip to content

Commit

Permalink
Fix some issues with lineWidth < 1 after transform (bug 1753075)
Browse files Browse the repository at this point in the history
 - 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.
  • Loading branch information
calixteman committed Feb 5, 2022
1 parent 8281e64 commit 8fc8f1c
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 86 deletions.
191 changes: 105 additions & 86 deletions src/display/canvas.js
Original file line number Diff line number Diff line change
Expand Up @@ -1110,7 +1110,7 @@ class CanvasGraphics {
// the transformation must already be set in canvasCtx._transformMatrix.
addContextCurrentTransform(canvasCtx);
}
this._cachedGetSinglePixelWidth = null;
this._cachedScaleForStroking = null;
}

beginDrawing({
Expand Down Expand Up @@ -1419,6 +1419,9 @@ class CanvasGraphics {

// Graphics state
setLineWidth(width) {
if (width !== this.current.lineWidth) {
this._cachedScaleForStroking = null;
}
this.current.lineWidth = width;
this.ctx.lineWidth = width;
}
Expand Down Expand Up @@ -1608,14 +1611,14 @@ class CanvasGraphics {
// Ensure that the clipping path is reset (fixes issue6413.pdf).
this.pendingClip = null;

this._cachedGetSinglePixelWidth = null;
this._cachedScaleForStroking = null;
}
}

transform(a, b, c, d, e, f) {
this.ctx.transform(a, b, c, d, e, f);

this._cachedGetSinglePixelWidth = null;
this._cachedScaleForStroking = null;
}

// Path
Expand Down Expand Up @@ -1751,33 +1754,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) {
Expand Down Expand Up @@ -2038,11 +2025,8 @@ class CanvasGraphics {
fillStrokeMode === TextRenderingMode.STROKE ||
fillStrokeMode === TextRenderingMode.FILL_STROKE
) {
if (resetLineWidthToOne) {
ctx.resetTransform();
ctx.lineWidth = Math.floor(this._combinedScaleFactor);
}
ctx.stroke();
this._cachedScaleForStroking = null;
this.rescaleAndStrokeNoSave();
}
ctx.restore();
} else {
Expand All @@ -2056,16 +2040,11 @@ 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);
}
this._cachedScaleForStroking = null;
ctx.save();
ctx.moveTo(x, y);
this.rescaleAndStrokeTextNoSave(character);
ctx.restore();
}
}

Expand Down Expand Up @@ -2156,7 +2135,6 @@ class CanvasGraphics {
}

let lineWidth = current.lineWidth;
let resetLineWidthToOne = false;
const scale = current.textMatrixScale;
if (scale === 0 || lineWidth === 0) {
const fillStrokeMode =
Expand All @@ -2165,9 +2143,7 @@ class CanvasGraphics {
fillStrokeMode === TextRenderingMode.STROKE ||
fillStrokeMode === TextRenderingMode.FILL_STROKE
) {
this._cachedGetSinglePixelWidth = null;
lineWidth = this.getSinglePixelWidth();
resetLineWidthToOne = lineWidth < 0;
lineWidth = 1;
}
} else {
lineWidth /= scale;
Expand All @@ -2178,6 +2154,8 @@ class CanvasGraphics {
lineWidth /= fontSizeScale;
}

const savedCurrentLineWidth = current.lineWidth;
current.lineWidth = lineWidth;
ctx.lineWidth = lineWidth;

let x = 0,
Expand Down Expand Up @@ -2235,13 +2213,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;
Expand All @@ -2251,8 +2223,7 @@ class CanvasGraphics {
accent.fontChar,
scaledAccentX,
scaledAccentY,
patternTransform,
resetLineWidthToOne
patternTransform
);
}
}
Expand All @@ -2277,6 +2248,9 @@ class CanvasGraphics {
}
ctx.restore();
this.compose();

current.lineWidth = savedCurrentLineWidth;

return undefined;
}

Expand All @@ -2300,7 +2274,7 @@ class CanvasGraphics {
if (isTextInvisible || fontSize === 0) {
return;
}
this._cachedGetSinglePixelWidth = null;
this._cachedScaleForStroking = null;

ctx.save();
ctx.transform.apply(ctx, current.textMatrix);
Expand Down Expand Up @@ -3103,47 +3077,92 @@ class CanvasGraphics {
ctx.beginPath();
}

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)|)
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 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
);
} else if (absDet > Number.EPSILON) {
this._cachedGetSinglePixelWidth = pixelHeight;
const { lineWidth } = this.current;

// 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]);
const scaledXLineWidth = normX * lineWidth;
const scaledYLineWidth = normY * lineWidth;
const scaleX =
scaledXLineWidth < this.outputScaleX
? this.outputScaleX / scaledXLineWidth
: 1;
const scaleY =
scaledYLineWidth < this.outputScaleY
? this.outputScaleY / scaledYLineWidth
: 1;

this._cachedScaleForStroking = [scaleX, scaleY];
} 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).
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]);
const baseArea = lineWidth * absDet;
const scaledNormY = normY * this.outputScaleX;
const scaledNormX = normX * this.outputScaleY;
const scaleX = scaledNormY > baseArea ? scaledNormY / baseArea : 1;
const scaleY = scaledNormX > baseArea ? scaledNormX / baseArea : 1;
this._cachedScaleForStroking = [scaleX, scaleY];
}
}
return this._cachedScaleForStroking;
}

// Rescale before stroking in order to have a final lineWidth
// with both thicknesse greater or equal to 1.
rescaleAndStroke() {
const { ctx } = this;
const [scaleX, scaleY] = this.getScaleForStroking();
if (scaleX === 1 && scaleY === 1) {
ctx.lineWidth = this.current.lineWidth;
ctx.stroke();
return;
}

return this._cachedGetSinglePixelWidth;
const savedMatrix = ctx.mozCurrentTransform.slice();
ctx.scale(scaleX, scaleY);
ctx.lineWidth = this.current.lineWidth;
ctx.stroke();
ctx.setTransform(...savedMatrix);
}

// 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 [scaleX, scaleY] = this.getScaleForStroking();
if (scaleX !== 1 || scaleY !== 1) {
ctx.scale(scaleX, scaleY);
}
ctx.lineWidth = this.current.lineWidth;
ctx.stroke();
}

rescaleAndStrokeTextNoSave(text) {
const { ctx } = this;
const [scaleX, scaleY] = this.getScaleForStroking();
if (scaleX !== 1 || scaleY !== 1) {
ctx.scale(scaleX, scaleY);
}
ctx.lineWidth = this.current.lineWidth;
ctx.strokeText(text, 0, 0);
}

getCanvasPosition(x, y) {
Expand Down
1 change: 1 addition & 0 deletions test/pdfs/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -510,3 +510,4 @@
!issue14307.pdf
!issue14497.pdf
!issue14502.pdf
!issue13211.pdf
1 change: 1 addition & 0 deletions test/pdfs/bug1753075.pdf.link
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
https://bugzilla.mozilla.org/attachment.cgi?id=9262522
Binary file added test/pdfs/issue13211.pdf
Binary file not shown.
14 changes: 14 additions & 0 deletions test/test_manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -6256,5 +6256,19 @@
"value": "PDF.js PDF.js PDF.js"
}
}
},
{ "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"
}
]

0 comments on commit 8fc8f1c

Please sign in to comment.