Bug 335819 - Zooming in doesn't work on certain documents.
Summary: Zooming in doesn't work on certain documents.
Status: RESOLVED FIXED
Alias: None
Product: okular
Classification: Applications
Component: PDF backend (show other bugs)
Version: 0.19.60
Platform: unspecified Linux
: NOR normal
Target Milestone: ---
Assignee: Okular developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-05 05:27 UTC by Markus Trippelsdorf
Modified: 2014-08-24 20:38 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.14.1


Attachments
small testcase (237.40 KB, application/octetstream)
2014-06-18 05:37 UTC, Markus Trippelsdorf
Details
screen capture (451.76 KB, video/x-matroska)
2014-06-19 21:17 UTC, Markus Trippelsdorf
Details
Please try this patch and attach the output you get (1.61 KB, patch)
2014-08-18 23:01 UTC, Albert Astals Cid
Details
Patch that fixes the issue. (478 bytes, patch)
2014-08-19 10:15 UTC, Markus Trippelsdorf
Details
Testcase for another issue (656.55 KB, application/pdf)
2014-08-19 13:38 UTC, Markus Trippelsdorf
Details
screen capture2 (1.13 MB, video/x-matroska)
2014-08-19 13:42 UTC, Markus Trippelsdorf
Details
New patch for both issues (1.98 KB, patch)
2014-08-20 06:18 UTC, Markus Trippelsdorf
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Markus Trippelsdorf 2014-06-05 05:27:33 UTC
Zooming in doesn't work with on certain files.
One has to zoom out first and only then zooming in further works.

Reproducible: Always

