Inhibit Applet: Fix excessive CPU use

[lukefromdc]

Fix https://github.com/mate-desktop/mate-power-manager/issues/248

Note that this partially reverts https://github.com/mate-desktop/mate-power-manager/commit/6ecb73c1a876e48d2d2ef05603087bd83ac7022f by returning to the previous cairo rendering of the icon, but only of the icon.

The code for backgrounds behind the icons was not restored, as all backgrounds render fine without it so long as the applet is hidden and reshown on each change of icon. This must be done ONLY when the icon is changed, not everytime the draw callback runs.

The use of the GtkImage required use of gtk_widget_show in a callback that seems to run constantly, thus causing excess CPU use

[raveit65]

I fixed those warnings.

inhibit-applet.c: In function 'gpm_applet_click_cb':
inhibit-applet.c:347:18: warning: passing argument 1 of 'gtk_widget_hide' from incompatible pointer type [-Wincompatible-pointer-types]
  gtk_widget_hide(applet);
                  ^~~~~~
In file included from /usr/include/gtk-3.0/gtk/gtkapplication.h:27,
                 from /usr/include/gtk-3.0/gtk/gtkwindow.h:33,
                 from /usr/include/gtk-3.0/gtk/gtkdialog.h:32,
                 from /usr/include/gtk-3.0/gtk/gtkaboutdialog.h:30,
                 from /usr/include/gtk-3.0/gtk/gtk.h:31,
                 from /usr/include/mate-panel-4.0/libmate-panel-applet/mate-panel-applet.h:29,
                 from inhibit-applet.c:31:
/usr/include/gtk-3.0/gtk/gtkwidget.h:628:65: note: expected 'GtkWidget *' {aka 'struct _GtkWidget *'} but argument is of type 'GpmInhibitApplet *' {aka 'struct <anonymous> *'}
 void       gtk_widget_hide                (GtkWidget           *widget);
                                            ~~~~~~~~~~~~~~~~~~~~~^~~~~~
inhibit-applet.c:348:22: warning: passing argument 1 of 'gtk_widget_show_all' from incompatible pointer type [-Wincompatible-pointer-types]
  gtk_widget_show_all(applet);
                      ^~~~~~
In file included from /usr/include/gtk-3.0/gtk/gtkapplication.h:27,
                 from /usr/include/gtk-3.0/gtk/gtkwindow.h:33,
                 from /usr/include/gtk-3.0/gtk/gtkdialog.h:32,
                 from /usr/include/gtk-3.0/gtk/gtkaboutdialog.h:30,
                 from /usr/include/gtk-3.0/gtk/gtk.h:31,
                 from /usr/include/mate-panel-4.0/libmate-panel-applet/mate-panel-applet.h:29,
                 from inhibit-applet.c:31:
/usr/include/gtk-3.0/gtk/gtkwidget.h:632:65: note: expected 'GtkWidget *' {aka 'struct _GtkWidget *'} but argument is of type 'GpmInhibitApplet *' {aka 'struct <anonymous> *'}
 void       gtk_widget_show_all            (GtkWidget           *widget);
                                            ~~~~~~~~~~~~~~~~~~~~~^~~~~~

[raveit65]

Commits can be squashed. Old previous cairo code introduced a deprecation warnings but it was there before and i don't know to fix it. So this can be ignored, imo.

[raveit65]

@lukefromdc Thanks for very quick reaction....

[muktupavels]

Why draw signal was removed in first place? Did you know that draw signal already has cairo_t *cr parameter?

I doubt that drawing at random times (5 without signals in master) with cairo is right thing, you should use draw signal and related callback should be called only for this signal not in random places. When you want to redraw you should call gtk_widget_queue_draw...

Also hiding / showing applet on click seems just wrong. Whatever you are trying to fix with it, it is just some workaround not real fix.

Also I am quite curious how you go this far? By merging 6ecb73c1a876e48d2d2ef05603087bd83ac7022f? I think it should not be hard to imagine that gtk_image_set_from_pixbuf affects drawing... Quick look in gtk:

