Bug 178540

Summary: zoom in previewwidget has poor behaviour
Product: [Applications] digikam Reporter: brandon <brandon>
Component: Preview-ImageAssignee: Digikam Developers <digikam-bugs-null>
Status: RESOLVED INTENTIONAL    
Severity: normal CC: brandon, marcel.wiesweg
Priority: NOR    
Version: 0.10.0   
Target Milestone: ---   
Platform: Microsoft Windows   
OS: Microsoft Windows   
Latest Commit: Version Fixed In: 1.0.0
Sentry Crash Report:
Attachments: Patch to improve scroll zooming.

Description brandon 2008-12-23 04:45:19 UTC
Version:            (using KDE 4.1.3)
Compiler:          N/A N/A
OS:                MS Windows
Installed from:    Compiled From Sources

source file: graphics/digikam/libs/widgets/common/previewwidget.cpp

previewwidget is used in a few places, imagepreviewview.cpp being one.

The zoom in out functionality has a few undesired behaviours.

Some to do with the use of:

QFrame::contentsRect().width()/.height()
instead of
QScrollView::visibleHeight()/visibleWidth()

or

conflicts between centering the image in extra vertical/horz. space and centering the contents scrolling within the viewport

also accounting for the appearance/disappearance of the scroll bars changing the visible area.

(Note: I have a patch that fixes all of these things, I just have to learn how to get it approved.)
Comment 1 caulier.gilles 2008-12-23 22:14:10 UTC
brandon,

just attach the patch to this bugzilla file. That all... We will review later...

Gilles Caulier
Comment 2 brandon 2008-12-24 02:00:27 UTC
Created attachment 29594 [details]
Patch to improve scroll zooming.

Here is the code I am playing around with so far. It will be less verbose after I test it a little more.

Basically to re-state the problem:

You should be able to put the mouse over a point on an image and use the scroll wheel to zoom all the way to max zoom right to that point without having to move the mouse.

As previewwidget.cpp works now you have to keep re-centering the mouse on the point you are trying to zoom in on.

This is because there are limitations to Q3ScrollView. It can't for example scroll negatively or, scroll so far that the right edge of an image is in the center of the viewport.

To overcome this you need to either 'bump' the mouse cursor, to keep it centered on the zoomCenterPoint, or adjust the pixmap x/y co-ordinates.

Right now I am only, 'bumping' the cursor, so you can smoothly zoom right in to any point on the image even the edges without having to reposition the mouse. 

In the future it might be worth adjusting the pixmap location as well, especially for the right and bottom edges. However the problem with adjusting the pixmap location is, it is once it has been adjusted it's impossible to know when the user would want the image be re-centered.

Anyway hope somebody likes using scroll-zoom and finds this useful.

I'm new to Qt/Kde so don't be shy about pointing out things I could have done better.
Comment 3 brandon 2008-12-24 02:02:32 UTC
I forgot to mention, this patch was created from the <svn-workspace>/graphics/digikam directory.

Is this the correct location to create patches from?
Comment 4 caulier.gilles 2008-12-24 13:08:17 UTC
<svn-workspace>/graphics/digikam directory sound fine... if you use trunk not branches :

http://websvn.kde.org/trunk/extragear/graphics/digikam/

Gilles Caulier
Comment 5 Marcel Wiesweg 2008-12-29 22:46:28 UTC
Brandon, I think we are positive about applying your patch, if you have it well tested and give us a version that you call final we will commit it.

(There is a comment in your code calling for configurability of the Ctrl modifier.  I think this was chosen due to being KDE standard, and we cannot endlessly add options)
Comment 6 caulier.gilles 2009-01-01 14:30:00 UTC
Andi,

Are you tested this patch ?

Gilles
Comment 7 Andi Clemens 2009-01-01 14:37:47 UTC
Not now, I will test it soon, just fixing something with the menu entries in digiKam. After that I will do it.

Andi
Comment 8 Andi Clemens 2009-01-01 15:27:03 UTC
The patch seems to work quite well, but sometimes the point I'm focusing is not centered when zooming in, although it should be possible to center the image.
When I select a point I want to zoom to, it can happen that this point moves out of the visible area.
In general I would say the patch is fine, maybe Q3ScrollView is not capable of doing it better?

Andi
Comment 9 Marcel Wiesweg 2009-01-08 22:59:39 UTC
Well I'm afraid but I have to say that the current implementation works much better for me than with the patch applied.
With the patch there is a tendency of the cursor to move top-left. E.g. if I start in the bottom-right corner and zoom in and out ten or fifteen times, the cursor finally ends up in the top-left corner. This does not happen with the current implementation. Maybe there's a difference between Linux and Windows?

Apart from this, I get these compilation warnings:
/home/marcel/freshmeat/multimedia/kde4/src/extragear/graphics/digikam/libs/widgets/common/previewwidget.cpp: In member function ‘void Digikam::PreviewWidget::setZoomFactor(double, bool)’:                                                                                                               
/home/marcel/freshmeat/multimedia/kde4/src/extragear/graphics/digikam/libs/widgets/common/previewwidget.cpp:252: warning: unused variable ‘oldZoom’  
/home/marcel/freshmeat/multimedia/kde4/src/extragear/graphics/digikam/libs/widgets/common/previewwidget.cpp: In member function ‘virtual void Digikam::PreviewWidget::contentsWheelEvent(QWheelEvent*)’:                                                                                                  
/home/marcel/freshmeat/multimedia/kde4/src/extragear/graphics/digikam/libs/widgets/common/previewwidget.cpp:680: warning: comparison between signed and unsigned integer expressions                                                                                                                      
/home/marcel/freshmeat/multimedia/kde4/src/extragear/graphics/digikam/libs/widgets/common/previewwidget.cpp: In member function ‘void Digikam::PreviewWidget::setZoomFactor(double, bool)’:                                                                                                               
/home/marcel/freshmeat/multimedia/kde4/src/extragear/graphics/digikam/libs/widgets/common/previewwidget.cpp:253: warning: ‘cpy’ may be used uninitialized in this function                                                                                                                                
/home/marcel/freshmeat/multimedia/kde4/src/extragear/graphics/digikam/libs/widgets/common/previewwidget.cpp:253: warning: ‘cpx’ may be used uninitial
Comment 10 caulier.gilles 2009-07-22 15:15:28 UTC
Well, closed as won't fix ???

Gilles Caulier
Comment 11 Andi Clemens 2009-10-08 01:37:07 UTC
Push.... :)