Introduction and comments about bug:137320

  hi all!

I have used digikam for a long time and I decided to start contributing to the
project. The first thing I would like to do is to get rid of the stoping
thumbnail bug:
http://bugs.kde.org/show_bug.cgi?id=137320

Do any of the core developers believe this is too dificult to begin or are
working on it?

But I am having a dificult time trying to understand the scheme of the
program. Which modules are involved in the process?
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

2008/3/5, Pau Rul·lan Ferragut <[hidden email]>:

  hi all!

hi

I have used digikam for a long time and I decided to start contributing to the
project.

Thanks to contribute...

Please read this page before (just to be sure) :
<a href="http://www.digikam.org/?q=contrib" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)">http://www.digikam.org/?q=contrib

...especially "Submitting patches" section.
 

The first thing I would like to do is to get rid of the stoping
thumbnail bug:
<a href="http://bugs.kde.org/show_bug.cgi?id=137320" target="_blank" onclick="return top.js.OpenExtLink(window,event,this)">http://bugs.kde.org/show_bug.cgi?id=137320

Do any of the core developers believe this is too dificult to begin or are
working on it?

Me and Marcel we work on KDE4 port.

I know than Arnd said to me by IRC than he will be interested to fix this problem too.
 

But I am having a dificult time trying to understand the scheme of the
program. Which modules are involved in the process?

It's not to difficult, but code to touch is indeep in core implementation from digikam/digikam subfolder. Look here :

- a class named PixmapManager handle thumbs image for each icon item.
- a class named AlbumIconView manage view of icon from main interface.

in AlbumIconView, we have a method named nextItemToThumbnail() which is called by PixmapManager::slotComplete() when a thumb have been found in cache (or regenerate in background if necessary by a kio-slave)

When you select a new album or if you scrool icon view, PixmapManager will call AlbumIconView::nextItemToThumbnail() to get the next icon instance to thumbnailize.

Now if you look how AlbumIconView::nextItemToThumbnail() method work, you will see than the first and the last _visible_ items are used to limit regeneration of thumbs...

If you want to change the behaviours of thumbnails genration done in background the code from AlbumIconView::nextItemToThumbnail() need to be fixed to handle the first and the last iconview item item from current album, not the visible ones.

But take a care : regenerate all icon thumbs in background take a while, especially with slow computer. Of course a kioslave is a separate process which run outside of digiKam but we don't have any control to set the priority (as a separate thread)

So, for huge album and slow computer (using a single processor especially) thumb generation will slow down digiKam. This is why the current thumbs generation behaviours have been implemented to only generate current visible items.

This is my proposal :

- added a new option in "Album Settings" page from Setup dialog, into "Interface Options". A checkbox is enough here. The new option must be disable by default.
- patch AlbumSettings class to manage this new option.
- patch AlbumIconView::nextItemToThumbnail() to handle this option (thrue an AlbumSettings method) to change behaviours accordingly (only visible items or all items)

Note : use KDE3 branch from svn to patch source code. Code will be backported later by me later to KDE4. Please use the bugzilla entry to post patch. Message will be automatically dispatched to this mailing list...

Best

Gilles Caulier

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Arnd Baecker]

Hi Pau,

On Wed, 5 Mar 2008, Gilles Caulier wrote:

> 2008/3/5, Pau Rul·lan Ferragut <[hidden email]>:

[...]
>> I have used digikam for a long time and I decided to start contributing
>> to the project.
>
> Thanks to contribute...

Yes, contributions are indeed very welcome!!

> Please read this page before (just to be sure) :
> http://www.digikam.org/?q=contrib
>
> ...especially "Submitting patches" section.
>
>
> The first thing I would like to do is to get rid of the stoping
>> thumbnail bug:
>> http://bugs.kde.org/show_bug.cgi?id=137320
>>
>> Do any of the core developers believe this is too dificult to begin or are
>> working on it?
>
> Me and Marcel we work on KDE4 port.
>
> I know than Arnd said to me by IRC than he will be interested to fix this
> problem too.

Yes, it is an annoying one ;-), but so far I did not have any time
to look into this, so, Pau, your contribution would be very much
appreciated!

