GitLab

If a window doesn't have a desktop file associated with it and gnome-shell wants to display the "blah is now ready" notification, it spews to the log this message:

value "-1.000000" of type 'gfloat' is invalid or out of range for property 'height' of type 'gfloat'`

This is because since commit 3eb80dc6 gnome-shell has been passing -1 for the icon size. This isn't normally a problem since the size gets pushed into an StIcon which interprets -1 to mean "pull from css".

Unfortunately, if there is no app info associated with the application, an StIcon is not used and instead the icon is pulled from the MetaWindow as a cairo surface and rendered into a StWidget. In this case, -1 is used to set the size of the ClutterImage which leads to the above message.

We could fix this by making the MetaWindow have a new property gicon to returns a GIcon instead of a cairo surface. We could then hook it straight in an StIcon just like we do in the case the app has app info.

relevant irc discussion

[15:14:23] <halfline> fmuellner: hey are you around ?
[15:14:42] <fmuellner> halfline: yup
[15:15:22] <halfline> cool, so i'm looking at this 7.5 memory leak thing (bug 1717000) and there's a bit of a side tangent in the bug where it prints
[15:15:23] <halfline> value "-1.000000" of type 'gfloat' is invalid or out of range for property 'height' of type 'gfloat'
[15:15:23] <bugbot> Bug https://bugzilla.gnome.org/show_bug.cgi?id=1717000 was not found.
[15:15:26] <halfline> to the log
[15:15:32] <halfline> (red hat bugzilla)
[15:15:54] <halfline> and from some debugging the customer got a stack trace of the problem
[15:16:22] <halfline> turns out we're passing -1 to ...
[15:16:40] <halfline> shell_app_create_icon_texture
[15:17:03] <halfline> which would normally be okay except shell_app_create_icon_texture will sometimes ask mutter for the icon to use
[15:17:48] <halfline> normally -1 would mean "ask the theme"
[15:18:06] <halfline> but when we're getting the icon from mutter we're not putting it in an st icon
[15:18:12] <halfline> so it's not getting themed 
[15:18:43] <halfline> at one point, we just hard coded "16" for the number
[15:18:49] <halfline> but then ... hang on
[15:20:45] <halfline> someone complained about the way the icon looked here: http://bugzilla.gnome.org/show_bug.cgi?id=788265
[15:20:46] <bugbot> Bug 788265: gnome-shell, normal, Normal, ---, gnome-shell-maint, RESOLVED FIXED, notification icon too small on high resolution displays
[15:22:01] <halfline> so i think the best way to fix this is to make sure we always stuff the icon in an st icon
[15:22:43] <halfline> even in the window_backed_app_get_icon case
[15:22:59] <fmuellner> mmh
[15:23:02] <halfline> but that seems kind of tricky since the mutter icon is a cairo surface
[15:23:08] <halfline> do you agree that's the best way to go ?
[15:23:26] <halfline> if so, i guess we should add a new GICON property on MetaWindow next to the ICON property maybe ?
[15:25:23] <fmuellner> the whole -1 thing is a bit fishy - in StIcon, it means "use size from theme", in parts of StTextureCache it means "use the 'natural' image size", and in other parts it means "I'm gonna blow up" :-(
[15:26:13] <fmuellner> so yeah, off-hand using an StIcon consistently looks like an improvement
[15:26:18] <halfline> right
[15:29:00] <halfline> oh wait
[15:29:09] <halfline> i think this may already be dealt with upstream
[15:29:58] <halfline> yea actually it is
[15:30:19] <halfline> commit 19c60ff5c52fc866ad3c24c7bd6065e8b28df3e5 got rid of the g_object_set call that's causing the problem
[15:30:52] <halfline> and commit 4f65283f3160bba4beb1202fbd7d3436fbdb34fc provides a way to theme it
[15:31:59] <halfline> alright good enough for me
[15:33:42] <fmuellner> halfline: I don't think that's true
[15:34:14] <halfline> don't think it's fixed?
[15:34:41] <fmuellner> commit 6f794738e82a33f356f5069c36cbffff846c7acf adds back a g_object_new(..., "height", size, ...) call
[15:35:17] <halfline> heh
[15:38:12] <fmuellner> i suppose a quick fix would be to move height/width out of the new() call, and use clutter_actor_set_size()
[15:39:20] <fmuellner> but that probably doesn't end up with an icon size from the theme, but with whatever size the cairo surfaces uses
[15:39:48] <halfline> right
[15:39:55] <halfline> which is probably 48px or something
[15:40:14] <halfline> so it'll be a big icon next to a bunch of little icons
[15:41:07] <fmuellner> there's also another issue - some recent commit removed the fallback icon code in mutter
[15:41:18] <fmuellner> so that property may actually be unset
[15:41:27] <fmuellner> (for example for all wayland windows)
[15:42:24] <fmuellner> if we use an StIcon, then there's a fallback-icon-name property we could use in the shell
[15:42:48] <fmuellner> (nothing breaks when the icon is empty, it just shows up empty)
[15:44:37] <halfline> alright so maybe should back to my original proposal
[15:44:46] <halfline> add a GIcon property to MetaWindow
[15:45:22] <halfline> and just st_icon_set_gicon from it instead of from g_app_info_get_icon
[15:45:31] <halfline> if there's no app info available
[15:47:03] <halfline> i guess need to be a little smarter to catch the icon changing
[15:54:00] <fmuellner> halfline: window.bind_property('gicon', icon, 'gicon', GObject.BindingFlags.SYNC_CREATE)?
[16:03:42] <halfline> fmuellner: yea i guess that would work... well g_object_bind_property (window, "gicon", icon, "gicon", G_BINDING_FLAGS_SYNC_CREATE);
[16:03:55] <halfline> since this is C code
[16:05:25] <halfline> alright, i'll start by filing the issue with the plan and see if i can hammer it out real quick before the customer gets back with more info on the main issue