Bug 323885 - Regression: Icons are only shown in the pager if its size is rather large. Solution proposed.
Summary: Regression: Icons are only shown in the pager if its size is rather large. So...
Status: RESOLVED FIXED
Alias: None
Product: plasmashell
Classification: Plasma
Component: Pager (show other bugs)
Version: 5.3.0
Platform: Ubuntu All
: NOR normal
Target Milestone: 1.0
Assignee: Plasma Bugs List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-08-22 09:23 UTC by Mitko
Modified: 2015-05-20 13:59 UTC (History)
6 users (show)

See Also:
Latest Commit:
Version Fixed In:
kde: VisualDesign+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mitko 2013-08-22 09:23:18 UTC
Since KDE 4.10 icons in the pager are only shown if it is big enough to show the entire icon. This is a regression from previous versions which always showed icons when the "Display icons" option was set. See below for the cause of the problem and a solution.

This is not a duplicate of bug 316577 or bug 318892. These other bugs claim that icons are never shown.

Reproducible: Always

Steps to Reproduce:
1. Configure multiple virtual desktops.
2. Add a pager and enable the "Display icons" option.
3. Resize the panel where the pager is. Try both a large panel and a very small one.
Actual Results:  
Icons are only shown if the panel size is large. Especially for vitual desktops with 2 or more rows, rather large panel sizes are required.

Expected Results:  
Icons should always be shown like it used to be in KDE < 4.10.

The problem is in the main.qml file of the pager. The setting controlling the visibility of the icon used to be:

visible: pager.showWindowIcons

but after commit 64a17885b22a13ae73e2a89d0e29216a5cf2b481 it changed to 

visible: pager.showWindowIcons && (windowRect.width > icon.width) && (windowRect.height > icon.height)

That commit was in response to bug 310475 which seems completely unrelated to the visibility to the icon. For some reason the commit author decided that there is a problem with the icon's visibilty and made the change above, but there is no explanation why this was necessary. I think the proper solution would be to simply revert that change.

Luckily, the problem is not in a compiled file, and those of use who don't want to wait for a new release can fix the behavior by simply modifying our local qml file. On my computer the file is here:

/usr/share/kde4/apps/plasma/packages/org.kde.pager/contents/ui/main.qml

I reversed the change mentioned above and now I can enjoy pager icons at all sizes again. Note that if you do this, you might need to reenable the "Display icons" settings. I had to do this since the setting was automatically turned off after the change. I also restarted the computer - don't know if that was really necessary.

I would submit a bug fix myself, but I've never contributed to KDE before and I don't want to go through all the effort of the commit procedures etc.
Comment 1 Mitko 2013-08-22 09:42:09 UTC
Added  Weng Xuetian to the cc list (author of the original commit).
Comment 2 sparhawk 2013-08-22 10:41:24 UTC
Thanks for the fix! I can confirm that reverting that line fixes this bug completely. This makes KDE usable once more — no more lost windows for me!
Comment 3 sparhawk 2013-10-18 01:13:48 UTC
Just to confirm that this regression is still in Kubuntu 13.10 (KDE 4.11.2).

The fix still works, but the original line has changed subtley and is now:

    visible: pager.showWindowIcons && (windowRect.width >= icon.width) && (windowRect.height >= icon.height)

Just change it to

    visible: pager.showWindowIcons

And the fix is complete.
Comment 4 sparhawk 2013-12-21 01:12:25 UTC
Just confirming that this regression is still in KDE 4.12. (I think I installed it from the Kubuntu backports.)

