Fwd: [Amarok] Huge performance improvements for the Cover Manage

[Seb Ruiz]

---------- Forwarded message ----------
From: Michael Reiher <redm at gmx.de>
Date: 2009/10/17
Subject: Re: [Amarok] Huge performance improvements for the Cover Manage
To: Seb Ruiz <ruiz at kde.org>


On Friday 16 October 2009 08:25:31 you wrote:
> 2009/10/16 Mark Kretschmann <kretschmann at kde.org>:
> > -    if( !m_hasImageChecked )
> > -        m_hasImage = ! const_cast<SqlAlbum*>( this )->image( size
> > ).isNull(); +    if( !m_hasImageChecked ) {
> > +
> > +        m_hasImageChecked = true;
> > +
> > +        QString image = const_cast<SqlAlbum*>( this )->findImage( size
> > ); +
> > +        // The user has explicitly set no cover
> > +        if( image == AMAROK_UNSET_MAGIC )
> > +            m_hasImage = false;
> > +        else m_hasImage = !image.isEmpty();
> > +    }
> > +
>
> Long lived caches can be dangerous. This introduces a defect: you
> aren't clearing the value of m_hasImage and m_hasImageChecked  when
> unsetting an album cover.
>
Hi

I'm not sure I understand what you mean here... I didn't introduce those
m_hasImageChecked and m_hasImage members. I actually made hasImage() work more
or less along the lines of image() in this regard. If you are referring to
removeImage(), you might be right, it should probably reset these... (see
attached patch). That's not my fault though ;) I didn't touch this.

Regarding the patch: I removed setting the cached image. This shouldn't be
necessary, as its done when fetching the updated image anyway.

Greets Michael




-- 
Seb Ruiz

http://www.sebruiz.net/
http://amarok.kde.org/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: amarok_remove_image.diff
Type: text/x-patch
Size: 477 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/amarok-devel/attachments/20091018/554ad39d/attachment-0001.diff