Bug 157314 - Zoom-slider has no steps
Summary: Zoom-slider has no steps
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: ImageEditor-Canvas (show other bugs)
Version: 0.9.3
Platform: unspecified Linux
: NOR wishlist
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-02-07 04:31 UTC by Fri13
Modified: 2017-08-07 08:40 UTC (History)
0 users

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


Attachments
make zoom values snap to specific values, v0 (5.84 KB, patch)
2008-04-18 18:09 UTC, Arnd Baecker
Details
make zoom values snap to specific values, v1 (5.48 KB, patch)
2008-04-19 14:22 UTC, Arnd Baecker
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Fri13 2008-02-07 04:31:36 UTC
Version:           0.9.3 (using KDE 3.5.8)
OS:                Linux

Currently digikam zoom-slider on different windows does not have any steps, it is dificult to get 100% zoom ration from a "fit-to-window" picture. If user clicks "zoom in" button, zoom ration goes over 100%.

Suggestion that zoom-slider would get few stops like every 2% or 5% so it would be easy to stop to 100% zoom, mayby add just one stop to that position. Then it would be like a fit-100%-fullzoom and user could still zoom between all these three positions easily.

It seems when user scrolls mouse wheel over zoom-slider, he gets different zoom steps for everytime when going back/forward
Comment 1 Arnd Baecker 2008-04-18 18:09:32 UTC
Created attachment 24391 [details]
make zoom values snap to specific values, v0

The patch works for the + and - buttons at the slider,
CTRL+mousewheel and changes of the slider itself.

For dealing with changes of the slider itself,
modifying PreviewWidget::setZoomFactor a la

  //If we are near 100% zoom, then we force 100% zoom
      if (fabs(zoom-1.0)<0.05) 
    {
	zoom = 1.0;
    }
(as done in the previous patch on the ML) is not a good idea.
Reason: this routine is used at various places throughout the code.
In particular, when a fit-to-window would be 96% then this should not
be set to 100% automatically, just because of 100% snapping .... ;-)

Also, if the user changed the ZoomText
(e.g.: EditorWindow::slotZoomTextChanged)
this must not be changed afterwards ;-)

To deal with the slider, the places to be adapted are:

#void DigikamView::setThumbSize(int size)
void ImagePannelWidget::slotZoomSliderChanged(int size)
void LightTableView::slotLeftZoomSliderChanged(int size)
void LightTableView::slotRightZoomSliderChanged(int size)

All these call setZoomFactor(z) in the end.
Therefore, instead a setZoomFactorSnapped(z) is
introduced, which does the proper handling of "snapping"
to 50%, 100% and fit-to-window.

Of course, this needs testing - feedback welcome!
(Once everything is considered fine, I will clean up the few DWarning()
statements left in for debugging)

Best, Arnd
Comment 2 Mikolaj Machowski 2008-04-19 00:03:46 UTC
Thanks. Now buttons works :)
Comment 3 Mikolaj Machowski 2008-04-19 10:07:35 UTC
Dnia Saturday 19 of April 2008, napisa
Comment 4 Arnd Baecker 2008-04-19 14:14:56 UTC
1.) > Doesn't work with Mouse-wheel hovering over slider.

Both when moving the slider and when using the mouse wheel, 
void DigikamView::slotZoomFactorChanged(double zoom)
is called, which in turn calls void DigikamView::setThumbSize(int size).
Thus the problem is the following: when moving the slider directly,
usually small steps are are used such that a maximum 5% 
difference condition to the snap values (50%, 100%, fit) is fulfilled
to make it snap. When using the mouse-wheel, larger steps are
used, so that it may happen, that no snapping occurs.
This maybe considered as a bug, or as a feature ;-).
However, I currently don't see any way to overcome this ...
So may I declare this as a feature?

About the other two points: could you please file separate wishes?



Comment 5 Arnd Baecker 2008-04-19 14:22:33 UTC
Created attachment 24404 [details]
make zoom values snap to specific values, v1

cleaned up patch.

Gilles, could you have a brief look at the code?
Comment 6 caulier.gilles 2008-04-23 08:46:47 UTC
Arnd,

Patch is fine for me. Just remove the #include <stdlib.h> from top of imagepreview.cpp...

C Ansi header must be limited if possible to improve portability.

Another tip : if you really need to use C Ansi header, i recommend to always use C++ header instead, we it's possible (it not always the case). For ex.:

#include <math.h> ==> #include <cmath>

C++ header don't use .h extension and generally add 'c' char on front of file name.

Note than not all C Ansi header file have been converter to C++ like (for Ex. #include <stdint.h>). In this case the C Ansi header need to be wrapped around "extern "C" {...}"...

Gilles
Comment 7 Arnd Baecker 2008-04-23 20:22:16 UTC
SVN commit 800239 by abaecker:

Make the zoom values of the slider snap to specific values 
(fit-to-window, 50% view, and 100% view).

CCBUGS: 157314
TODO:KDE4PORT



 M  +2 -1      NEWS  
 M  +5 -0      digikam/albumwidgetstack.cpp  
 M  +1 -0      digikam/albumwidgetstack.h  
 M  +1 -1      digikam/digikamview.cpp  
 M  +62 -2     libs/widgets/common/previewwidget.cpp  
 M  +3 -0      libs/widgets/common/previewwidget.h  
 M  +1 -1      libs/widgets/imageplugins/imagepannelwidget.cpp  
 M  +2 -2      utilities/lighttable/lighttableview.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=800239
Comment 8 Arnd Baecker 2008-04-23 20:33:05 UTC
Gilles, thanks for the review -
the #include <stdlib.h> was indeed a left-over from testing ...

I think that the main point of the wish is fulfilled.

The same will have to be done for the image editor, see
http://bugs.kde.org/show_bug.cgi?id=161085
Comment 9 caulier.gilles 2008-04-24 08:43:21 UTC
Arnd,

I will take a look into 161085.

Note look here about C Ansi header and C++: 

http://www.cs.sunysb.edu/facilities/FAQ/gccfaq/standard_header_files.html

Gilles
Comment 10 caulier.gilles 2008-04-24 09:20:50 UTC
Arnd,

Qt3 => Qt4 port feedback : 

QValueList<double> ==> QList<double>
qHeapSort(QValueList<double>) ==> qSort(QList<double>)

Gilles
Comment 11 caulier.gilles 2008-04-24 09:22:16 UTC
SVN commit 800397 by cgilles:

backport commit #800239 from KDE3 branch
BUG: 157314


 M  +5 -0      digikam/albumwidgetstack.cpp  
 M  +1 -0      digikam/albumwidgetstack.h  
 M  +7 -7      digikam/digikamview.cpp  
 M  +63 -3     libs/widgets/common/previewwidget.cpp  
 M  +4 -1      libs/widgets/common/previewwidget.h  
 M  +1 -1      libs/widgets/imageplugins/imagepannelwidget.cpp  
 M  +3 -3      utilities/lighttable/lighttableview.cpp  
 M  +1 -1      utilities/lighttable/lighttableview.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=800397