[api-minor] Remove the use of (get/put)ImageData when drawing SMasks (bug 1874013) #18029

calixteman · 2024-04-30T15:16:22Z

and implement then in using some SVG filters and composition. Composing in using destination-in in order to multiply RGB components by the alpha from the mask isn't perfect: it'd be a way better to natively have alpha masks support, it induces some small rounding errors and consequently computed RGB are approximatively correct.
In term of performance, it's a real improvement, for example, the pdf in issue #17779 is now rendered in few seconds.
There are still some room for improvement, but overall it should be a way better.

calixteman · 2024-04-30T15:16:36Z

/botio test

moz-tools-bot · 2024-04-30T15:16:39Z

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.241.84.105:8877/0feb0b21b97d482/output.txt

moz-tools-bot · 2024-04-30T15:16:39Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/0a021c7af3d4282/output.txt

src/display/canvas.js

moz-tools-bot · 2024-04-30T15:44:12Z

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/0feb0b21b97d482/output.txt

Total script time: 27.54 mins

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

  different ref/snapshot: 65
  different first/second rendering: 2

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

moz-tools-bot · 2024-04-30T15:59:01Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/0a021c7af3d4282/output.txt

Total script time: 42.37 mins

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

  different ref/snapshot: 27

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

Snuffleupagus

This seems generally fine, with the movement in the reference-tests inline with what you'd expect from this sort of patch (since there's mostly off-by-one changes in the pixel-values).

One thing to note, that however should not block this PR: This will "break", and essentially regress, SMask rendering in Node.js environments.
Given that the PDF.js library is still usable for general PDF parsing in Node.js, with only rendering affected, combined with Node.js being listed as "mostly" supported in the FAQ this seems fine to me.
Users affected by this should thus be encouraged to implement/polyfill (general) filter-support for Node.js environments.

src/display/display_utils.js

src/display/canvas.js

Snuffleupagus · 2024-05-01T08:15:47Z

/botio-linux preview

moz-tools-bot · 2024-05-01T08:15:50Z

From: Bot.io (Linux m4)

Received

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

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

moz-tools-bot · 2024-05-01T08:17:01Z

From: Bot.io (Linux m4)

Success

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

Total script time: 1.18 mins

Published

Viewer: http://54.241.84.105:8877/a3d493b8457d6dc/web/viewer.html
Viewer (legacy): http://54.241.84.105:8877/a3d493b8457d6dc/legacy/web/viewer.html

calixteman · 2024-05-01T13:15:23Z

One thing to note, that however should not block this PR: This will "break", and essentially regress, SMask rendering in Node.js environments.

Did you already test pdf.js with https://www.npmjs.com/package/skia-canvas ? or with CanvasKit: https://www.npmjs.com/package/canvaskit-wasm ?

CanvasKit looks interesting:
https://www.npmjs.com/package/canvaskit-wasm#drop-in-canvas2d-replacement

Snuffleupagus · 2024-05-01T13:23:22Z

Did you already test pdf.js with https://www.npmjs.com/package/skia-canvas ? or with CanvasKit: https://www.npmjs.com/package/canvaskit-wasm ?

I've not done that, however it'd probably not help without additional work on the PDF.js side of things. (But as said above that should not block this PR, I just wanted to mention that to prevent surprises later.)

Note how nothing filter-related currently works in Node.js, since it's using

pdf.js/src/display/node_utils.js

Line 83 in 77e2182

class NodeFilterFactory extends BaseFilterFactory {}

which simply extends

pdf.js/src/display/base_factory.js

Lines 18 to 38 in 77e2182

    
           class BaseFilterFactory { 
        
             constructor() { 
        
               if (this.constructor === BaseFilterFactory) { 
        
                 unreachable("Cannot initialize BaseFilterFactory."); 
        
               } 
        
             } 
        
             addFilter(maps) { 
        
               return "none"; 
        
             } 
        
             addHCMFilter(fgColor, bgColor) { 
        
               return "none"; 
        
             } 
        
             addHighlightHCMFilter(filterName, fgColor, bgColor, newFgColor, newBgColor) { 
        
               return "none"; 
        
             } 
        
             destroy(keepHCM = false) {} 
        
           }

…(bug 1874013) and implement then in using some SVG filters and composition. Composing in using destination-in in order to multiply RGB components by the alpha from the mask isn't perfect: it'd be a way better to natively have alpha masks support, it induces some small rounding errors and consequently computed RGB are approximatively correct. In term of performance, it's a real improvement, for example, the pdf in issue mozilla#17779 is now rendered in few seconds. There are still some room for improvement, but overall it should be a way better.

Snuffleupagus

r=me, thank you!

calixteman · 2024-05-01T14:12:52Z

/botio makeref

moz-tools-bot · 2024-05-01T14:12:54Z

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.241.84.105:8877/31cf2e12253af63/output.txt

moz-tools-bot · 2024-05-01T14:12:54Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/706e735560c221e/output.txt

moz-tools-bot · 2024-05-01T14:32:54Z

From: Bot.io (Linux m4)

Success

Full output at http://54.241.84.105:8877/31cf2e12253af63/output.txt

Total script time: 19.98 mins

Lint: Passed
Make references: Passed
Check references: Passed

moz-tools-bot · 2024-05-01T14:38:15Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/706e735560c221e/output.txt

Total script time: 25.33 mins

Lint: Passed
Make references: FAILED

calixteman · 2024-05-01T14:39:46Z

/botio-windows makeref