> But I am having a dificult time trying to understand the scheme of the
>> program. Which modules are involved in the process?
>
> It's not to difficult, but code to touch is indeep in core implementation
> from digikam/digikam subfolder. Look here :
>
> - a class named PixmapManager handle thumbs image for each icon item.
> - a class named AlbumIconView manage view of icon from main interface.
>
> in AlbumIconView, we have a method named nextItemToThumbnail() which is
> called by PixmapManager::slotComplete() when a thumb have been found in
> cache (or regenerate in background if necessary by a kio-slave)
>
> When you select a new album or if you scrool icon view, PixmapManager will
> call AlbumIconView::nextItemToThumbnail() to get the next icon instance to
> thumbnailize.
>
> Now if you look how AlbumIconView::nextItemToThumbnail() method work, you
> will see than the first and the last _visible_ items are used to limit
> regeneration of thumbs...
>
> If you want to change the behaviours of thumbnails genration done in
> background the code from AlbumIconView::nextItemToThumbnail() need to be
> fixed to handle the first and the last iconview item item from current
> album, not the visible ones.
>
> But take a care : regenerate all icon thumbs in background take a while,
> especially with slow computer. Of course a kioslave is a separate process
> which run outside of digiKam but we don't have any control to set the
> priority (as a separate thread)
>
> So, for huge album and slow computer (using a single processor especially)
> thumb generation will slow down digiKam. This is why the current thumbs
> generation behaviours have been implemented to only generate current visible
> items.

Just a comment on the possible performance:
What happens, if one displays a huge album (e.g. all images from 2007),
where the thumbnails have not been generated?
Possible problems:
- moving to a different place (with the slider) in the same view:
   would those images thumbnails only be displayed after all other ones
   have been generated?
   (Or are thumbnails for images currently in view generated first?)
- if one then moves to a different view (e.g. 2006),
   will the 2007 thumbnails be still generated in the background?
   (I.e., is there any way to stop the thumbnail generation when
    changing views, so that full priority goes to the currently wanted
    thumbnails?)

> This is my proposal :
>
> - added a new option in "Album Settings" page from Setup dialog, into
> "Interface Options". A checkbox is enough here. The new option must be
> disable by default.

Yes, something like: "[ ] Load all thumbnails of the current view"

> - patch AlbumSettings class to manage this new option.
> - patch AlbumIconView::nextItemToThumbnail() to handle this option (thrue an
> AlbumSettings method) to change behaviours accordingly (only visible items
> or all items)

Of course, for a start (to try out that feature), one
could just change AlbumIconView::nextItemToThumbnail()
and see how it performs. Maybe some (user-definable) limitation
(like 20 images etc.) is needed.

Best, Arnd
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

2008/3/5, Gilles Caulier <[hidden email]>:
> 2008/3/5, Pau Rul·lan Ferragut <[hidden email]>:

> > The first thing I would like to do is to get rid of the
> >stoping thumbnail bug:

> - a class named PixmapManager handle thumbs image for each icon item.
> - a class named AlbumIconView manage view of icon from main interface.

I am stuck on AlbumIconView::nextItemToThumbnail(). I would like to
use Iconview::firstItem() to get the first item of the album but I
cannot find any
object or way to do it. Any ideas?
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Arnd Baecker]

On Wed, 5 Mar 2008, Pau Rul·lan Ferragut wrote:

> 2008/3/5, Gilles Caulier <[hidden email]>:
>> 2008/3/5, Pau Rul·lan Ferragut <[hidden email]>:
>
>>> The first thing I would like to do is to get rid of the
>>> stoping thumbnail bug:
>
>> - a class named PixmapManager handle thumbs image for each icon item.
>> - a class named AlbumIconView manage view of icon from main interface.
>
> I am stuck on AlbumIconView::nextItemToThumbnail(). I would like to
> use Iconview::firstItem() to get the first item of the album but I
> cannot find any
> object or way to do it. Any ideas?

Looking at iconview.cpp it *seems* (Gilles, correct me!),
that two new public methods
    firstItem()  and lastItem()
