[api-minor] Simplify how the list of points are structured #18163

calixteman · 2024-05-24T13:54:12Z

Instead of sending to the main thread an array of Objects for a list of points (or quadpoints), we'll send just a basic float buffer.
It should slightly improve performances (especially when cloning the data) and use slightly less memory.

calixteman · 2024-05-24T13:54:23Z

/botio test

moz-tools-bot · 2024-05-24T13:54:25Z

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

moz-tools-bot · 2024-05-24T13:54:28Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/3b67265eba93c9e/output.txt

moz-tools-bot · 2024-05-24T13:57:23Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/3b67265eba93c9e/output.txt

Total script time: 2.90 mins

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

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

calixteman · 2024-05-24T14:20:29Z

/botio-windows test

moz-tools-bot · 2024-05-24T14:20:33Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/384ec49f12783b3/output.txt

moz-tools-bot · 2024-05-24T14:22:12Z

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/10e5dd63bf23fe4/output.txt

Total script time: 27.78 mins

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

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

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

moz-tools-bot · 2024-05-24T14:33:37Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/384ec49f12783b3/output.txt

Total script time: 13.06 mins

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

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

calixteman · 2024-05-24T15:31:24Z

/botio test

moz-tools-bot · 2024-05-24T15:31:26Z

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

moz-tools-bot · 2024-05-24T15:31:26Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/78252ef7bea0fa7/output.txt

moz-tools-bot · 2024-05-24T15:59:13Z

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/9854630ec7de34d/output.txt

Total script time: 27.78 mins

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

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

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

calixteman · 2024-05-24T16:36:08Z

/botio-windows test

moz-tools-bot · 2024-05-24T16:36:11Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/63baf1d26558a4f/output.txt

moz-tools-bot · 2024-05-24T16:49:09Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/63baf1d26558a4f/output.txt

Total script time: 12.97 mins

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

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

Snuffleupagus · 2024-05-24T20:33:44Z

Two questions:

Are the ref-test "failures" actually expected?
If so, what's the explanation for the changes?

calixteman · 2024-05-26T17:09:03Z

Two questions:

* Are the ref-test "failures" actually expected?

* If so, what's the explanation for the changes?

I didn't expect the changes and their are due to the fact I chose to set the data in some Float32Array which makes the check failing:

pdf.js/src/display/annotation_layer.js

Lines 535 to 539 in 17e09e5

    
           if ( 
        
             rectTrX === trX && 
        
             rectTrY === trY && 
        
             rectBlX === blX && 
        
             rectBlY === blY

So in last version of the patch I changed for Float64Array.
I think we don't need to have 64-bits precision here and 32-bits is enough but it can be done in a follow-up.
And it'll require to make few other changes especially when we test the equality of two numbers with different precisions or we could just try to pass every floating data into Float32Array (which could make sense for data.rect).

calixteman · 2024-05-26T17:09:19Z

/botio test

moz-tools-bot · 2024-05-26T17:09:22Z

From: Bot.io (Linux m4)

Received

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

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

moz-tools-bot · 2024-05-26T17:09:22Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/6bfc227e0be2cc2/output.txt

moz-tools-bot · 2024-05-26T17:37:09Z

From: Bot.io (Linux m4)

Failed

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

Total script time: 27.74 mins

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

  different ref/snapshot: 18
  different first/second rendering: 4

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

moz-tools-bot · 2024-05-26T18:10:34Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/6bfc227e0be2cc2/output.txt

Total script time: 60.00 mins

calixteman · 2024-05-26T18:16:59Z

/botio-windows test

moz-tools-bot · 2024-05-26T18:17:01Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/5e113b023186972/output.txt

moz-tools-bot · 2024-05-26T19:00:23Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/5e113b023186972/output.txt

Total script time: 43.35 mins

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

  different ref/snapshot: 4

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

Snuffleupagus · 2024-05-28T08:40:08Z

I didn't expect the changes and their are due to the fact I chose to set the data in some Float32Array which makes the check failing:

Interesting, thank you!
This leads me to a, potentially stupid, question: Could we use Float32Array and utilize Math.fround(...) in that check?

calixteman · 2024-05-28T09:43:47Z

I didn't expect the changes and their are due to the fact I chose to set the data in some Float32Array which makes the check failing:

Interesting, thank you! This leads me to a, potentially stupid, question: Could we use Float32Array and utilize Math.fround(...) in that check?

Yes (thx for the tip: I didn't know fround) or just use a Float32Array for the data.rect, but as said I think it's worth doing that in follow-up, or do you prefer we switch to 32-bits precision in this patch ?

Snuffleupagus · 2024-05-28T10:06:42Z

[...] or just use a Float32Array for the data.rect,

Off the top of my head I cannot tell how much else that'd affect, so perhaps we shouldn't do that here.

My untested idea was that we'd use Float32Array as originally in this patch, and simply change

pdf.js/src/display/annotation_layer.js

Line 531 in 95a7de9

const [rectBlX, rectBlY, rectTrX, rectTrY] = this.data.rect;

into

const [rectBlX, rectBlY, rectTrX, rectTrY] = this.data.rect.map(e => Math.fround(e));

Instead of sending to the main thread an array of Objects for a list of points (or quadpoints), we'll send just a basic float buffer. It should slightly improve performances (especially when cloning the data) and use slightly less memory.

calixteman · 2024-05-30T13:36:31Z

/botio test

moz-tools-bot · 2024-05-30T13:36:34Z

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

moz-tools-bot · 2024-05-30T13:36:34Z

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/8eada7f5d96d4af/output.txt

moz-tools-bot · 2024-05-30T14:04:32Z

From: Bot.io (Linux m4)

Failed

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

Total script time: 27.96 mins

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

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

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

moz-tools-bot · 2024-05-30T14:17:39Z

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/8eada7f5d96d4af/output.txt

Total script time: 41.07 mins

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

  different ref/snapshot: 6

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

Snuffleupagus

r=me, thank you!

timvandermeij · 2024-05-30T16:33:33Z

Given that this is the first [api-minor] patch for the new release, could you create a patch to bump the library version to 4.4 with a reference to the merge commit of this patch, similar to 3ff93ed? Thanks!

calixteman force-pushed the simplify_list_points branch from 9213955 to 5b6aebb Compare May 24, 2024 15:30

mozilla deleted a comment from moz-tools-bot May 24, 2024

Snuffleupagus added the annotations label May 24, 2024

calixteman force-pushed the simplify_list_points branch from 5b6aebb to 28455c9 Compare May 26, 2024 17:03

timvandermeij added the performance label May 28, 2024

calixteman force-pushed the simplify_list_points branch from 28455c9 to 6fa98ac Compare May 30, 2024 13:36

Snuffleupagus approved these changes May 30, 2024

View reviewed changes

calixteman merged commit fdb3617 into mozilla:master May 30, 2024
9 checks passed

calixteman deleted the simplify_list_points branch May 30, 2024 16:25

ZeroXClem mentioned this pull request Aug 12, 2024

[Snyk] Upgrade pdfjs-dist from 4.0.269 to 4.4.168 ZeroXClem/ChunkViz#4

Closed

npanchal108 mentioned this pull request Aug 31, 2024

[Snyk] Upgrade pdfjs-dist from 4.0.269 to 4.5.136 npanchal108/ecommercebase#5

Open

[api-minor] Simplify how the list of points are structured #18163

[api-minor] Simplify how the list of points are structured #18163

Conversation

calixteman commented May 24, 2024

calixteman commented May 24, 2024

moz-tools-bot commented May 24, 2024

From: Bot.io (Linux m4)

Received

moz-tools-bot commented May 24, 2024

From: Bot.io (Windows)

Received

moz-tools-bot commented May 24, 2024

From: Bot.io (Windows)

Failed

calixteman commented May 24, 2024

moz-tools-bot commented May 24, 2024

From: Bot.io (Windows)

Received

moz-tools-bot commented May 24, 2024

From: Bot.io (Linux m4)

Failed

moz-tools-bot commented May 24, 2024

From: Bot.io (Windows)

Failed

calixteman commented May 24, 2024

moz-tools-bot commented May 24, 2024

From: Bot.io (Linux m4)

Received

moz-tools-bot commented May 24, 2024

From: Bot.io (Windows)

Received

moz-tools-bot commented May 24, 2024

From: Bot.io (Linux m4)

Failed

calixteman commented May 24, 2024

moz-tools-bot commented May 24, 2024

From: Bot.io (Windows)

Received

moz-tools-bot commented May 24, 2024

From: Bot.io (Windows)

Failed

Snuffleupagus commented May 24, 2024

calixteman commented May 26, 2024

calixteman commented May 26, 2024

moz-tools-bot commented May 26, 2024

From: Bot.io (Linux m4)

Received

moz-tools-bot commented May 26, 2024

From: Bot.io (Windows)

Received

moz-tools-bot commented May 26, 2024

From: Bot.io (Linux m4)

Failed

moz-tools-bot commented May 26, 2024

From: Bot.io (Windows)

Failed

calixteman commented May 26, 2024

moz-tools-bot commented May 26, 2024

From: Bot.io (Windows)

Received

moz-tools-bot commented May 26, 2024

From: Bot.io (Windows)

Failed

Snuffleupagus commented May 28, 2024

calixteman commented May 28, 2024

Snuffleupagus commented May 28, 2024

calixteman commented May 30, 2024

moz-tools-bot commented May 30, 2024

From: Bot.io (Linux m4)

Received

moz-tools-bot commented May 30, 2024

From: Bot.io (Windows)

Received

moz-tools-bot commented May 30, 2024

From: Bot.io (Linux m4)

Failed

moz-tools-bot commented May 30, 2024

From: Bot.io (Windows)

Failed

Snuffleupagus left a comment