Summary: | KIconTheme icon theme matching doesn't use icons that need to be scaled | ||
---|---|---|---|
Product: | [Frameworks and Libraries] kdelibs | Reporter: | Alexander Larsson <alexl> |
Component: | general | Assignee: | James Richard Tyrer <tyrerj> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | esigra, kde-artists, rdieter, tyrerj |
Priority: | NOR | ||
Version: | SVN | ||
Target Milestone: | --- | ||
Platform: | Compiled Sources | ||
OS: | Linux | ||
Latest Commit: | Version Fixed In: | ||
Attachments: |
Don't scale icons up unless you have to, but do scale icons.
Minor change to fix the problem |
Description
Alexander Larsson
2003-01-16 09:49:27 UTC
Created attachment 757 [details]
Don't scale icons up unless you have to, but do scale icons.
Subject: kdelibs/kdecore CVS commit by zrusin: Patch by Alexander Larsson, fixes 53052 "KIconTheme icon matching doesn't find icons that need to be scaled". Thanks Alex! CCMAIL: 53052-done@bugs.kde.org M +33 -9 kicontheme.cpp 1.55 --- kdelibs/kdecore/kicontheme.cpp #1.54:1.55 @@ -360,7 +360,31 @@ KIcon KIconTheme::iconPath(const QString } else { + // dw < 0 means need to scale up to get an icon of the requested size + if (dir->type() == KIcon::Fixed) + { dw = dir->size() - size; - if (dir->type() != KIcon::Threshold && - ((dw > 7) || (abs(dw) >= abs(delta)))) + } else if (dir->type() == KIcon::Scalable) + { + if (size < dir->minSize()) + dw = dir->minSize() - size; + else if (size > dir->maxSize()) + dw = dir->maxSize() - size; + else + dw = 0; + } else if (dir->type() == KIcon::Threshold) + { + if (size < dir->size() - dir->threshold()) + dw = dir->size() - dir->threshold() - size; + else if (size > dir->size() + dir->threshold()) + dw = dir->size() + dir->threshold() - size; + else + dw = 0; + } + /* Skip this if we've found a closer one, unless + it's a downscale, and we only had upscales befores. + This is to avoid scaling up unless we have to, + since that looks very ugly */ + if ((abs(dw) >= abs(delta)) && + !(delta > 0 && dw < 0)) continue; } Please see new BUG #68625 This is not a duplicate but a very similar problem. -- JRT Why was this closed??? It has NOT been fixed!!! UPDATE: I still have this problem with KDE-3.2.3. This bug has been partially fixed in 3.3 ALPHA 1, but there are still problems. Test case: Select Icon Theme of KDE Classic. In Konqueror, show the BookMark ToolBar with at least one folder on it. Select Icon Size of Medium (22x22) for the BookMark ToolBar. Then the icons shown is Crystal SVG rather than KDE Classic. This is just an instance of what is a general design problem. -- JRT To provide additional information: http://standards.freedesktop.org/icon-theme-spec/icon-theme-spec-latest.html << The lookup is done first in the current theme, and then recursively in each of the current theme's parents, and finally in the default theme called "hicolor" (implementations may add more default themes before "hicolor", but "hicolor" must be last). As soon as there is an icon of any size that matches in a theme, the search is stopped. Even if there may be an icon with a size closer to the correct one in an inherited theme, we don't want to use it. Doing so may generate an inconsistant change in an icon when you change icon sizes (e.g. zoom in). >> This seems very clear to me. To make this very clear, KDE does not do this -- it does not follow the spec. If you are using KDEClassic and a size is missing, you get a CrystalSVG icon not a scaled KDEClassic icon. The first fix for this bug caused a new bug which now needs to be fixed. *** Bug 68625 has been marked as a duplicate of this bug. *** Created attachment 20748 [details]
Minor change to fix the problem
This appears to fix the problem.
SVN commit 670335 by tyrerj: BUG:53052 M +5 -12 kiconloader.cpp --- branches/KDE/3.5/kdelibs/kdecore/kiconloader.cpp #670334:670335 @@ -460,19 +460,17 @@ static const QString &xpm_ext = KGlobal::staticQString(".xpm"); ext[count++]=&xpm_ext; - /* antlarr: Multiple inheritance is a broken concept on icon themes, so - the next code doesn't support it on purpose because in fact, it was - never supported at all. This makes the order in which we look for an - icon as: + /* JRT: To follow the XDG spec, the order in which we look for an + icon 1s: png, svgz, svg, xpm exact match + png, svgz, svg, xpm best match next theme in inheritance tree : png, svgz, svg, xpm exact match + png, svgz, svg, xpm best match next theme in inheritance tree : png, svgz, svg, xpm exact match + png, svgz, svg, xpm best match and so on - And if the icon couldn't be found then it tries best match in the same - order. - */ for ( KIconThemeNode *themeNode = d->links.first() ; themeNode ; themeNode = d->links.next() ) @@ -484,11 +482,6 @@ return icon; } - } - - for ( KIconThemeNode *themeNode = d->links.first() ; themeNode ; - themeNode = d->links.next() ) - { for (int i = 0 ; i < count ; i++) { icon = themeNode->theme->iconPath(name + *ext[i], size, KIcon::MatchBest); |