Use more natural zooming on mobile

snorp commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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

wesj commented 11 years ago

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 commented 11 years ago

@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 commented 11 years ago

@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 commented 11 years ago

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 commented 11 years ago

@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.

snorp commented 11 years ago

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 commented 10 years ago

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 commented 10 years ago

@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 commented 10 years ago

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

skruse commented 10 years ago

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 commented 9 years ago

+2 for this

MickL commented 8 years ago

Someone got a solution for this 2 year old issue?

timvandermeij commented 8 years ago

Nothing has been done about this as far as I know. I refer to my previous comment in https://github.com/mozilla/pdf.js/issues/2582#issuecomment-30316908. We invite anyone to submit a PR for this once there is working code.

rorysmorris commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

+1 Would love to see this here.

sporkman commented 7 years ago

+1, also would the example elsewhere that uses hammer.js to capture the event and then call the pdf.js zoom functions not be a relatively "clean" method?

rorysmorris commented 7 years ago

@sporkman what i did in the end was to render the canvas really large, then to use native browser zooming to allow zooming (my app was designed for touch devices). i can share the source if needed.

toplay3 commented 7 years ago

@rorysmorris how did you manage to get around disabling native browser zoom when pinching while scrolling? I have also implemented hammer.js on pdf.js but cannot get around that particular issue

Rob--W commented 7 years ago

Have you tried to use the touch-action CSS property? https://developer.mozilla.org/en-US/docs/Web/CSS/touch-action

On Oct 5, 2016 3:15 PM, "toplay3" notifications@github.com wrote:

@rorysmorris https://github.com/rorysmorris how did you manage to get around disabling native browser zoom when pinching while scrolling? I have also implemented hammer.js on pdf.js but cannot get around that particular issue

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mozilla/pdf.js/issues/2582#issuecomment-251670785, or mute the thread https://github.com/notifications/unsubscribe-auth/ABTUT83E287dv4LSs4T_TGnzwe8yqCMYks5qw6LmgaJpZM4AXvZ2 .

toplay3 commented 7 years ago

@Rob--W wow genius! works perfectly now! thanks so much!

yahya-uddin commented 7 years ago

When is this issue planning to be implemented? Its been quite some time since this issue was originally posted. Needless to say +1!

MickL commented 7 years ago

I also check this issue every week since a year now. Would be awesome to have it in core or as a addon with hammer js.

yurydelendik commented 7 years ago

To be more constructive on this thread, here is requirements for UI (if somebody wants to really help out and speed out the resolution of the issue) and there is nothing specific to PDF.js:

to zoom and pan set of DIVs (with different sizes) that are vertically stacked
not allowing user to get lost in viewport at different scales
detect and inform when a div visibility or visible scale changes.

The latter is important since we don't want paint all pages at max resolution on low powered devices. If somebody will have prototype, find to me at IRC I'll help to hook PDFPageView for visibility and scale events. Shall I also set 5-good-beginner-bug?

timvandermeij commented 7 years ago

I agree that this would be a good beginner bug, so I'm labeling it as such.

Rob--W commented 7 years ago

This is not just for mobile, I'm also getting feature requests to support two-finger trackpad zooming (on Mac).

From my brief investigation, I found that there is no cross-browser way to support zoom gestures yet.

Safari has some specific gesture events: https://developer.apple.com/reference/webkitjs/gestureevent
Firefox add-on code has access to gesture events: https://developer.mozilla.org/en-US/docs/Web/Guide/Events/Mouse_gesture_events
Some bugzilla discussion on converting pinch events to ctrl-wheel (with input and references to other browsers): https://bugzilla.mozilla.org/show_bug.cgi?id=1052253

Goldenflamer commented 7 years ago

+1

Manmade commented 7 years ago

@toplay3 and @Rob--W What did you do to get it working? Adding "touch-action: auto;"? To what? And what else did you do? Thanks!

squallstar commented 7 years ago

Hi everyone, like most of you I'm interested in getting pinch to zoom working, has anyone been able to do so, even by using external libs like hammer? If you made it, what is the set up? @rorysmorris @toplay3 @Rob--W thanks

kosssi commented 7 years ago

👍

squallstar commented 7 years ago

@rorysmorris any advice about the above question? thanks :)

rorysmorris commented 7 years ago

Hi @squallstar - at first I tried to use a JS pinch and zoom library to handle this for me, but I found a few problems. It didn't zoom how i wanted, i.e. it would allow the PDF canvas to go completely off-screen, when i never wanted the edges to be away from the screen edge, if that makes sense. In the end, I just used the native zooming that's built into the mobile browser to pinch/zoom/navigate. I setup PDF.js to render the PDF at 3x the pixel size it was being displayed, then scaled down with CSS, so it would remain fairly crisp when zoomed in. If you need any help, I could show you a working example of my implementation.

squallstar commented 7 years ago

