Bug 366047

Summary: sni proxy scaling not always appropriate
Product: [Plasma] plasmashell Reporter: Oswald Buddenhagen <ossi>
Component: XembedSNIProxyAssignee: Plasma Bugs List <plasma-bugs>
Status: RESOLVED FIXED    
Severity: minor CC: franksouza183, kde, materka, nate
Priority: NOR    
Version: 5.6.5   
Target Milestone: 1.0   
Platform: Debian unstable   
OS: Linux   
Latest Commit: Version Fixed In: 5.17.1

Description Oswald Buddenhagen 2016-07-24 18:12:48 UTC
the sni proxy apparently scales the embedded window's contents, presumably to make them visually fit into the surroundings. however, if the window contains sharp edges and/or text, the scaling can be rather unhelpful. an example of such an application is gnome-mail-notification, which shows a fairly tiny icon with the number of new messages.

i suggest to provide a per-app option to disable scaling which is not an integral multiple, and just centering the icon instead.
i'm not quite sure how to integrate it gui-wise, though. i suppose one could react to alt-rightclick, as that's a window manager gesture which is otherwise meaningless in this context.


Reproducible: Always
Comment 1 David Edmundson 2016-08-18 23:01:43 UTC
it's not so much a visual thing, but a workaround because some apps behave really really weirdly.

I'll see about doing this in the config file at least.
Comment 2 Konrad Materka 2019-10-09 11:43:06 UTC
I think we can remove the code responsible for icon scaling from xembedsniproxy:

if (w != s_embedSize || h != s_embedSize) {
    qCDebug(SNIPROXY) << "Scaling pixmap of window" << m_windowId << Title() << "from w*h" << w << h;
    m_pixmap = m_pixmap.scaled(s_embedSize, s_embedSize, Qt::KeepAspectRatio, Qt::SmoothTransformation);
}

As we are scaling the client window down, it will only be performed on icons smaller that 32x32. It will be scaled up anyway in the StatusNofierItem.qml, from systemtray applet - this is a better place for that.
Comment 3 David Edmundson 2019-10-09 11:57:10 UTC
>As we are scaling the client window down


IIRC this code exists because some client windows ignored me and decided to be a massive size regardless
Comment 4 Konrad Materka 2019-10-10 10:20:02 UTC
> IIRC this code exists because some client windows ignored me and decided to be a massive size regardless

I was not able to find any application that (still) behaves this way. In coments you mentioned Spotify (no longer has icon, feature entirely removed) and Chromium (I was not able to get icon, maybe my fault). The only case is KeePass2 (bug 358240), which is a Mono application. It indeed ignores window resize, because it is too soon to do that in the constructor. With a pause in debug mode it was working correctly. As a workaround I added client window resize in the update() method: https://phabricator.kde.org/D24529. It works perfectly fine, I tested it with several apps with no regression.

We can safely disable up-scaling - QML, which is showing the icon, can do (is doing) that automatically. As a bonus we can add additional logic/configuration option in System Tray to disable up-scaling. Personally I don't think it is very important, icon will look bad anyway, it will be too small. Definitely it will solve a situation when we are up-scaling from 24x24 to 32x32 and System Tray is down-scaling it to 24x24 again.
Comment 5 Konrad Materka 2019-10-10 11:54:16 UTC
Patch: https://phabricator.kde.org/D24531
Comment 6 Konrad Materka 2019-10-10 11:58:56 UTC
*** Bug 389990 has been marked as a duplicate of this bug. ***
Comment 7 Nate Graham 2019-10-10 14:40:17 UTC
Git commit 13efbfca67f8270458d103e128ba76525f663329 by Nate Graham, on behalf of Konrad Materka.
Committed on 10/10/2019 at 14:39.
Pushed by ngraham into branch 'Plasma/5.17'.

[XembedSNIProxy] Scale only big icons

Summary:
Do not scale up small icons to avoid quality detoriation. For small icons scaling should be done in SystemTray only. Currently XembedSNIProxy scales to 32x32, then SystemTray (usually) scales it down to 24x24.
FIXED-IN: 5.17.1

Test Plan:
I've run few applications with small icons:
  - keepassx (22x22)
  - liferea (16x16)
  - tuxguitar (16x16)

It looks much better without scaling in xemebdsniproxy.

Reviewers: davidedmundson, #plasma, #plasma_workspaces

Reviewed By: davidedmundson, #plasma, #plasma_workspaces

Subscribers: plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D24531

M  +1    -1    xembed-sni-proxy/sniproxy.cpp

https://commits.kde.org/plasma-workspace/13efbfca67f8270458d103e128ba76525f663329