[prev in list] [next in list] [prev in thread] [next in thread]
List: gwenview-devel
Subject: Re: Review Request 119549: Allow different ways to handle zoom and position between images
From: Aurélien Gâteau <agateau () kde ! org>
Date: 2014-08-07 16:28:30
Message-ID: 20140807162830.20974.80578 () probe ! kde ! org
[Download RAW message or body]
--===============5118399826738306668==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
> On août 4, 2014, 10:44 matin, Aurélien Gâteau wrote:
> > Hi, looks great, thanks for the patch!
> >
> > I only have minor requests for the configuration dialog:
> > - I suggest renaming "Shared zoom and position" to "Keep zoom and \
> > position" (and rename the enum value to Keep) and "Individual (per \
> > image) zoom and position" to "Per image zoom and \
> > position".
> > - Move the "Zoom mode" block above the "Enlarge smaller images" \
> > checkbox to keep all zooming options together
>
> John Zaitseff wrote:
> * Moving the zoom mode block is probably a good idea. Did you want me to \
> redo the patch, or will you? I can post the "git request-pull" here if \
> you like.
> * Renaming "Individual (per image) zoom and position" is also a good \
> idea.
> * I'm not so sure about renaming "Shared zoom and position": the point is \
> that the zoom and position are *shared* between different images. I do \
> think it can be worded better, however, so I'm open to suggestions. What \
> about "Keep same zoom and position" (with enum renamed to KeepSame)?
> I can also provide a patch for the documentation (handbook, etc.) to \
> explain all this to the user; I just didn't want to do that before this \
> patch was accepted :-)
> Aurélien Gâteau wrote:
> "Keep same zoom and position" and a KeepSame enum sounds good to me. A \
> documentation update sounds good as well :)
> Would be great if you could update the merge request with these changes \
> as it makes it easy for me to merge them in without having to use git \
> author command line options to attribute the changes to you.
> John Zaitseff wrote:
> I've done all the above, except for the documentation update: that I'll \
> do as a separate patch, as I may well modify more than just my little \
> section. I also plan to add tooltips, etc, to the dialog boxes, as part \
> of the documentation patch.
> John Zaitseff wrote:
> 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 zoom-modes
>
> for you to fetch changes up to a32c9c127eebe2e90c4af6a9c84a662321d1f80e:
>
> Move the "Zoom mode" block above "Enlarge smaller images" checkbox \
> (2014-08-06 08:56:18 +1000)
> ----------------------------------------------------------------
> John Zaitseff (13):
> 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
> Remove trailing whitespace
> Replace tabs with spaces in source code
> Rename label "Individual (per-image)" to "Per image zoom and position"
> Rename Shared zoom mode to KeepSame
> Move the "Zoom mode" block above "Enlarge smaller images" checkbox
>
> app/configdialog.cpp | 7 ++
> app/imageviewconfigpage.ui | 226 \
> ++++++++++++++++++++++++++++++-------- app/kipiinterface.h \
> | 1 - app/viewmainpage.cpp | 19 ++--
> cmake/FindLCMS2.cmake | 3 +-
> devdoc/CONTRIBUTING.md | 1 -
> doc/index.docbook | 38 +++----
> lib/archiveutils.h | 1 -
> lib/cms/iccjpeg.c | 2 +-
> lib/documentview/documentview.cpp | 6 +-
> lib/gwenviewconfig.kcfg | 28 +++--
> lib/zoommode.h | 49 +++++++++
> lib/zoomwidget.cpp | 29 -----
> lib/zoomwidget.h | 6 -
> part/gvpart.rc | 1 -
> 15 files changed, 289 insertions(+), 128 deletions(-)
> create mode 100644 lib/zoommode.h
Thanks a lot! I just merged it in master (it's too late for 4.14 \
unfortunately).
- Aurélien
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/119549/#review63738
-----------------------------------------------------------
On août 7, 2014, 6:27 après-midi, John Zaitseff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119549/
> -----------------------------------------------------------
>
> (Updated août 7, 2014, 6:27 après-midi)
>
>
> Review request for Gwenview.
>
>
> Bugs: 337262
> http://bugs.kde.org/show_bug.cgi?id=337262
>
>
> Repository: gwenview
>
>
> Description
> -------
>
> (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!).
>
> Diffs
> -----
>
> app/configdialog.cpp a582695
> app/imageviewconfigpage.ui f5e1e5a
> app/kipiinterface.h 45fb03b
> app/viewmainpage.cpp 7caf099
> cmake/FindLCMS2.cmake cc17e9f
> devdoc/CONTRIBUTING.md ffdb7f7
> doc/index.docbook d3266b8
> lib/archiveutils.h db79771
> lib/cms/iccjpeg.c 1f6c4b1
> lib/documentview/documentview.cpp 1ab8bbb
> lib/gwenviewconfig.kcfg 92d39f7
> lib/zoommode.h PRE-CREATION
> lib/zoomwidget.h b181f5a
> lib/zoomwidget.cpp da66a56
> part/gvpart.rc 35703a7
>
> Diff: https://git.reviewboard.kde.org/r/119549/diff/
>
>
> Testing
> -------
>
> Have compiled Gwenview GIT head with this patch on my system; tested \
> successfully with all variants.
>
> Thanks,
>
> John Zaitseff
>
>
--===============5118399826738306668==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 8bit
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: \
1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; \
-webkit-border-radius: 6px;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/119549/">https://git.reviewboard.kde.org/r/119549/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; \
padding-left: 10px;"> <p style="margin-top: 0;">On août 4th, 2014, 10:44 \
matin CEST, <b>Aurélien Gâteau</b> wrote:</p> <blockquote \
style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">Hi, looks great, thanks for \
the patch!</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">I only have minor requests \
for the configuration dialog:<br style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: \
normal;" />
- I suggest renaming "Shared zoom and position" to "Keep zoom and position" \
(and rename the enum value to Keep) and "Individual (per image) zoom and \
position" to "Per image zoom and position".<br style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;" />
- Move the "Zoom mode" block above the "Enlarge smaller images" checkbox to \
keep all zooming options together</p></pre> </blockquote>
<p>On août 5th, 2014, 2:20 matin CEST, <b>John Zaitseff</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; \
padding-left: 10px;"> <pre style="white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 \
1em;line-height: inherit;white-space: normal;"> <li style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;"> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">Moving the zoom mode block is \
probably a good idea. Did you want me to redo the patch, or will you? I \
can post the "git request-pull" here if you like.</p> </li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;"> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">Renaming \
"Individual (per image) zoom and position" is also a good idea.</p> </li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;"> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm not so \
sure about renaming "Shared zoom and position": the point is that the zoom \
and position are <em style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;">shared</em> between different \
images. I do think it can be worded better, however, so I'm open to \
suggestions. What about "Keep same zoom and position" (with enum renamed \
to KeepSame)?</p> </li>
</ul>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">I can also provide a patch for the \
documentation (handbook, etc.) to explain all this to the user; I just \
didn't want to do that before this patch was accepted :-)</p></pre> \
</blockquote>
<p>On août 5th, 2014, 9:33 matin CEST, <b>Aurélien Gâteau</b> \
wrote:</p> <blockquote style="margin-left: 1em; border-left: 2px solid \
#d0d0d0; padding-left: 10px;"> <pre style="white-space: pre-wrap; \
white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: \
-o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">"Keep same \
zoom and position" and a KeepSame enum sounds good to me. A documentation \
update sounds good as well :)</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">Would be \
great if you could update the merge request with these changes as it makes \
it easy for me to merge them in without having to use git author command \
line options to attribute the changes to you.</p></pre> </blockquote>
<p>On août 6th, 2014, 1:06 matin CEST, <b>John Zaitseff</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; \
padding-left: 10px;"> <pre style="white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">I've done all the above, \
except for the documentation update: that I'll do as a separate patch, as I \
may well modify more than just my little section. I also plan to add \
tooltips, etc, to the dialog boxes, as part of the documentation \
patch.</p></pre> </blockquote>
<p>On août 6th, 2014, 1:20 matin CEST, <b>John Zaitseff</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; \
padding-left: 10px;"> <pre style="white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">The following changes since \
commit fde8adf02b8c7ce6b5f2975af3a628daabbe567d:</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">SVN_SILENT made messages (.desktop file) (2014-07-06 04:29:41 \
+0000)</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">are available in the git \
repository at:</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: \
inherit;">git://git.zap.org.au/data/git/gwenview zoom-modes</p> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">for you to fetch changes up to \
a32c9c127eebe2e90c4af6a9c84a662321d1f80e:</p> <p style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
inherit;">Move the "Zoom mode" block above "Enlarge smaller images" \
checkbox (2014-08-06 08:56:18 +1000)</p> <hr style="text-rendering: \
inherit;margin: 0;padding: 0;white-space: normal;border: 1px solid \
#ddd;line-height: inherit;" /> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">John Zaitseff \
(13):<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;" /> Add radio buttons to set the default zoom \
mode. Also renumber all controls on the page to be systematic.<br \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;" />
Rename Single zoom mode to Shared zoom mode<br style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;" />
Add the ZoomMode config file option<br style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;" />
Remove the superfluous Lock Zoom button in the zoom widget<br \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;" />
Completely remove traces of ZoomLocked<br style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;" />
Use saved zoom and position only for individual zoom mode<br \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;" />
Fix issue where "0% zoom" is shown (bug #335675)<br style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;" />
Add missing ">" to e-mail address<br style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;" />
Remove trailing whitespace<br style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: \
normal;" />
Replace tabs with spaces in source code<br style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;" /> Rename label "Individual (per-image)" to "Per image zoom and \
position"<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" />
Rename Shared zoom mode to KeepSame<br style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;" />
Move the "Zoom mode" block above "Enlarge smaller images" \
checkbox</p> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">app/configdialog.cpp \
| 7 ++<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" /> app/imageviewconfigpage.ui \
| 226 ++++++++++++++++++++++++++++++--------<br style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;" /> app/kipiinterface.h | 1 -<br style="padding: \
0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: \
normal;" /> app/viewmainpage.cpp | 19 ++--<br \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;" /> cmake/FindLCMS2.cmake | 3 \
+-<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;" /> devdoc/CONTRIBUTING.md | 1 \
-<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;" /> doc/index.docbook | 38 \
+++----<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;" /> lib/archiveutils.h | 1 \
-<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;" /> lib/cms/iccjpeg.c | 2 \
+-<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;" /> lib/documentview/documentview.cpp | 6 \
+-<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;" /> lib/gwenviewconfig.kcfg | 28 \
+++--<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;" /> lib/zoommode.h | 49 \
+++++++++<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" /> lib/zoomwidget.cpp \
| 29 -----<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" /> lib/zoomwidget.h \
| 6 -<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;" /> part/gvpart.rc | 1 \
-<br style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;" /> 15 files changed, 289 insertions(+), 128 \
deletions(-)<br style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;" /> create mode 100644 \
lib/zoommode.h</p></pre> </blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Thanks a lot! I just merged it in master \
(it's too late for 4.14 unfortunately).</p></pre> <br />
<p>- Aurélien</p>
<br />
<p>On août 7th, 2014, 6:27 après-midi CEST, John Zaitseff wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" \
style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: \
6px; -webkit-border-radius: 6px;"> <tr>
<td>
<div>Review request for Gwenview.</div>
<div>By John Zaitseff.</div>
<p style="color: grey;"><i>Updated août 7, 2014, 6:27 après-midi</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=337262">337262</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
gwenview
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description \
</h1> <table width="100%" bgcolor="#ffffff" cellspacing="0" \
cellpadding="10" style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">(Taken from bug #337262 )</p> \
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">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...</p> \
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">I have come to realise that there are THREE \
main ways people like to have the zoom and position settings applied to \
successive images:</p> <ol style="padding: 0;text-rendering: \
inherit;margin: 0 0 0 2em;line-height: inherit;white-space: normal;"> <li \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;"> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p> </li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;"> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p> </li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;"> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p> </li>
</ol>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">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).</p> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">I am \
submitting a patch that does the following:</p> <ol style="padding: \
0;text-rendering: inherit;margin: 0 0 0 2em;line-height: \
inherit;white-space: normal;"> <li style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: normal;"> <p \
style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">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).</p> </li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;"> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">Remove the \
now-obsolete ShowLockZoomButton and LockZoom config options.</p> </li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;"> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">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 :-)</p> </li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;"> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">Set the zoom \
and position settings for each image depending on the ZoomMode \
selected.</p> </li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;"> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">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!).</p> </li>
</ol></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing \
</h1> <table width="100%" bgcolor="#ffffff" cellspacing="0" \
cellpadding="10" style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">Have compiled Gwenview GIT \
head with this patch on my system; tested successfully with all \
variants.</p></pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> \
</h1> <ul style="margin-left: 3em; padding-left: 0;">
<li>app/configdialog.cpp <span style="color: grey">(a582695)</span></li>
<li>app/imageviewconfigpage.ui <span style="color: \
grey">(f5e1e5a)</span></li>
<li>app/kipiinterface.h <span style="color: grey">(45fb03b)</span></li>
<li>app/viewmainpage.cpp <span style="color: grey">(7caf099)</span></li>
<li>cmake/FindLCMS2.cmake <span style="color: grey">(cc17e9f)</span></li>
<li>devdoc/CONTRIBUTING.md <span style="color: grey">(ffdb7f7)</span></li>
<li>doc/index.docbook <span style="color: grey">(d3266b8)</span></li>
<li>lib/archiveutils.h <span style="color: grey">(db79771)</span></li>
<li>lib/cms/iccjpeg.c <span style="color: grey">(1f6c4b1)</span></li>
<li>lib/documentview/documentview.cpp <span style="color: \
grey">(1ab8bbb)</span></li>
<li>lib/gwenviewconfig.kcfg <span style="color: \
grey">(92d39f7)</span></li>
<li>lib/zoommode.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>lib/zoomwidget.h <span style="color: grey">(b181f5a)</span></li>
<li>lib/zoomwidget.cpp <span style="color: grey">(da66a56)</span></li>
<li>part/gvpart.rc <span style="color: grey">(35703a7)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/119549/diff/" \
style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
--===============5118399826738306668==--
_______________________________________________
Gwenview-devel mailing list
Gwenview-devel@kde.org
https://mail.kde.org/mailman/listinfo/gwenview-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic