Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

workspace switcher: Add option to display windows icons #11530

Conversation

anaximeno
Copy link
Contributor

@anaximeno anaximeno commented Feb 24, 2023

Fixes #11455

The goal is to show the icon of the windows on the workspace-switcher applet.

To do:

  • Get the icon of the windows
  • Show the icon of the window in the center of its respective rectangle
  • Add settings to enable and disable this feature

@anaximeno anaximeno changed the title Draft: workspace-switcher applet: feat: display window's icons workspace-switcher applet: feat: display window's icons Feb 24, 2023
@anaximeno anaximeno marked this pull request as draft February 24, 2023 01:28
@anaximeno
Copy link
Contributor Author

So far, I've managed to get the St.Icon (or St.Bin) object that represents the icons of the windows, but I didn't find how to draw those icons in the Cairo context.

@lestcape
Copy link

@anaximeno While maybe your idea can be done, this probably is not the right way to go. You should avoid complex task of painting things from the cjs side and instead create it as an actor and let Muffin to draw it from the c side.

So, I think that instead of try to find a way to draw a texture/St.Icon you should probably try to find a way to set a Clutter actor (your texture) at top of the drawing area. Maybe this code should help you:

https://github.com/linuxmint/cinnamon/blob/master/js/ui/boxpointer.js#L47-L52

The other possibility that i not recommended is try to find the real icon of the app instead of a texture of it and then load it as a pixbuf, to then you can write it in the cairo context.

@anaximeno
Copy link
Contributor Author

Thanks for the suggestion @lestcape.

@anaximeno anaximeno changed the title workspace-switcher applet: feat: display window's icons [WIP] workspace-switcher applet: feat: display window's icons Apr 18, 2023
@anaximeno

This comment was marked as outdated.

@anaximeno anaximeno force-pushed the feat/workspace-switcher/show-window-icon branch from c4f29b2 to ca2d657 Compare August 24, 2023 20:13
@anaximeno anaximeno force-pushed the feat/workspace-switcher/show-window-icon branch from 33635e3 to 4d1675c Compare December 23, 2023 23:27
@anaximeno anaximeno force-pushed the feat/workspace-switcher/show-window-icon branch from 4d1675c to 1d86985 Compare January 4, 2024 23:48
This is not perfect yet. I have yet to handle some cases:
1. When the window is moved to an area close to the borders of the workspace representation,
2. Improve the position of the icon to be centered according to the size of the window representation,
3. Move the icon to the remaining position if there are any when the window is moved to a position close to the border
@anaximeno anaximeno marked this pull request as ready for review January 6, 2024 06:36
@anaximeno anaximeno changed the title [WIP] workspace-switcher applet: feat: display window's icons workspace-switcher applet: feat: display window's icons Jan 6, 2024
@anaximeno anaximeno changed the title workspace-switcher applet: feat: display window's icons workspace-switcher: add option to display windows icons Jan 6, 2024
This way we avoid code duplication and simplify the settings to activate this option, since the user will easilly see the option to activate and deactivate the window icons option
Now it will center the icon according to the intersected rectangle between the scaled window rectangle and the workspace graph. Had to use a bit of my geometry knowledge :)
This may also be more performant as we will calculate the intersection rect only once
It is not necessary
@anaximeno anaximeno changed the title workspace-switcher: add option to display windows icons workspace switcher: add option to display windows icons Jan 8, 2024
@mtwebster
Copy link
Member

Hi,

So this seems to work pretty well, but with one big issue - the icons are blurry.

I took a look at things, and you're loading the icons correctly, but you end up scaling the entire applet (or some other parent actor) which ruins the icons. You can see this by slowly increasing panel height (like mouse-wheel the slider) and observing the icons' sharpness changing as you go...

I didn't look at how the applet is graphically constructed, but possibly you need to add/improve size calculations so the layout is rendered without scaling (accounting for theme padding/margins maybe), or render the icons separately, on top of the scaled WindowGraphs.

@anaximeno
Copy link
Contributor Author

The icon is rendered separately I believe, it is added at the top of the window graph drawing area after drawing the window graph in the correct position on the workspace graph, the size of the icons is determined in the workspace-switcher settings so it is not modified according to the size of the workspace on the panel but rather manually by the user, maybe automatically updating the size of the icon according to the size of the workspace switcher applet on the panel could be a solution?

@mtwebster
Copy link
Member

Hmm assuming these are svgs (most app icons will be I think), maybe it's related to it being rendered 'in between' pixels. Maybe try rounding their position to a whole number? I'm not really well-versed in that sort of thing, but I know we've had icon sharpness issues in the past, when an icon wasn't aligned properly on its canvas.

@lestcape
Copy link

@anaximeno the icon have 12 px by default. It's difficult that it will not be blurry at that size. The smallest icon size Cinnamon uses have 24 pixels and normal app icon have at less 16px, while most of common apps icons are 32px at less. So, use 12px means that the icon was scaled for sure.

I recommended you select always standard icons sizes with more than 24px, and do not allow other size, because other size means a re-scaled the icons. You should see: /usr/share/icons/hicolor for the standard icons size. And also you can have an idea of who may apps have an icon in the select a size and how it luck like if you enter to the folder of the size you want. For example for the size 32px it should be: /usr/share/icons/hicolor/32x32/apps

I don´t think you can improve your implementation in that regards, but you can smart selected the preferred icon size to improve the result.

@anaximeno
Copy link
Contributor Author

I would prefer not limiting the feature to only sizes greater or equal to 24px because that would make it almost unusable on smaller screens and panels (here's an example with the panel height = 28):

  • 24 px
    screenshot-auto_1707924409

  • 8 px
    screenshot-auto_1707924437

And to be fair the blurriness is barely noticeable on smaller screens, though I suspect it might be more noticeable on wider screens, I will test with a wider monitor and see how bad it is.

@lestcape
Copy link

Separate point: You should take the scalefactor on account in the icon size. Currently you only have on account the position, not the size. It probably should help to be possible display the icon with a better resolution in a screen with a big scalefactor.

@anaximeno
Copy link
Contributor Author

anaximeno commented Feb 15, 2024

Separate point: You should take the scalefactor on account in the icon size. Currently you only have on account the position, not the size. It probably should help to be possible display the icon with a better resolution in a screen with a big scalefactor.

I believe you are talking about the global.ui_scale of the monitor, for what I've observed, I believe Cinnamon (or Muffin) already takes care of applying scaling to the icons, so I believe it wouldn't be necessary to scale the size of the icon on the code when instantiating the icon. But I have to take it into account while calculating the position where the icon should be placed because the real size of the icon is bigger (because of the scaling) than the one I sent while creating the icon instance.

@anaximeno anaximeno force-pushed the feat/workspace-switcher/show-window-icon branch from bb3cfe9 to 8f882c3 Compare February 15, 2024 05:05
chore: Improve focus window update
@anaximeno anaximeno force-pushed the feat/workspace-switcher/show-window-icon branch from 3945ca6 to f0dfd1d Compare April 11, 2024 03:59
@anaximeno anaximeno changed the title workspace switcher: add option to display windows icons workspace switcher: Add option to display windows icons May 17, 2024
@anaximeno anaximeno force-pushed the feat/workspace-switcher/show-window-icon branch from 0659d1f to e77e3ac Compare May 17, 2024 20:52
@anaximeno
Copy link
Contributor Author

I will recreate this PR with a cleaner commit log.

@anaximeno
Copy link
Contributor Author

Replaced by #12202

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show an Icon representation of the window in the workspace-switcher applet
3 participants