Bug 268317

In fact Kipiplugins::PreviewImage

https://projects.kde.org/projects/extragear/graphics/kipi-plugins/repository/revisions/master/entry/common/libkipiplugins/widgets/previewimage.h

...mustt be used instead ImageDisplay in plugin :

https://projects.kde.org/projects/extragear/graphics/kipi-plugins/repository/revisions/master/entry/timeadjust/clockphotodialog.cpp#L198

It's really an easy job...

Gilles Caulier

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?

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

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?

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

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.

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 :)

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!

Smit,

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

Best

Gilles Caulier

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

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.
>

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

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

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