Fix color of image masks inside uncolored patterns #8808

janpe2 · 2017-08-22T15:38:49Z

This is a patch for uncolored (/PaintType 2) tiling patterns that contain image masks. When uncolored patterns are used as a fill or stroke, the operator scn or SCN specifies the color of the pattern. If the pattern contains an image mask, PDF.js incorrectly draws it using black instead of the color specified by scn or SCN.

In my patch, I assign the proper color to the fillColor property of the CanvasExtraState object. That property is used as the color of image masks in functions paintImageMaskXObject...() in canvas.js.

yurydelendik · 2017-08-22T21:15:58Z

src/display/pattern_helper.js

@@ -360,7 +360,7 @@ var TilingPattern = (function TilingPatternClosure() {
      var graphics = canvasGraphicsFactory.createCanvasGraphics(tmpCtx);
      graphics.groupLevel = owner.groupLevel;

-      this.setFillAndStrokeStyleToContext(tmpCtx, paintType, color);
+      this.setFillAndStrokeStyleToContext(tmpCtx, paintType, color, graphics);


let's change signature to setFillAndStrokeStyleToGraphics(graphic, paintType, color) -- context is accessible via graphics.ctx.

yurydelendik · 2017-08-22T21:18:52Z

src/display/pattern_helper.js

@@ -412,6 +413,8 @@ var TilingPattern = (function TilingPatternClosure() {
            var cssColor = Util.makeCssRgb(color[0], color[1], color[2]);
            context.fillStyle = cssColor;
            context.strokeStyle = cssColor;
+            // Set color needed by image masks (fixes issues 3226 and 8741).
+            graphics.current.fillColor = cssColor;


do we need to do that for this.current.strokeColor =? do we need to do the same for PaintType.COLORED case?

janpe2 · 2017-08-29T16:36:35Z

do we need to do that for this.current.strokeColor =?

I don't know if any PDF would need this but I added it, just to be sure.

do we need to do the same for PaintType.COLORED case?

I added lines similar to the other case. Colored tiling patterns are a bit different: now the color argument in setFillAndStrokeStyleToContext() is meaningless (or null) because the PDF operators scn and SCN have no color operands.

timvandermeij · 2017-08-29T20:44:51Z

/botio-linux preview

timvandermeij · 2017-08-29T20:47:26Z

/botio test

Snuffleupagus · 2017-08-30T17:56:16Z

Fixes #8741, #3226.

Would it be possible to include the PDF file from either of those issues as a linked eq test-case, to prevent future regressions?

janpe2 · 2017-09-05T16:38:40Z

I added the test case.

By the way, my patch seems to fix also issue #8860.

Snuffleupagus · 2017-09-05T16:44:20Z

I added the test case.

Thank you; one question though, wouldn't it be enough to just test the first page of the document?
If so, please add "lastPage": 1, new test entry in test_manifest.json.

Snuffleupagus · 2017-09-05T18:11:59Z

/botio test

Snuffleupagus · 2017-09-05T18:19:31Z

@janpe2 Looking at the test logs above, it appears that the server prevented the PDF file from being downloaded:

Downloading http://arxiv.org/pdf/0705.4298.pdf to pdfs/issue3226.pdf...
Error during downloading of http://arxiv.org/pdf/0705.4298.pdf: HTTP 403
WARNING: File was not downloaded. See "pdfs/issue3226.pdf.error" file.
Unable to verify the checksum for the files that are used for testing.
Please re-download the files, or adjust the MD5 checksum in the manifest for the files listed above.

Would it be possible to link to the PDF file through e.g. https://archive.org/web/ instead, or do we need to use another PDF file for the test-case?

Edit: This prevents the new test-case from actually running, with messages such as:

TEST-SKIPPED | PDF was not downloaded issue3226 | in firefox | page1 round 1 | Loading PDF document: UnknownErrorException: PDFDocument: stream must have data
...
TEST-SKIPPED | PDF was not downloaded issue3226 | in chrome | page1 round 1 | Loading PDF document: UnknownErrorException: PDFDocument: stream must have data

timvandermeij · 2017-09-05T19:31:57Z

/botio test

janpe2 · 2017-09-05T20:08:12Z

What are those failed regression tests? Should I be worried about them?

timvandermeij · 2017-09-05T21:09:38Z

The problem is that the old file is not replaced. @yurydelendik Could you remove the old (cached) issue3226.pdf file from both bots so we can try again?

Snuffleupagus · 2017-09-06T08:12:36Z

The problem is that the old file is not replaced. @yurydelendik Could you remove the old (cached) issue3226.pdf file from both bots so we can try again?

I don't think that's going to help much, since the (Internet Archive) link does not actually point to a PDF file.
If you open https://web.archive.org/web/20170830175223/http://arxiv.org/pdf/0705.4298.pdf directly in a browser, you'll get the same thing that the test-suite gets when downloading the file: a HTML page which references a PDF file.

It seems to me that linking to this particular PDF file will probably not work, and that the easiest solution here would be to try and use a PDF file from one of the other referenced issues as a test-case instead.

pdfjsbot · 2017-09-11T13:56:12Z

From: Bot.io (Linux m4)

Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.67.70.0:8877/d3ecbe51f41b9e9/output.txt

pdfjsbot · 2017-09-11T13:56:12Z

From: Bot.io (Windows)

Received

Command cmd_test from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.215.176.217:8877/6b875cc3ee3d9bb/output.txt

pdfjsbot · 2017-09-11T14:12:45Z

From: Bot.io (Linux m4)

Success

Full output at http://54.67.70.0:8877/d3ecbe51f41b9e9/output.txt

Total script time: 16.55 mins

Font tests: Passed
Unit tests: Passed
Regression tests: Passed

pdfjsbot · 2017-09-11T14:25:56Z

From: Bot.io (Windows)

Failed

Full output at http://54.215.176.217:8877/6b875cc3ee3d9bb/output.txt

Total script time: 29.73 mins

Font tests: Passed
Unit tests: Passed
Regression tests: FAILED

Image differences available at: http://54.215.176.217:8877/6b875cc3ee3d9bb/reftest-analyzer.html#web=eq.log

Snuffleupagus

Looks good, thank you for the patch!

Snuffleupagus · 2017-09-12T11:27:54Z

/botio makeref

pdfjsbot · 2017-09-12T11:27:55Z

From: Bot.io (Linux m4)

Received

Command cmd_makeref from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.67.70.0:8877/c5a69837639ea9f/output.txt

pdfjsbot · 2017-09-12T11:27:55Z

From: Bot.io (Windows)

Received

Command cmd_makeref from @Snuffleupagus received. Current queue size: 1

Live output at: http://54.215.176.217:8877/ebe8b13be2ac83a/output.txt

pdfjsbot · 2017-09-12T11:43:35Z

From: Bot.io (Linux m4)

Success

Full output at http://54.67.70.0:8877/c5a69837639ea9f/output.txt

Total script time: 15.65 mins

Lint: Passed
Make references: Passed
Check references: Passed

pdfjsbot · 2017-09-12T12:25:55Z

From: Bot.io (Windows)

Success

Full output at http://54.215.176.217:8877/ebe8b13be2ac83a/output.txt

Total script time: 28.13 mins

Lint: Passed
Make references: Passed
Check references: Passed

Fix color of image masks inside uncolored patterns

yurydelendik reviewed Aug 22, 2017

View reviewed changes

timvandermeij added the tiling-type label Aug 22, 2017

janpe2 force-pushed the issue8741 branch from 26c4c6f to b881ffd Compare August 29, 2017 16:34

janpe2 force-pushed the issue8741 branch from b881ffd to e96104a Compare September 5, 2017 16:34

janpe2 force-pushed the issue8741 branch from e96104a to 36cb38e Compare September 5, 2017 16:50

janpe2 force-pushed the issue8741 branch from 36cb38e to 4deb3bd Compare September 5, 2017 18:47

mozilla deleted a comment from pdfjsbot Sep 5, 2017

Fix color of image masks inside uncolored patterns

8602039

janpe2 force-pushed the issue8741 branch from 4deb3bd to 8602039 Compare September 6, 2017 10:44

mozilla deleted a comment from pdfjsbot Sep 6, 2017

Snuffleupagus requested a review from yurydelendik September 6, 2017 11:34

mozilla deleted a comment from pdfjsbot Sep 11, 2017

Snuffleupagus approved these changes Sep 12, 2017

View reviewed changes

Snuffleupagus removed the request for review from yurydelendik September 12, 2017 11:27

Snuffleupagus merged commit f2618eb into mozilla:master Sep 12, 2017

janpe2 deleted the issue8741 branch December 19, 2017 19:36

movsb pushed a commit to movsb/pdf.js that referenced this pull request Jul 14, 2018

Merge pull request mozilla#8808 from janpe2/issue8741

b3e6933

Fix color of image masks inside uncolored patterns

Snuffleupagus mentioned this pull request Dec 2, 2020

Update the link for the "pr8808" test-case (issue 12680) #12681

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix color of image masks inside uncolored patterns #8808

Fix color of image masks inside uncolored patterns #8808

janpe2 commented Aug 22, 2017 •

edited by timvandermeij

Loading

yurydelendik Aug 22, 2017

yurydelendik Aug 22, 2017

janpe2 commented Aug 29, 2017

timvandermeij commented Aug 29, 2017

timvandermeij commented Aug 29, 2017

Snuffleupagus commented Aug 30, 2017 •

edited

Loading

janpe2 commented Sep 5, 2017

Snuffleupagus commented Sep 5, 2017 •

edited

Loading

Snuffleupagus commented Sep 5, 2017

Snuffleupagus commented Sep 5, 2017 •

edited

Loading

timvandermeij commented Sep 5, 2017

janpe2 commented Sep 5, 2017

timvandermeij commented Sep 5, 2017 •

edited

Loading

Snuffleupagus commented Sep 6, 2017 •

edited

Loading

pdfjsbot commented Sep 11, 2017

pdfjsbot commented Sep 11, 2017

pdfjsbot commented Sep 11, 2017

pdfjsbot commented Sep 11, 2017

Snuffleupagus left a comment

Snuffleupagus commented Sep 12, 2017

pdfjsbot commented Sep 12, 2017

pdfjsbot commented Sep 12, 2017

pdfjsbot commented Sep 12, 2017

pdfjsbot commented Sep 12, 2017

Fix color of image masks inside uncolored patterns #8808

Fix color of image masks inside uncolored patterns #8808

Conversation

janpe2 commented Aug 22, 2017 • edited by timvandermeij Loading

yurydelendik Aug 22, 2017

Choose a reason for hiding this comment

yurydelendik Aug 22, 2017

Choose a reason for hiding this comment

janpe2 commented Aug 29, 2017

timvandermeij commented Aug 29, 2017

timvandermeij commented Aug 29, 2017

Snuffleupagus commented Aug 30, 2017 • edited Loading

janpe2 commented Sep 5, 2017

Snuffleupagus commented Sep 5, 2017 • edited Loading

Snuffleupagus commented Sep 5, 2017

Snuffleupagus commented Sep 5, 2017 • edited Loading

timvandermeij commented Sep 5, 2017

janpe2 commented Sep 5, 2017

timvandermeij commented Sep 5, 2017 • edited Loading

Snuffleupagus commented Sep 6, 2017 • edited Loading

pdfjsbot commented Sep 11, 2017

From: Bot.io (Linux m4)

Received

pdfjsbot commented Sep 11, 2017

From: Bot.io (Windows)

Received

pdfjsbot commented Sep 11, 2017

From: Bot.io (Linux m4)

Success

pdfjsbot commented Sep 11, 2017

From: Bot.io (Windows)

Failed

Snuffleupagus left a comment

Choose a reason for hiding this comment

Snuffleupagus commented Sep 12, 2017

pdfjsbot commented Sep 12, 2017

From: Bot.io (Linux m4)

Received

pdfjsbot commented Sep 12, 2017

From: Bot.io (Windows)

Received

pdfjsbot commented Sep 12, 2017

From: Bot.io (Linux m4)

Success

pdfjsbot commented Sep 12, 2017

From: Bot.io (Windows)

Success

janpe2 commented Aug 22, 2017 •

edited by timvandermeij

Loading

Snuffleupagus commented Aug 30, 2017 •

edited

Loading

Snuffleupagus commented Sep 5, 2017 •

edited

Loading

Snuffleupagus commented Sep 5, 2017 •

edited

Loading

timvandermeij commented Sep 5, 2017 •

edited

Loading

Snuffleupagus commented Sep 6, 2017 •

edited

Loading