Summary: | Patch to allow different ways to handle zoom and position between images | ||
---|---|---|---|
Product: | [Applications] gwenview | Reporter: | John Zaitseff <j.zaitseff> |
Component: | general | Assignee: | Gwenview Bugs <gwenview-bugs-null> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | bugs, groszdanielpub, myriam, someuniquename, walch.martin |
Priority: | NOR | ||
Version: | Git (add output of "git log -1 --oneline" to description) | ||
Target Milestone: | --- | ||
Platform: | Compiled Sources | ||
OS: | Linux | ||
Latest Commit: | http://commits.kde.org/gwenview/5572aded5dfedd7219138166c26b9d01cadebe84 | Version Fixed In: | |
Sentry Crash Report: | |||
Attachments: | Allow different modes of operation for zoom and position preservation |
Description
John Zaitseff
2014-07-09 08:17:36 UTC
Created attachment 87658 [details]
Allow different modes of operation for zoom and position preservation
The following changes since commit fde8adf02b8c7ce6b5f2975af3a628daabbe567d: SVN_SILENT made messages (.desktop file) (2014-07-06 04:29:41 +0000) are available in the git repository at: git://git.zap.org.au/data/git/gwenview.git zoom-modes for you to fetch changes up to 1bf8690fcebf1bd01bc64df2beb7e2b154a4fa9b: Add missing ">" to e-mail address (2014-07-09 17:26:15 +1000) ---------------------------------------------------------------- John Zaitseff (8): Add radio buttons to set the default zoom mode. Also renumber all controls on the page to be systematic. Rename Single zoom mode to Shared zoom mode Add the ZoomMode config file option Remove the superfluous Lock Zoom button in the zoom widget Completely remove traces of ZoomLocked Use saved zoom and position only for individual zoom mode Fix issue where "0% zoom" is shown (bug #335675) Add missing ">" to e-mail address app/configdialog.cpp | 7 ++ app/imageviewconfigpage.ui | 182 ++++++++++++++++++++++++++++++-------- app/viewmainpage.cpp | 19 ++-- lib/documentview/documentview.cpp | 6 +- lib/gwenviewconfig.kcfg | 29 ++++-- lib/zoommode.h | 49 ++++++++++ lib/zoomwidget.cpp | 29 ------ lib/zoomwidget.h | 6 -- 8 files changed, 231 insertions(+), 96 deletions(-) create mode 100644 lib/zoommode.h This patch potentially closes the following still-open bugs: * bug 215734 * bug 334530 * bug 335675 * bug 337037 John, thanks for the investigation and patch! I suggest to create a review request at https://git.reviewboard.kde.org/ I have submitted a review request (https://git.reviewboard.kde.org/r/119549/) for this issue. Git commit 5572aded5dfedd7219138166c26b9d01cadebe84 by Aurélien Gâteau, on behalf of John Zaitseff. Committed on 07/08/2014 at 16:24. Pushed by gateau into branch 'master'. Allow different ways to handle zoom and position between images (Taken from bug #337262 ) Over the years, there has been much discussion about whether the zoom and position should be kept the same between images (see, for example, bug 293103, which I submitted, or bugs 291759, 294915, 321122, 327889, 331412, 334530, 337037 --- there may be more!). I have even submitted a patch (with bug 293103) which was applied in modified form---thanks! However, it has been subsequently broken... I have come to realise that there are THREE main ways people like to have the zoom and position settings applied to successive images: 1. Each image should be opened in Zoom to Fit mode, even if the previous image was zoomed in or out and was panned to a different position. This is Aurélien Gâteau's preferred mode of operation: call it Autofit zoom mode. 2. The zoom and position settings should be shared across all images. New images should be opened with the previous image's settings. If you go back to a previous image (from image B to image A, previously opened), image A's settings should be updated to be those of image B. This allows you to set zoom and position on an image, then go back and forward to the previous and next image while retaining the settings. This is MY preferred mode of operation: call it Shared mode. 3. Each image should remember its own zoom and position settings. New images should be opened with the previous image's position and zoom, but if you then change image B's zoom/position, this is NOT passed back to image A, if you go back to that image. This is Abhinav Gangwar's preferred mode of operation, and is what LockZoom implements in Gwenview: call it Individual mode. I think people don't mind Autofit being the default, as long as it can be changed. At the moment, doing so is very non-obvious, and the Shared mode of operation simply does not exist (as of 12th February 2014). I am submitting a patch that does the following: 1. Create a config file option "ZoomMode" with ZoomMode::Autofit, ZoomMode::Shared and ZoomMode::Individual being the choices (Autofit is the default, per Aurélien's preferences). 2. Remove the now-obsolete ShowLockZoomButton and LockZoom config options. 3. Remove the Lock Zoom button: it is not visible in Autofit mode anyway, and when a user wants the other modes, they want it on ALL the time, not just some of the time! Besides, it clutters up the interface :-) 4. Set the zoom and position settings for each image depending on the ZoomMode selected. 5. Add a group of three Zoom Mode radio buttons to the Image Settings configuration dialog page: "Autofit each image", "Shared zoom and position" and "Individual (per-image) zoom and position". I think adding these radio buttons is the most elegant way for users to change this setting, given how non-obvious the current method is (as can be seen by the continual stream of bug reports!). REVIEW: 119549 M +7 -0 app/configdialog.cpp M +181 -45 app/imageviewconfigpage.ui M +0 -1 app/kipiinterface.h M +7 -12 app/viewmainpage.cpp M +1 -2 cmake/FindLCMS2.cmake M +0 -1 devdoc/CONTRIBUTING.md M +19 -19 doc/index.docbook M +0 -1 lib/archiveutils.h M +1 -1 lib/cms/iccjpeg.c M +5 -1 lib/documentview/documentview.cpp M +19 -9 lib/gwenviewconfig.kcfg A +49 -0 lib/zoommode.h [License: GPL (v2)] M +0 -29 lib/zoomwidget.cpp M +0 -6 lib/zoomwidget.h M +0 -1 part/gvpart.rc http://commits.kde.org/gwenview/5572aded5dfedd7219138166c26b9d01cadebe84 Thanks John and Aurélien. Which of the bugs mentioned in comment #3 can be marked resolved? All the bugs in Comment #3 may be closed: most are duplicates of this one. I have confirmed that zoom in full-screen mode does work. *** Bug 215734 has been marked as a duplicate of this bug. *** *** Bug 334530 has been marked as a duplicate of this bug. *** *** Bug 335675 has been marked as a duplicate of this bug. *** *** Bug 337037 has been marked as a duplicate of this bug. *** Information: The commit is not part of the 4.14 applications release due to string freezes. The version number might get bumped to 5.x, in case Gwenview developers decide to make the next release based on the KF5 frameworks. There is a further issue, which I reported in Bug 215734 (Comment 2 in particular): WHAT DOES MIDDLE-CLICK DO? In KDE 3: - If the zoom ratio is not zoom-to-fit (but e. g. 60%), it goes to zoom-to-fit (e. g. 25%). - If it is zoomed to fit, it goes back to the last zoom ratio used before (e. g. 60%). In KDE 4: - If it is not zoom-to-fit, it goes to zoom-to-fit. - If it is zoomed to fit, it goes to 100%. I strongly prefer the KDE 3 version. With today's high resolution photos, often something like 60% and not 100% is preferable even for viewing details. So one would frequently switch between zoom-to-fit (for seeing the whole image) and 60% (for viewing details). Similarly, when looking at scanned documents, one would switch between the whole page and fit-to-width. Toggling between zoom-to-fit and the last-used zoom sounds preferable, as I can make the last-used zoom be 100%. Aurelien Gateau you nailed the issue on the head in your comment from 2014-08-07. IMHO re-using one zoom setting while changing images (your option 2) is the most logical and most widely implemented in professional photo software way of doing this. I like that you gave power to the users by letting them choose their preferred method - I've always considered this as "the KDE way", which is why I prefer using KDE over other suites. Can't wait for the next version of Gwenview. |