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.

Sign up for GitHub

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

Jump to bottom

Some icons can't be found #1116

Open
ByteDrummer opened this issue Oct 8, 2022 · 3 comments · May be fixed by #1302
Open

Some icons can't be found #1116

ByteDrummer opened this issue Oct 8, 2022 · 3 comments · May be fixed by #1302

Comments

@ByteDrummer
Copy link

Issue description

Run notify-send --icon=nm-no-connection hgfhgfhgf. This icon wont be found

Installation info

  • Version: 1.9.0
  • Install type: Arch official package
  • Window manager / Desktop environment: BSPWM
Minimal dunstrc 
[global]
width="(0, 1200)"
height=9999
enable_recursive_icon_lookup=yes
icon_theme=Papirus
max_icon_size=50
@apprehensions
Copy link
Contributor

Try setting a specific icon_path with the relative icon name and path root directory

@ByteDrummer
Copy link
Author

Yeah, that works. For some reason, the recursive lookup algorithm is skipping the /usr/share/icons/Papirus/24x24/panel/ directory where this icon is contained unless I specify min_icon_size = 24. This isn't ideal, though, because then all theme icons are 24x24 when enable_recursive_icon_lookup is enabled.

@just-max
Copy link

just-max commented May 12, 2023 •
edited

The problem is the use of n->min_icon_size here:

dunst/src/notification.c

Line 340 in dfab9f0

n->icon_path = get_path_from_icon_name(new_icon, n->min_icon_size);

which, assuming recursive icon lookup is enabled, looks for icons with the given size:

dunst/src/icon.c

Lines 217 to 221 in dfab9f0

if (settings.enable_recursive_icon_lookup) {
char *path = find_icon_path(iconname, size);
LOG_I("Found icon at %s", path);
return path;
}

which then calls, in turn find_icon_in_theme_with_inherit and find_icon_in_theme, which only matches directories for exactly the given size, unless the icon directory is scalable[1] or scalable within a threshold[2]:

dunst/src/icon-lookup.c

Lines 250 to 263 in dfab9f0

switch (dir.type) {
case THEME_DIR_FIXED:
match_size = dir.size == size;
break;
case THEME_DIR_SCALABLE:
match_size = dir.min_size <= size && dir.max_size >= size;
break;
case THEME_DIR_THRESHOLD:
match_size = (float)dir.size / dir.threshold <= size
&& dir.size * dir.threshold >= size;
break;
}

I think the problem is that this part of the XDG spec isn't implemented:

As soon as there is an icon of any size that matches in a theme, the search is stopped. Even if there may be an icon with a size closer to the correct one in an inherited theme, we don't want to use it. Doing so may generate an inconsistant change in an icon when you change icon sizes (e.g. zoom in).

Dunst only looks for icons of exactly matching sizes, and doesn't fall back to other sizes.

So basically there are two problems:

  1. Using min_icon_size as the icon size [3]
  2. Not looking up alternative sizes

The fix for 2 is obvious, just find the "best" sized icon when the exact match is not available as in the XDG spec.

Not sure what the proper fix to 1 would be: we could add an icon_size option which is the size to prefer. min_icon_size and max_icon_size would remain to scale icons that are too small/big.

[1] Although Dunst doesn't seem to find scalable icons either, unless XDG_DATA_DIRS=/usr/local/share/:/usr/share/ is set manually...

[2] Not relevant here, but thresholds are actually supposed to be checked by offset, not by a factor:

if Type is Threshold
    return Size - Threshold <= iconsize <= Size + Threshold

[3] Apparently using min_icon_size as the icon size is intentional behaviour: #1094 (comment).

@bynect bynect mentioned this issue Mar 2, 2024
Open
@bynect bynect linked a pull request Apr 9, 2024 that will close this issue
Make recursive icon lookup work with size == 0 #1302
Draft
tobyvin added a commit to tobyvin/.dotfiles that referenced this issue May 14, 2024
@tobyvin
fix(dunst): add workaround to fix recursive...
80e3a19 
...icon search

Ref: dunst-project/dunst#1116
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make recursive icon lookup work with size == 0 bynect/dunst
3 participants
@ByteDrummer @just-max @apprehensions