Bug 268317 - "Determine from clock photo" dialog: Allow zooming with +/- or ctrl+mousewheel in the preview
Summary: "Determine from clock photo" dialog: Allow zooming with +/- or ctrl+mousewhee...
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: Plugin-Generic-TimeAdjust (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR wishlist
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords: junior-jobs
Depends on:
Blocks:
 
Reported: 2011-03-12 21:41 UTC by Michael G. Hansen
Modified: 2018-09-22 08:33 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 2.6.0


Attachments
Proposed patch. (12.73 KB, patch)
2012-03-20 19:49 UTC, Andrei Datcu
Details
Final patch - fixed resize problems (13.26 KB, patch)
2012-03-24 21:30 UTC, Andrei Datcu
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Michael G. Hansen 2011-03-12 21:41:34 UTC
Version:           2.0.0 (using Devel) 
OS:                Linux

The zoom level in the preview in the "Determine from clock photo" dialog can only be changed using the buttons/slider below the preview image. It would be nice if one could also change the zoom level using ctrl+scrollwheel or the +/- keys.

Reproducible: Always
Comment 2 Andrei Datcu 2012-03-17 09:39:12 UTC
Hello, I am working on this bug and I have a few questions:
1. Where can I see a usage of Kipiplugins::kppreviwimage?
2. From what I've seen, when zooming in/out KPPreviewimage::slotZoomIn() uses a constant 1.5 zoom factor. Here the zoom factor is determined by a QSlider. Would it be ok to actually modify the ImageDisplay class rather than using kppreviewimage?
Comment 3 caulier.gilles 2012-03-19 11:51:58 UTC
1/ Very simple. Grep PreviewManager (not PreviewImage) in kipi-plugins source code. It's used in Panorama, ExpoBlending and RawConverter (single mode) tool.

Ex : https://projects.kde.org/projects/extragear/graphics/kipi-plugins/repository/revisions/master/entry/rawconverter/dialogs/singledialog.cpp#L137

2/ To be more clear, ImageDisplay class must be fully dropped from source code. Simply use PreviewManager class as well instead, and adapt dialog source code to use it properlly. We don't care about slider for zoom control, PreviewManager has already all in place.

I waiting your patch against git/master to review. Thanks in advance to contribute

Best

Gilles Caulier
Comment 4 Veaceslav Munteanu 2012-03-19 13:13:55 UTC
Well, we looked into PreviewManager and it contains a lot of this that we don't need(labels etc..)

Can we define a new class that will inherit Kppreviewimage and will contain an extra method for smooth zoom?
Comment 5 caulier.gilles 2012-03-19 13:38:05 UTC
I recommend to patch current PreviewManager to add new method to setup which options can be show as customization. We already do it into KPImageList class for ex.

Gilles Caulier
Comment 6 Andrei Datcu 2012-03-20 19:49:35 UTC
Created attachment 69771 [details]
Proposed patch.

I used KPPreviewManager class instead do display the image. I did not patch the class itself, as you proposed, because I think it looks ok. This is my first patch I've ever submitted so please forgive me if I made obvious mistakes.
Comment 7 Veaceslav Munteanu 2012-03-20 22:54:59 UTC
Andrei, I've tested your patch :)

It look really good with with new previewimage :)

But first, you must resize de window so all elements could be visible.

Also, the patch you sent was hard to apply git apply said it was corrupted and patch < yourpatch.patch  asked me to manully show the paths.

Please don't merge/split patches manually :)
Comment 8 Andrei Datcu 2012-03-24 21:30:09 UTC
Created attachment 69866 [details]
Final patch - fixed resize problems

Hello! I've managed to fix the resize problem. I think everything is ok now. Please review my patch! Thanks!
Comment 9 caulier.gilles 2012-03-25 16:23:28 UTC
Smit,

There is another patch to review and apply to git/master. Can you do it ?

Best

Gilles Caulier
Comment 10 caulier.gilles 2012-03-25 16:25:35 UTC
Smit,

I take a look to patch, and it sound fine for me (not compiled and tested)

Can you apply it and check if all work fine. If yes, feel free to apply it to git/master

Thanks in advance

Gilles Caulier
Comment 11 Smit Mehta 2012-03-25 16:25:55 UTC
Hi Gilles

Yes, I shall do it in a couple of hours.

On Sun, Mar 25, 2012 at 9:53 PM, Gilles Caulier <caulier.gilles@gmail.com>wrote:

> https://bugs.kde.org/show_bug.cgi?id=268317
>
> Gilles Caulier <caulier.gilles@gmail.com> changed:
>
>           What    |Removed                     |Added
>
> ----------------------------------------------------------------------------
>                 CC|                            |smit.meh@gmail.com
>
> --- Comment #9 from Gilles Caulier <caulier.gilles@gmail.com> ---
> Smit,
>
> There is another patch to review and apply to git/master. Can you do it ?
>
> Best
>
> Gilles Caulier
>
> --
> You are receiving this mail because:
> You are on the CC list for the bug.
>
Comment 12 caulier.gilles 2012-03-25 16:36:01 UTC
Andrei,

In kipiplugins BatchProcessImages tool, there is another class to port to Kipiplugins::kppreviewimage :

https://projects.kde.org/projects/extragear/graphics/kipi-plugins/repository/revisions/master/entry/batchprocessimages/common/imagepreview.h

ImagePreview is used to render in a widget an ImageMagick effect before to apply it to a queue of file to batch. ImageMagick is started in a QProcess to render a preview temp image. This one is displayed in ImagePreview as well. Using KPPreviewImage can be better and will factorize source code...

Relevant bugzilla entries are these :

https://bugs.kde.org/show_bug.cgi?id=220221

Post your patch to review in this entry. 

Best

Gilles Caulier
Comment 13 Smit Mehta 2012-03-25 17:48:17 UTC
Hi Andrei

I was trying to apply your patch, but it always gave some error or the other. Have you merged / split your patches manually? Or in any case, kindly submit your patch again (dont do any changes manually in your patch).

Smit
Comment 14 caulier.gilles 2012-03-25 21:02:55 UTC
Git commit 3d2ba880ed4cb626f0c92097cd10b8937d31af63 by smit mehta.
Committed on 25/03/2012 at 20:25.
Pushed by smitmehta into branch 'master'.

For solving bug no. 268317 (Patch proposed by Andrei Datcu, tested my smitmehta)
BUGS 268317

M  +15   -197  timeadjust/clockphotodialog.cpp
M  +0    -48   timeadjust/clockphotodialog.h

http://commits.kde.org/kipi-plugins/3d2ba880ed4cb626f0c92097cd10b8937d31af63