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.

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

Fix decoding of JPX images having an alpha channel #18204

Merged
merged 1 commit into from
Jun 3, 2024

Conversation

calixteman
Copy link
Contributor

When an image has a non-zero SMaskInData it means that the image has an alpha channel.
With JPX images, the colorspace isn't required (by spec) so when we don't have it, the JPX decoder will handle the conversion in RGBA format.

@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/d0917ce728a316f/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/6e11bb66521501b/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Failed

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

Total script time: 28.29 mins

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

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

Total script time: 43.58 mins

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

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

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, with a couple of nits; thank you!

src/core/decode_stream.js Outdated Show resolved Hide resolved
src/core/decode_stream.js Outdated Show resolved Hide resolved
src/core/image.js Outdated Show resolved Hide resolved
src/core/colorspace.js Show resolved Hide resolved
test/test_manifest.json Outdated Show resolved Hide resolved
@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/359d4e88db737e0/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/a963b4b79f4a32f/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/359d4e88db737e0/output.txt

Total script time: 28.02 mins

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

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

@timvandermeij
Copy link
Contributor

timvandermeij commented Jun 3, 2024

I think there a regression in issue11004-page1 and issue12579-page3 since they don't render anymore in the reference tests. Given that the earlier test run didn't seem to have it, it looks like the new changes changed the behavior; I've provided more information in the comment below.

With that addressed this patch also LGTM; thank you for fixing this issue!

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Failed

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

Total script time: 43.25 mins

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

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

When an image has a non-zero SMaskInData it means that the image
has an alpha channel.
With JPX images, the colorspace isn't required (by spec) so when we
don't have it, the JPX decoder will handle the conversion in RGBA
format.
@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/431f00638cfc18a/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/e79df2894c62088/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/431f00638cfc18a/output.txt

Total script time: 28.14 mins

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

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

Total script time: 42.91 mins

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

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

@calixteman calixteman merged commit 21e6227 into mozilla:master Jun 3, 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/fe3c6903b42bbf3/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/0d236a4599ad1ad/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Success

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

Total script time: 19.32 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/0d236a4599ad1ad/output.txt

Total script time: 24.15 mins

  • Make references: Passed
  • Check references: Passed

@calixteman calixteman deleted the issue16782 branch June 3, 2024 20:49
jollaitbot pushed a commit to sailfishos-mirror/poppler that referenced this pull request Nov 11, 2024
Handle transparent JPX images, they may contain RGBA data
when no ColorSpace pdf dict is defined or when SMaskInData
is non-zero.

PDF files posted in below issues are fixed by this commit:
https://gitlab.freedesktop.org/poppler/poppler/-/issues/1486
mozilla/pdf.js#16782
mozilla/pdf.js#11306
mozilla/pdf.js#17416

Inspired by related fix in pdf.js:
mozilla/pdf.js#18204

While working on this commit we also succesfully addressed
regressions that emerged for the following files:
mozilla/pdf.js#18896
https://bugs.freedesktop.org/attachment.cgi?id=49294

Issue #1486
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants