From fc6448d18ce6000ef6711f5485bd982bddc0eb54 Mon Sep 17 00:00:00 2001 From: Rob Wu Date: Mon, 19 Jun 2017 12:40:48 +0200 Subject: [PATCH] Move svg:clipPath generation from clip to endPath In the PDF from issue 8527, the clip operator (W) shows up before a path is defined. The current SVG backend however expects a path to exist before generating a `` element. In the example, the path was defined after the clip, followed by a endPath operator (n). So this commit fixes the bug by moving the path generation logic from clip to endPath. Our canvas backend appears to use similar logic: `CanvasGraphics_endPath` calls `consumePath`, which in turn draws the clip and resets the `pendingClip` state. The canvas backend calls `consumePath` from multiple other places, so we probably need to check whether doing so is also necessary for the SVG backend. I scanned our corpus of PDF files in test/pdfs, and found that in every instance (except for one), the "W" PDF operator (clip) is immediately followed by "n" (endPath). The new test from this commit (clippath.pdf) starts with "W", followed by a path definition and then "n". # Commands used to find some of the clipping commands: grep -ra '^W$' -C7 | less -S grep -ra '^W ' -C7 | less -S grep -ra ' W$' -C7 | less -S test/pdfs/issue6413.pdf is the only file where "W" (a tline 55) is not followed by "n". In fact, the "W" is the last operation of a series of XObject painting operations, and removing it does not have any effect on the rendered PDF (confirmed by looking at the output of PDF.js's canvas backend, and ImageMagick's convert command). --- src/display/svg.js | 18 +++++++++++++----- test/pdfs/.gitignore | 1 + test/pdfs/clippath.pdf | 37 +++++++++++++++++++++++++++++++++++++ test/test_manifest.json | 8 ++++++++ 4 files changed, 59 insertions(+), 5 deletions(-) create mode 100644 test/pdfs/clippath.pdf diff --git a/src/display/svg.js b/src/display/svg.js index 2aa214824..0430c6650 100644 --- a/src/display/svg.js +++ b/src/display/svg.js @@ -361,6 +361,7 @@ SVGGraphics = (function SVGGraphicsClosure() { this.extraStack = []; this.commonObjs = commonObjs; this.objs = objs; + this.pendingClip = null; this.pendingEOFill = false; this.embedFonts = false; @@ -389,6 +390,7 @@ SVGGraphics = (function SVGGraphicsClosure() { this.transformMatrix = this.transformStack.pop(); this.current = this.extraStack.pop(); + this.pendingClip = null; this.tgrp = null; }, @@ -894,9 +896,10 @@ SVGGraphics = (function SVGGraphicsClosure() { current.setCurrentPoint(x, y); }, - endPath: function SVGGraphics_endPath() {}, - - clip: function SVGGraphics_clip(type) { + endPath: function SVGGraphics_endPath() { + if (!this.pendingClip) { + return; + } var current = this.current; // Add current path to clipping path var clipId = 'clippath' + clipCount; @@ -905,17 +908,18 @@ SVGGraphics = (function SVGGraphicsClosure() { clipPath.setAttributeNS(null, 'id', clipId); clipPath.setAttributeNS(null, 'transform', pm(this.transformMatrix)); var clipElement = current.element.cloneNode(); - if (type === 'evenodd') { + if (this.pendingClip === 'evenodd') { clipElement.setAttributeNS(null, 'clip-rule', 'evenodd'); } else { clipElement.setAttributeNS(null, 'clip-rule', 'nonzero'); } + this.pendingClip = null; clipPath.appendChild(clipElement); this.defs.appendChild(clipPath); if (current.activeClipUrl) { // The previous clipping group content can go out of order -- resetting - // cached clipGroup's. + // cached clipGroups. current.clipGroup = null; this.extraStack.forEach(function (prev) { prev.clipGroup = null; @@ -926,6 +930,10 @@ SVGGraphics = (function SVGGraphicsClosure() { this.tgrp = null; }, + clip: function SVGGraphics_clip(type) { + this.pendingClip = type; + }, + closePath: function SVGGraphics_closePath() { var current = this.current; var d = current.path.getAttributeNS(null, 'd'); diff --git a/test/pdfs/.gitignore b/test/pdfs/.gitignore index abaf1da18..774e4d31a 100644 --- a/test/pdfs/.gitignore +++ b/test/pdfs/.gitignore @@ -90,6 +90,7 @@ !issue3879r.pdf !issue5686.pdf !issue3928.pdf +!clippath.pdf !close-path-bug.pdf !issue6019.pdf !issue6621.pdf diff --git a/test/pdfs/clippath.pdf b/test/pdfs/clippath.pdf new file mode 100644 index 000000000..03d5316b7 --- /dev/null +++ b/test/pdfs/clippath.pdf @@ -0,0 +1,37 @@ +%PDF-1.1 +1 0 obj +<> +endobj +2 0 obj +<> +endobj +3 0 obj +<> +endobj +4 0 obj +<> +stream +W +40 20 m +160 20 l +160 80 l +40 80 l +h +n +0 0 0 sc +0 0 200 100 re +f +endstream +endobj +xref +0 5 +0000000000 65535 f +0000000009 00000 n +0000000054 00000 n +0000000128 00000 n +0000000186 00000 n +trailer +<> +startxref +299 +%%EOF diff --git a/test/test_manifest.json b/test/test_manifest.json index 2241eae98..4f2f176a9 100644 --- a/test/test_manifest.json +++ b/test/test_manifest.json @@ -2776,6 +2776,14 @@ "type": "eq", "about": "CFF font that is drawn with clipping." }, + { "id": "clippath", + "file": "pdfs/clippath.pdf", + "md5": "7ab95c0f106dccd90d6569f241fe8771", + "rounds": 1, + "link": false, + "type": "eq", + "about": "Clipping before a path exists, followed by adding a path and then drawing a rectangle." + }, { "id": "annotation-tx", "file": "pdfs/annotation-tx.pdf", "md5": "56321ea830be9c4f8437ca17ac535b2d",