have to be introduced, which just return
   d->firstContainer and    d->lastContainer ,
respectively.

Best, Arnd
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

On Wed, Mar 05, 2008 at 10:06:32PM +0100, Arnd Baecker wrote:
> On Wed, 5 Mar 2008, Pau Rul·lan Ferragut wrote:
>> 2008/3/5, Gilles Caulier <[hidden email]>:
>>> 2008/3/5, Pau Rul·lan Ferragut <[hidden email]>:

>>>> The first thing I would like to do is to get rid of the stoping thumbnail
>>>> bug:

>>> - a class named PixmapManager handle thumbs image for each icon item.
>>> - a class named AlbumIconView manage view of icon from main interface.

>> I am stuck on AlbumIconView::nextItemToThumbnail(). I would like to use
>> Iconview::firstItem() to get the first item of the album but I cannot find
>> any object or way to do it. Any ideas?

> Looking at iconview.cpp it *seems* (Gilles, correct me!),
> that two new public methods
>    firstItem()  and lastItem()
> have to be introduced, which just return
>   d->firstContainer and    d->lastContainer ,
> respectively.

These methods exists. My problem is that my experience with C++ is like zero
and I do not find the way to those methods without a IconView object. Another
way would be to get this IconView but I do not find from where.

I have this from the actual implementation:
        AlbumIconItem* firstItem = static_cast<AlbumIconItem*>(fItem);
        AlbumIconItem* lastItem  = static_cast<AlbumIconItem*>(lItem);
        AlbumIconItem* item      = firstItem;


and want to add something like these:
       AlbumIconItem* firstItem = firstItem();
       AlbumIconItem* lastItem  = lastItem();
       AlbumIconItem* item      = firstItem;

--
Do not be too quick to decide what is impossible.

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Arnd Baecker]

On Thu, 6 Mar 2008, Pau Ruŀlan Ferragut wrote:

> On Wed, Mar 05, 2008 at 10:06:32PM +0100, Arnd Baecker wrote:
>> On Wed, 5 Mar 2008, Pau Rul·lan Ferragut wrote:
>>> 2008/3/5, Gilles Caulier <[hidden email]>:
>>>> 2008/3/5, Pau Rul·lan Ferragut <[hidden email]>:
>
>>>>> The first thing I would like to do is to get rid of the stoping thumbnail
>>>>> bug:
>
>>>> - a class named PixmapManager handle thumbs image for each icon item.
>>>> - a class named AlbumIconView manage view of icon from main interface.
>
>>> I am stuck on AlbumIconView::nextItemToThumbnail(). I would like to use
>>> Iconview::firstItem() to get the first item of the album but I cannot find
>>> any object or way to do it. Any ideas?
>
>> Looking at iconview.cpp it *seems* (Gilles, correct me!),
>> that two new public methods
>>    firstItem()  and lastItem()
>> have to be introduced, which just return
>>   d->firstContainer and    d->lastContainer ,
>> respectively.
>
> These methods exists.

Hmm, you are right, they exist.
But do they return  d->firstContainer and    d->lastContainer ?
They  return d->firstGroup->firstItem() and  d->lastGroup->lastItem() .

Well, I better shut up, because I don't know
the meaning of Groups and Containers
(and I could not find any explaining comments in the code ...)

Gilles, could you maybe explain?

> My problem is that my experience with C++ is like zero
> and I do not find the way to those methods without a IconView object. Another
> way would be to get this IconView but I do not find from where.
>
> I have this from the actual implementation:
>        AlbumIconItem* firstItem = static_cast<AlbumIconItem*>(fItem);
>        AlbumIconItem* lastItem  = static_cast<AlbumIconItem*>(lItem);
>        AlbumIconItem* item      = firstItem;
>
>
> and want to add something like these:
>       AlbumIconItem* firstItem = firstItem();
>       AlbumIconItem* lastItem  = lastItem();
>       AlbumIconItem* item      = firstItem;

In principle
     IconItem *fItem = firstItem();
     IconItem *lItem = lastItem();
would technically work, but I am not sure, whether that would
do what we want ...

Best, Arnd
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-4]

