Replace css color rgb(...) by #...

[calixteman]

• it's faster to generate the color code in using a table for components
• it's very likely a way faster to parse (when setting the color in the canvas)

[Snuffleupagus]

Interesting find; should the cases in /~https://github.com/mozilla/pdf.js/blob/master/src/display/svg.js be converted as well and possibly the Util.makeCssRgb method removed if it becomes unused?

[Snuffleupagus]

/botio-linux test

[pdfjsbot]

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/bf51015794c5981/output.txt

[pdfjsbot]

From: Bot.io (Linux m4)

---

Failed

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

Total script time: 24.93 mins

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

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

[timvandermeij]

Looks good, but this really needs a benchmark since this code is called very often and the question is what the effect of this change is; see /~https://github.com/mozilla/pdf.js/wiki/Benchmarking-your-changes for how to do that, and make sure to use different PDF files from the test/pdf folder for good coverage (especially image-heavy ones or other files that trigger this utility function often).

[calixteman]

I ran benchmarks with this file:
foo.pdf
I generated using a python script.
In this file, makeHexColors is called 406,890 times and the perf comparison before/after the change:

-- Grouped By browser, stat --
browser | stat         | Count | Baseline(ms) | Current(ms) | +/- |    %  | Result(P<.05)
------- | ------------ | ----- | ------------ | ----------- | --- | ----- | -------------
firefox | Overall      |    50 |         4578 |        4509 | -69 | -1.51 |        faster
firefox | Page Request |    50 |            6 |           8 |   2 | 25.81 |              
firefox | Rendering    |    50 |         4571 |        4501 | -71 | -1.54 |        faster

[timvandermeij]

Good improvement! If you push the fix for the comment above and re-run the benchmark with that fix applied, I'll also trigger the tests here.

[calixteman]

I already ran the benchmark with the template string.

[timvandermeij]

/botio test

[pdfjsbot]

From: Bot.io (Linux m4)

---

Received

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

Live output at: http://54.67.70.0:8877/35fe18a84d7ba8e/output.txt

[pdfjsbot]

From: Bot.io (Windows)

---

Received

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

Live output at: http://3.101.106.178:8877/ff8eee58c5ba6d1/output.txt

[pdfjsbot]

From: Bot.io (Linux m4)

---

Failed

Full output at http://54.67.70.0:8877/35fe18a84d7ba8e/output.txt

Total script time: 25.07 mins

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

Image differences available at: http://54.67.70.0:8877/35fe18a84d7ba8e/reftest-analyzer.html#web=eq.log

[pdfjsbot]

From: Bot.io (Windows)

---

Failed

Full output at http://3.101.106.178:8877/ff8eee58c5ba6d1/output.txt

Total script time: 29.46 mins

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

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

[timvandermeij]

Looks good!

/botio makeref

[pdfjsbot]

From: Bot.io (Linux m4)

---

Received

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

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

[pdfjsbot]

From: Bot.io (Windows)

---

Received

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

Live output at: http://3.101.106.178:8877/8e97dfbedf68794/output.txt

[pdfjsbot]

From: Bot.io (Windows)

---

Success

Full output at http://3.101.106.178:8877/8e97dfbedf68794/output.txt

Total script time: 26.83 mins

• Lint: Passed
• Make references: Passed
• Check references: Passed

[pdfjsbot]

From: Bot.io (Linux m4)

---

Failed

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

Total script time: 60.00 mins

• Lint: Passed

[timvandermeij]

/botio-linux makeref

[pdfjsbot]

From: Bot.io (Linux m4)

---

Received

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

Live output at: http://54.67.70.0:8877/3333c796a6c1372/output.txt

[pdfjsbot]

From: Bot.io (Linux m4)

---

Success

Full output at http://54.67.70.0:8877/3333c796a6c1372/output.txt

Total script time: 23.99 mins

• Lint: Passed
• Make references: Passed
• Check references: Passed

[SamyCookie]

just for information see also this https://stackoverflow.com/a/55200387 that also approve the idea to make a pre created table for performance.