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

Use more natural zooming on mobile #2582

Open
snorp opened this issue Jan 18, 2013 · 83 comments

Comments

@snorp
Copy link

commented Jan 18, 2013

Right now you have to hit the plus/minus buttons even on mobile in order to zoom the document. This is really strange especially since pinch-to-zoom is active as well. I think the best way to go on mobile is maybe to render at a "natural" size, and let the browser control the zoom, similar to a web page.

@brendandahl

This comment has been minimized.

Copy link
Contributor

commented Jan 18, 2013

This is what we wanted to do, but for V1 the pinch to zoom was having issues so we had to use the zoom buttons.

@snorp

This comment has been minimized.

Copy link
Author

commented Jan 18, 2013

Can you elaborate on the issues? I think we could probably prioritize those for you in order to get PDF.js working well.

@wesj

This comment has been minimized.

Copy link

commented Feb 4, 2013

I really don't think you want the browser to do the zooming in this case. If pdf.js renders a document at 800x600, and you pinch zoom in, the browser is just going to continue to render that 800x600 content at a new higher resolution which may or may not look like crap depending on the resolution of the device and how much you've zoomed. (I assume pdf.js still uses canvas for rendering and not svg?)

@brendandahl

This comment has been minimized.

Copy link
Contributor

commented Sep 17, 2013

@Snuffleupagus @timvandermeij Are either of you interested in working on this? It seems it shouldn't be too hard using the shared library in gaia https://github.com/mozilla-b2g/gaia/blob/master/shared/js/gesture_detector.js

@timvandermeij

This comment has been minimized.

Copy link
Contributor

commented Sep 17, 2013

@brendandahl I'll definitely put it on my to-do list and start experimenting with it soon. https://github.com/mozilla-b2g/gaia/blob/862de8489b648a9af7e8a5b88be031b5479404ba/apps/camera/js/panzoom.js#L15 seems to have a good example, as 'transform' is used for 2-finger pinch events. It would be really great if pinch to zoom would work, as it's used quite often on mobile.

@timvandermeij

This comment has been minimized.

Copy link
Contributor

commented Sep 17, 2013

I'm working on this. Interested users can keep an eye on https://github.com/timvandermeij/pdf.js/tree/pinch-to-zoom for the progress. It is already catching the pinch-to-zoom command on my tablet and phone, but the accuracy must be improved, as well as the actual re-rendering (I'll need to find a way to calculate the new scale with the old scale and the new middle point, or some other way).

@brendandahl

This comment has been minimized.

Copy link
Contributor

commented Sep 17, 2013

@timvandermeij One thing we may need to do before we implement this is improve the zoom. We've talked about first just using css transforms to to scale the canvas, then start re-rendering and then once re-rendering is done show the new canvas.

@timvandermeij timvandermeij referenced this issue Sep 18, 2013
0 of 5 tasks complete
@snorp

This comment has been minimized.

Copy link
Author

commented Sep 19, 2013

I strongly believe the best solution here is going to be one that relies on the browser's compositor to do the transient zoom (the animation during the pinch or double tap) and then just let pdf.js redraw at the new resolution. The sad part here is that right now content is oblivious to zoom changes, and I don't think resizing the canvas to the new resolution will work. We may need to enhance the canvas spec to handle this.

@skruse

This comment has been minimized.

Copy link
Contributor

commented Dec 11, 2013

I had some success with hammer.js. I allowed the "native" pinch of the browser (which leads to blurry PDF) and upon pinchend I redraw the PDF canvas, with scale = scale*zoom and give the canvas the css "transform: scale(1/zoom)". So all will be in the same place (especially text and anchors). Looks neat.

@timvandermeij

This comment has been minimized.

Copy link
Contributor

commented Dec 11, 2013

