Skip to content

[breaking] Treat pts as pts#1717

Open
courtneypattison wants to merge 1 commit into
wojtekmaj:mainfrom
courtneypattison:#1219
Open

[breaking] Treat pts as pts#1717
courtneypattison wants to merge 1 commit into
wojtekmaj:mainfrom
courtneypattison:#1219

Conversation

@courtneypattison

@courtneypattison courtneypattison commented Feb 11, 2024

Copy link
Copy Markdown

Sets the scale to the actual size of PDF :)

Fixes #1219

@wojtekmaj

Copy link
Copy Markdown
Owner

Oooooo that's just lovely! Thank you for that!

I'll happily release this, but in the next major, as this change might end up being very impactful on people's UIs.

Also, we may need some documentation change? Not sure, will have to look into that.

@courtneypattison

Copy link
Copy Markdown
Author

You're welcome! I'm sorry, I was rushing and messed up the commit. It should be pixels instead of points since the conversion is from pdf to css pixels. Also, the util from pdfjs isn't accessible so I just copied it out. I think it makes what is going on more explicit anyway. Hopefully it's correct this time!

@wojtekmaj

Copy link
Copy Markdown
Owner

That looks good to me! My point stands though - I'll be merging this in the next major release. :) Thanks!

@courtneypattison

Copy link
Copy Markdown
Author

Great, thank you @wojtekmaj!

@wojtekmaj wojtekmaj changed the title Treats pts as pts [breaking] Treat pts as pts Feb 13, 2024
@wojtekmaj
wojtekmaj force-pushed the main branch 7 times, most recently from 2d488c1 to a077571 Compare May 14, 2024 09:32
@wojtekmaj
wojtekmaj force-pushed the main branch 5 times, most recently from 1f758cc to ce6db03 Compare June 10, 2024 23:04
@wojtekmaj
wojtekmaj force-pushed the main branch 2 times, most recently from 42a8c3c to 0d68fee Compare June 13, 2024 08:22
@wojtekmaj
wojtekmaj force-pushed the main branch 4 times, most recently from d0aabb2 to 736f3f3 Compare July 9, 2024 15:54
@gfargo

This comment has been minimized.

@wojtekmaj wojtekmaj left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Hey @courtneypattison - I'm preparing a new major release so I'm coming back to this long standing PR. I rebased your PR and made it align with breaking changes in PDF.js. Thanks for your patience with this.

It seems like this PR is not doing the trick fully; we're only making the canvas larger while we're rendering it at a smaller scale, making it blurry, and we're also leaving text and annotation layers unchanged.

@courtneypattison

Copy link
Copy Markdown
Author

@wojtekmaj thank you for your hard work on this project! It's likely blurry because the device pixel ratio isn't being accounted for in the canvas's render size. You may also want to account for userUnit, which redefines the PDF's scale in some scenarios (e.g. over 200 inch PDFs). You can see how we get an accurate scale in Pattern Projector. Ignore patternScale: that's just user supplied scaling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PDFs render too small treating pts as px

3 participants