Skip to content

[Editor] Allow Float32Array for quadpoints in annotations (bug 1907958)#18526

Merged
calixteman merged 1 commit into
mozilla:masterfrom
calixteman:bug1907958
Jul 31, 2024
Merged

[Editor] Allow Float32Array for quadpoints in annotations (bug 1907958)#18526
calixteman merged 1 commit into
mozilla:masterfrom
calixteman:bug1907958

Conversation

@calixteman

Copy link
Copy Markdown
Contributor

Added annotations could have some quadpoints (highlight, ink). The isNumberArray check was returning false and consequently the annotation wasn't printable.
The tests didn't catch this issue because the quadpoints were passed as Array.
So driver.js has been updated in order to pass them as Float32Array in order to be in a situation similar to the real life one.

@calixteman

Copy link
Copy Markdown
Contributor Author

/botio test

@moz-tools-bot

Copy link
Copy Markdown
Collaborator

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

@moz-tools-bot

Copy link
Copy Markdown
Collaborator

From: Bot.io (Windows)


Received

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

Live output at: http://54.193.163.58:8877/46053aafd269bc9/output.txt

@moz-tools-bot

Copy link
Copy Markdown
Collaborator

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/60b5d7476401114/output.txt

Total script time: 30.02 mins

  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  different ref/snapshot: 15

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

Comment thread src/core/core_utils.js
Added annotations could have some quadpoints (highlight, ink).
The isNumberArray check was returning false and consequently the annotation wasn't
printable.
The tests didn't catch this issue because the quadpoints were passed as Array.
So driver.js has been updated in order to pass them as Float32Array in order
to be in a situation similar to the real life one.
@moz-tools-bot

Copy link
Copy Markdown
Collaborator

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/46053aafd269bc9/output.txt

Total script time: 44.22 mins

  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: FAILED
  different ref/snapshot: 6

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

@calixteman

Copy link
Copy Markdown
Contributor Author

/botio test

@moz-tools-bot

Copy link
Copy Markdown
Collaborator

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

@moz-tools-bot

Copy link
Copy Markdown
Collaborator

From: Bot.io (Windows)


Received

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

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

@moz-tools-bot

Copy link
Copy Markdown
Collaborator

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/51bccbf9f9b5b07/output.txt

Total script time: 30.16 mins

  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  different ref/snapshot: 16
  different first/second rendering: 1

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

@moz-tools-bot

Copy link
Copy Markdown
Collaborator

From: Bot.io (Windows)


Failed

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

Total script time: 45.37 mins

  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  different ref/snapshot: 3

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

@Snuffleupagus Snuffleupagus left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was an interesting bug, good that we have proper test-coverage for it now!

r=me, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants