Bug 337262 - Patch to allow different ways to handle zoom and position between images
Summary: Patch to allow different ways to handle zoom and position between images
Status: RESOLVED FIXED
Alias: None
Product: gwenview
Classification: Applications
Component: general (show other bugs)
Version: Git (add output of "git log -1 --oneline" to description)
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: Gwenview Bugs
URL:
Keywords:
: 334530 335675 337037 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-07-09 08:17 UTC by John Zaitseff
Modified: 2014-12-01 14:40 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Allow different modes of operation for zoom and position preservation (21.89 KB, patch)
2014-07-09 08:18 UTC, John Zaitseff
Details

Note You need to log in before you can comment on or make changes to this bug.
Description John Zaitseff 2014-07-09 08:17:36 UTC
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!).

Could you please apply the patch.  I will also submit a "git request-pull".

Reproducible: Always
Comment 1 John Zaitseff 2014-07-09 08:18:28 UTC
Created attachment 87658 [details]
Allow different modes of operation for zoom and position preservation
Comment 2 John Zaitseff 2014-07-09 08:19:40 UTC
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
Comment 3 John Zaitseff 2014-07-09 08:26:20 UTC
This patch potentially closes the following still-open bugs:

* bug 215734
* bug 334530
* bug 335675
* bug 337037
Comment 4 Christoph Feck 2014-07-20 20:12:26 UTC
John, thanks for the investigation and patch! I suggest to create a review request at https://git.reviewboard.kde.org/
Comment 5 John Zaitseff 2014-07-31 02:41:45 UTC
I have submitted a review request (https://git.reviewboard.kde.org/r/119549/) for this issue.
Comment 6 Aurelien Gateau 2014-08-07 16:27:33 UTC
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
Comment 7 Christoph Feck 2014-08-07 16:51:08 UTC
Thanks John and Aurélien.

Which of the bugs mentioned in comment #3 can be marked resolved?
Comment 8 John Zaitseff 2014-08-07 21:45:25 UTC
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.
Comment 9 Christoph Feck 2014-08-16 14:35:41 UTC
*** Bug 215734 has been marked as a duplicate of this bug. ***
Comment 10 Christoph Feck 2014-08-16 14:35:55 UTC
*** Bug 334530 has been marked as a duplicate of this bug. ***
Comment 11 Christoph Feck 2014-08-16 14:36:42 UTC
*** Bug 335675 has been marked as a duplicate of this bug. ***
Comment 12 Christoph Feck 2014-08-16 14:36:59 UTC
*** Bug 337037 has been marked as a duplicate of this bug. ***
Comment 13 Christoph Feck 2014-08-16 14:45:01 UTC
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.
Comment 14 Grósz Dániel 2014-08-16 18:09:47 UTC
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.
Comment 15 DrSlony 2014-08-18 10:10:30 UTC
Toggling between zoom-to-fit and the last-used zoom sounds preferable, as I can make the last-used zoom be 100%.
Comment 16 DrSlony 2014-12-01 14:40:23 UTC
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.