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

Fix missing dominant color #2045

Merged
merged 3 commits into from
Aug 14, 2023
Merged

Fix missing dominant color #2045

merged 3 commits into from
Aug 14, 2023

Conversation

jrom99
Copy link
Contributor

@jrom99 jrom99 commented Jun 16, 2023

This is the same attempt to fix #1493, in which we separate Gio.ThemedIcon from Gio.LoadableIcon but now with the const keyword and using list unpacking, as is done in similar pieces around the file.

I apologise for closing the previous merge request, GitHub asked to remove my previous commit when I clicked to sync my branch.

I read the documentation on how to cache and async it, but I couldn't make sense of the documentation, sorry.

When iconTexture is a Gio.ThemedIcon, it seems we can use choose_icon or lookup_icon, and the former is better suited as it's able to check for all possible names (which is useful as Papirus theme doesn't use the first name available for drive-harddisk).

There's also a fourth case that I have trouble replicating, in which the icon is an emblem (I don't know the icon type), such as when an external HD is mounted. This doesn't apply to other emblem-like icons, such as mounted network devices and the trash can. This emblem raises a iconTexture.load is not a function error.

jrom99 added 3 commits June 16, 2023 10:38
Some icons are not `Gio.ThemedIcon`, but ` Gio.LoadableIcon`, so we can get their pixel buffer from memory. Updated with `const` and list unpacking as used elsewhere in the file.
If the dominant color is missing (icon cannot be loaded), then a sensible fallback is used so that a running dot is still displayed.
Replaced `lookup_icon` with `choose_icon`, since it keeps looking in the current theme for a themed icon using all of its available names instead of only one. While `Gio.ThemedIcon` offers a `iconTexture.load_icon()`, it only looks for the first name available and fails otherwise.

Also, `iconInfo` has been renamed to `iconBuffer` since `Gio.LoadableIcon` returns a stream/buffer (or should outright fail, which I could not observe yet).

Could not find an example of app that uses a `Gio.EmblemedIcon` or any other implementation (i.e.: lacking `iconTexture.load`) that would justify another branching.
@3v1n0
Copy link
Collaborator

3v1n0 commented Aug 14, 2023

Looks good, thanks a lot!

@3v1n0 3v1n0 merged commit cc089fe into micheleg:master Aug 14, 2023
@scarlion1
Copy link

I'm on v99 on GNOME Shell 47.2 and getting many JS ERROR: TypeError: iconTexture.load is not a function messages (below) with external USB LUKS encrypted device unlocked and mounted.  In this state it has an unlocked padlock emblem on the dock icon.
ℰ.ℊ.:
image

I couldn't find iconTexture.load mentioned anywhere else.  Is it related to this issue, or something I should put in a new issue?  Thx