2008/3/6, Arnd Baecker <[hidden email]>:

On Thu, 6 Mar 2008, Pau Ruŀlan Ferragut wrote:

> On Wed, Mar 05, 2008 at 10:06:32PM +0100, Arnd Baecker wrote:
>> On Wed, 5 Mar 2008, Pau Rul·lan Ferragut wrote:
>>> 2008/3/5, Gilles Caulier <[hidden email]>:
>>>> 2008/3/5, Pau Rul·lan Ferragut <[hidden email]>:
>
>>>>> The first thing I would like to do is to get rid of the stoping thumbnail
>>>>> bug:
>
>>>> - a class named PixmapManager handle thumbs image for each icon item.
>>>> - a class named AlbumIconView manage view of icon from main interface.
>
>>> I am stuck on AlbumIconView::nextItemToThumbnail(). I would like to use
>>> Iconview::firstItem() to get the first item of the album but I cannot find
>>> any object or way to do it. Any ideas?
>
>> Looking at iconview.cpp it *seems* (Gilles, correct me!),
>> that two new public methods
>>    firstItem()  and lastItem()
>> have to be introduced, which just return
>>   d->firstContainer and    d->lastContainer ,
>> respectively.
>
> These methods exists.


Hmm, you are right, they exist.
But do they return  d->firstContainer and    d->lastContainer ?
They  return d->firstGroup->firstItem() and  d->lastGroup->lastItem() .

Well, I better shut up, because I don't know
the meaning of Groups and Containers
(and I could not find any explaining comments in the code ...)

Gilles, could you maybe explain?

Groups of items are used when multiple album are displayed in the same time in icon view for ex. when you use searches tools or recursive album mode.

firstItem() return the first item from the first group available on icon view, and lastItem() the last item from the last group. If there is just one group you have the first item and the last item as well.


 

> My problem is that my experience with C++ is like zero
> and I do not find the way to those methods without a IconView object. Another
> way would be to get this IconView but I do not find from where.
>
> I have this from the actual implementation:
>        AlbumIconItem* firstItem = static_cast<AlbumIconItem*>(fItem);
>        AlbumIconItem* lastItem  = static_cast<AlbumIconItem*>(lItem);
>        AlbumIconItem* item      = firstItem;
>
>
> and want to add something like these:
>       AlbumIconItem* firstItem = firstItem();
>       AlbumIconItem* lastItem  = lastItem();
>       AlbumIconItem* item      = firstItem;


In principle
     IconItem *fItem = firstItem();
     IconItem *lItem = lastItem();
would technically work, but I am not sure, whether that would
do what we want ...

Yes this is right, but to have tested this patch on my computer, this is not enough to solve the problem. thumbs rendering is broken.

Gilles

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Arnd Baecker]

On Thu, 6 Mar 2008, Gilles Caulier wrote:

> 2008/3/6, Arnd Baecker <[hidden email]>:

[...]

>> In principle
>>      IconItem *fItem = firstItem();
>>      IconItem *lItem = lastItem();
>> would technically work, but I am not sure, whether that would
>> do what we want ...
>
>
> Yes this is right, but to have tested this patch on my computer, this is not
> enough to solve the problem. thumbs rendering is broken.

Yes, I can confirm this
(Essentially, in an album with many entries, just move
to a place in the middle - only the thumbnail of the first visible
item is shown).

Things are better if one uses
         IconItem *fItem = findFirstVisibleItem(r);;
         IconItem *lItem = lastItem();
but somehow it does not loop over all images, i.e.,
     while (item)
     {
         if (item->isDirty())
             return item;
         if (item == lastItem)
             break;
         item = (AlbumIconItem*)item->nextItem();
         DWarning() << "nextItemToThumbnail: " << item->imageInfo()->name() << endl;
     }

only shows a few "nextItemToThumbnail: ..." messages on the konsole,
but not more.

So is there something in PixmapManager which does not
trigger it correctly ?
(I don't really understand the logic in
PixmapManager::slotCompleted()
where d->thumbJob->kill() is called when a thumbJob is not Null  -
again a few more comments in the code would really help ;-)

Best, Arnd

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel