GNOME Bugzilla – Bug 645648
don't redraw the calendar/clock when they haven't changed
Last modified: 2011-04-11 18:02:01 UTC
while debugging network menu stuff, I noticed that the calendar boxpointer was getting relayouted every second. No need for that.
Created attachment 184204 [details] [review] dateMenu: only update the date/time labels if they have changed Setting the contents of the labels triggers reallocation and redrawing, which is pointless if we're just setting it to what it already was.
Review of attachment 184204 [details] [review]: Looks good. (Up to you to request a freeze break or wait until the release is through)
Review of attachment 184204 [details] [review]: 3.0.1 in my opinion. Patch looks good, but maybe it would make more sense to do this inside StLabel? (Consider iteration with label.get_clutter_text().set_markup() followed by label.set_text() in that case)
Created attachment 185232 [details] [review] st_label_set_text: no-op if the text is unchanged If a caller sets an StLabel's text to what it already is (as, eg, the clock menu does), do nothing. Unless the label is editable, in which case, setting the text has a visible side effect (dropping the selection), so we don't optimize that out.
Review of attachment 185232 [details] [review]: ::: src/st/st-label.c @@ +387,2 @@ + if (clutter_text_get_editable (ctext) || + strcmp (clutter_text_get_text (ctext), text) != 0) Shouldn't that be g_strcmp0(), as clutter_text_get_text() may return NULL?
Created attachment 185426 [details] [review] st_label_set_text: no-op if the text is unchanged Wacky. Calling clutter_text_set_text(ctext NULL) will set its text to "", but setting its "text" property to NULL will set it to NULL. This seems like it's probably a ClutterText bug, but...
Review of attachment 185426 [details] [review]: Looks good to me.
Comment on attachment 185232 [details] [review] st_label_set_text: no-op if the text is unchanged my bad. setting the text property to NULL actually causes a crash. so in fact, there is no way for clutter_text_get_text() to return NULL
(In reply to comment #9) > (From update of attachment 185232 [details] [review]) > my bad. setting the text property to NULL actually causes a crash. so in fact, > there is no way for clutter_text_get_text() to return NULL Mmmh, okay ... still a bit wary, as the code in clutter-text.c checks for NULL before accessing the text member. But even new Clutter.Text().get_text() returns "" rather than null, so I guess it's safe after all.
I ended up going with the strcmp0 patch anyway. Clutter bug: http://bugzilla.clutter-project.org/show_bug.cgi?id=2629 Attachment 185426 [details] pushed as a56bc9d - st_label_set_text: no-op if the text is unchanged