Discussion:
[KPhotoAlbum] suggested solution to the icon problem
Andreas Schleth
2018-01-04 14:08:44 UTC
Permalink
Hi Johannes, Tobias,

attached is "not a patch", but a possible path to a solution of the icon
problem with Mint and Ubuntu:

a) We should provide at least one thumbnail of size 64x64, better
several for all the items on the home screen that are not categories.

I do not know how to install these correctly. Therefore, my patch is not
a patch :-)

b) The QIcon-call should be amended as shown in the patch file. This
patch works only for starting the executable directly from the build
directory.

QIcon::fromTheme has a variant with 2 parameters: 1. as before some
named icon from a theme, 2. a fallback QIcon. I suggest, we use this
variant and provide fallback icons where needed.

Also I think that all the cases with icon names as in
QString::fromLatin1("...") oder QString::fromUtf8("...") could probably
replaced with QLatin1String("...") as long as the strings will not be
translated.

As to the reason for the fail in Ubuntu/Mint: it is the theme (or
missing icon names in the Mint/Ubuntu themes). If I choose a KDE Theme
in Mint, the problem gets better (more icons show), but the are shown in
different sizes. However, we cannot force the user to choose a certain
desktop icon theme just for KPA. Therefore, I vote for the fallback
solution.

I could chip in and provide a real patch, if you could show me how to
install custom icons and how to correctly reference them. For the time
being, I think the home screen of KPA needs this kind of attention.
There are other parts of the program that exhibit the missing icon
syndrome (settings dialog, ...), but there we always have a textual
description.

Cheers, Andreas
Johannes Zarl-Zierl
2018-01-04 15:13:22 UTC
Permalink
Hi Andreas,

Sorry for not responding to your previous mail in a timely manner (I have a
draft response sitting in my mailbox, but have had no time to finish it, yet).
Also: thanks for sharing the insights into mangled icons with us. It enabled
me to recreate the issue in a virtualbox Ubuntu image.
Post by Andreas Schleth
QIcon::fromTheme has a variant with 2 parameters: 1. as before some
named icon from a theme, 2. a fallback QIcon. I suggest, we use this
variant and provide fallback icons where needed.
You're right: Providing fallback icons (in a resource file) is probably the
way to go.
Post by Andreas Schleth
Also I think that all the cases with icon names as in
QString::fromLatin1("...") oder QString::fromUtf8("...") could probably
replaced with QLatin1String("...") as long as the strings will not be
translated.
Yes - though I would be surprised if there was a measurable performance
difference for our usage of strings.
Post by Andreas Schleth
As to the reason for the fail in Ubuntu/Mint: it is the theme (or
missing icon names in the Mint/Ubuntu themes). If I choose a KDE Theme
in Mint, the problem gets better (more icons show), but the are shown in
different sizes. However, we cannot force the user to choose a certain
desktop icon theme just for KPA. Therefore, I vote for the fallback
solution.
I'm still wondering how this happens in the first place: On my virtualized
Ubuntu 16.04 LTS I see that:

KPA uses the ubuntu-mono-dark icon theme, with:
Available icon sizes: (QSize(16, 16), QSize(22, 22), QSize(24, 24), QSize(32,
32), QSize(48, 48))
Actual icon size: QSize(48, 48)
Pixmap rect QRect(0,0 70x70)

Yet, the displayed pixmap has (by the way of a screen ruler) ~20px in height.

I would really like to know how this can happen…

Cheers,
Johannes
Johannes Zarl-Zierl
2018-01-05 00:01:59 UTC
Permalink
Hi Anreas,
Post by Johannes Zarl-Zierl
[...]
Yet, the displayed pixmap has (by the way of a screen ruler) ~20px in height.
Yes, this is approximately the height of the text line. If I introduce
just one icon 64x64 für the thumbnail view via a fallback, all the other
items show (more or less) OK, except for the missing icons. It might be
a QT issue with QListView.
"Height of the text line" was a good clue, and now I think I have an idea
what's happening:
Because the item "view-preview" (i.e. thumbnail view) is missing, it falls
back to text view. It seems that the QListView in Qt 5.5 then assumes that all
items have only the DisplayRole instead of DecorationRole.

Forcing the issue in Qt 5.9 by setting an invalid icon did not work.
I opted for the QLatin1String because that would make it clearer, that
this is a fixed string, that does not need translation. I was just
confused because of the mixture of ::fromUtf8 and ::fromLatin1.
Ok.
Could You show me, how to add ressources to KPA and how to call them?
I have to admit I did not use resources with a cmake-based Qt project before.
I need to take a look myself…

Cheers,
Johannes
Tobias Leupold
2018-01-05 11:47:00 UTC
Permalink
Post by Johannes Zarl-Zierl
I have to admit I did not use resources with a cmake-based Qt project
before. I need to take a look myself…
I only use resource files for a Windows build of one of my projects that is
built with qmake (because with cmake, I didn't manage to build a statically
linked Windows binary), but apparently, it's not a problem with cmake:

http://doc.qt.io/qt-5/cmake-manual.html and
https://cmake.org/cmake/help/v3.2/manual/cmake-qt.7.html
Johannes Zarl-Zierl
2018-01-05 12:09:05 UTC
Permalink
Hello Andreas,

I've pushed a fix for the issue of QListView messing with the icons. Seeing
that the remaining issue is "only" missing icons, I think that we don't
need(*) to rush pre-packaged fallback icons into KPA.

From my point of view, I think that warning messages for missing icons are
probably a better solution for the problem, because they enable the user to
just install the right package and get consistent icons for other applications
as well.


(*) There is a krazy2 check for non-standard icons which did not complain
about "view-preview" in the past. Getting a fresh overview which icons we can
rely on is certainly a good idea. Then we can make an informed choice and
selectively provide fallbacks or use different icons.

Cheers,
Johannes
Post by Johannes Zarl-Zierl
Hi Anreas,
Post by Johannes Zarl-Zierl
[...]
Yet, the displayed pixmap has (by the way of a screen ruler) ~20px in height.
Yes, this is approximately the height of the text line. If I introduce
just one icon 64x64 für the thumbnail view via a fallback, all the other
items show (more or less) OK, except for the missing icons. It might be
a QT issue with QListView.
"Height of the text line" was a good clue, and now I think I have an idea
Because the item "view-preview" (i.e. thumbnail view) is missing, it falls
back to text view. It seems that the QListView in Qt 5.5 then assumes that
all items have only the DisplayRole instead of DecorationRole.
Forcing the issue in Qt 5.9 by setting an invalid icon did not work.
I opted for the QLatin1String because that would make it clearer, that
this is a fixed string, that does not need translation. I was just
confused because of the mixture of ::fromUtf8 and ::fromLatin1.
Ok.
Could You show me, how to add ressources to KPA and how to call them?
I have to admit I did not use resources with a cmake-based Qt project
before. I need to take a look myself…
Cheers,
Johannes
Loading...