paulmcauley / klassy

Klassy is a highly customizable binary Window Decoration, Application Style and Global Theme plugin for recent versions of the KDE Plasma desktop.
843 stars 24 forks source link

System icons recoloring issue #56

Closed Dejweed closed 11 months ago

Dejweed commented 2 years ago

System icons aren't being recolored if they are white (most dark icon themes use white). When using light color scheme with dark icon theme buttons can become invisible (since it's white on white). I assume this is not an icon theme issue since it gets recolored just fine in GTK applications with Adwaita theme, but I did notice this issue isn't present with icons that use current color scheme as fill (like Breeze icon theme). However those icons don't change color when hovered.

klassy_recolor Notice the low visibility with Papirus-dark and Breeze

paulmcauley commented 11 months ago

Hi, this should be fixed on master now. Please test.

I had to write a pixel-manipulation algorithm myself, rather than just relying on a Qt effect.

There is still a bug where the icons will look bad on mixed-dpi multi-monitor setups but this will be fixed in the Qt6 version.

Dejweed commented 11 months ago

Now they aren't getting recolored when hovered (see top right of the image in my previous comment: normally white, black when hovered). I'm also getting some pixels at top left of the button when using system icons: image

Also light on light close button with JasperDark color scheme (background colors: accent color, always show icons and backgrounds, translucent backgrounds off): image Another example (almost same settings as last one, only difference always show: icons): image

Unrelated to the colors, I would appreciate some UI "quality of life" improvements. When I was checking out different presets each time the preset gets loaded the window closes, so every time I had to reopen presets again. Some kind of apply/preview button would be nice. Same with general settings, you can't apply them without the window closing. I feel like the new button customization (always show) could become difficult to navigate with drop down menus. Ticking checkboxes sounds easier to me, but I haven't tried it in practice yet. And since many settings focus on close button specifically, it could have it's own settings for those options (always show and background colors. Then there would also be an option for it to look just like other buttons (example libadwaita). Just an idea, I don't know how viable it would be codewise.

paulmcauley commented 11 months ago

