This revision was automatically updated to reflect the committed changes.
Closed by commit R102:e7e44a7af825: [about-distro] run absolute paths through
qicon intead of qpixmap (authored by sitter).
REPOSITORY
R102 KInfoCenter
CHANGES SINCE LAST UPDATE
davidedmundson added a comment.
The SVG issue is only relevant in QIcon::fromTheme(name, fallback) because
it detects
if (icon.isNull() || icon.availableSizes().isEmpty())
return fallback;
and availableSizes is problematic for the SVG QIcon backend.
You're not using
sitter added a comment.
If I understand it correctly, the bug would only show its ugly head with
`QIcon::fromTheme("foo.svg")` but not `QIcon::fromTheme("foo")`. So, if that
seems a concern we can just add a comment in the README. Distribution devs
would set LogoPath's value manually, so we
ngraham added a comment.
The issue Kai is worried about is
https://bugreports.qt.io/browse/QTBUG-63187. If we're not hitting that bug
here, we need to understand why, or else there's a chance that the logo will
not appear.
REPOSITORY
R102 KInfoCenter
REVISION DETAIL
sitter added a comment.
works fine here
REPOSITORY
R102 KInfoCenter
REVISION DETAIL
https://phabricator.kde.org/D19033
To: sitter, #plasma
Cc: broulik, cfeck, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot,
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
broulik added a comment.
isn't `QIcon::fromTheme` buggy when passed an absolute path to an SVG file?
REPOSITORY
R102 KInfoCenter
REVISION DETAIL
https://phabricator.kde.org/D19033
To: sitter, #plasma
Cc: broulik, cfeck, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot,
lesliezhai,
sitter updated this revision to Diff 51741.
sitter added a comment.
throw away the comment. upon reflection I actually find it unnecessary
REPOSITORY
R102 KInfoCenter
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D19033?vs=51740=51741
BRANCH
Plasma/5.15
REVISION DETAIL
sitter added a comment.
Well, it won't resolve the path, it will simply load it. I do see the
ambiguity though. ;)
REPOSITORY
R102 KInfoCenter
REVISION DETAIL
https://phabricator.kde.org/D19033
To: sitter, #plasma
Cc: cfeck, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot,
cfeck added a comment.
The comment is confusing. If QIcon does not resolve absolute paths, but I can
stil pass absolute paths, the comment could say that it ignores the path?
Shouldn't this comment rather be part of the README, instead of the code?
REPOSITORY
R102 KInfoCenter
REVISION
sitter created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
sitter requested review of this revision.
REVISION SUMMARY
qicon::fromtheme internally skips resolution of absolute paths and instead
simply opens it by path. so, simplify the code by only
10 matches
Mail list logo
