Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve parseAppearanceStream to handle more "complex" ColorSpaces #16648

Merged
merged 1 commit into from
Jul 6, 2023
Merged

Improve parseAppearanceStream to handle more "complex" ColorSpaces #16648

merged 1 commit into from
Jul 6, 2023

Conversation

Snuffleupagus
Copy link
Collaborator

The existing code is unable to correctly extract the color from the appearance-stream when the ColorSpace-data is "complex". To reproduce this:

  • Open freetexts.pdf in the viewer.
  • Note the purple color of the "Hello World from Preview" annotation.
  • Enable any of the Editors.
  • Note how the relevant annotation is now black.

@Snuffleupagus Snuffleupagus requested a review from calixteman July 6, 2023 11:32
@calixteman
Copy link
Contributor

it broke the tests tracemonkey-stamp-editor.

@Snuffleupagus
Improve parseAppearanceStream to handle more "complex" ColorSpaces
6442a6c 
The existing code is unable to *correctly* extract the color from the appearance-stream when the ColorSpace-data is "complex". To reproduce this:
 - Open `freetexts.pdf` in the viewer.
 - Note the purple color of the "Hello World from Preview" annotation.
 - Enable any of the Editors.
 - Note how the relevant annotation is now black.
@mozilla mozilla deleted a comment from moz-tools-bot Jul 6, 2023
@mozilla mozilla deleted a comment from moz-tools-bot Jul 6, 2023
@mozilla mozilla deleted a comment from moz-tools-bot Jul 6, 2023
@mozilla mozilla deleted a comment from moz-tools-bot Jul 6, 2023
@Snuffleupagus Snuffleupagus force-pushed the AppearanceStreamEvaluator-setFillColorSpace branch from 29f77b2 to 6442a6c Compare July 6, 2023 14:01
@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Received

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

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

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Received

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

Live output at: http://54.241.84.105:8877/3b8cb014c0ff187/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/3b8cb014c0ff187/output.txt

Total script time: 25.19 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: FAILED
  different ref/snapshot: 19
  different first/second rendering: 2

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

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Failed

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

Total script time: 37.48 mins

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

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

@Snuffleupagus
Copy link
Collaborator Author

it broke the tests tracemonkey-stamp-editor.

Fixed now, please re-review.

calixteman
calixteman approved these changes Jul 6, 2023
View reviewed changes
Copy link
Contributor

@calixteman calixteman left a comment

Choose a reason for hiding this comment

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

Cool stuff, thank you.
Preview (mac) usually exports in using colorspace so it'll be useful.

@Snuffleupagus Snuffleupagus merged commit 091266c into mozilla:master Jul 6, 2023
@Snuffleupagus Snuffleupagus deleted the AppearanceStreamEvaluator-setFillColorSpace branch July 6, 2023 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@calixteman calixteman calixteman approved these changes

Assignees
No one assigned
Labels
annotations editor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Snuffleupagus @calixteman @moz-tools-bot