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

Raise max prop len #85

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

sgraf812
Copy link

256000 bytes was not enough to store 512x512 icons onwards.
The current setting allows up to 2048x2048 icons and should be quite future proof.

256000 bytes was not enough to store 512x512 icons onwards.
The current setting allows up to 2048x2048 icons and should be quite
future proof.
@sagb
Copy link
Owner

sagb commented May 12, 2019

This limit is for icons in _NET_WM_ICON property.
They are parsed, transformed to Pixmap format, copied and resized on-the-fly, right after user press alt-tab and waits for something to appear on the screen.
Are you sure we should allow this data be 32 MB per window?

@sgraf812
Copy link
Author

Well, it's not like the 16MB will need to be initialised, at least I don't think that's what XGetWindowProperty does. So we aren't really paying for it at this point. Rather the current behavior opens up room for surprising behavior, where icons are present but weren't loaded.

So I think the right thing would be to make the buffer as big as the data needs it to be.

The actual question here is whether icons of size 512x512 are supported or not. Spotify and VSCode are just two applications that sadly don't provide supersampled versions of their icons. Since I use both, I'd very much like them to show up properly.

I just spun up 10 instances of VSCode (which has 1024x1024 = 4MB icons) and the delay is noticable (a few hundred ms). We could work around that with some amount of caching, I guess, but I probably won't be the one to implement it, so...

@sagb
Copy link
Owner

sagb commented May 12, 2019

I've installed spotify and vscode. Indeed, they have 512x and 1024x icons in _NET_WM_ICON property.
There are also badly named file icons. The following makes them recognized by alttab:

ln /usr/share/spotify/icons/spotify-linux-32.png /usr/share/icons/hicolor/32x32/apps/spotify.png
mkdir -p /usr/share/icons/hicolor/1024x1024/apps && ln /usr/share/pixmaps/com.visualstudio.code.png /usr/share/icons/hicolor/1024x1024/apps/code.png 

sagb added a commit that referenced this pull request Mar 14, 2021
The memory that is being allocated for each icon is limited for speed
(related: #85).
When the application has huge icons (1024x1024), this triggered a bug:
the allocated memory was not freed (#107).
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.

2 participants