Create shading patterns the size of the current path. (bug 1722807) #14230

brendandahl · 2021-11-03T21:22:23Z

Previously, when we created a shading pattern canvas we created it
as the same size as the page. This was good for caching if the same
pattern was used over and over again, but when lots of different
shadings are created that caused us to create many full page
canvases.

Instead of creating the full page canvses, instead create the canvas
as the same size as the current path bounding box. This reduces memory
consumption by a lot since most paths are pretty small. Also, in real world
PDFs it's rare for a shading (non shading fill) to be reused over and over again.
Bug 1721949 is an example where the same pattern is reused and it will be slightly
slower than before.

brendandahl · 2021-11-03T21:25:28Z

Some notes from performance testing:

bug1722807_page2 is MUCH faster going from 62,066ms to 330ms in Firefox and the several GB memory increase is gone
bug1721949 is a bit slower, but a pretty small difference

-- Grouped By pdf, stat, browser --

pdf	stat	browser	Count	Baseline(ms)	Current(ms)	+/-	%	Result(P<.05)
bug1721949	Overall	chrome	50	136	168	32	23.27	slower
bug1721949	Overall	firefox	50	175	185	10	5.87
bug1721949	Page Request	chrome	50	2	2	0	4.82
bug1721949	Page Request	firefox	50	2	2	0	-8.57
bug1721949	Rendering	chrome	50	135	166	32	23.56	slower
bug1721949	Rendering	firefox	50	172	183	10	6.06
bug1722807_page2	Overall	chrome	5	868	308	-561	-64.56
bug1722807_page2	Overall	firefox	5	62069	332	-61737	-99.47	faster
bug1722807_page2	Page Request	chrome	5	6	4	-2	-27.59
bug1722807_page2	Page Request	firefox	5	3	2	-1	-38.46
bug1722807_page2	Rendering	chrome	5	863	303	-559	-64.83
bug1722807_page2	Rendering	firefox	5	62066	330	-61736	-99.47	faster
issue13325_reduced	Overall	chrome	50	16	16	0	0.00
issue13325_reduced	Overall	firefox	50	24	11	-13	-54.85	faster
issue13325_reduced	Page Request	chrome	50	1	1	0	9.26
issue13325_reduced	Page Request	firefox	50	1	1	0	-37.04
issue13325_reduced	Rendering	chrome	50	15	15	0	-0.81
issue13325_reduced	Rendering	firefox	50	23	10	-13	-55.87	faster

brendandahl · 2021-11-03T21:27:42Z

/botio test

pdfjsbot · 2021-11-03T21:27:43Z

From: Bot.io (Linux m4)

Received

Command cmd_test from @brendandahl received. Current queue size: 1

Live output at: http://54.241.84.105:8877/2bfc0cde59f7be6/output.txt

pdfjsbot · 2021-11-03T21:27:43Z

From: Bot.io (Windows)

Received

Command cmd_test from @brendandahl received. Current queue size: 1

Live output at: http://54.193.163.58:8877/4fa737c913b1a67/output.txt

pdfjsbot · 2021-11-03T21:53:28Z

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/2bfc0cde59f7be6/output.txt

Total script time: 23.11 mins

Font tests: Passed
Unit tests: Passed
Integration Tests: Passed
Regression tests: FAILED

  errors: 3
  different ref/snapshot: 47
  different first/second rendering: 1

Image differences available at: http://54.241.84.105:8877/2bfc0cde59f7be6/reftest-analyzer.html#web=eq.log

src/display/canvas.js

src/display/pattern_helper.js

pdfjsbot · 2021-11-03T22:28:29Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/4fa737c913b1a67/output.txt

Total script time: 34.45 mins

Font tests: Passed
Unit tests: Passed
Integration Tests: FAILED
Regression tests: FAILED

  errors: 643
  different ref/snapshot: 40
  different first/second rendering: 1

Image differences available at: http://54.193.163.58:8877/4fa737c913b1a67/reftest-analyzer.html#web=eq.log

brendandahl · 2021-11-04T00:56:44Z

Are these chrome annotation and form failures new? They seem to work fine locally in tests and in the viewer....

