Bug 297459 - Cancel button doesn't work when items are being processed
Summary: Cancel button doesn't work when items are being processed
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: Faces-RedEyes (show other bugs)
Version: 5.1.0
Platform: Ubuntu Linux
: NOR normal
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-04-04 10:45 UTC by Smit Mehta
Modified: 2016-08-23 10:29 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.2.0


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Smit Mehta 2012-04-04 10:45:13 UTC
The cancel button doesnt stop the thread, in both the cases, be it "Try Run" or "Correct Photos"

Reproducible : Always
Steps to reproduce :
Select >=2 images
Go to tools->redeyeremoval
Click on "Correct Photos"
Try to cancel the process in middle.

It just updates the cursor, but doesnt cancel the process itself.
Comment 1 caulier.gilles 2012-04-04 10:54:49 UTC
removeredeyeswindow.cpp::line 720. cancelClicked() signal is not connected...

Gilles Caulier
Comment 2 Smit Mehta 2012-04-04 11:38:52 UTC
Hi Gilles

I dont think thats the problem. First of all the lines 719-721 are useless as there is no "Cancel" button declared. We always deal with "Close" throughout the code. Also, once the process starts, setBusy(true) is called, which sets the slots for myCloseClicked() properly. And connection is fine as I see, because the mouse cursor changes when we click it. There should be some problem with the thread.

Smit
Comment 3 caulier.gilles 2012-04-04 11:55:41 UTC
Thread must be canceled at line 506

Look in WorkerThread::cancel() : pd->cancel is switched to true. This affect Task::run() to be returned if a NEW task is started. It do not stop any ld->runtype action in fact.

I suspect that operation will be canceled when current image processed will be done. Can you confirm ?

Gilles Caulier
Comment 4 Smit Mehta 2012-04-04 12:09:57 UTC
Hi Gilles

No, the thread doesnt get canceled after current image also. I just tried with 4 images, and canceled it after 1st image, but the thread still completes all the 4 images.

Smit
Comment 5 caulier.gilles 2012-04-04 12:35:43 UTC
Don't forget that WorkedThread use paralelization thread class based on KDE ThreadWeaver. This want mean that more than one image will be proecessed at the same time. On my i5 computer i can process 4 images at the same time with JPEGLossLess tool which is also based on ThreadWeaver.

We need to implement operation cancelization into sub image processing classes of remove red eyes...

Gilles Caulier
Comment 6 Smit Mehta 2012-04-04 12:51:28 UTC
Hi Gilles

Okay, I tested it again with 50 images. On canceling, it gets canceled after processing 5 images. How can we implement cancel operation into sub image processing classes of remove red eyes? Can you tell me to look for some other code/tool where similar thing is already implemented?

Smit
Comment 7 caulier.gilles 2012-04-05 08:50:18 UTC
Smit,

I take a look into RedEyesRemoval tool source code to see how it's implemented.

Look like the object used to process image is Locator :

https://projects.kde.org/projects/extragear/graphics/kipi-plugins/repository/revisions/master/entry/removeredeyes/plugin/workerthread.h#L117

It's called by 3 possible operations provided by tool :

https://projects.kde.org/projects/extragear/graphics/kipi-plugins/repository/revisions/master/entry/removeredeyes/plugin/workerthread.cpp#L99

https://projects.kde.org/projects/extragear/graphics/kipi-plugins/repository/revisions/master/entry/removeredeyes/plugin/workerthread.cpp#L108

https://projects.kde.org/projects/extragear/graphics/kipi-plugins/repository/revisions/master/entry/removeredeyes/plugin/workerthread.cpp#L113

To be able to stop Locator operation, cancel variable need to be shared with Locator instance. We can pass cancel memory address to Locator instance by a dedicated method.

In Locator implementation, here haarclassifierlocator class :

https://projects.kde.org/projects/extragear/graphics/kipi-plugins/repository/revisions/master/entry/removeredeyes/detection/locators/haarclassifier/haarclassifierlocator.cpp

... we need to check if cancel address content pass to true in each processing loop to cancel computations.

I use this technic into KPWriteImage class to be able to cancel writing image data  to file :

https://projects.kde.org/projects/extragear/graphics/kipi-plugins/repository/revisions/master/entry/common/libkipiplugins/tools/imageio/kpwriteimage.h#L71

Gilles Caulier
Comment 8 caulier.gilles 2015-05-19 07:33:42 UTC
Maik,

In my previous comment i explain how to fix the problem...

Gilles
Comment 9 caulier.gilles 2016-08-18 15:12:43 UTC
This problem still valid with new red-eyes correction filter implemented in digiKam core and used in BQM.

The filter implementation do not permit to cancel operation. It require only few lines of code to do in loop control. Very easy to fix now compared to obsolete kipi tool.

Gilles Caulier
Comment 10 Maik Qualmann 2016-08-20 20:25:27 UTC
Git commit 6849adf429369d97c9fc421f1d01dd9d55c70c2e by Maik Qualmann.
Committed on 20/08/2016 at 20:24.
Pushed by mqualmann into branch 'master'.

add cancel function to BQM red eye correction tool

M  +2    -1    NEWS
M  +11   -8    libs/facesengine/redeye/redeyecorrectionfilter.cpp
M  +17   -4    utilities/queuemanager/tools/enhance/redeyecorrection.cpp
M  +7    -0    utilities/queuemanager/tools/enhance/redeyecorrection.h

http://commits.kde.org/digikam/6849adf429369d97c9fc421f1d01dd9d55c70c2e
Comment 11 Maik Qualmann 2016-08-23 10:29:48 UTC
Git commit 1e4c8e99c3d88423dd491136f6edb5d74afae0f1 by Maik Qualmann.
Committed on 23/08/2016 at 10:28.
Pushed by mqualmann into branch 'master'.

fix cancel DImg filter when the filter is started directly

M  +3    -1    libs/threads/dynamicthread.cpp

http://commits.kde.org/digikam/1e4c8e99c3d88423dd491136f6edb5d74afae0f1