Skip to content

Don't clip when the clip path is empty (issue #12306) #14797

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
Merged
merged 1 commit into from
Apr 18, 2022
Merged

Don't clip when the clip path is empty (issue #12306) #14797

merged 1 commit into from
Apr 18, 2022

Conversation

calixteman
Copy link
Contributor

No description provided.

@Snuffleupagus
Copy link
Collaborator

/botio test

@pdfjsbot
Copy link

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

@pdfjsbot
Copy link

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/2a1b8241e14d801/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/5ec16099bf0a268/output.txt

Total script time: 2.10 mins

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

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

@pdfjsbot
Copy link

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/2a1b8241e14d801/output.txt

Total script time: 3.01 mins

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

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

@calixteman
Copy link
Contributor Author

/botio test

@pdfjsbot
Copy link

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/17b7161325149b7/output.txt

@pdfjsbot
Copy link

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

@pdfjsbot
Copy link

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/78ebb3110fe4a13/output.txt

Total script time: 23.91 mins

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

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

@pdfjsbot
Copy link

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/17b7161325149b7/output.txt

Total script time: 24.65 mins

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

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

@Snuffleupagus
Copy link
Collaborator

Unfortunately there appears to be a number of regressions, see an incomplete list below:

  • issue13447-eq-page1
  • bug1669099-page1
  • There's also movement in what appears to be most, if not all, Type3-related tests

@calixteman
Copy link
Contributor Author

Unfortunately there appears to be a number of regressions, see an incomplete list below:

* issue13447-eq-page1

* bug1669099-page1

* There's also movement in what appears to be most, if not all, Type3-related tests

Likely related to

pdf.js/src/display/canvas.js

Line 2823 in 379125c

this.clip();

I changed it to this.ctx.clip() which should be better.

@calixteman
Copy link
Contributor Author

/botio test

@pdfjsbot
Copy link

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

@pdfjsbot
Copy link

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/2904f6d2ab59ee2/output.txt

@Snuffleupagus
Copy link
Collaborator

Likely related to

pdf.js/src/display/canvas.js

Line 2823 in 379125c

this.clip();

I changed it to this.ctx.clip() which should be better.

That really cannot explain all of the movement in Type3-tests though, since the code-path in question only matter for annotations...

@pdfjsbot
Copy link

From: Bot.io (Linux m4)

Failed

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

Total script time: 23.83 mins

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

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

@pdfjsbot
Copy link

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/2904f6d2ab59ee2/output.txt

Total script time: 24.52 mins

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

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

@calixteman
Copy link
Contributor Author

/botio test

@pdfjsbot
Copy link

From: Bot.io (Windows)

Received

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

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

@pdfjsbot
Copy link

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

@pdfjsbot
Copy link

From: Bot.io (Linux m4)

Failed

Full output at http://54.241.84.105:8877/77ca0619a771917/output.txt

Total script time: 22.90 mins

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

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

@pdfjsbot
Copy link

From: Bot.io (Windows)

Failed

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

Total script time: 24.36 mins

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

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

@Snuffleupagus Snuffleupagus merged commit 5bbed40 into mozilla:master Apr 18, 2022
@Snuffleupagus Snuffleupagus linked an issue Apr 18, 2022 that may be closed by this pull request
All empty pages in pdf file #12306
Closed
@Snuffleupagus
Copy link
Collaborator

/botio makeref

@pdfjsbot
Copy link

From: Bot.io (Linux m4)

Received

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

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

@pdfjsbot
Copy link

From: Bot.io (Windows)

Received

Command cmd_makeref from @Snuffleupagus received. Current queue size: 1

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

@pdfjsbot
Copy link

From: Bot.io (Windows)

Failed

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

Total script time: 3.75 mins

  • Lint: Passed
  • Make references: Passed
  • Check references: FAILED (no refs found)

@pdfjsbot
Copy link

From: Bot.io (Linux m4)

Success

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

Total script time: 21.54 mins

  • Lint: Passed
  • Make references: Passed
  • Check references: Passed

@Snuffleupagus
Copy link
Collaborator

/botio-windows makeref

@pdfjsbot
Copy link

From: Bot.io (Windows)

Received

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

Live output at: http://54.193.163.58:8877/12db1e008b70f15/output.txt

@Snuffleupagus
Copy link
Collaborator

Snuffleupagus commented Apr 18, 2022 •
edited
Loading

I completely missed that there was a regressions in the tiling-pattern-large-steps-page1 test-case (look at its right edge).

@calixteman Can you please look into that, and fix it in a follow-up?

@calixteman
Copy link
Contributor Author

Yep, it's likely because of

pdf.js/src/display/pattern_helper.js

Line 622 in 5bbed40

graphics.clip();
(it's just a guess).

@pdfjsbot
Copy link

From: Bot.io (Windows)

Success

Full output at http://54.193.163.58:8877/12db1e008b70f15/output.txt

Total script time: 21.11 mins

  • Lint: Passed
  • Make references: Passed
  • Check references: Passed

calixteman added a commit to calixteman/pdf.js that referenced this pull request Apr 18, 2022
@calixteman
Fix clipping issue with pattern (follow-up of mozilla#14797)
742bf0d
calixteman added a commit to calixteman/pdf.js that referenced this pull request Apr 18, 2022
@calixteman
Fix clipping issue with pattern (follow-up of mozilla#14797)
c2aa03e
Snuffleupagus added a commit that referenced this pull request Apr 18, 2022
@Snuffleupagus
Merge pull request #14799 from calixteman/12306-followup
32ae0e4 
Fix clipping issue with pattern (follow-up of #14797)
@Snuffleupagus Snuffleupagus mentioned this pull request May 8, 2022
Closed
bh213 pushed a commit to bh213/pdf.js that referenced this pull request Jun 3, 2022
@calixteman @bh213
Fix clipping issue with pattern (follow-up of mozilla#14797)
de1c1c6
rousek pushed a commit to signosoft/pdf.js that referenced this pull request Aug 10, 2022
@calixteman @rousek
Fix clipping issue with pattern (follow-up of mozilla#14797)
6cc9325
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Snuffleupagus Snuffleupagus Snuffleupagus left review comments

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

Successfully merging this pull request may close these issues.

All empty pages in pdf file
3 participants
@calixteman @Snuffleupagus @pdfjsbot