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

Use a transparent color when setting fill/stroke colors in a pattern context but with no colorspace #18467

Merged
merged 1 commit into from
Jul 22, 2024
Merged

Use a transparent color when setting fill/stroke colors in a pattern context but with no colorspace #18467

merged 1 commit into from
Jul 22, 2024

Conversation

calixteman
Copy link
Contributor

No description provided.

@calixteman calixteman linked an issue Jul 20, 2024 that may be closed by this pull request
[Bug]: Invalid rendering because of no colorspace #18466
Closed
@calixteman
Copy link
Contributor Author

/botio test

@moz-tools-bot
Copy link
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/319e40c5a30bbe1/output.txt

@moz-tools-bot
Copy link
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/23fcad13e2f64a4/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/23fcad13e2f64a4/output.txt

Total script time: 30.17 mins

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

Image differences available at: http://54.241.84.105:8877/23fcad13e2f64a4/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/319e40c5a30bbe1/output.txt

Total script time: 44.40 mins

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

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

@calixteman
Copy link
Contributor Author

/botio test

@moz-tools-bot
Copy link
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/9553bf4eebc57ff/output.txt

@moz-tools-bot
Copy link
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/0ea6a48f5076615/output.txt

@calixteman
Copy link
Contributor Author

/botio test

@moz-tools-bot
Copy link
Collaborator

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

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/16758c66f66ce97/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/9553bf4eebc57ff/output.txt

Total script time: 30.24 mins

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

Image differences available at: http://54.241.84.105:8877/9553bf4eebc57ff/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/0ea6a48f5076615/output.txt

Total script time: 44.89 mins

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

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

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/7019f4fa170a184/output.txt

Total script time: 30.06 mins

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

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

@calixteman
Copy link
Contributor Author

There is a regression in issue6707.pdf.
It's due to the fact we've a previous valid colorspace which is not replaced by the default gray one.
On master, I just applied this diff:

diff --git a/src/core/colorspace.js b/src/core/colorspace.js
index 74b43a1c4..ee2e8493c 100644
--- a/src/core/colorspace.js
+++ b/src/core/colorspace.js
@@ -396,7 +396,8 @@ class ColorSpace {
               }
             }
           }
-          throw new FormatError(`Unrecognized ColorSpace: ${cs.name}`);
+          return this.singletons.gray;
+          //throw new FormatError(`Unrecognized ColorSpace: ${cs.name}`);
       }
     }
     if (Array.isArray(cs)) {

and interestingly the above pdf shows stuff which was invisible before.
I know this pdf is a bit corrupted, but I do wonder if instead of throwing in ColorSpace::_parse when we've a wrong cs we could fallback on DeviceGray.
In the last months, we removed few throw ... to just fallback on something (or sometimes just to do nothing), maybe we could do the same here.
@Snuffleupagus wdyt ?

@Snuffleupagus
Copy link
Collaborator

I know this pdf is a bit corrupted, but I do wonder if instead of throwing in ColorSpace::_parse when we've a wrong cs we could fallback on DeviceGray.

Without testing I really don't know what the general effect would be if we change all the relevant cases in that method to just "swallow" errors, since it might "fix" some things and break others.

Maybe we should just update this patch to address the referenced issue, without regressing anything, and then we could look into those ideas in a follow-up instead to reduce the number of "moving parts" in this PR?

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/16758c66f66ce97/output.txt

Total script time: 45.02 mins

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

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

Snuffleupagus
Snuffleupagus reviewed Jul 20, 2024
View reviewed changes
src/core/evaluator.js Show resolved Hide resolved
src/core/evaluator.js Show resolved Hide resolved
@calixteman
Copy link
Contributor Author

/botio test

@moz-tools-bot
Copy link
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/edb8b6eae34032f/output.txt

@moz-tools-bot
Copy link
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/a5321997a773d60/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Failed

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

Total script time: 29.91 mins

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

Image differences available at: http://54.241.84.105:8877/a5321997a773d60/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/edb8b6eae34032f/output.txt

Total script time: 44.63 mins

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

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

Snuffleupagus
Snuffleupagus approved these changes Jul 22, 2024
View reviewed changes
Copy link
Collaborator

@Snuffleupagus Snuffleupagus left a comment

Choose a reason for hiding this comment

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

r=me, thank you!

@calixteman calixteman merged commit 91655aa into mozilla:master Jul 22, 2024
9 checks passed
@calixteman
Copy link
Contributor Author

/botio makeref

@moz-tools-bot
Copy link
Collaborator

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

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Received

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

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

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)

Success

Full output at http://54.241.84.105:8877/9b69194e2ae546e/output.txt

Total script time: 20.36 mins

  • Make references: Passed
  • Check references: Passed

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)

Success

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

Total script time: 25.01 mins

  • Make references: Passed
  • Check references: Passed

@npanchal108 npanchal108 mentioned this pull request Aug 31, 2024
Open
@Snuffleupagus Snuffleupagus mentioned this pull request Oct 13, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Snuffleupagus Snuffleupagus Snuffleupagus approved these changes

Assignees
No one assigned
Labels
core
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: Invalid rendering because of no colorspace
4 participants
@calixteman @moz-tools-bot @Snuffleupagus @timvandermeij