[Development] Some QIcon::fromTheme() enhancements
antti.kaijanmaki at canonical.com
Fri Aug 23 13:23:23 CEST 2013
On 23.08.2013 11:32, Olivier Goffart wrote:
> Stable means it does not change much.
I was very careful not to introduce any big changes. I'm only a default
path and providing a fallback in the case that the normal loading from
theme directories fails. I'm not touching the way theme directories are
travelled or how the engine handles the search or matching of the found
icons or the icon cache.
As I stated in my original mail there is a bigger problem of QIconLoader
not being able to handle themes expanding over multiple base dirs and
fixing that is clearly something for dev.
> The bug was already there since the beginning of the feature in Qt 4.7. They
> already had to wait so many years, they can wait a few moth more.
> However, a regression in Qt 5.1.2 would be much more damageable. Maybe there
> is a broken small .xpm in one style that makes an otherwise good png in
> another theme not taken. Maybe this happens to be a huge performance
> bottleneck with some setup, maybe there is a bug in XPM parsing. Or any other
> problem we don't think about now.
If there is a broken XPM in some of the themes then it's a bug in the
theme and not Qt. The worst thing that could happen is that you get a
low resolution XPM icon instead of a PNG one (from the parent theme),
but that would mean that the XPM file has been put to the theme on
purpose to override the parent PNG.
And then again if there is a bug in XPM parsing then it's more severe
bug that needs to be addressed. QIcon uses XPM files in it's examples so
XPM support is a first class citizen in Qt.
> You would have to have good arguments that this bug is urgent enough.
> Following a specification is important of course, but it is only some details
> about it and it does not seem an urgent issue to me.
My only argument would be that QIcon::fromTheme() does not do what it's
documented to do. That's partly because the documentation does not
actually describe what is implemented and what is not. All it says is that
Returns the QIcon corresponding to name in the current icon theme. If no
such icon is found in the current theme fallback is returned instead.
The latest version of the freedesktop icon specification and naming
specification can be obtained here:
These changes bring the functionality of the function closer to what
it's documented to do with very little changes.
I don't have any strong feelings about getting this to stable. dev is
also fine, but I don't see a reason why this could not be included in
stable. The chances of introducing regression is IMO very low.
More information about the Development