Snuffleupagus · 2021-11-04T11:23:10Z

Are these chrome annotation and form failures new?

I cannot recall having seen them before, unfortunately.
Also, note e.g. ShowText-ShadingPattern-page1 which is a regular eq-test without any annotations/forms.

They seem to work fine locally in tests and in the viewer....

When testing this patch in Google Chrome locally, I can reproduce the test-failures. However, there's no errors in the console that'd explain the rendering failures.

src/display/pattern_helper.js

brendandahl · 2021-11-05T15:41:19Z

I haven't fixed the annotation/form bug yet. Just pushing so I can try this out on linux/windows...

Snuffleupagus · 2021-11-05T16:01:53Z

src/display/pattern_helper.js

+      const ownerBBox = owner.current.getClippedPathBoundingBox(
+        pathType,
+        ctx.mozCurrentTransform
+      );


Looking at the previous test-results, there's also a breaking error which I assume is caused by this line (since this method uses Util.intersect which can return null).

TEST-UNEXPECTED-FAIL | test failed issue6961 | in firefox | page1 round 1 | render : TypeError: can't access property 2, ownerBBox is null

Nice catch, I totally missed that! It seems to fix the other issues. However, that PDF is now displaying wrong in a new way (it's wrong as is too). The computed bounding box for the shading is slightly too small. I'm not sure why yet. It's not going to be fun to debug, since that PDF has thousands of paths. :(

I still don't quite understand why breaking that PDF, breaks other PDFs. We shouldn't be reusing any canvases, but we must not be cleaning up something correctly when an error happens.

Could the problem be that we're actually re-using the same <canvas> for every page/document during tests, thus possibly keeping some bad graphics-state alive?

A while back I wanted to see if we could get rid of some intermittent failures by always creating a new <canvas> during tests, please see PR #12960; maybe we should see if that'd fixed these problems and potentially land that?

That brings up a good point! The canvas state should be getting reset when we assign to the width/height in driver.js, but in chrome we add our polyfill for mozCurrentTransform and that does NOT get reset. That would explain why this was only an issue in chrome. I'm thinking we should either override the width/height attributes of the canvas to reset the transform stuff or manually wipe out the transform stack and reset the current transform. The former is probably less error prone since we wouldn't have to remember to do it.

As for the bounding box being wrong in issue6961, that is because of #14241.

Previously, when we created a shading pattern canvas we created it as the same size as the page. This was good for caching if the same pattern was used over and over again, but when lots of different shadings are created that caused us to create many full page canvases. Instead of creating the full page canvses, create the canvas as the same size as the current path bounding box. This reduces memory consumption by a lot since most paths are pretty small. Also, in real world PDFs it's rare for a shading (non shading fill) to be reused over and over again. Bug 1721949 is an example where the same pattern is reused and it will be slightly slower than before.

brendandahl · 2021-11-06T03:44:55Z

/botio test

pdfjsbot · 2021-11-06T03:44:56Z

From: Bot.io (Linux m4)

Received

Command cmd_test from @brendandahl received. Current queue size: 1

Live output at: http://54.241.84.105:8877/648f5c85255a8e3/output.txt

pdfjsbot · 2021-11-06T03:44:56Z

From: Bot.io (Windows)

Received

Command cmd_test from @brendandahl received. Current queue size: 2

Live output at: http://54.193.163.58:8877/4137d3fe75c1c05/output.txt

pdfjsbot · 2021-11-06T04:19:22Z

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/648f5c85255a8e3/output.txt

Total script time: 21.10 mins

Font tests: Passed
Unit tests: Passed
Integration Tests: Passed
Regression tests: FAILED

  errors: 675
  different ref/snapshot: 749
  different first/second rendering: 3

Image differences available at: http://54.241.84.105:8877/648f5c85255a8e3/reftest-analyzer.html#web=eq.log

pdfjsbot · 2021-11-06T05:05:09Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/4137d3fe75c1c05/output.txt

Total script time: 41.97 mins

Font tests: Passed
Unit tests: Passed
Integration Tests: Passed
Regression tests: FAILED

  different ref/snapshot: 27
  different first/second rendering: 1

Image differences available at: http://54.193.163.58:8877/4137d3fe75c1c05/reftest-analyzer.html#web=eq.log

Snuffleupagus · 2021-11-06T07:54:48Z

/botio browsertest

pdfjsbot · 2021-11-06T07:54:49Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/749ae76a4cf4950/output.txt

pdfjsbot · 2021-11-06T07:54:49Z

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.241.84.105:8877/39c04ff731af90b/output.txt

pdfjsbot · 2021-11-06T08:13:20Z

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/39c04ff731af90b/output.txt

Total script time: 18.50 mins

Regression tests: FAILED

  errors: 635
  different ref/snapshot: 22

Image differences available at: http://54.241.84.105:8877/39c04ff731af90b/reftest-analyzer.html#web=eq.log

Snuffleupagus · 2021-11-06T08:14:30Z

/botio-linux browsertest

pdfjsbot · 2021-11-06T08:14:31Z

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.241.84.105:8877/c93d4e50c70dd2b/output.txt

Snuffleupagus · 2021-11-06T08:26:39Z

/botio-linux browsertest

pdfjsbot · 2021-11-06T08:26:40Z

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.241.84.105:8877/f4cf9759fe79ca9/output.txt

pdfjsbot · 2021-11-06T08:32:26Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/749ae76a4cf4950/output.txt

Total script time: 37.61 mins

Regression tests: FAILED

  different ref/snapshot: 30

Image differences available at: http://54.193.163.58:8877/749ae76a4cf4950/reftest-analyzer.html#web=eq.log

pdfjsbot · 2021-11-06T08:33:06Z

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/c93d4e50c70dd2b/output.txt

Total script time: 18.58 mins

Regression tests: FAILED

  errors: 614
  different ref/snapshot: 20
  different first/second rendering: 2

Image differences available at: http://54.241.84.105:8877/c93d4e50c70dd2b/reftest-analyzer.html#web=eq.log

Snuffleupagus · 2021-11-06T08:44:52Z

/botio-linux browsertest

pdfjsbot · 2021-11-06T08:44:54Z

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.241.84.105:8877/5fbcfe3d3546bce/output.txt

pdfjsbot · 2021-11-06T08:51:42Z

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/f4cf9759fe79ca9/output.txt

Total script time: 18.60 mins

Regression tests: FAILED

  errors: 614
  different ref/snapshot: 22
  different first/second rendering: 1

Image differences available at: http://54.241.84.105:8877/f4cf9759fe79ca9/reftest-analyzer.html#web=eq.log

pdfjsbot · 2021-11-06T09:11:56Z

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/5fbcfe3d3546bce/output.txt

Total script time: 20.23 mins

Regression tests: FAILED

  different ref/snapshot: 28
  different first/second rendering: 1

Image differences available at: http://54.241.84.105:8877/5fbcfe3d3546bce/reftest-analyzer.html#web=eq.log

Snuffleupagus

r=me, thank you!

Snuffleupagus · 2021-11-06T09:18:17Z

/botio makeref

pdfjsbot · 2021-11-06T09:18:19Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/39fe3bab8910866/output.txt

pdfjsbot · 2021-11-06T09:18:19Z

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.241.84.105:8877/a195abca852cd53/output.txt

pdfjsbot · 2021-11-06T09:39:12Z

From: Bot.io (Linux m4)

Success

Full output at http://54.241.84.105:8877/a195abca852cd53/output.txt

Total script time: 20.87 mins

Lint: Passed
Make references: Passed
Check references: Passed

pdfjsbot · 2021-11-06T09:54:44Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/39fe3bab8910866/output.txt

Total script time: 31.42 mins

Lint: Passed
Make references: FAILED

Snuffleupagus · 2021-11-06T10:05:27Z

/botio-windows makeref

pdfjsbot · 2021-11-06T10:05:28Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/330df8d5dadae8f/output.txt

pdfjsbot · 2021-11-06T10:43:42Z

From: Bot.io (Windows)

Success

Full output at http://54.193.163.58:8877/330df8d5dadae8f/output.txt

Total script time: 38.22 mins

Lint: Passed
Make references: Passed
Check references: Passed

timvandermeij · 2021-11-06T12:57:44Z

Awesome work on this and the other PRs; thank you!

Snuffleupagus reviewed Nov 3, 2021

View reviewed changes

src/display/canvas.js Outdated Show resolved Hide resolved

src/display/canvas.js Outdated Show resolved Hide resolved

src/display/pattern_helper.js Outdated Show resolved Hide resolved

src/display/pattern_helper.js Outdated Show resolved Hide resolved

Snuffleupagus added shading-pattern performance labels Nov 4, 2021

Snuffleupagus reviewed Nov 4, 2021

View reviewed changes

src/display/pattern_helper.js Show resolved Hide resolved

brendandahl force-pushed the reduce-gradient-size branch from 9f71e42 to 4e772f2 Compare November 5, 2021 15:40

Snuffleupagus reviewed Nov 5, 2021

View reviewed changes

brendandahl force-pushed the reduce-gradient-size branch from 4e772f2 to b56cca0 Compare November 6, 2021 03:44

Snuffleupagus approved these changes Nov 6, 2021

View reviewed changes

Snuffleupagus merged commit 38efd13 into mozilla:master Nov 6, 2021

Create shading patterns the size of the current path. (bug 1722807) #14230

Create shading patterns the size of the current path. (bug 1722807) #14230

Conversation

brendandahl commented Nov 3, 2021

brendandahl commented Nov 3, 2021 • edited Loading

brendandahl commented Nov 3, 2021

pdfjsbot commented Nov 3, 2021

From: Bot.io (Linux m4)

Received

pdfjsbot commented Nov 3, 2021

From: Bot.io (Windows)

Received

pdfjsbot commented Nov 3, 2021

From: Bot.io (Linux m4)

Failed

pdfjsbot commented Nov 3, 2021

From: Bot.io (Windows)

Failed

brendandahl commented Nov 4, 2021

Snuffleupagus commented Nov 4, 2021 • edited Loading

brendandahl commented Nov 5, 2021

Snuffleupagus Nov 5, 2021

Choose a reason for hiding this comment

brendandahl Nov 5, 2021

Choose a reason for hiding this comment

Snuffleupagus Nov 5, 2021 • edited Loading

Choose a reason for hiding this comment

brendandahl Nov 5, 2021

Choose a reason for hiding this comment

brendandahl Nov 5, 2021

Choose a reason for hiding this comment

brendandahl commented Nov 6, 2021

pdfjsbot commented Nov 6, 2021

From: Bot.io (Linux m4)

Received

pdfjsbot commented Nov 6, 2021

From: Bot.io (Windows)

Received

pdfjsbot commented Nov 6, 2021

From: Bot.io (Linux m4)

Failed

pdfjsbot commented Nov 6, 2021

From: Bot.io (Windows)

Failed

Snuffleupagus commented Nov 6, 2021

pdfjsbot commented Nov 6, 2021

From: Bot.io (Windows)

Received

pdfjsbot commented Nov 6, 2021

From: Bot.io (Linux m4)

Received

pdfjsbot commented Nov 6, 2021

From: Bot.io (Linux m4)

Failed

Snuffleupagus commented Nov 6, 2021

pdfjsbot commented Nov 6, 2021

From: Bot.io (Linux m4)

Received

Snuffleupagus commented Nov 6, 2021

pdfjsbot commented Nov 6, 2021

From: Bot.io (Linux m4)

Received

pdfjsbot commented Nov 6, 2021

From: Bot.io (Windows)

Failed

pdfjsbot commented Nov 6, 2021

From: Bot.io (Linux m4)

Failed

Snuffleupagus commented Nov 6, 2021

pdfjsbot commented Nov 6, 2021

From: Bot.io (Linux m4)

Received

pdfjsbot commented Nov 6, 2021

From: Bot.io (Linux m4)

Failed

pdfjsbot commented Nov 6, 2021

From: Bot.io (Linux m4)

Failed

Snuffleupagus left a comment

Choose a reason for hiding this comment

brendandahl commented Nov 3, 2021 •

edited

Loading

Snuffleupagus commented Nov 4, 2021 •

edited

Loading

Snuffleupagus Nov 5, 2021 •

edited

Loading