[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: KFileItem API problem (Was: Re: [PATCH] Highlighting thumbnails in konqueror)
From:       Martijn Klingens <mklingens () yahoo ! com>
Date:       2001-07-14 12:52:11
[Download RAW message or body]

On Saturday 14 July 2001 00:07, David Faure wrote:
> On Saturday 14 July 2001 00:39, Martijn Klingens wrote:
> > On Friday 13 July 2001 22:31, David Faure wrote:
> > > Well, I see nothing wrong with KFileItem::pixmap(). It knows nothing
> > > about effects, it just passes the state around after adding the
> > > overlays etc.
> >
> > Yes and no. KFileItem::pixmap() calls the global function DesktopIcon()
> > which internally _always_ applies the IconEffect for desktop icons
> > (KIcon::Desktop). In this case it should be KIcon::NoGroup.
>
> No, no, that's only the fallback case (no mimetype known).
> In the usual case, KFileItem::pixmap() calls KMimeType::pixmap(),
> which in turn calls KIconLoader::loadIcon, with all the necessary
> parameters. In short, it makes the pixmap completely ready to be used,
> which made sense until those discussions about optimizing effects :)

from KFileItem::pixmap():
  QPixmap p = mime->pixmap( m_url, KIcon::Desktop, _size, _state );

KIcon::Desktop is the bad boy here, because it indicates we want to apply the 
effect for desktop icons here. The effect code is applied deep down the icon 
API in KIconLoader::loadIcon(), which is IMO not a good idea in this case. It 
makes it impossible to retrieve the raw, unprocessed icon in code that wants 
it.

_BUT_: I just thought about it again, and I could as well use a QIconSet 
instead of a QPixmap. I can probably even make the iconset code 'lazy' in 
that it only loads the icon when first accessed. That might be a better 
solution anyway.

I am still not entirely convinced that the current API is perfect, but I 
think that I just discovered a very nice and elegant work-around, so at least 
for now it doesn't really matter anymore.

> > Actually, that even applies to more parts of KDE. When I was testing
> > these settings I encountered a few related bugs: set the normal
> > (inactive) effect for Desktop icons to something, preferably something
> > that is easily recognizable like colorize. After applying at least the
> > alt-tab-window, minicli and the KJanusWidget for configuration dialogs
> > all take this new settings whereas they shouldn't.
>
> Well, that's arguable I guess. At least for those who designed the
> iconloading system (Antonio, Torsten etc.), this must have been the wanted
> behaviour. Or maybe they didn't think about that ;-)

It probably is designed like this, but some effects are only nice on the 
Konqueror background and not on the window background in minicli.

Also, I'd suggest for KDE 3.0 to change the groups into Program (kicker 
icons, but also minicli for example), DirEntry (Files and dirs in Konq and 
the file dialogs), Icon (toolbar icons, KJanusWidget icons, etc.), NoGroup 
and User. Applying a file item's effect to minicli's icon is kinda strange 
IMO.

> > That is another option, but I'm not sure if that doesn't break anything
> > either.
>
> Well, it'd be a new value for the enum, as you suggested.
> So it wouldn't break anything, since no existing code uses this
> not-yet-existant value (maybe I misnamed it, if something's already called
> NoEffect - I meant a new value for state).

I'll try that if the QIconSet approach doesn't work. It would be by far the 
fastest if CPU is more the bottleneck than memory, because it only has to 
apply any given effect once for any given icon. Since QPixmap uses lazy 
copying, the overall memory usage is probably hardly increased anyway, if at 
all.

(Yeah, I'm beginning to like this new idea... :-)

Martijn

[prev in list] [next in list] [prev in thread] [next in thread]