moz-tools-bot · 2024-05-01T14:39:48Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/adec702218bff64/output.txt

moz-tools-bot · 2024-05-01T15:05:11Z

From: Bot.io (Windows)

Success

Full output at http://54.193.163.58:8877/adec702218bff64/output.txt

Total script time: 25.37 mins

Lint: Passed
Make references: Passed
Check references: Passed

luckymore · 2024-05-14T03:41:38Z

Good job, and When are we going to release it?

calixteman added performance smask labels Apr 30, 2024

github-advanced-security bot found potential problems Apr 30, 2024

View reviewed changes

src/display/canvas.js Fixed Show fixed Hide fixed

calixteman force-pushed the simplify_smask branch from b2a7b21 to f9bf4bf Compare April 30, 2024 16:31

calixteman requested a review from Snuffleupagus April 30, 2024 18:06

Snuffleupagus changed the title ~~Remove the use of (get/put)ImageData when drawing smaks (bug 1874013)~~ [api-minor] Remove the use of (get/put)ImageData when drawing smasks (bug 1874013) May 1, 2024

Snuffleupagus changed the title ~~[api-minor] Remove the use of (get/put)ImageData when drawing smasks (bug 1874013)~~ [api-minor] Remove the use of (get/put)ImageData when drawing SMasks (bug 1874013) May 1, 2024

Snuffleupagus reviewed May 1, 2024

View reviewed changes

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

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

src/display/canvas.js Show resolved Hide resolved

src/display/canvas.js Show resolved Hide resolved

Snuffleupagus linked an issue May 1, 2024 that may be closed by this pull request

Slow rendering of PDF with time spent in genericComposeSMask #17838

Closed

calixteman force-pushed the simplify_smask branch from f9bf4bf to 82989e6 Compare May 1, 2024 13:41

Snuffleupagus approved these changes May 1, 2024

View reviewed changes

calixteman merged commit 96ce111 into mozilla:master May 1, 2024
8 of 9 checks passed

This was referenced May 1, 2024

A single page pdf is taking 4 min to render #17779

Closed

New soft mask method causes some text not to be rendered #14982

Closed

Snuffleupagus linked an issue May 1, 2024 that may be closed by this pull request

Slow to render the 1st page of MagPi magazine issue 87 #17785

Closed

Snuffleupagus mentioned this pull request May 3, 2024

fixed memory leak when layer offset lower than mask offset #15102

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[api-minor] Remove the use of (get/put)ImageData when drawing SMasks (bug 1874013) #18029

[api-minor] Remove the use of (get/put)ImageData when drawing SMasks (bug 1874013) #18029

calixteman commented Apr 30, 2024

calixteman commented Apr 30, 2024

moz-tools-bot commented Apr 30, 2024

moz-tools-bot commented Apr 30, 2024

moz-tools-bot commented Apr 30, 2024

moz-tools-bot commented Apr 30, 2024

Snuffleupagus left a comment •

edited

Snuffleupagus commented May 1, 2024

moz-tools-bot commented May 1, 2024

moz-tools-bot commented May 1, 2024

calixteman commented May 1, 2024 •

edited

Snuffleupagus commented May 1, 2024 •

edited

Snuffleupagus left a comment

calixteman commented May 1, 2024

moz-tools-bot commented May 1, 2024

moz-tools-bot commented May 1, 2024

moz-tools-bot commented May 1, 2024

moz-tools-bot commented May 1, 2024

calixteman commented May 1, 2024

moz-tools-bot commented May 1, 2024

moz-tools-bot commented May 1, 2024

luckymore commented May 14, 2024

[api-minor] Remove the use of (get/put)ImageData when drawing SMasks (bug 1874013) #18029

[api-minor] Remove the use of (get/put)ImageData when drawing SMasks (bug 1874013) #18029

Conversation

calixteman commented Apr 30, 2024

calixteman commented Apr 30, 2024

moz-tools-bot commented Apr 30, 2024

From: Bot.io (Linux m4)

Received

moz-tools-bot commented Apr 30, 2024

From: Bot.io (Windows)

Received

moz-tools-bot commented Apr 30, 2024

From: Bot.io (Linux m4)

Failed

moz-tools-bot commented Apr 30, 2024

From: Bot.io (Windows)

Failed

Snuffleupagus left a comment • edited

Choose a reason for hiding this comment

Snuffleupagus commented May 1, 2024

moz-tools-bot commented May 1, 2024

From: Bot.io (Linux m4)

Received

moz-tools-bot commented May 1, 2024

From: Bot.io (Linux m4)

Success

Published

calixteman commented May 1, 2024 • edited

Snuffleupagus commented May 1, 2024 • edited

Snuffleupagus left a comment

Choose a reason for hiding this comment

calixteman commented May 1, 2024

moz-tools-bot commented May 1, 2024

From: Bot.io (Linux m4)

Received

moz-tools-bot commented May 1, 2024

From: Bot.io (Windows)

Received

moz-tools-bot commented May 1, 2024

From: Bot.io (Linux m4)

Success

moz-tools-bot commented May 1, 2024

From: Bot.io (Windows)

Failed

calixteman commented May 1, 2024

moz-tools-bot commented May 1, 2024

From: Bot.io (Windows)

Received

moz-tools-bot commented May 1, 2024

From: Bot.io (Windows)

Success

luckymore commented May 14, 2024

Snuffleupagus left a comment •

edited

calixteman commented May 1, 2024 •

edited

Snuffleupagus commented May 1, 2024 •

edited