New commit 6c1e6c5e7dbe17c900ea9569b6d5f66595c3d3f9 should have fixed the glitches in the top-left (I hadn't even noticed these on a HiDPI screen!)

Dejweed commented 11 months ago

Okay, with Jasper it might be the color scheme's issue, it also happens with menubars. But using Breeze Dark doesn't change colors as it used to. This also applies to non-system icons. Here's before and now: old⁄new I'm using slightly different settings but you get the idea.

paulmcauley commented 11 months ago

Now they aren't getting recolored when hovered (see top right of the image in my previous comment: normally white, black when hovered). image

I don't know what you mean here - can you elaborate what the exact settings are and what you expect?

Also light on light close button with JasperDark color scheme (background colors: accent color, always show icons and backgrounds, translucent backgrounds off): image

This is a bug. I have it fixed on my local machine and will upload the fix later

Another example (almost same settings as last one, only difference always show: icons): image

These are just colours directly from the colour scheme. I am going to eventually add a custom colour configuration where you can override the colour-scheme and Klassy's hard-coded colours.

Unrelated to the colors, I would appreciate some UI "quality of life" improvements. When I was checking out different presets each time the preset gets loaded the window closes, so every time I had to reopen presets again. Some kind of apply/preview button would be nice. Same with general settings, you can't apply them without the window closing.

Yes, a preset apply is a good idea. I can't add Apply to the main config in System Settings as the dialog that initially loads in System Settings when you click the pencil is provided by Plasma, so we need to file a bug there at http://bugs.kde.org . In the meantime you can run klassy-settings where there is an Apply button (/usr/bin/klassy-settings): Screenshot_20231209_150831

I feel like the new button customization (always show) could become difficult to navigate with drop down menus. Ticking checkboxes sounds easier to me, but I haven't tried it in practice yet. And since many settings focus on close button specifically, it could have it's own settings for those options (always show and background colors. Then there would also be an option for it to look just like other buttons (example libadwaita). Just an idea, I don't know how viable it would be codewise.

Yes, I think checkboxes would be easier for Always Show too, but I am going to move all the Button Behaviour (Always Show/Highlight Using) options to a new window so will change it later. I also want behaviours and colours to be able to be set differently for active and inactive windows.

But using Breeze Dark doesn't change colors as it used to. This also applies to non-system icons. Here's before and now: old⁄new I'm using slightly different settings but you get the idea.

I think this is the contrast heuristic where it sets the icon colour to either black/white if there is poor contrast with the background - it only happens when translucent is off. I will eventually add a toggle to control this. I made a change to cope with always show backgrounds, so need to check...

Dejweed commented 11 months ago

Awesome, I'm looking forward to these changes. Thank you for your work and for making such a great theme.

paulmcauley commented 11 months ago

a412c70eba1c1c4eb4dfd107b09487b42a191029 should fix the colouring issues you mentioned. Let me know if any more problems

paulmcauley commented 11 months ago

Some kind of apply/preview button would be nice. Same with general settings, you can't apply them without the window closing.

The Plasma bug/wishlist for this is at: https://bugs.kde.org/show_bug.cgi?id=463176

Add your +1 there!

Dejweed commented 11 months ago

Now it works as it should! But I think the translucent hovered accent color isn't opaque enough, and it gets inconsistent with the menubar colors (translucent backgrounds off, background color: accent color, always show icons):

https://github.com/paulmcauley/klassy/assets/65261588/f19a0c8b-354d-47ad-9e47-a3386a1f1ef2

Looking at the colorscheme (gruvbox in this case), it looks like menubar uses hover and focus colors, while titlebar icons both look like hover color, one more transparent, but neither match the window's border color. On some other colorschemes it does match window's border color. Is this intentional? image

Add your +1 there!

How do I do that? Do I just comment +1 like you did?

I found another example where the recoloring could be better: background: titlebar text color, always show icons and backgrounds, translucent off image

Unrelated to colors I also noticed outlines on square buttons look blurry (not 1 pixel wide).

paulmcauley commented 11 months ago

But I think the translucent hovered accent color isn't opaque enough,

It was designed to match the Breeze "Blue Ocean" changes a couple of years ago. I may make there a slider for opacity.

and it gets inconsistent with the menubar colors (translucent backgrounds off, background color: accent color, always show icons): hover.mp4

Looking at the colorscheme (gruvbox in this case), it looks like menubar uses hover and focus colors, while titlebar icons both look like hover color, one more transparent, but neither match the window's border color. On some other colorschemes it does match window's border color. Is this intentional? image

Yes, that was a bug. They were the wrong way round. Fixed in 1b0a9fdf2116aee2ce4011487f83bdaf5a3df0f7 I also rewrote the entire code that selects the colour for the icon so that animations work properly, so let me know if you spot anything else!

The button accent colours are based off the "Button" group Hover and Focus. I was not sure whether the "Window" or "Button" group is more correct. The window outline accent is from QPalette::Highlight which is the same used in the titlebar separator from the original Breeze - it gives a slight more intense colour.

Add your +1 there!

How do I do that? Do I just comment +1 like you did?

Whatever you feel appropriate!

I found another example where the recoloring could be better: background: titlebar text color, always show icons and backgrounds, translucent off image

I have now changed this to be a mix between the titlebar and font colours.

Unrelated to colors I also noticed outlines on square buttons look blurry (not 1 pixel wide).

This is fixed in 58e6bd06eb966d06077714c62053f67aeb86580e

Dejweed commented 11 months ago

Utterly nord dark (background: accent colors, always show icons, translucent off): image I think black would be better when hovered.

Nice work with presets UI!

Also unrelated to colors: When using certain color scheme (like Breeze light), using custom accent color (in KDE settings) draws outlines when there should only be background. My klassy-settings window size is too small by default. I'm also missing the icon for "Use system icon theme". image

paulmcauley commented 11 months ago

Utterly nord dark (background: accent colors, always show icons, translucent off): image I think black would be better when hovered.

A black background? I think the background colour here is just taken from the colour scheme.

Nice work with presets UI!

Also unrelated to colors: When using certain color scheme (like Breeze light), using custom accent color (in KDE settings) draws outlines when there should only be background.

This is when poor contrast is detected between the button background and the titlebar. Currently it draws an outline in such cases, but I am thinking of changing it so that instead the button background colour gets mixed with some of the titlebar text colour, and also making it configurable.

My klassy-settings window size is too small by default.

I know about that and will change it before the next release, but no point now as there are going to be UI changes

I'm also missing the icon for "Use system icon theme". image

What icon theme are you using? It might not have included an icon for that.

Dejweed commented 11 months ago

A black background? I think the background colour here is just taken from the colour scheme.

No no, the icon would be black.

What icon theme are you using? It might not have included an icon for that.

Win98SE. Oh okay, it was the icon theme's issue. The icon used to be there so I thought it was related to klassy.

Are there plans to use system icons for shade and other buttons? From what is see, not every icon theme has them so that could be an issue, but those that do would look more coherent. And different colors for remaining traffic light buttons (shade, keep above/below...)?

paulmcauley commented 11 months ago

A black background? I think the background colour here is just taken from the colour scheme.

No no, the icon would be black.

The icon colour is just from the colour scheme here. The contrast threshold I have set thinks its OK and I don't want to mess with it. There is going to be the option to override all button colours.

What icon theme are you using? It might not have included an icon for that.

Win98SE. Oh okay, it was the icon theme's issue. The icon used to be there so I thought it was related to klassy.

OK, there was a code change on that icon. I will change it back and see if there is a fallback icon can be put there from Breeze.

Are there plans to use system icons for shade and other buttons? From what is see, not every icon theme has them so that could be an issue, but those that do would look more coherent.

If I can find a way to detect whether a system icon theme has an icon or not I would implement it differently. Have you found any icon themes with shade? (Keep above and below already use the system icon theme)

And different colors for remaining traffic light buttons (shade, keep above/below...)?

I want to make colours overridable for each button type.

Dejweed commented 11 months ago

Have you found any icon themes with shade? (Keep above and below already use the system icon theme)

Looking at Papirus, I found two icons, window-shade and xfce-wm-shade. Also window-pin and xfce-wm-stick. These are most common, but I feel like they are split between GTK/QT, so no single standard icon. If we take a look at breeze, it has window-shade/window-unshade and window-pin/window-unpin. Window-unpin is different, I couldn't find an icon that resembles PinnedOnAllDesktops. So yeah, I don't know if it's worth it.

paulmcauley commented 11 months ago

I see there is a function: bool QIcon::hasThemeIcon(const QString &name)

I can maybe do a more intelligent icon lookup than at present

paulmcauley commented 11 months ago

Are there plans to use system icons for shade and other buttons? From what is see, not every icon theme has them so that could be an issue, but those that do would look more coherent.

ebd6f878cf32d5e11d5ee9804f8f60d30f2e166c now allows all button types to use the system icon theme. Papirus now looks like this (though I'm not sure this is a good idea for all icon themes): Screenshot_20231215_193342

paulmcauley commented 11 months ago

Master now has many more colour configuration options (with more coming). These should address all outstanding issues