search

bbidulock / icewm

A window manager designed for speed, usability, and consistency
Other
590 stars 100 forks source link

Changed icons size behavior in 1.8.0 version #483

Closed tim77 closed 4 years ago

tim77 commented 4 years ago

After 1.8.0 version now 24px icon size variant downscaled from 16px variant, but previously it was upscaled from 32px version. This is intentional or regression?

Here is two pics which could better explain this issue:

24px variant dowscaled from 16px:

image

32px variant which upscaled onto 24px before 1.8.0 update:

image

So before 1.8.0 update 24px version looked nice as you can understand and not pixelated.

gijsbers commented 4 years ago

A new icon loader module was written for 1.8.0 by @Code7R. To help pinpoint the problem: in your preferences set Trace=icon. Then restart a 1.7.0 and a 1.8.1 and compare their logging output for icon files. Under GDM you can get the icewm output with: journalctl _UID=$(id -u).

tim77 commented 4 years ago

@gijsbers i am afraid this is not what you asking for but here logs from Rawhide:

Also here is how 24px version was looked in old 1.7.0: image

gijsbers commented 4 years ago

Tracing of icon loading is not enabled. In your preferences do you have Trace=icon? You must have this:

$ grep -C 1 Trace= ~/.icewm/preferences
#  Enable tracing for the given modules
Trace=icon

Then restart. What we are looking for is lines which contain "Icewm: icon". I will give you two examples. For 1.7.0: 170.txt and for 1.8.1: 181.txt

In your case this may look differently and hopefully we can infer which icons are not properly loaded by the new icon loader. Maybe @Code7R could try Fedora32 and see?

tim77 commented 4 years ago

Tracing of icon loading is not enabled. In your preferences do you have Trace=icon? You must have this:

$ grep -C 1 Trace= ~/.icewm/preferences
#  Enable tracing for the given modules
Trace=icon

I did exactly like that but on system-wide prefs and there is no user preferences on this Rawhide VM which could override something:

grep -C 1 Trace= /usr/share/icewm/preferences 
#  Enable tracing for the given modules
Trace="icon"

Tried also with Trace=icon but still. :man_shrugging: Maybe i should try this in non-Rawhide Fedora...

gijsbers commented 4 years ago

Maybe /usr/share/icewm/preferences is only an example and not actually processed. If you run icewm --postpreferences | grep Trace you should see your setting. It is safer to do it in your user preferences. Or you can run icewm like: icewm --replace --trace=icon In any case you must see output lines with "IceWM: icon".

tim77 commented 4 years ago

I would really like to say that this was a reason but seems like /usr/share/icewm/preferences still valid:

LANG=en_US.UTF-8 icewm --postpreferences | grep Trace
IceWM: Warning: Session Manager: Init error: Could not open network socket
Trace="icon"
IceWM: A window manager is already running, use --replace to replace it

But good to know this anyway, thanks. Also if i try icewm --replace --trace=icon i end up in GDM and thats it. Maybe this is GDM issues since this Rawhide and there is pre-released 3.38 version? I should try thus with stable Fedora branches...

gijsbers commented 4 years ago

You can also run a Xephyr. This will create a new display for testing. Then you can start icewm directly with: icewm -d :1 --trace=icon The output will then be directly visible in your terminal.

tim77 commented 4 years ago

@gijsbers you awesome, Xephyr helps a lot here:

03:08:00.968: IceWM: icon open: /usr/share/icewm/themes/IceAdwaita-Medium/icons/web-browser_32x32.svg
03:08:00.983: IceWM: icon open: /usr/share/icewm/themes/IceAdwaita-Medium/icons/system-file-manager_32x32.svg
03:08:00.988: IceWM: icon open: /usr/share/icewm/themes/IceAdwaita-Medium/icons/utilities-terminal_32x32.svg
03:08:00.999: IceWM: icon open: /usr/share/icewm/icons/icewm_32x32.png
03:08:19.965: IceWM: icon open: /usr/share/icewm/themes/IceAdwaita-Medium/icons/folder_32x32.png
03:08:19.970: IceWM: icon open: /usr/share/icewm/themes/IceAdwaita-Medium/icons/windows_32x32.png
03:08:19.971: IceWM: icon open: /usr/share/icewm/themes/IceAdwaita-Medium/icons/help_32x32.png
03:08:19.974: IceWM: icon open: /usr/share/icewm/themes/IceAdwaita-Medium/icons/focus_32x32.png
03:08:19.975: IceWM: icon open: /usr/share/icewm/themes/IceAdwaita-Medium/icons/pref_32x32.png
03:08:19.977: IceWM: icon open: /usr/share/icewm/themes/IceAdwaita-Medium/icons/themes_32x32.png
03:08:19.978: IceWM: icon open: /usr/share/icewm/themes/IceAdwaita-Medium/icons/logout_32x32.png
03:10:54.048: IceWM: icon open: /usr/share/icewm/themes/IceAdwaita-Medium/icons/web-browser_16x16.png
03:10:54.049: IceWM: icon open: /usr/share/icewm/themes/IceAdwaita-Medium/icons/system-file-manager_16x16.png
03:10:54.051: IceWM: icon open: /usr/share/icewm/themes/IceAdwaita-Medium/icons/utilities-terminal_16x16.png
03:10:54.062: IceWM: icon open: /usr/share/icewm/icons/icewm_16x16.png
gijsbers commented 4 years ago

OK, this shows clearly the difference. This theme IceAdwaita-Medium has the unusual settings:

SmallIconSize = 24
MenuIconSize = 24

Perfectly acceptable, but apparently not tested for the new module. @Code7R should look into this.

Code7R commented 4 years ago

@gijsbers So that last commit was the fix, I guess? If so, thanks. I can reproduce the issue and after your changes the problem is gone. Hard to see what triggered it, though, due to massive code reformatting.


12:30:02.723: IceWM: icon open: /usr/local/share/icewm/themes/IceAdwaita-Medium/icons/windows_32x32.png
12:30:02.724: IceWM: icon open: /usr/local/share/icewm/themes/IceAdwaita-Medium/icons/help_32x32.png
12:30:02.724: IceWM: icon open: /usr/local/share/icewm/themes/IceAdwaita-Medium/icons/focus_32x32.png
tim77 commented 4 years ago

I can try and test build from master. I thought it still WIP. :)

gijsbers commented 4 years ago

I explained before that one can't fill a global static array on startup with static values when later on the configuration is read which might be much different. Hence the constructor and the lazy. I must say that I did find it hard to follow and needed to simplify here and there. Still difficult to read though.

Yes @tim77 it works now. I'm using this theme all the time. Looks good.

tim77 commented 4 years ago

Can confirm, work as expected with this patch and master snapshot. Thanks a lot @gijsbers @Code7R I сan't wait new 1.8.2 update and consider this major fix. :)

Screenshot_fedora-rawhide-gnome_2020-09-05_14:38:12

gijsbers commented 4 years ago

Can you again git pull and check latest additions?

tim77 commented 4 years ago

Can you again git pull and check latest additions?

:soon:

tim77 commented 4 years ago

LGTM. Tested with all three variants: 16px, 24px, 32px.

tim77 commented 4 years ago

Closing, since everything works as expected now in >= 1.8.2.