Bug 166440 - Removing images from Light Table is not working properly
Summary: Removing images from Light Table is not working properly
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: LightTable-Workflow (show other bugs)
Version: 0.10.0
Platform: unspecified Linux
: NOR normal
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-07-13 13:31 UTC by Andi Clemens
Modified: 2022-02-01 09:18 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In: 0.10.0


Attachments
before (33.91 KB, image/png)
2008-07-23 23:33 UTC, Andi Clemens
Details
after (35.10 KB, image/png)
2008-07-23 23:34 UTC, Andi Clemens
Details
valgrind (11.81 KB, text/plain)
2008-07-25 12:26 UTC, Andi Clemens
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andi Clemens 2008-07-13 13:31:42 UTC
Version:           0.10.0-beta2 (rev.: 831503M) (using 4.00.98 (KDE 4.0.98 (4.1 RC1)), compiled sources)
Compiler:          gcc
OS:                Linux (i686) release 2.6.25-ARCH

When removing images from the lighttable, it behaves strange. For example if I have 6 images in the lighttable and I remove one image, the total counter at the bottom of the lighttable window tells me that 2 images have been removed, but in the thumbbar only one image is actually removed.
If I continue removing images, the thumbbar is cleared after a while, although there are still images listed in there.
Here is a video showing this behavior:
http://digikam3rdparty.free.fr/misc.tarballs/temp/lighttable-0.10-removal-problem.ogv

Andi
Comment 1 caulier.gilles 2008-07-13 13:49:34 UTC
Andy,

It's reproductible into KDE3 code ?

Gilles
Comment 2 Andi Clemens 2008-07-13 13:58:26 UTC
no...
That was fixed by me weeks ago.... and it was a different issue... in KDE3 it 
only occurred when removing the last image in Light table...

see http://bugs.kde.org/show_bug.cgi?id=162814#c17

Andi

On Sunday 13 July 2008 13:49:35 Gilles Caulier wrote:
[bugs.kde.org quoted mail]
Comment 3 Gandalf Lechner 2008-07-13 20:02:10 UTC
I can confirm this bug, same here
Comment 4 Andi Clemens 2008-07-23 23:14:28 UTC
I guess I found the problem, but I can't explain it myself. I had the same issue when working on the UndoManager before in KDE3: The deconstructor of the ThumbBarItem is never called (could this be a Qt3 bug in general?).
In Qt4 (KDE4) the deconstructor is hit:

http://lxr.kde.org/source/extragear/graphics/digikam/libs/widgets/common/thumbbar.cpp#845

removeItem has been called before and is again called by the BarItem deconstructor. So the item is removed twice. This can also be seen when running in the debugger and watching the call stack.

The KDE3 code is actually the same but the deconstructor is not hit, so the second call for removeItem is never executed.

So there are two solutions: remove the described source code line in the deconstructor (this works, I have tested it) or remove the other function call in the appropriate slot.

This again gives the question why nearly all deconstructors I tested in digiKam KDE3 are never executed... like I said before I've seen the same behavior in other deconstructors like the one from UndoManager and UndoCache (maybe a thread problem?).
Comment 5 caulier.gilles 2008-07-23 23:17:35 UTC
Hum,

I have never seen that here. Destructor is always called.

Marcel, Can you reproduce this on your computer ?

Gilles
Comment 6 Andi Clemens 2008-07-23 23:24:02 UTC
Well maybe other deconstructors do work, but the one for ThumbBarItem is definitely not called... I set a breakpoint and did other debugging tasks but it is never ever executed. In KDE4 / Qt4 it is and so the image gets removed twice... which breaks the whole lighttable thumbbar after a while.
Comment 7 Andi Clemens 2008-07-23 23:33:56 UTC
Created attachment 26366 [details]
before

Here you can see the first call to removeItem right after the slot is called
(pressing CTRL+K to remove image from thumbbar)
Comment 8 Andi Clemens 2008-07-23 23:34:57 UTC
Created attachment 26367 [details]
after

And here after the constructor is called... removeItem is called twice... this
is not the case for the KDE3 version, although the code is the same.
Comment 9 Marcel Wiesweg 2008-07-24 14:44:52 UTC
I did not look into the KDE3 problem, but for KDE4 I would suggest this:

add a method takeItem to Thumbbar which gets all the code from removeItem, except for the word "delete" in line 689.

Make removeItem only call "delete item", and make the destructor call takeItem.

So you can still remove items with delete, and if removeItem is called at various places we keep it for compatibility. If it is used only in a few places, we could remove it and use delete everywhere.
Comment 10 Andi Clemens 2008-07-24 20:57:20 UTC
Marcel,

sounds fine for me...I will try this today... Or have you patched it already?

Andi
Comment 11 Andi Clemens 2008-07-25 11:05:02 UTC
SVN commit 837619 by aclemens:

Images get removed twice from the thumbbar view when removing them via keyboard shortcut or context menu. This happened due to the use of the ThumbBarView::removeItem() method in the appropriate slot as well as in the destructor of a ThumbBarItem.
To obtain backward compatibility we move the context of removeItem() into a new method takeItem() and call this new method from within the destructor of a ThumbBarItem, removeItem only calls delete now.
This way we can remove an item "the old way" but also by calling just delete.
Somehow the KDE3 branch is not affected by this because the destructor is never called at all (I don't know why).

CCBUGS:166440

 M  +23 -20    thumbbar.cpp  
 M  +3 -2      thumbbar.h  


WebSVN link: http://websvn.kde.org/?view=rev&revision=837619
Comment 12 Andi Clemens 2008-07-25 11:37:56 UTC
I moved the last commit into the KDE3 source, too. But again the destructor is not called. The only difference I found was in the old removeItem() method:

d->itemDict.remove(item->url().url());

In KDE4 we call

d->itemDict.take(item->url().url());

So I used the take method as well, but still the destructor is not called. The only thing that happens now is that the dictionary is cleared after some time, but the images stay in thumbbar and light table.

But since the removing of images from light table is working for KDE3 anyway, I will close this bug now.
Comment 13 caulier.gilles 2008-07-25 11:43:33 UTC
Andi,

But, in KDE3 we have a memory leak ? Try to use valgrind to be sure. Look in HACKING file to see how to use valgrind...

Best

Gilles Caulier
Comment 14 Andi Clemens 2008-07-25 12:26:52 UTC
Created attachment 26404 [details]
valgrind

This is what I get. Should I also use "--show-reachable=yes"?