@rorysmorris thanks, it would be great if you show me (or send a snippet) or how you implemented it. Much appreciated 👍

rorysmorris commented 7 years ago

@squallstar send me an email through the contact form on my site and I'll get a demo over to you later on today :) http://rorymorris.co.uk

MickL commented 7 years ago

@squallstar This is not really a solution but kind of a workaround. Disadvantages are: Performance, result has devices dependency, zoom is not endless.

rorysmorris commented 7 years ago

The performance is greatly improved by using native scrolling and panning, as opposed to using JavaScript to perform those tasks. Maximum zoom-level is easily configurable with a meta viewport tag, and who wants endless zoom on a rasterised image anyway? Great looking "JS-ImageResizer" library by the way! @MickL

MickL commented 7 years ago

Performance was more a guess. Lets say using an iPhone Plus in landscape @ 2208px width. Now you would have a 6624px Canvas. Depending on the PDF complexity and count of pages you may have 3 rendered 6000px Canvas with HTML-text, graphics etc.

I guess your solution is great for a lot of people. But as you described its more a fake-zoom than it actually triggers PDF.js zoom and rerendering :)

rorysmorris commented 7 years ago

True, performance-wise from that aspect isn't great. In my particular use-case for this, I had to render out 300+ page PDF's at iPad (retina) size, as well as have a drawing canvas on top of each one!

PDF.js would crash the browser (iOS Safari) at around 10 rendered PDF pages, so in the end I had to only show one PDF page at a time with previous/next buttons. Not ideal! And I agree, native zoom functionality built into PDF.js would've been a lot nicer.

toplay3 commented 7 years ago

checkout kamihq.com for working pinch-zoom example

squallstar commented 7 years ago

@rorysmorris I've sent you my email yesterday evening through the contact form, any chance you can send me the example you were talking about? :) thanks

ltullman commented 7 years ago

Can anyone who has a working example (@rorysmorris, @Rob--W, etc) please share?

hetalv985 commented 7 years ago

@rorysmorris I tried to contact you through your website. If anyone has a working example of this, can you please share?

squallstar commented 7 years ago

@ltullman @hetalv985 I managed make it working implement it myself, check my gist here:

https://gist.github.com/squallstar/1d720e93eabe7f60dc61b547d2c19228 simply paste that to the end of viewer.html 👍 that's it.

hetalv985 commented 7 years ago

Thanks squallstar. But we are not using the viewer.html. We are rendering the pdf in our iOS mobile app inside a div tag. How do we use this feature in that case? Do we need to include the viewer files?

MickL commented 7 years ago

So we have the function handleMouseWheel which is very similar. Why not use the exact same function with touch gesture oder hammer js? Isnt this fixed in 1-2 hours then?

timvandermeij commented 7 years ago

Unfortunately it's not that easy. Please refer to my previous comment at https://github.com/mozilla/pdf.js/issues/2582#issuecomment-30316908. We need to limit the amount of touch gestures so that the canvas is not re-rendered on every delta change, which is a major performance issue. If someone is willing to work on this, feel free to submit a PR and we'll review it.

MickL commented 7 years ago

If thats the only issue... :)

I also noticed on "ctrl + mousewheel" it scrolls to the cursor-position like on Google Maps. But this won't work if there are no scrollbars (which is default on page-load). So to let this feel natural we have to create a padding if needed for pinch-zoom and mousewheel-zoom.

A nice addition would be if mousewheel-zoom and pinch-zoom would use the same function.

Your commit looks good already. Maybe performance is no more an issue, it is more than 3 years ago. Ofcourse the best solution would be: start-pinch -> blurred zoom -> end-pinch -> really zoom pdf. Also user-scalable=no is not working on latest iOS anymore. So we may have to preventDefault(), too.

MickL commented 7 years ago

I invested alot of time in this. Actually it seems possible but there are several problems i couldn't solve:

hammer.js only works on pinchend (pinchmove always gets canceled when pdf.js zooms)
I could not scroll perfectly to the center of pinchend even though i used the same method as mousewheel-zoom does
Sometimes the center is not set correctly by hammer.js
Sometimes pinchend is not called by hammer.js and no zoom happens
I tried to add css transformation on pinchmove(css zoom on pinchmove, pdf js zoom on pinchend) which would give great performance but again i was not able to scroll perfectly (position #viewer with css translate)
hammer.js prevents any other touch-action(in our case regular page scroll) so i had to modify hammer.js to only prevent when 2 fingers are used
(Same for mousewheel zoom:) If there are no scrollbars (zoomed out so you see the whole document) we cant position (scroll) the document to the center of pinch.

Probably all of this is solvable but iOS 10 allows page-zoom even when it is disabled with user-scalable=no. So a few times on pinch preventDefault() is not working and the whole page is zoomed. This would lead into major issue for the end user (only way to get out is to double-tap the toolbar).

mozilla / pdf.js

Use more natural zooming on mobile #2582