GNOME Bugzilla – Bug 743743
display: Replace CcRRLabeler with gnome-shell's DBus monitor labeler
Last modified: 2015-02-05 13:37:31 UTC
CcRRLabeler assumes an X11 session using override redirect toplevel windows which are positioned in global desktop coordinates. Since this isn't available on Wayland sessions, we're moving to a DBus API which gnome-shell implements and works the same on both X11 an Wayland sessions. As side-effects, we get native looking gnome-shell OSD labels without having to mimic the style ourselves and we're also now able to easily show labels on mirrored setups.
Created attachment 295806 [details] [review] display: Replace CcRRLabeler with gnome-shell's DBus monitor labeler
Review of attachment 295806 [details] [review]: ::: panels/display/cc-display-panel.c @@ +127,3 @@ + for (info = infos; *info; info++) + { + gint number = cc_display_panel_get_output_id (*info); Separate the declaration and assignment. @@ +133,3 @@ + output = gnome_rr_screen_get_output_by_name (priv->screen, + gnome_rr_output_info_get_name (*info)); + str = g_strdup_printf ("%d", number); Any reason why the API takes a string instead of a number? @@ +170,3 @@ + windows = gtk_window_list_toplevels (); + + for (w = windows; w; w = w->next) Add braces for the multi-line code block (the for loop) @@ +2408,3 @@ + { + if (!g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) + g_warning ("Failed to contact gnome-shell: %s\n", error->message); No need for the extra linefeed. @@ +2414,3 @@ + + priv = self->priv; + priv->shell_proxy = proxy; self->priv->shell_proxy = proxy; ?
Created attachment 295925 [details] [review] display: Replace CcRRLabeler with gnome-shell's DBus monitor labeler -- (In reply to comment #2) > Any reason why the API takes a string instead of a number? Just because it's a bit more flexible. Currently we just number the outputs but in the past (gnome 2.x, not sure when it changed exactly) we used to show the monitor vendor and model. If we ever want to change it again then we won't have to change the DBus API. All the other comments are fixed, thanks.
(In reply to comment #3) > Created an attachment (id=295925) [details] [review] > display: Replace CcRRLabeler with gnome-shell's DBus monitor labeler > > -- > (In reply to comment #2) > > Any reason why the API takes a string instead of a number? > > Just because it's a bit more flexible. Currently we just number the > outputs but in the past (gnome 2.x, not sure when it changed exactly) > we used to show the monitor vendor and model. If we ever want to > change it again then we won't have to change the DBus API. Then I'd rather it be passed as a GVariant, so that we can put either a string or an int in it. > All the other comments are fixed, thanks.
Created attachment 296193 [details] [review] display: Replace CcRRLabeler with gnome-shell's DBus monitor labeler -- Ok, the DBus API has been changed to take a{uv} now.
Created attachment 296194 [details] [review] display: Replace CcRRLabeler with gnome-shell's DBus monitor labeler -- Sending an int inside the variant instead of a string now.
Review of attachment 296194 [details] [review]: Looks good.
Attachment 296194 [details] pushed as c6970ec - display: Replace CcRRLabeler with gnome-shell's DBus monitor labeler