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

Scaling SVG performance #138

Closed
halvard-cognite opened this issue Jan 23, 2018 · 12 comments
Closed

Scaling SVG performance #138

halvard-cognite opened this issue Jan 23, 2018 · 12 comments
Assignees
Labels
enhancement New feature or request question Further information is requested
Milestone

Comments

@halvard-cognite
Copy link

Hey,

Thanks for a great library!

I'm trying out the experimental svg rendering, and found it to solve my initial problems with text annotations that render in the wrong place.

However when trying to add a zoom feature that uses scaling, I ran into a few problems.
At first I could not get the page to scale at all, I figured this out to be a style that sets the max-width to 100% on the svg.
Overriding that style and setting it to initial made scaling work as it did on canvas.

My second problem is that the performance is really bad when changing the scale, it seems like all child components re-render when i change the scale. I don't think this is necessary? Would it not be possible to just change the scale on the svg element directly without triggering a complete re-render?

Maybe I'm using this wrong and there is a better way to achieve zooming with this library.
If you have any thoughts on this I could possibly help with a PR.

@wojtekmaj
Copy link
Owner

Hey @halvard-cognite,
thank you for your feedback! It's especially valueable to me since SVG is still implemented experimentally but I watch Mozilla's steps closely and I'd like to be ready whenever they are ready.

Should you want to create your own scaling solution, you will have to overwrite default styles indeed. I don't think I'm planning to change this as I believe vast majority will actually benefit from max-width.

Speaking of performance, indeed that's pretty idiotic to cause re-render each time. I'll improve its performance in incoming version 3.0 :)

@wojtekmaj wojtekmaj self-assigned this Jan 23, 2018
@wojtekmaj wojtekmaj added enhancement New feature or request question Further information is requested labels Jan 23, 2018
@wojtekmaj wojtekmaj added this to the 3.0.0 milestone Jan 23, 2018
@wojtekmaj
Copy link
Owner

wojtekmaj commented Jan 23, 2018

Improvement has been merged; 3.0.0-alpha.2 is the first version that has the fix. Let me know what you think!

Note: To use alpha version for testing purposes, you can run npm install react-pdf@next.

@halvard-cognite
Copy link
Author

Thats awesome! Thanks for the quick response :)

I'm perfectly fine with overriding a style that is useful for most people.

But I might be misunderstanding something, is it possible to scale the document without changing the max-width of the svg? The only other option i can think of is to dynamically change the size of the parent container.

@wojtekmaj
Copy link
Owner

wojtekmaj commented Jan 24, 2018

Hmmm. Hope I understand correctly what you want to achieve.

You can pass width prop to both Document and Page components (if provided in both, the one provided to Page component will take priority).

You can also change CSS width using JavaScript, or just the width attribute. Mind you, the latter would not work for canvas rendering.

@halvard-cognite
Copy link
Author

I think I might be misunderstanding something basic about this.

Is the scaling not supposed to be used as a zoom feature?

Maybe adding a zoom example to the docs would be a good idea, I think that would be something a lot of people would want to implement.

@wojtekmaj
Copy link
Owner

React-PDF is not a fully blown PDF reader, it's only a bridge to build one. I have plans to make one, but this is very time consuming process.

The simplest zooming solution you can build is some kind of buttons or a dropdown with width values, which then you set to your application state, and then you pass it to <Document> component as width prop. This will resize all the pages rendered inside it.

@halvard-cognite
Copy link
Author

@wojtekmaj thank you!
I'll try that out and compare it to using scale + no max-width and see what is better.

We have some advanced use-cases for handling pdfs, maybe we can contribute in some way on this. It would be great if any effort we put into it could benefit the community :)

@wojtekmaj
Copy link
Owner

That would be awesome!

@sallyhill
Copy link

Hiya, I'm using the latest version and finding that it seems to repaint all the text when I change the page width on pinch. Am I implementing zoom incorrectly, or is it just laggy with pdf.js?

@wojtekmaj
Copy link
Owner

Depends on what you mean by repaints. If you mean the browser doing the painting - that's entirely expected. You need updated content on the screen after all. If you mean that React Components are updated, that would seem odd.

@sallyhill
Copy link

I mean that It seems like the react components are rerendered. It's a tricky thing, trying to get a smooth pinch zoom and then at the end of the zoom letting the pdf quality actually enhance. Right now I just track when I'm pinching and don't render the text layer then, but getting a nice interaction with it has been a bit awkward. I have been wishing there were two different widths I could adjust - the page width, which rerenders completely, and some style element on the canvas object or something.

@wojtekmaj
Copy link
Owner

Repainting PDF on changing zoom is expected, if you are in Canvas mode. Try out SVG mode, as SVGs, unlikes canvas, can be resized without re-painting.

You could also play around with rendering high res PDF in Canvas and use transforms to rescale it, but that seems like a lot of work to me.

alexandernanberg pushed a commit to alexandernanberg/react-pdf that referenced this issue Jun 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants