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

Normalize BBox of form XObjects in SVG back-end #10162

Merged
merged 1 commit into from
Oct 28, 2018

Conversation

janpe2
Copy link
Contributor

@janpe2 janpe2 commented Oct 16, 2018

In issue #10151, footnotes are missing because they are form XObjects that have unnormalized BBox entries, for example /BBox [0.0 765.354 595.276 0.0]. This makes the height of SVG <rect> element negative, so the clipping path fails and the form XObject becomes invisible.

This PR makes the footnote texts visible but some graphics are still missing because shadings haven't been implemented in the SVG back-end.

@Snuffleupagus
Copy link
Collaborator

Nice find!
It might not work, but I wanted to ask nonetheless: Provided that it doesn't break the (default) canvas back-end, would it perhaps make sense to simply do the normalization on the /core side instead?

@janpe2
Copy link
Contributor Author

janpe2 commented Oct 16, 2018

I think that would be unnecessary because the canvas back-end has no problems when BBox is unnormalized.

@Snuffleupagus
Copy link
Collaborator

I think that would be unnecessary because the canvas back-end has no problems when BBox is unnormalized.

Perhaps, but there's some value in having the API return data in a format that works directly in both back-ends without having to remember to transform it one of them, hence my question.

@timvandermeij
Copy link
Contributor

I don't think there is a use case for needing an unnormalized bounding box, so I agree that it is probably better to handle this in core so any consumers don't need to worry about normalization anymore.

@janpe2 janpe2 force-pushed the svg-normalize-bbox branch from ed2356c to 9cd5f94 Compare October 22, 2018 11:25
@timvandermeij
Copy link
Contributor

/botio test

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.67.70.0:8877/e95ea9c212b3383/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

Live output at: http://54.215.176.217:8877/c2adc671feb7178/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Failed

Full output at http://54.67.70.0:8877/e95ea9c212b3383/output.txt

Total script time: 6.65 mins

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

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

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

Full output at http://54.215.176.217:8877/c2adc671feb7178/output.txt

Total script time: 24.02 mins

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

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

@timvandermeij
Copy link
Contributor

/botio test

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

Live output at: http://54.215.176.217:8877/66aa03f63ede96c/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.67.70.0:8877/803a7856f1eeeee/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/803a7856f1eeeee/output.txt

Total script time: 19.15 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: Passed

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://54.215.176.217:8877/66aa03f63ede96c/output.txt

Total script time: 23.99 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: Passed

@timvandermeij timvandermeij merged commit ed4ac1b into mozilla:master Oct 28, 2018
@timvandermeij
Copy link
Contributor

Nice work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants