mate-desktop / mate-panel

MATE panel
https://mate-desktop.org
GNU General Public License v2.0
185 stars 118 forks source link

status-notifier: Show AttentionIcon when Status is NeedsAttention #1413

Closed cwendling closed 1 year ago

cwendling commented 1 year ago

Fixes #1412.

This PR also includes 2 unrelated typo fixes extracted from gnome-panel. I can move remove them from this PR if wanted, as they are unrelated (and not necessary for anything but not having typos in the code).

cwendling commented 1 year ago

The attention icon from #1412 is rendered a bit ugly (scaled up), but that's orthogonal to the changes here. Possibly we should separately avoid scaling up the icon if it's "close enough" to the expected size.

cwendling commented 1 year ago

@lukefromdc simplest is to use the test case from the OP of #1412.

cwendling commented 1 year ago

Running the test case proved this works: icon changes occur only with this PR in place.

Thanks for the test, merged.

On my setup, all the icons from the test case come out way too small, but all my other tray and SNI icons are unchanged in rendering. Might be a test case issue, or we may need to force icon sizes when the provided icon is more than say, a 2px mismatch but that may be something to worry about for another PR, if for no more reason than to make any later bisects easier

Yeah this is orthogonal to the AttentionIcon support, which uses the exact same code to render the icons as the normal icon (just the source is changed when needed).

However, them being too small is kind of odd… or maybe you've got HiDPI and we have a HiDPI issue in the code path using a file path instead of an icon name? That's possible as I doubt many people tried it, worth investigating separately.

However, it's a fairly tricky situation, because for me, with a 26px tall panel, I'd rather have the 16x16 icon (the attention one from the test) at it's real size than scaled up to 24x24 which makes it very ugly and blurry. So I was thinking on my end not to scale up an icon if it's some percentage of the expected size (well, here it's about 33%, but for 16 to 24 it still looks better)… or maybe scale to a close enough size that is likely to result in less blurring, e.g. usually ×2 is cleaner than ×2.1 -- but "good" values likely depends on the scaling algorithm.

Anyway, this again should be discussed separately :)

lukefromdc commented 1 year ago

Thanks, found and used it in my last test

lukefromdc commented 1 year ago

Yes I am using a 4K monitor and window-scaling=2

cwendling commented 1 year ago

Yes I am using a 4K monitor and window-scaling=2

OK, could be that this part of the code is not handling that well, we'd have to investigate (in another PR) [edit] Unfortunately I don't have access to 4K setup, so I probably won't really be able to help here. But it looks like get_surface() is not handling window scaling, so that's probably it.