Re: [Okular-devel] Review Request 110003: Best-fit zoom
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110003/#review38075 --- This review has been submitted with commit e4aa8317b5d49ad1129e23bda52415ff7dc0a290 by Albert Astals Cid on behalf of Thomas Fischer to branch master. - Commit Hook On Aug. 17, 2013, 8:24 p.m., Thomas Fischer wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110003/ --- (Updated Aug. 17, 2013, 8:24 p.m.) Review request for Okular. Description --- Attached patch implements best-fit zoom for Okular. It is a refined version of the patch submitted in bug report 249364, attachment 51069. The refinement addresses the scrollbar issues as observed in continuous view mode. This addresses bug 249364. http://bugs.kde.org/show_bug.cgi?id=249364 Diffs - conf/dlggeneralbase.ui f2c9efd conf/okular.kcfg 1e23d61 doc/index.docbook 432aee2 part.rc 64aeffb ui/pageview.h 5484cc5 ui/pageview.cpp 5944072 Diff: http://git.reviewboard.kde.org/r/110003/diff/ Testing --- Thanks, Thomas Fischer ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110003: Best-fit zoom
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110003/ --- (Updated Aug. 18, 2013, 3:20 p.m.) Status -- This change has been marked as submitted. Review request for Okular. Description --- Attached patch implements best-fit zoom for Okular. It is a refined version of the patch submitted in bug report 249364, attachment 51069. The refinement addresses the scrollbar issues as observed in continuous view mode. This addresses bug 249364. http://bugs.kde.org/show_bug.cgi?id=249364 Diffs - conf/dlggeneralbase.ui f2c9efd conf/okular.kcfg 1e23d61 doc/index.docbook 432aee2 part.rc 64aeffb ui/pageview.h 5484cc5 ui/pageview.cpp 5944072 Diff: http://git.reviewboard.kde.org/r/110003/diff/ Testing --- Thanks, Thomas Fischer ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110003: Best-fit zoom
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110003/ --- (Updated Aug. 17, 2013, 8:24 p.m.) Review request for Okular. Changes --- Addressing Albert Astals Cid's recent comments on constness, version numbers, and documentation (handbook). Description --- Attached patch implements best-fit zoom for Okular. It is a refined version of the patch submitted in bug report 249364, attachment 51069. The refinement addresses the scrollbar issues as observed in continuous view mode. This addresses bug 249364. http://bugs.kde.org/show_bug.cgi?id=249364 Diffs (updated) - conf/dlggeneralbase.ui f2c9efd conf/okular.kcfg 1e23d61 doc/index.docbook 432aee2 part.rc 64aeffb ui/pageview.h 5484cc5 ui/pageview.cpp 5944072 Diff: http://git.reviewboard.kde.org/r/110003/diff/ Testing --- Thanks, Thomas Fischer ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110003: Best-fit zoom
On Aug. 14, 2013, 9:55 p.m., Albert Astals Cid wrote: Second, I am trying to add Auto Fit as one of the default zoom settings in the configuration dialog. Although I was quite sure I did not miss a spot and the option turns up in the settings dialog, choosing Auto Fit does not get activated when restarting Okular. Any hints what goes wrong? You aware this setting only applies for files you've never opened? Is this your problem? Indeed. I tested it with another file I had never opened before and AutoFit was active as expected. So, is there anything left or is my patch good to be accepted? - Thomas --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110003/#review37805 --- On July 7, 2013, 11:04 p.m., Thomas Fischer wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110003/ --- (Updated July 7, 2013, 11:04 p.m.) Review request for Okular. Description --- Attached patch implements best-fit zoom for Okular. It is a refined version of the patch submitted in bug report 249364, attachment 51069. The refinement addresses the scrollbar issues as observed in continuous view mode. This addresses bug 249364. http://bugs.kde.org/show_bug.cgi?id=249364 Diffs - conf/dlggeneralbase.ui f2c9efd conf/okular.kcfg 1e23d61 part.rc 64aeffb ui/pageview.h 5484cc5 ui/pageview.cpp 16b00ab Diff: http://git.reviewboard.kde.org/r/110003/diff/ Testing --- Thanks, Thomas Fischer ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110003: Best-fit zoom
On Aug. 14, 2013, 9:55 p.m., Albert Astals Cid wrote: Second, I am trying to add Auto Fit as one of the default zoom settings in the configuration dialog. Although I was quite sure I did not miss a spot and the option turns up in the settings dialog, choosing Auto Fit does not get activated when restarting Okular. Any hints what goes wrong? You aware this setting only applies for files you've never opened? Is this your problem? Thomas Fischer wrote: Indeed. I tested it with another file I had never opened before and AutoFit was active as expected. So, is there anything left or is my patch good to be accepted? Looks ok from the code POV (well you need to update part.rc version and adding a few const to variables that never change like uiAspect/pageAspect/etc would be cool) What I would like you is editing the docbook manual to explain the new zoom mode. Can you do that? - Albert --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110003/#review37805 --- On July 7, 2013, 11:04 p.m., Thomas Fischer wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110003/ --- (Updated July 7, 2013, 11:04 p.m.) Review request for Okular. Description --- Attached patch implements best-fit zoom for Okular. It is a refined version of the patch submitted in bug report 249364, attachment 51069. The refinement addresses the scrollbar issues as observed in continuous view mode. This addresses bug 249364. http://bugs.kde.org/show_bug.cgi?id=249364 Diffs - conf/dlggeneralbase.ui f2c9efd conf/okular.kcfg 1e23d61 part.rc 64aeffb ui/pageview.h 5484cc5 ui/pageview.cpp 16b00ab Diff: http://git.reviewboard.kde.org/r/110003/diff/ Testing --- Thanks, Thomas Fischer ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110003: Best-fit zoom
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110003/#review37805 --- Second, I am trying to add Auto Fit as one of the default zoom settings in the configuration dialog. Although I was quite sure I did not miss a spot and the option turns up in the settings dialog, choosing Auto Fit does not get activated when restarting Okular. Any hints what goes wrong? You aware this setting only applies for files you've never opened? Is this your problem? - Albert Astals Cid On July 7, 2013, 11:04 p.m., Thomas Fischer wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110003/ --- (Updated July 7, 2013, 11:04 p.m.) Review request for Okular. Description --- Attached patch implements best-fit zoom for Okular. It is a refined version of the patch submitted in bug report 249364, attachment 51069. The refinement addresses the scrollbar issues as observed in continuous view mode. This addresses bug 249364. http://bugs.kde.org/show_bug.cgi?id=249364 Diffs - conf/dlggeneralbase.ui f2c9efd conf/okular.kcfg 1e23d61 part.rc 64aeffb ui/pageview.h 5484cc5 ui/pageview.cpp 16b00ab Diff: http://git.reviewboard.kde.org/r/110003/diff/ Testing --- Thanks, Thomas Fischer ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110003: Best-fit zoom
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110003/ --- (Updated July 7, 2013, 11:04 p.m.) Review request for Okular. Changes --- The patch reflects two changes based on comments in the Forum. First, it renames the feature to auto fit which a majority of comments support as a better name than best fit. Second, I am trying to add Auto Fit as one of the default zoom settings in the configuration dialog. Although I was quite sure I did not miss a spot and the option turns up in the settings dialog, choosing Auto Fit does not get activated when restarting Okular. Any hints what goes wrong? Description --- Attached patch implements best-fit zoom for Okular. It is a refined version of the patch submitted in bug report 249364, attachment 51069. The refinement addresses the scrollbar issues as observed in continuous view mode. This addresses bug 249364. http://bugs.kde.org/show_bug.cgi?id=249364 Diffs (updated) - conf/dlggeneralbase.ui f2c9efd conf/okular.kcfg 1e23d61 part.rc 64aeffb ui/pageview.h 5484cc5 ui/pageview.cpp 16b00ab Diff: http://git.reviewboard.kde.org/r/110003/diff/ Testing --- Thanks, Thomas Fischer ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110003: Best-fit zoom
On April 14, 2013, 10:07 p.m., Albert Astals Cid wrote: Can you please describe your best fit algorithm? I am finding that in my 1920x1080 resolution best fit is giving me a Fit to width while in evince i get a Fit to page that may actually make more sense since i am seeing more info in that Fit to page than in this Fit to width and i can still read it. Thomas Fischer wrote: My best-fit code switches between three zoom types depending on the relation of aspect ratio of Okular's area to show content and the aspect ratio of the shown page. Depending on this relation, it assumes that one of the following three cases is true: (1) Both ratios are very similar, e.g. when you are looking on landscape-oriented PDF slides in a maximized Okular window (4:3 aspect ratio). Fit-page is the best viewing mode here. (2) You have have a wide Okular window (e.g. maximize on a wide screen) viewing a portrait-oriented A4 document such as a letter. If you eye sight and your screen is good, you can still read it, but don't assume everyone else can. So a better choice may be to fit-width zoom the page, even if only a small section is visible, but at least the text should be readable. (3) You have a split-screen Okular window (e.g. right or left half of your screen), but you are looking at a landscape-oriented document. Again, it may be a better choice to fit-height zoom your document than to fit-page zoom it if you want to read the fineprint. On extreme cases, best-fit may zoom in a way that the document becomes even hard to read, but in most cases it has its benefits being a good approximation for typical use-cases (in my personal view). Albert Astals Cid wrote: Do you think you could take a few screenshots of the various scenarios with various kind of document formats in both okular+your patch and evince and post them to forums.kde.org so that people can comment (even the forums have poll support IIRC) which of the two Best Fit algorithms they like more? If we're going to do a Best Fit, lets make sure it's Best Fit by all people's opinion and not just based or your or my personal view. Wrote a posting in KDE Forum: http://forum.kde.org/viewtopic.php?f=251t=111777 - Thomas --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110003/#review31033 --- On April 14, 2013, 11:26 a.m., Thomas Fischer wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110003/ --- (Updated April 14, 2013, 11:26 a.m.) Review request for Okular. Description --- Attached patch implements best-fit zoom for Okular. It is a refined version of the patch submitted in bug report 249364, attachment 51069. The refinement addresses the scrollbar issues as observed in continuous view mode. This addresses bug 249364. http://bugs.kde.org/show_bug.cgi?id=249364 Diffs - part.rc 64aeffb ui/pageview.h 5e839f2 ui/pageview.cpp 6e093ef Diff: http://git.reviewboard.kde.org/r/110003/diff/ Testing --- Thanks, Thomas Fischer ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110003: Best-fit zoom
On April 14, 2013, 10:07 p.m., Albert Astals Cid wrote: Can you please describe your best fit algorithm? I am finding that in my 1920x1080 resolution best fit is giving me a Fit to width while in evince i get a Fit to page that may actually make more sense since i am seeing more info in that Fit to page than in this Fit to width and i can still read it. My best-fit code switches between three zoom types depending on the relation of aspect ratio of Okular's area to show content and the aspect ratio of the shown page. Depending on this relation, it assumes that one of the following three cases is true: (1) Both ratios are very similar, e.g. when you are looking on landscape-oriented PDF slides in a maximized Okular window (4:3 aspect ratio). Fit-page is the best viewing mode here. (2) You have have a wide Okular window (e.g. maximize on a wide screen) viewing a portrait-oriented A4 document such as a letter. If you eye sight and your screen is good, you can still read it, but don't assume everyone else can. So a better choice may be to fit-width zoom the page, even if only a small section is visible, but at least the text should be readable. (3) You have a split-screen Okular window (e.g. right or left half of your screen), but you are looking at a landscape-oriented document. Again, it may be a better choice to fit-height zoom your document than to fit-page zoom it if you want to read the fineprint. On extreme cases, best-fit may zoom in a way that the document becomes even hard to read, but in most cases it has its benefits being a good approximation for typical use-cases (in my personal view). - Thomas --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110003/#review31033 --- On April 14, 2013, 11:26 a.m., Thomas Fischer wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110003/ --- (Updated April 14, 2013, 11:26 a.m.) Review request for Okular. Description --- Attached patch implements best-fit zoom for Okular. It is a refined version of the patch submitted in bug report 249364, attachment 51069. The refinement addresses the scrollbar issues as observed in continuous view mode. This addresses bug 249364. http://bugs.kde.org/show_bug.cgi?id=249364 Diffs - part.rc 64aeffb ui/pageview.h 5e839f2 ui/pageview.cpp 6e093ef Diff: http://git.reviewboard.kde.org/r/110003/diff/ Testing --- Thanks, Thomas Fischer ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110003: Best-fit zoom
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110003/#review31033 --- Can you please describe your best fit algorithm? I am finding that in my 1920x1080 resolution best fit is giving me a Fit to width while in evince i get a Fit to page that may actually make more sense since i am seeing more info in that Fit to page than in this Fit to width and i can still read it. - Albert Astals Cid On April 14, 2013, 11:26 a.m., Thomas Fischer wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110003/ --- (Updated April 14, 2013, 11:26 a.m.) Review request for Okular. Description --- Attached patch implements best-fit zoom for Okular. It is a refined version of the patch submitted in bug report 249364, attachment 51069. The refinement addresses the scrollbar issues as observed in continuous view mode. This addresses bug 249364. http://bugs.kde.org/show_bug.cgi?id=249364 Diffs - part.rc 64aeffb ui/pageview.h 5e839f2 ui/pageview.cpp 6e093ef Diff: http://git.reviewboard.kde.org/r/110003/diff/ Testing --- Thanks, Thomas Fischer ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
Re: [Okular-devel] Review Request 110003: Best-fit zoom
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110003/#review31007 --- Needs fixing: * Choose 100% in the combobox. * Get 75% instead - Albert Astals Cid On April 13, 2013, 6:59 p.m., Thomas Fischer wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110003/ --- (Updated April 13, 2013, 6:59 p.m.) Review request for Okular. Description --- Attached patch implements best-fit zoom for Okular. It is a refined version of the patch submitted in bug report 249364, attachment 51069. The refinement addresses the scrollbar issues as observed in continuous view mode. This addresses bug 249364. http://bugs.kde.org/show_bug.cgi?id=249364 Diffs - ui/pageview.h 5e839f2 ui/pageview.cpp 6e093ef Diff: http://git.reviewboard.kde.org/r/110003/diff/ Testing --- Thanks, Thomas Fischer ___ Okular-devel mailing list Okular-devel@kde.org https://mail.kde.org/mailman/listinfo/okular-devel
