Created attachment 51069 [details] Best-fit zoom option, mostly diff against kdegraphics/okular/ui/pageview.cpp Version: unspecified (using Devel) OS: Linux I have written a small patch for Okular's ui/pageview.cpp which adds "Fit Best" as one of the zooming options. This new zoom option compares the current page's aspect ratio with the window's aspect ratio and switches effectively between fit-width, fit-height, and fit-page. The code reuses partially the (no longer) not used "Fit Text" code, adds some new lines, and enhances the infinite resizing loop patch to cover the new zoom option, too. Reproducible: Always
Thanks for the patch. Is that 1.25 supposed to be the aspectRatio of the users screen?
> Is that 1.25 supposed to be the aspectRatio of the users screen? No, not directly. The patch first computes the aspect ratios of the page and the aspect ratio of the space available to display the page (as determined by rowHeight and colWidth). Then both aspect ratios are compared by dividing one with another, the result is stored in variable rel. If both aspect ratios are very similar, such as when viewing presentations in a full-screen okular on a 3:4 or 16:9 monitor, rel is somewhere around 1 and the best zoom option would be "fit page" (third case in the following if block). Now, this "very similar" has to be put in a numeric value and this is aspectRatioRelation. Having a value of 1.25, it states that if 0.8<rel<1.25 holds, best "fit page" is the best zoom option. If rel is below 0.8, the page to view is relatively higher than the available space and "fit width" is most appropriate to utilize the space best. "Fit page" would render any text on the page unreadable (assuming I would open an A4 page/portrait with normal 10-12pt text). The same principle holds for rel >1.25 resulting in a "fit height" zoom. Another advantage with best fit is, that in the "page fit" case no scrolling is necessary and the mouse wheel allows to jump between pages (very useful for slide sets), and in the other two cases, only one scroll bar is used making browsing through the document much easier. I hope this explains the patch...
Ok, so 1.25, 1/1.25 is the aspect ratio boundary where it decides to either go for zoom width/page/page i see, the name of the variable got me a bit lost. There is a problem with the scrollbar code and i can make it quite easily remove the vertical scrollbar when it should be there. Screenshot attached.
Created attachment 51310 [details] Screenshot of the problem
> There is a problem with the scrollbar code and i can make it quite easily > remove the vertical scrollbar when it should be there. Screenshot attached. Haven't found a good solution yet. Could be necessary to rewrite/improve the current code for zooming and scrollbar visibility. I will continue to look into a simple solution, though...
Any luck?
(In reply to comment #6) > Any luck? It looks like the problem you observed is related to mixing best-fit and continuous view. Both features mess with the vertical scroll bar. The problem does not appear if you use single-page view.
Please try this clone here, it should work better now: git@git.kde.org:clones/okular/thomasfischer/bestfit.git
Can you please use reviewboard to submit patches? It's much easier to comment in there and also to keep track what is waiting, etc.
Submitted as review 110003: https://git.reviewboard.kde.org/r/110003/
Git commit e4aa8317b5d49ad1129e23bda52415ff7dc0a290 by Albert Astals Cid, on behalf of Thomas Fischer. Committed on 18/08/2013 at 15:19. Pushed by aacid into branch 'master'. Auto-fit zoom REVIEW: 110003 M +5 -0 conf/dlggeneralbase.ui M +1 -1 conf/okular.kcfg M +12 -0 doc/index.docbook M +2 -1 part.rc M +72 -8 ui/pageview.cpp M +2 -1 ui/pageview.h http://commits.kde.org/okular/e4aa8317b5d49ad1129e23bda52415ff7dc0a290
Albert, Version-Fixed-In == 4.12, or do you want to use the Okular version now?
4.12 or 4.12.0 is both fine