@skruse I prepared a patch for implementing pinch to zoom a while ago (see #3708), also with Hammer.js, but I have not yet succeeded in getting it to work properly on mobile/tablet devices. The pinch motions caused a lot of performance and stability problems. Do you mind sharing your implementation with us? If not, could you create a pull request with your pinch to zoom implementation? Perhaps it could replace mine if it works more fluently on mobile/tablet devices. :)

@rodoabad

This comment has been minimized.

Copy link

commented Dec 19, 2013

Hey skruse, how were you able to calculate the zoom ratio at end of zoom?

@skruse

This comment has been minimized.

Copy link
Contributor

commented Dec 19, 2013

var zoom = document.documentElement.clientWidth / window.innerWidth;

And I got performance issues too: One should not zoom in "too far" on a mobile device, in terms of the "scale" parameter. I guess 2 or 3 is the very maximum.

@sasikanth513

This comment has been minimized.

Copy link

commented Sep 7, 2015

+2 for this

@MickL

This comment has been minimized.

Copy link

commented Nov 19, 2015

Someone got a solution for this 2 year old issue?

@timvandermeij

This comment has been minimized.

Copy link
Contributor

commented Nov 19, 2015

Nothing has been done about this as far as I know. I refer to my previous comment in #2582 (comment). We invite anyone to submit a PR for this once there is working code.

@rorysmorris

This comment has been minimized.

Copy link

commented Mar 10, 2016

Would love to see a solution for this. At the moment this is the only thing that is stopping me from using pdf.js

:(

@Manmade

This comment has been minimized.

Copy link

commented Mar 20, 2016

Pinch zoom would be great! I found this jquery plugin that uses pdf.js and has pinch zoom and swiping pages. http://touchpdf.net/demo/index.htm But it would be good if it was build in pdf.js from the start :-)

@ltullman

This comment has been minimized.

Copy link

commented Aug 4, 2016

+1 Would love to see this here.

@amurillo17

This comment has been minimized.

Copy link

commented May 31, 2018

I saw that "Box Content Preview" (which uses PDFJS to show PDFs) does a good implementation for this. This is a Codepen of their viewer:

https://codepen.io/box-platform/pen/rmZdjm

And this is the pull request for their fix:

box/box-content-preview#567

I thought that maybe someone could make something similar for PDFJS, based on their solution. I'll try to implement it myself and post the code if I do. I'll be great if someone else (that probably have more experience than me) can also collaborate and try it!

@prohtex

This comment has been minimized.

Copy link

commented Jun 1, 2018

@amurillo17 thanks for the link. The Box solution looks fantastic. I'd love to see something like this working with pdfjs. Please keep us updated on your progress!

@tefracky

This comment has been minimized.

Copy link

commented Sep 2, 2018

Is there any good solution for this problem?

@chrisgillis

This comment has been minimized.

Copy link

commented Oct 26, 2018

Any updates for this? Would be a great feature to have!

@rcarretteGroupeBRIAND

This comment has been minimized.

Copy link

commented Mar 18, 2019

any updates ? Thanks

@AnReZa

This comment has been minimized.

Copy link

commented Apr 10, 2019

Are there any simple ways to add this feature to the existing viewer?

@prohtex

This comment has been minimized.

Copy link

commented Apr 10, 2019

Are there any simple ways to add this feature to the existing viewer?

No.

@AnReZa

This comment has been minimized.

Copy link

commented Apr 10, 2019

@prohtex Hm, that's pretty sad. I hope, that such a feature will be added soon after all these years.

@prohtex

This comment has been minimized.

Copy link

commented Apr 10, 2019

@prohtex Hm, that's pretty sad. I hope, that such a feature will be added soon after all these years.

It will only be added when someone steps up and integrates code into a PR (not me, sadly), and even then, it could take years.

@M-Reimer

This comment has been minimized.

Copy link

commented Apr 22, 2019

I have a workaround, but it is "Android only" as I don't care about iOS in my "Firefox for Android" addon:
https://github.com/M-Reimer/android-pdf-js/blob/master/patches/pdfjs-pinch-gestures.js
My Makefile just appends this to "web/viewer.js". It registers to the touch events and translates the pinch gestures to button clicks.

@AnReZa

This comment has been minimized.

Copy link

commented Apr 23, 2019

I have a workaround, but it is "Android only" as I don't care about iOS in my "Firefox for Android" addon:

It works on my iPad as well, but it feels a little bit sluggish and not very natural. It zooms one step after each pinch. So you have to release the fingers to make it zoom and it doesn't matter how far of a distance you have pinched. Is this the normal behavior, or just a device-specific bug? Thank you anyways, cause this is a step into the right direction.

@prohtex

This comment has been minimized.

Copy link

commented Apr 23, 2019

It works on my iPad as well, but it feels a little bit sluggish and not very natural. It zooms one step after each pinch. So you have to release the fingers to make it zoom and it doesn't matter how far of a distance you have pinched. Is this the normal behavior, or just a device-specific bug? Thank you anyways, cause this is a step into the right direction.

Looking at the code, this is not a viable solution or even a good workaround. All this does is watch for zoom events and simulate clicking on the [-] [+] buttons with jQuery.

@AnReZa

This comment has been minimized.

Copy link

commented Apr 23, 2019

@prohtex Exactly. If you try it, it does something, but it doesn't work well at all.

@M-Reimer

This comment has been minimized.

Copy link

commented Apr 23, 2019

As I said: Not perfect but better than nothing.
At least for me it's easier to do the pinch gesture than to hit a small zoom button on a small mobile screen. It makes gestures work that every mobile device user expects to work.

It works on my iPad as well, but it feels a little bit sluggish and not very natural. It zooms one step after each pinch. So you have to release the fingers to make it zoom and it doesn't matter how far of a distance you have pinched. Is this the normal behavior, or just a device-specific bug? Thank you anyways, cause this is a step into the right direction.

It hits the buttons. "Natural zooming" is not possible with this simple approach and I don't want to mess too much with PDF.js. This is not the goal of my Addon.

But it should be possible to do multiple button presses depending on how far the fingers have moved. I'll try that.

Looking at the code, this is not a viable solution or even a good workaround. All this does is watch for zoom events and simulate clicking on the [-] [+] buttons with jQuery.

I'm open to suggestions, but as this is part of an Addon which just wants to make PDF.js usable as PDF viewer inside Firefox, I won't do deep modifications.

BTW: There is not a single line of jQuery in my code. This is raw JavaScript.

@M-Reimer

This comment has been minimized.

Copy link

commented Apr 23, 2019

It should be actually possible to do the zoom steps while moving the fingers...
I'll try to do some more improvements, but the only communication path, I'll use, are the two buttons that I'll remote-control. This way my code doesn't interfere with PDF.js.

@prohtex

This comment has been minimized.

Copy link

commented Apr 23, 2019

BTW: There is not a single line of jQuery in my code. This is raw JavaScript.

I stand corrected. Hoping for a mature solution soon.

@MickL

This comment has been minimized.

Copy link

commented Apr 23, 2019

The way to accomplish this is by using css transformation while pinch-zoom. Also the movement of the fingers needs to be applied not just the zoom. Then on pinch-zoom-end you can reset css transformation and apply actual zoom and scroll to pdf.js.

P.S. Using jQuery inside a non-jQuery project already seems like a bad practice to me.

@AnReZa

This comment has been minimized.

Copy link

commented Apr 23, 2019

P.S. Using jQuery inside a non-jQuery project already seems like a bad practice to me.

Where does @M-Reimer use JQuery? As he already stated, his workaround does only use plain Javascript.

@prohtex

This comment has been minimized.

Copy link

commented Apr 23, 2019

Yeah.. my bad. Point being, this solution doesn't work well.

@M-Reimer

This comment has been minimized.

Copy link

commented Apr 27, 2019

Updated so the distance between fingers changes the zoom amount:
https://github.com/M-Reimer/android-pdf-js/blob/master/patches/pdfjs-pinch-gestures.js
It is not possible to zoom while moving the fingers as, for some reason, the actual zoom and redraw causes the touch event to somehow "end".
Again: Better than not handling the gesture at all.

@AnReZa

This comment has been minimized.

Copy link

commented May 7, 2019

@M-Reimer Thank you very much for your work. At least, it's an improvement compared to the last version. Maybe someone finds a solution to make it redraw while zooming. Or it might be possible to temporarily zoom using css transformations and then redraw it eventually after the touch gesture ends.

@AnReZa

This comment has been minimized.

Copy link

commented May 15, 2019

@anvaka Looked into the case and it seems like there might be a rather simple solution. Unfortunately I currently don't have the time to test it, but maybe one wants to give it a try.

@jsprpalm

This comment has been minimized.

Copy link

commented May 23, 2019

I made a version based on @squallstar solution that also uses css transform durring the pinch move to give more user feedback. The rendering is only done on touchend. It's not perfect, there are some small issues with scroll positioning after rendering but maybe someone could use it as a start.

https://gist.github.com/jsprpalm/12217feab2f1acc14bd8e8508291619e

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.