Again, the fix still works.
Comment 5 sparhawk 2014-01-30 22:58:35 UTC
This regression is still in KDE 4.12.1, installed via Kubuntu backports. Again the fix works.
Comment 6 sparhawk 2014-02-19 00:09:26 UTC
This regression is still in KDE 4.12.2, installed via Kubuntu backports. Again the fix works. This bug seems very easy to fix, even if it is still "unconfirmed".
Comment 7 sparhawk 2014-04-04 03:53:33 UTC
Still here in KDE 4.12.97, installed in Kubuntu 14.04 (beta). The fix still works.
Comment 8 Mitko 2014-04-09 07:44:38 UTC
I wonder if there is any way to get more attention to this bug. It's even still marked as unconfirmed.
Comment 9 Mitko 2014-04-09 07:48:08 UTC
Added Martin Gräßlin to the CC list as he marked the previous, related bug report as resolved.
Comment 10 sparhawk 2014-04-11 22:32:27 UTC
@Mitko I'm really not too sure. To me, it's such an obvious regression, and it's such an easy fix. I would have thought cc:ing the author of the regression would have been enough, but there's been no response, or even justification for that change. I'm not sure what else to do.
Comment 11 sparhawk 2014-05-20 13:02:29 UTC
Still here in 4.13.1. The fix is still valid.
Comment 12 sparhawk 2014-06-12 06:26:47 UTC
Still here in 4.13.2, but the config file has moved to /usr/share/apps/plasma/packages/org.kde.pager/contents/ui/main.qml