Steps to Reproduce:
1. Open attached pdf
2. Try to zoom in (doesn't work)
3. Zoom out and then zoom in (this works fine)
Comment 1 Markus Trippelsdorf 2014-06-05 05:37:26 UTC
File is too big. Please find it here: trippelsdorf.de/foo.pdf
Comment 2 Albert Astals Cid 2014-06-17 21:36:00 UTC
No, it's not there.
Comment 3 Markus Trippelsdorf 2014-06-18 05:37:10 UTC
Created attachment 87241 [details]
small testcase

(In reply to comment #2)
> No, it's not there.

Yes. I've deleted the file after a few days, because it contained copyrighted material.

Here's a smaller example.
Comment 4 Albert Astals Cid 2014-06-18 23:14:22 UTC
Can you define "Try to zoom in (doesn't work)" ?
Comment 5 Markus Trippelsdorf 2014-06-19 04:58:29 UTC
(In reply to comment #4)
> Can you define "Try to zoom in (doesn't work)" ?

Sure. Pressing Ctrl-+ or hitting the "Zoom in" button. Okular just doesn't react to these inputs.

(My default zoom level is set to: "Fit Width")
Comment 6 Albert Astals Cid 2014-06-19 20:05:33 UTC
I understand you've compiled 0.19.60 from git. Can you compile 0.19.2 and see if you still have the same problem?
Comment 7 Markus Trippelsdorf 2014-06-19 20:18:19 UTC
(In reply to comment #6)
> I understand you've compiled 0.19.60 from git. Can you compile 0.19.2 and
> see if you still have the same problem?

Yes. Same issue with 0.19.2.  (and poppler-0.26.1-21-g1b705331019b)
Comment 8 Albert Astals Cid 2014-06-19 20:29:56 UTC
Can you enable all okular debug in kdebugdialog and attach the debug output when running okular from a terminal?
Comment 9 Markus Trippelsdorf 2014-06-19 20:33:24 UTC
okular(21821) Okular::Utils::realDpiY: Pix: 1680 MM: 441
okular(21821) Okular::Utils::realDpiX: Pix: 1050 MM: 276
okular(21821) Okular::DocumentPrivate::openDocumentInternal: Output DPI: QSizeF(96.6304, 96.7619)
okular(21821)/okular (app) Okular::Document::requestPixmaps: request observer=0x1d40468 1044x824@0
okular(21821)/okular (app) Okular::Document::requestPixmaps: request observer=0x1d40468 1044x789@1
okular(21821)/okular (app) Okular::DocumentPrivate::sendGeneratorPixmapRequest: sending request observer=0x1d40468 1044x824@0 async == true isTile == false
okular(21821)/okular (app) Okular::Document::requestPixmaps: request observer=0x1d40468 1044x824@0
okular(21821)/okular (app) Okular::Document::requestPixmaps: request observer=0x1d40468 1044x789@1
okular(21821)/okular (app) Okular::Document::requestPixmaps: request observer=0x1d40468 1044x824@0
okular(21821)/okular (app) Okular::Document::requestPixmaps: request observer=0x1d40468 1044x789@1
okular(21821)/okular (app) Okular::Document::requestPixmaps: request observer=0x1c2efe8 80x63@0
okular(21821)/okular (app) Okular::Document::requestPixmaps: request observer=0x1d40468 1044x789@1
okular(21821)/okular (app) Okular::DocumentPrivate::sendGeneratorPixmapRequest: sending request observer=0x1c2efe8 80x63@0 async == true isTile == false
okular(21821)/okular (app) Okular::Document::requestPixmaps: request observer=0x1d40468 1044x789@1
okular(21821)/okular (app) Okular::Document::requestPixmaps: request observer=0x1d40468 1044x789@1
okular(21821)/okular (app) Okular::DocumentPrivate::sendGeneratorPixmapRequest: sending request observer=0x1d40468 1044x789@1 async == true isTile == false
Comment 10 Albert Astals Cid 2014-06-19 21:05:21 UTC
Nothing of interest here. Do you think you could record the desktop and host it somewhere so we can see what's going on?
Comment 11 Markus Trippelsdorf 2014-06-19 21:17:38 UTC
Created attachment 87303 [details]
screen capture
Comment 12 Albert Astals Cid 2014-07-10 21:17:38 UTC
Oh damn, i was able to reproduce something similar one time and then when i went to add debug it went away :/

Do i understand that you know how to compile okular? Would you be able to add some extra debug lines and give me the output so i can try to figure out what's going on?
Comment 13 Markus Trippelsdorf 2014-07-11 05:24:52 UTC
(In reply to comment #12)
> Oh damn, i was able to reproduce something similar one time and then when i
> went to add debug it went away :/
> 
> Do i understand that you know how to compile okular? Would you be able to
> add some extra debug lines and give me the output so i can try to figure out
> what's going on?

Yes, I could give it a try.
Comment 14 Markus Trippelsdorf 2014-07-11 05:33:36 UTC
BTW if I observed correctly it only happens in documents that contain a "rotated" page in them.
An example is a document which consists of almost only "portrait" pages, but contains one "landscape" page.
Comment 15 Christoph Feck 2014-08-15 14:48:19 UTC
What is the status of this bug? If I understand it correctly, some debug lines were to be added?
Comment 16 Markus Trippelsdorf 2014-08-15 15:07:06 UTC
(In reply to Christoph Feck from comment #15)
> What is the status of this bug? If I understand it correctly, some debug
> lines were to be added?

Yes, but Albert never posted a patch...
Comment 17 Albert Astals Cid 2014-08-18 23:01:30 UTC
Created attachment 88313 [details]
Please try this patch and attach the output you get
Comment 18 Markus Trippelsdorf 2014-08-19 06:13:34 UTC
markus@x4 tmp % okular out.pdf
(open document)
PageView::updateZoom 6 
PageView::updateZoom2  6 1.02034 
PageView::updateZoom 1 
PageView::updateZoom2  1 1.02034 
PageView::updateZoom 1 
PageView::updateZoom2  1 1.02034 
(zoom in attempt, not working)
PageView::updateZoom 4 
PageView::updateZoom2  4 1.02034 
PageView::updateZoom3  QVector(0.12, 0.25, 0.33, 0.5, 0.66, 0.75, 1, 1.06228, 1.06228, 1.25, 1.5, 2, 4, 8, 16) 1.06228 
PageView::updateZoom4  
(zoom out)
PageView::updateZoom 5 
PageView::updateZoom2  5 1.02034 
PageView::updateZoom3  QVector(0.12, 0.25, 0.33, 0.5, 0.66, 0.75, 1, 1.06228, 1.06228, 1.25, 1.5, 2, 4, 8, 16) 1 
PageView::updateZoom6  
(zoom in)
PageView::updateZoom 4 
PageView::updateZoom2  4 1 
PageView::updateZoom3  QVector(0.12, 0.25, 0.33, 0.5, 0.66, 0.75, 1, 1.06228, 1.06228, 1.25, 1.5, 2, 4, 8, 16) 1.06228 
PageView::updateZoom4  
PageView::updateZoom 1 
PageView::updateZoom2  1 1.06228 
(zoom in)
PageView::updateZoom 4 
PageView::updateZoom2  4 1.06228 
PageView::updateZoom3  QVector(0.12, 0.25, 0.33, 0.5, 0.66, 0.75, 1, 1.06228, 1.06228, 1.25, 1.5, 2, 4, 8, 16) 1.25 
PageView::updateZoom6
Comment 19 Markus Trippelsdorf 2014-08-19 10:02:01 UTC
The following hack fixes the issue for me:

diff --git a/ui/pageview.cpp b/ui/pageview.cpp
index 48d5d8df191c..eb1bd25226ae 100644
--- a/ui/pageview.cpp
+++ b/ui/pageview.cpp
@@ -55,6 +55,7 @@
 // system includes
 #include <math.h>
 #include <stdlib.h>
+#include <algorithm>
 
 // local includes
 #include "formwidgets.h"
@@ -3667,6 +3668,7 @@ void PageView::updateZoom( ZoomMode newZoomMode )
             }
             else
             {
+                newFactor=*std::lower_bound(zoomValue.begin(), zoomValue.end(),newFactor );
                 i = qUpperBound(zoomValue.begin(), zoomValue.end(), newFactor);
             }
             const float tmpFactor = *i;
Comment 20 Markus Trippelsdorf 2014-08-19 10:07:56 UTC
Posted the wrong patch. This one fixes the issue for me:

diff --git a/ui/pageview.cpp b/ui/pageview.cpp
index 48d5d8df191c..025163466894 100644
--- a/ui/pageview.cpp
+++ b/ui/pageview.cpp
@@ -3667,7 +3667,7 @@ void PageView::updateZoom( ZoomMode newZoomMode )
             }
             else
             {
-                i = qUpperBound(zoomValue.begin(), zoomValue.end(), newFactor);
+                i = qLowerBound(zoomValue.begin(), zoomValue.end(), newFactor);
             }
             const float tmpFactor = *i;
             if ( tmpFactor == zoomFactorFitWidth )
Comment 21 Markus Trippelsdorf 2014-08-19 10:14:00 UTC
This one is the actually working fix. Sorry for the noise.

diff --git a/ui/pageview.cpp b/ui/pageview.cpp
index 48d5d8df191c..383e5e4d2f8a 100644
--- a/ui/pageview.cpp
+++ b/ui/pageview.cpp
@@ -3667,6 +3667,7 @@ void PageView::updateZoom( ZoomMode newZoomMode )
             }
             else
             {
+                newFactor = *qLowerBound(zoomValue.begin(), zoomValue.end(), newFactor);
                 i = qUpperBound(zoomValue.begin(), zoomValue.end(), newFactor);
             }
             const float tmpFactor = *i;
Comment 22 Markus Trippelsdorf 2014-08-19 10:15:05 UTC
Created attachment 88316 [details]
Patch that fixes the issue.
Comment 23 Markus Trippelsdorf 2014-08-19 13:38:25 UTC
Created attachment 88318 [details]
Testcase for another issue

Another bug caused by the same commit fad9c4e6fd532e0:

Trying to zoom in the title-page of the attached testcase results 
in an endless loop : "Fit Width" -> "150%" -> "200%" ->  "Fit Width" -> etc.

markus@x4 tmp % okular out2.pdf
PageView::updateZoom 6 
PageView::updateZoom2  6 1.44576 
PageView::updateZoom 1 
PageView::updateZoom2  1 1.44576 
PageView::updateZoom 1 
PageView::updateZoom2  1 1.44576 
PageView::updateZoom 4 
PageView::updateZoom2  4 1.44576 
PageView::updateZoom3  QVector(0.12, 0.25, 0.33, 0.5, 0.66, 0.75, 1, 1.25, 1.5, 2, 2.60237, 2.60237, 4, 8, 16) 1.5 
PageView::updateZoom6  
PageView::updateZoom 4 
PageView::updateZoom2  4 1.5 
PageView::updateZoom3  QVector(0.12, 0.25, 0.33, 0.5, 0.66, 0.75, 1, 1.25, 1.5, 2, 2.60237, 2.60237, 4, 8, 16) 2 
PageView::updateZoom6  
PageView::updateZoom 4 
PageView::updateZoom2  4 2 
PageView::updateZoom3  QVector(0.12, 0.25, 0.33, 0.5, 0.66, 0.75, 1, 1.25, 1.5, 2, 2.60237, 2.60237, 4, 8, 16) 2.60237 
PageView::updateZoom4  
PageView::updateZoom 1 
PageView::updateZoom2  1 2.60237 
PageView::updateZoom 4 
PageView::updateZoom2  4 1.44576 
PageView::updateZoom3  QVector(0.12, 0.25, 0.33, 0.5, 0.66, 0.75, 1, 1.25, 1.5, 2, 2.60237, 2.60237, 4, 8, 16) 1.5 
PageView::updateZoom6
Comment 24 Markus Trippelsdorf 2014-08-19 13:42:09 UTC
Created attachment 88319 [details]
screen capture2
Comment 25 tingnan.zhang 2014-08-19 22:19:37 UTC
The problem actually may not be in the function Markus modified. The problem lies in the function 

double PageView::zoomFactorFitMode( ZoomMode mode );

Each time you click zoom in, the values of zoomFactorFitWidth and zoomFactorFitPage are computed from this function. I assumed that those values would remain the same when the document portion of the window remains the same.  

However, I just did a quick check and found that the value returned from viewport()->height() is changing even though the window size remains the same. Is it a designed behaviour?
Comment 26 Albert Astals Cid 2014-08-19 22:51:55 UTC
Well, have a look at the code, it depends on the current page and some more stuff, it's just 24 lines of code ;)
Comment 27 Markus Trippelsdorf 2014-08-20 06:18:01 UTC
Created attachment 88329 [details]
New patch for both issues

The attached patch fixes both issues.
Please review.
Comment 28 Albert Astals Cid 2014-08-20 21:43:06 UTC
Can you please use https://git.reviewboard.kde.org/ for patches? It is so much easier to keep track and discuss patches over there.
Comment 29 Albert Astals Cid 2014-08-21 21:44:11 UTC
Quick question, does converting 
float zoomFactor;
to
double zoomFactor;
solve your problem?
Comment 30 Albert Astals Cid 2014-08-21 21:57:16 UTC
No, that's not going to do anything
Comment 31 Albert Astals Cid 2014-08-21 22:08:51 UTC
Can you guys review https://git.reviewboard.kde.org/r/119894/ ?
Comment 32 Albert Astals Cid 2014-08-24 20:38:45 UTC
Git commit 3dfd172375cab9352fe73713df11219cb03ed199 by Albert Astals Cid.
Committed on 24/08/2014 at 20:37.
Pushed by aacid into branch 'KDE/4.14'.

Only update zoomFactor for current page

REVIEW: 119894
FIXED-IN: 4.14.1

Reviewed by Fabio and Markus Trippelsdorf

M  +10   -7    ui/pageview.cpp

http://commits.kde.org/okular/3dfd172375cab9352fe73713df11219cb03ed199