One set of JS ERROR: TypeError: iconTexture.load is not a function messages (with diff's highlighted)
_getIconPixBuf@file:///home/scar/.local/share/gnome-shell/extensions/dash-to-dock@micxgx.gmail.com/appIconIndicators.js:1174:42
_getColorPalette@file:///home/scar/.local/share/gnome-shell/extensions/dash-to-dock@micxgx.gmail.com/appIconIndicators.js:1190:29
_computeStyle@file:///home/scar/.local/share/gnome-shell/extensions/dash-to-dock@micxgx.gmail.com/appIconIndicators.js:383:67
_updateIndicator@file:///home/scar/.local/share/gnome-shell/extensions/dash-to-dock@micxgx.gmail.com/appIconIndicators.js:409:14
_onOverviewHiding@file:///home/scar/.local/share/gnome-shell/extensions/dash-to-dock@micxgx.gmail.com/theming.js:91:21
_callHandlers@resource:///org/gnome/gjs/modules/core/_signals.js:130:42
_emit@resource:///org/gnome/gjs/modules/core/_signals.js:119:10
_changeShownState@resource:///org/gnome/shell/ui/overview.js:257:14
_animateNotVisible@resource:///org/gnome/shell/ui/overview.js:578:14
hide@resource:///org/gnome/shell/ui/overview.js:562:14
activateWindow@resource:///org/gnome/shell/ui/main.js:834:14
_onCloneSelected@resource:///org/gnome/shell/ui/workspace.js:1401:18
_activate@resource:///org/gnome/shell/ui/windowPreview.js:556:14
_init/<@resource:///org/gnome/shell/ui/windowPreview.js:117:51
@resource:///org/gnome/shell/ui/init.js:21:20

_getIconPixBuf@file:///home/scar/.local/share/gnome-shell/extensions/dash-to-dock@micxgx.gmail.com/appIconIndicators.js:1174:42
_getColorPalette@file:///home/scar/.local/share/gnome-shell/extensions/dash-to-dock@micxgx.gmail.com/appIconIndicators.js:1190:29
_computeStyle@file:///home/scar/.local/share/gnome-shell/extensions/dash-to-dock@micxgx.gmail.com/appIconIndicators.js:383:67
_updateIndicator@file:///home/scar/.local/share/gnome-shell/extensions/dash-to-dock@micxgx.gmail.com/appIconIndicators.js:409:14
_updateSolidStyle@file:///home/scar/.local/share/gnome-shell/extensions/dash-to-dock@micxgx.gmail.com/theming.js:452:29
_callHandlers@resource:///org/gnome/gjs/modules/core/_signals.js:130:42
_emit@resource:///org/gnome/gjs/modules/core/_signals.js:119:10
_changeShownState@resource:///org/gnome/shell/ui/overview.js:257:14
_animateNotVisible@resource:///org/gnome/shell/ui/overview.js:578:14
hide@resource:///org/gnome/shell/ui/overview.js:562:14
activateWindow@resource:///org/gnome/shell/ui/main.js:834:14
_onCloneSelected@resource:///org/gnome/shell/ui/workspace.js:1401:18
_activate@resource:///org/gnome/shell/ui/windowPreview.js:556:14
_init/<@resource:///org/gnome/shell/ui/windowPreview.js:117:51
@resource:///org/gnome/shell/ui/init.js:21:20

_getIconPixBuf@file:///home/scar/.local/share/gnome-shell/extensions/dash-to-dock@micxgx.gmail.com/appIconIndicators.js:1174:42
_getColorPalette@file:///home/scar/.local/share/gnome-shell/extensions/dash-to-dock@micxgx.gmail.com/appIconIndicators.js:1190:29
_computeStyle@file:///home/scar/.local/share/gnome-shell/extensions/dash-to-dock@micxgx.gmail.com/appIconIndicators.js:383:67
_updateIndicator@file:///home/scar/.local/share/gnome-shell/extensions/dash-to-dock@micxgx.gmail.com/appIconIndicators.js:409:14
_updateSolidStyle@file:///home/scar/.local/share/gnome-shell/extensions/dash-to-dock@micxgx.gmail.com/theming.js:453:29
_callHandlers@resource:///org/gnome/gjs/modules/core/_signals.js:130:42
_emit@resource:///org/gnome/gjs/modules/core/_signals.js:119:10
_changeShownState@resource:///org/gnome/shell/ui/overview.js:257:14
_animateNotVisible@resource:///org/gnome/shell/ui/overview.js:578:14
hide@resource:///org/gnome/shell/ui/overview.js:562:14
activateWindow@resource:///org/gnome/shell/ui/main.js:834:14
_onCloneSelected@resource:///org/gnome/shell/ui/workspace.js:1401:18
_activate@resource:///org/gnome/shell/ui/windowPreview.js:556:14
_init/<@resource:///org/gnome/shell/ui/windowPreview.js:117:51
@resource:///org/gnome/shell/ui/init.js:21:20

_getIconPixBuf@file:///home/scar/.local/share/gnome-shell/extensions/dash-to-dock@micxgx.gmail.com/appIconIndicators.js:1174:42
_getColorPalette@file:///home/scar/.local/share/gnome-shell/extensions/dash-to-dock@micxgx.gmail.com/appIconIndicators.js:1190:29
_computeStyle@file:///home/scar/.local/share/gnome-shell/extensions/dash-to-dock@micxgx.gmail.com/appIconIndicators.js:383:67
_updateIndicator@file:///home/scar/.local/share/gnome-shell/extensions/dash-to-dock@micxgx.gmail.com/appIconIndicators.js:409:14
_onOverviewHidden@file:///home/scar/.local/share/gnome-shell/extensions/dash-to-dock@micxgx.gmail.com/docking.js:769:14
_callHandlers@resource:///org/gnome/gjs/modules/core/_signals.js:130:42
_emit@resource:///org/gnome/gjs/modules/core/_signals.js:119:10
_changeShownState@resource:///org/gnome/shell/ui/overview.js:257:14
_hideDone@resource:///org/gnome/shell/ui/overview.js:594:18
_animateNotVisible/<@resource:///org/gnome/shell/ui/overview.js:579:55
onStopped@resource:///org/gnome/shell/ui/overviewControls.js:748:21
_makeEaseCallback/<@resource:///org/gnome/shell/ui/environment.js:65:22
_easeActorProperty/<@resource:///org/gnome/shell/ui/environment.js:247:60
@resource:///org/gnome/shell/ui/init.js:21:20

@jrom99
Copy link
Contributor Author

jrom99 commented Dec 31, 2024

It seems that you've found the hard-to-replicate bug lol. From what I've understood before, in this bug the icon's emblem (usually the notification counter, but maybe the padlock in your case?) is retrieved as the iconTexture instead of the actual icon.

So, to be clear, does the icon not appear on your dock, does it have the wrong dominant color, or are you only concerned about the messages?

if (iconTexture instanceof Gio.FileIcon) {
// Use GdkPixBuf to load the pixel buffer from the provided file path
return GdkPixbuf.Pixbuf.new_from_file(iconTexture.get_file().get_path());
} else if (iconTexture instanceof Gio.ThemedIcon) {
// Get the first pixel buffer available in the icon theme
const iconNames = iconTexture.get_names();
const iconInfo = themeLoader.choose_icon(iconNames, DOMINANT_COLOR_ICON_SIZE, 0);
if (iconInfo)
return iconInfo.load_icon();
else
return null;
}
// Use GdkPixBuf to load the pixel buffer from memory
// iconTexture.load is available unless iconTexture is not an instance of Gio.LoadableIcon
// this means that iconTexture is an instance of Gio.EmblemedIcon,
// which may be converted to a normal icon via iconTexture.get_icon?
const [iconBuffer] = iconTexture.load(DOMINANT_COLOR_ICON_SIZE, null);
return GdkPixbuf.Pixbuf.new_from_stream(iconBuffer, null);

@scarlion1
Copy link

It seems that you've found the hard-to-replicate bug lol.

seems to be something i'm cursed with 😩

to be clear, does the icon not appear on your dock,

The icon (and emblem) appear on my dock, that's where i got the picture from in my post.

does it have the wrong dominant color,

The color is fine but not sure what you mean by dominant color.  I checked the settings and it seems to only be mentioned in regards to the window counter indicators that show up under the icons, which are working fine and use an appropriate color with "Use dominant color" option on.

or are you only concerned about the messages?

Yes, this, since I'm running Debian testing and monitoring the system messages/journal.

I guess maybe I was confused about the error message initially and how it relates to this pull, but this was the only place i found iconTexture.load mentioned.  Now with your question and going through all the settings, it seems the same error has resurfaced but it's not related to the window counter indicators but the icon emblem instead?  I don't really need to see the volumes/devices, so if I turn off that option (in Launchers tab), the messages stop at least.  Maybe this should be in a new issue though?

@jrom99
Copy link
Contributor Author

jrom99 commented Jan 1, 2025

The dominant color is used for these window counters as well as a glossy highlight replicating the old Unity dock.

My theory is that dash-to-dock is trying and failing to get the dominant color of the padlock emblem (probably yellow) and apply it over the original dominant color (probably blue). I agree that this should be a new issue, because I don't know what would be the expected behavior here (ie: should we ignore the emblem, interpolate both colors, use only the emblem color?), and also because it has a different behavior (even if the same cause).

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.

Dominant color CSS doesn't work for some apps
3 participants