The fix still works though.
Comment 13 sparhawk 2014-07-27 05:24:06 UTC
I just wanted to post a screenshot of a small pager with the icons displayed (i.e. with Mitko's patch applied). I have ten virtual desktops, and the icons are clearly useful in distinguishing them, even at this size. Again, I can see no reason for this regression, assuming it was intentional.

http://i.imgur.com/7nmOndu.png
Comment 14 sparhawk 2014-09-29 12:56:34 UTC
I've moved to Arch, and this bug is still apparent, now in 4.14.1. The fix still works.
Comment 15 sparhawk 2014-10-20 22:58:36 UTC
Still here in 4.14.2. Fix still works.
Comment 16 sparhawk 2014-11-11 03:20:43 UTC
Some more information for others that are here to find a fix. I have two Linux systems: Arch and Kubuntu. The path of the file to modify varies based on distro.

Arch: /usr/share/apps/plasma/packages/org.kde.pager/contents/ui/main.qml
Kubuntu: /usr/share/kde4/apps/plasma/packages/org.kde.pager/contents/ui/main.qml

Also, I got bored of applying this fix regularly over the last year, so I relearnt how to make patches. Download the patch [1], then after a KDE upgrade, run:

    cd /; sudo patch -i /path/to/patch

Obviously, modify the paths in the patch if your file is elsewhere.

[1] http://paste.kde.org/piry4am1p
Actually, items in the KDE pastebin expire after a year. Conceivably this bug will take longer than that to fix, so here is a mirrored link:
http://pastebin.com/nVa7E3Vb
Comment 17 sparhawk 2014-11-24 11:15:37 UTC
Ugh, I missed an option. It should be:

cd /; sudo patch -p0 -i /path/to/patch
Comment 18 sparhawk 2015-05-04 02:23:48 UTC
I just upgraded to Plasma 5.3, and this bug is still here. The pertinent file appears to have moved to /usr/share/plasma/plasmoids/org.kde.plasma.pager/contents/ui, although the offending line is still present. Unfortunately the fix no longer works! Any suggestions?
Comment 19 sparhawk 2015-05-19 11:03:05 UTC
Oh, it might have been some issue with my system. The fix still works in Plasma 5. I think I can't edit this bug, but it might be worth changing it to Plasma 5 instead of Plasma 4. Because otherwise the devs might not pay attention to this bug. Um…
Comment 20 David Edmundson 2015-05-19 11:10:06 UTC
Moving to Plasma5, where we are better at responding to bugs.

However on this patch.

I don't see how that's a solution, it has a clearly deliberate check to see if it's big enough to show the icon and this patch just removes that check. 

The icons are still at the native size which means they're just going to be overflowing out of their window. That's hardly better, I would be willing to bet we had a tonne of bug reports complaining about that, and this will just keep us going round in circles changing the behaviour
Comment 21 sparhawk 2015-05-19 11:25:25 UTC
David, thank you so much for the response. 

I actually agree in part. The icons definitely overflow a bit more in Plasma 5 [1] than in Plasma 4 [2]. However, I have two comments about this.

Firstly, having the icons visible to me is extremely important. As you can see, I have ten virtual desktops, and I easily lose track of my windows without the icons. Surely it's better to have overflowing icons than none at all? Part of the reason that I love KDE/Plasma is that it offers so much choice. If people don't like the overflowing icons, then they could just turn it off.

Secondly, if people complain about the icons enough to make bug reports, then this suggests that the instant fix (of disabling the icon) would be unsatisfactory to them. This suggests that they would not be placated with a response that disables icons entirely, and the real solution would be for the pager to just scale them down.

[1] http://i.imgur.com/xAz5A9w.png
[2] http://i.imgur.com/7nmOndu.png
Comment 22 David Edmundson 2015-05-19 12:21:06 UTC
>and the real solution would be for the pager to just scale them down.

Lets try that then and see how it looks? Could you give it a go?

Something like:
width: Math.min(windowRect.width, nativeWidth)
Comment 23 sparhawk 2015-05-19 12:41:04 UTC
Hm… The icons often look a little squashed, because the proportions are not maintained. I tried a few permutations.

No scaling: http://i.imgur.com/rLuehfE.png
Width scaled: https://i.imgur.com/DBg2AC8.png
Height scaled*: http://i.imgur.com/diByFlL.png
Both scaled*: http://i.imgur.com/48dqUSu.png

* i.e. with `height: Math.min(windowRect.height, nativeHeight)`

It's most obvious odd in the "Width scaled" screenshot in the top row, fourth desktop from the left. That icon should be a circle, but it's very squashed here. I guess the code would have to be slightly more sophisticated, working out the maximum scaling required in width or height, then scaling both down this constant amount.
Comment 24 David Edmundson 2015-05-19 13:11:17 UTC
Thanks, to keep proportions

if you take your both scaled patch

add
fillMode: KQuickControlsAddonsComponents.QPixmapItem.PreserveAspectFit
Comment 25 sparhawk 2015-05-19 13:21:39 UTC
Here it is with the fillMode line added: http://i.imgur.com/JoMJGIA.png

To be honest, at first glance, I think I prefer it cropped (with no scaling) for this low resolution. Perhaps cropped looks less attractive, but I think there is considerably more information in the cropped version. IMO having more of the colour and distinctive central part of the icons is more descriptive at a quick glance. However, I could see the argument for the other way.
Comment 26 David Edmundson 2015-05-19 23:27:14 UTC
Tagging the visual design team for opinions.

Marking as confirmed so it's not in my "to triage" list, but I'm not promising we'll change it.
Comment 27 Andrew Lake 2015-05-20 03:58:48 UTC
I think the cropped, non-scaled version looks and works better (http://i.imgur.com/rLuehfE.png). I think that beyond a certain point, the scaled-down icons just become less effective in servicing their primary recognition function than a cropped portion of the larger icon.
Comment 28 David Edmundson 2015-05-20 13:14:27 UTC
Sparhawk, so if we do what Andrew says that's just committing your patch at http://pastebin.com/nVa7E3Vb ?

What's your real name? I'll put you down as the patch author.
Comment 29 sparhawk 2015-05-20 13:21:31 UTC
That sounds perfect. Thanks for that. However, I think Mitko should get the credit, as he or she was the one that identified the problem, wrote the bug report, and identified the original commit, file and fix. I just diff'ed the file after applying Mitko's fix. If Mitko is non-responsive, I'm happy to be uncredited. Cheers.
Comment 30 Mitko 2015-05-20 13:44:57 UTC
Hi guys, sorry for being unresponsive, but I wasn't receiving any notifications regarding this bug until today when I received only the last two comments in my e-mail (but not the discussion before). Any idea why I didn't get e-mails before? Perhaps, I was not properly subscribed to the bug or something.

For what it's worth, I also prefer the cropped version :)

Anyway, my real name is Dimitar Asenov. However, I think it's only fair that sparhawk also gets credit, as it was his continued support of this bug that has lead to the patch being merged. I hope that's possible somehow.
Comment 31 David Edmundson 2015-05-20 13:59:39 UTC
remote: This commit is available for viewing at:
remote: http://commits.kde.org/plasma-desktop/2942aa6836e3bf3a1281e69ea86b938a6bf4a58b
remote: Closing bug 323885

Fixed in 5.4