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

[Michael Reiher]

On Sunday 18 October 2009 00:32:58 Seb Ruiz wrote:
> ---------- 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
> 

Could someone be so nice to commit this patch for me? Provided it's correct of 
course... 

Greets 

Michael