• https://gitlab.gnome.org/GNOME/gtk/blob/gtk-3-22/gtk/gtkimage.c#L1099
• https://gitlab.gnome.org/GNOME/gtk/blob/gtk-3-22/gtk/gtkiconhelper.c#L718
• https://gitlab.gnome.org/GNOME/gtk/blob/gtk-3-22/gtk/gtkiconhelper.c#L83
• https://gitlab.gnome.org/GNOME/gtk/blob/gtk-3-22/gtk/gtkiconhelper.c#L53

It was enough to look in just two files in gtk+ and you see that this function will queue redraw...

[lukefromdc]

The hide-and-show IS a workaround. Cairo docs say it cannot "erase," only paint over. Hiding and reshowing the applet when the icon changes works. As for the 5 calls to draw, that's not my code. Remember that this stuff dates back to the GTK2 days and we don't have the team size for a total refactoring

[lukefromdc]

Please retest again:

As per Albert's comments, I found a better way to do this: crib the relevent code from gnome-applets/inhibit for rendering the icon. It does use the GtkImage but it does not re-run gtk_widget_show over and over again, so CPU use stays down and the rendering is clean. Tested with system and custom panel backgrounds, both compiz and metacity set for the non-compositing case. Also works in both normal and highdpi cases, and though the icon scales in steps it is always cleanly rendered

I could not find the relevent GNOME commits that changed the code however, because the applets were stripped out of gnome-power-manager 3.0 and reappeared in gnome-applets only in 2015.

Will squash before merge, or revert and squash if others find the previous code to work better.

[lukefromdc]

Just fixed the copy-pasted double call of gpm_applet_update_tooltip and force-pushed so as not to add another commit

[raveit65]

This version works well here, low cpu load and scales nice with HIDPI. Both issues are fixed. Edit: ready to go.

[lukefromdc]

Squashed all before merging. As a rule I prefer to use code from GNOME when possible on backend bugfixes, as this reduces the difference between the forks, and because often the same bugs have already been squashed there, as was the case here.

[monsta]

For some reason the applet icon isn't zoomed when I increase panel size, but instead jumps from one fixed size to another. I tried brightness applet, it doesn't have this problem.

[raveit65]

For some reason the applet icon isn't zoomed when I increase panel size, but instead jumps from one fixed size to another.

Did that work before?

[monsta]

Yeah, it works if I build from previous commit in master. It zooms with panel size, even if panel is resized up to 100 px or more.

[raveit65]

Honestly, i prefer low cpu usage and a good looking icon with HIDIPi ;) So, i am fine with this commit.

[lukefromdc]

The current behavior is identical with the GNOME version, from which the code was copied. If you all want, I can add back code to force scaling but will be busy elsewhere today. Should not cause a CPU use problem, as that came from re-running gtk_widget_show over and over again

[lukefromdc]

I did some preliminary tests with calling gtk_image_set_pixel_size in gpm_applet_update_icon but found that on clicking the icon it would render as a thin line for some reason. Used this code:

static void gpm_applet_update_icon (GpmInhibitApplet applet) { const gchar icon; gint size;

size= applet->size - 2;

/* get icon */
if (applet->proxy == NULL) {
    icon = GPM_INHIBIT_APPLET_ICON_INVALID;
} else if (applet->cookie > 0) {
    icon = GPM_INHIBIT_APPLET_ICON_INHIBIT;
} else {
    icon = GPM_INHIBIT_APPLET_ICON_UNINHIBIT;
}
gtk_image_set_from_icon_name (GTK_IMAGE(applet->image),
                  icon,
                  GTK_ICON_SIZE_BUTTON);
gtk_image_set_pixel_size(GTK_IMAGE(applet->image), size);

}

I then found that adding gtk_image_set_pixel_size(GTK_IMAGE(applet->image), applet->size) to gpm_applet_click_cb would keep the icon showing-but render it nearly double-size! Dividing applet->size did nothing, a hardocded number worked. Seems that applet->size gets reset, possibly to zero when the icon is clicked

[muktupavels]

You probably want to modify gpm_applet_size_allocate_cb, it already calls gtk_image_set_pixel_size... And it seems that nothing sets applet->size...

[lukefromdc]

@Monstan, Try https://github.com/mate-desktop/mate-power-manager/pull/255 Which removes the GNOME code that forces icon size to original icon size