Merge lp:~3v1n0/gtk/unity-border-radius-support into lp:~ubuntu-desktop/gtk/ubuntugtk3

Proposed by Marco Trevisan (Treviño)
Status: Merged
Approved by: Sebastien Bacher
Approved revision: 511
Merged at revision: 509
Proposed branch: lp:~3v1n0/gtk/unity-border-radius-support
Merge into: lp:~ubuntu-desktop/gtk/ubuntugtk3
Diff against target: 363 lines (+279/-56)
4 files modified
debian/changelog (+9/-0)
debian/patches/series (+1/-1)
debian/patches/unity-border-radius.patch (+269/-0)
debian/patches/unity_rbga_tooltips.patch (+0/-55)
To merge this branch: bzr merge lp:~3v1n0/gtk/unity-border-radius-support
Reviewer Review Type Date Requested Status
Sebastien Bacher Approve
Allison Karlitskaya (community) Needs Fixing
Review via email: mp+288331@code.launchpad.net

Description of the change

Export windows corners radius as an X11 property in unity.

It works together with lp:~3v1n0/unity/gtk-border-radius-support/+merge/288358

To post a comment you must log in.
505. By Marco Trevisan (Treviño)

debian/patches/unity-border-radius.patch: Export windows corners radius as an X11 property in unity

506. By Marco Trevisan (Treviño)

debian/patches/unity-border-radius.patch:
 - Don't draw window border in unity for any theme. Add unity-csd style class.

507. By Marco Trevisan (Treviño)

Merging with lp:~ubuntu-desktop/gtk/ubuntugtk3

Revision history for this message
Allison Karlitskaya (desrt) wrote :

Looks pretty good, but there are some things that I would consider changing.

I am not an expert in GTK theming by any measure, however, so my suggestions might be pure insanity. I just took a look at how some other widgets are doing things.

One comment in general: do we really need to query the title widget to find out the borders that will be drawn on the toplevel? Something seems wrong here. I would have expected that we can query this information directly from the window itself in some way (and avoid the ugly hacks about assuming zeros).

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

> One comment in general: do we really need to query the title widget to find
> out the borders that will be drawn on the toplevel? Something seems wrong
> here. I would have expected that we can query this information directly from
> the window itself in some way (and avoid the ugly hacks about assuming zeros).

Not as far I know... The window itself has not rounded corners, but it can have a titlebox which has rounded corners.
By defining a border-radius to the window itself, gives no rounded corners by default. It might be something that will work in the future, though. But not right now AFAIK.

To remove the workaround we could get these values from the toplevel, and in case a titlebox is defined we get them from that, but we can't query them from the toplevel, from what I see.

Revision history for this message
Marco Trevisan (Treviño) (3v1n0) :
Revision history for this message
Allison Karlitskaya (desrt) :
508. By Marco Trevisan (Treviño)

debian/patches/unity-border-radius.patch:
  - use better style
  - take care of window scaling (and update values on scale changes)
  - use memcmp instead of manual checks
  - only check changes if border radius properties changed (using "style-updated" signal)
  - remove unwanted change

509. By Marco Trevisan (Treviño)

debian/patches/unity-border-radius.patch: don't cache window borders value

Since we only update when they change, we can avoid this.

510. By Marco Trevisan (Treviño)

Merging with lp:~ubuntu-desktop/gtk/ubuntugtk3

Revision history for this message
Allison Karlitskaya (desrt) :
review: Needs Fixing
Revision history for this message
Marco Trevisan (Treviño) (3v1n0) wrote :

Thanks for comments, patch updated.

511. By Marco Trevisan (Treviño)

debian/patches/unity-border-radius.patch: remove unneeded variables to keep track of what exported

Signals are already smart enough to update us only when needed

Revision history for this message
Allison Karlitskaya (desrt) wrote :

No further problems that I can see.

Revision history for this message
Sebastien Bacher (seb128) wrote :

thanks

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2016-03-14 12:51:46 +0000
3+++ debian/changelog 2016-03-17 13:35:43 +0000
4@@ -1,3 +1,12 @@
5+gtk+3.0 (3.18.9-1ubuntu2) UNRELEASED; urgency=medium
6+
7+ * debian/patches/unity-border-radius.patch:
8+ - Export windows corners radius as an X11 property in unity (LP: #1516403)
9+ * debian/patches/unity_rbga_tooltips.patch:
10+ - Removed as it's now included in the border-radius patch
11+
12+ -- Marco Trevisan (Treviño) <marco@ubuntu.com> Tue, 15 Mar 2016 18:40:42 +0100
13+
14 gtk+3.0 (3.18.9-1ubuntu1) xenial; urgency=low
15
16 * Merge with Debian. Remaining changes:
17
18=== modified file 'debian/patches/series'
19--- debian/patches/series 2016-03-14 12:44:16 +0000
20+++ debian/patches/series 2016-03-17 13:35:43 +0000
21@@ -21,7 +21,7 @@
22 restore_filechooser_typeaheadfind
23 0001-calendar-always-emit-day-selected-once.patch
24 git_icon_fallback.patch
25-unity_rbga_tooltips.patch
26 git-refresh-mir-backend.patch
27 0001-gtkwindow-set-transparent-background-color.patch
28 ubuntu_fileselector_behaviour.patch
29+unity-border-radius.patch
30
31=== added file 'debian/patches/unity-border-radius.patch'
32--- debian/patches/unity-border-radius.patch 1970-01-01 00:00:00 +0000
33+++ debian/patches/unity-border-radius.patch 2016-03-17 13:35:43 +0000
34@@ -0,0 +1,269 @@
35+Description: Export windows corners radius in the _UNITY_GTK_BORDER_RADIUS
36+ X11 property when such feature is supported by WM (so just in unity).
37+ This has to be removed when compiz will natively support _GTK_FRAME_EXTENTS
38+Author: Marco Trevisan <marco.trevisan@canonical.com>
39+Bug-Ubuntu: https://bugs.launchpad.net/bugs/1516403
40+
41+Index: gtk+3.0-3.18.8/gtk/gtkwindow.c
42+===================================================================
43+--- gtk+3.0-3.18.8.orig/gtk/gtkwindow.c
44++++ gtk+3.0-3.18.8/gtk/gtkwindow.c
45+@@ -232,6 +232,7 @@ struct _GtkWindowPrivate
46+ guint csd_requested : 1;
47+ guint client_decorated : 1; /* Decorations drawn client-side */
48+ guint use_client_shadow : 1; /* Decorations use client-side shadows */
49++ guint use_unity_border_radius : 1; /* Unity border radius is supported */
50+ guint maximized : 1;
51+ guint fullscreen : 1;
52+ guint tiled : 1;
53+@@ -3976,14 +3977,139 @@ gtk_window_set_geometry_hints (GtkWindow
54+ }
55+
56+ static void
57++unity_border_radius_update (GtkWindow *window)
58++{
59++#ifdef GDK_WINDOWING_X11
60++ Atom border_radius;
61++ GtkWindowPrivate *priv;
62++ GtkStyleContext *context;
63++ GdkDisplay *display;
64++ GdkWindow *gdk_window;
65++
66++ enum { TOP_LEFT, TOP_RIGHT, BOTTOM_LEFT, BOTTOM_RIGHT };
67++ gulong corners[4] = {0};
68++
69++ priv = window->priv;
70++ context = NULL;
71++
72++ if (!gtk_widget_get_realized (GTK_WIDGET (window)))
73++ return;
74++
75++ if (priv->type == GTK_WINDOW_POPUP)
76++ {
77++ context = gtk_widget_get_style_context (GTK_WIDGET (window));
78++ }
79++ else if (priv->title_box)
80++ {
81++ context = gtk_widget_get_style_context (priv->title_box);
82++ }
83++
84++ if (context)
85++ {
86++ corners[TOP_LEFT] = round (_gtk_css_corner_value_get_x (_gtk_style_context_peek_property (context, GTK_CSS_PROPERTY_BORDER_TOP_LEFT_RADIUS), 100) * priv->scale);
87++ corners[TOP_RIGHT] = round (_gtk_css_corner_value_get_x (_gtk_style_context_peek_property (context, GTK_CSS_PROPERTY_BORDER_TOP_RIGHT_RADIUS), 100) * priv->scale);
88++
89++ /* Bottom corners radius is not controlled by title box, and we can
90++ * assume it's always 0 for such windows. */
91++
92++ if (priv->type == GTK_WINDOW_POPUP)
93++ {
94++ corners[BOTTOM_LEFT] = round (_gtk_css_corner_value_get_x (_gtk_style_context_peek_property (context, GTK_CSS_PROPERTY_BORDER_BOTTOM_LEFT_RADIUS), 100) * priv->scale);
95++ corners[BOTTOM_RIGHT] = round (_gtk_css_corner_value_get_x (_gtk_style_context_peek_property (context, GTK_CSS_PROPERTY_BORDER_BOTTOM_RIGHT_RADIUS), 100) * priv->scale);
96++ }
97++ }
98++
99++ gdk_window = _gtk_widget_get_window (GTK_WIDGET (window));
100++ display = gdk_window_get_display (gdk_window);
101++ border_radius = gdk_x11_get_xatom_by_name_for_display (display,
102++ "_UNITY_GTK_BORDER_RADIUS");
103++
104++ if (corners[TOP_LEFT] || corners[TOP_RIGHT] || corners[BOTTOM_LEFT] || corners[BOTTOM_RIGHT])
105++ {
106++ XChangeProperty (GDK_DISPLAY_XDISPLAY (display),
107++ GDK_WINDOW_XID (gdk_window),
108++ border_radius,
109++ gdk_x11_get_xatom_by_name_for_display (display, "CARDINAL"),
110++ 32, PropModeReplace,
111++ (guchar *) &corners, G_N_ELEMENTS (corners));
112++ }
113++ else
114++ {
115++ XDeleteProperty (GDK_DISPLAY_XDISPLAY (display),
116++ GDK_WINDOW_XID (gdk_window),
117++ border_radius);
118++ }
119++#endif
120++}
121++
122++static void
123++on_decoration_style_changed (GtkWidget *widget,
124++ GtkWindow *window)
125++{
126++ GtkStyleContext *context = gtk_widget_get_style_context (widget);
127++ const GtkBitmask *changes = _gtk_style_context_get_changes (context);
128++
129++ if (!changes)
130++ return;
131++
132++ if (_gtk_bitmask_get(changes, GTK_CSS_PROPERTY_BORDER_TOP_LEFT_RADIUS) ||
133++ _gtk_bitmask_get(changes, GTK_CSS_PROPERTY_BORDER_TOP_RIGHT_RADIUS) ||
134++ _gtk_bitmask_get(changes, GTK_CSS_PROPERTY_BORDER_BOTTOM_RIGHT_RADIUS) ||
135++ _gtk_bitmask_get(changes, GTK_CSS_PROPERTY_BORDER_BOTTOM_LEFT_RADIUS))
136++ {
137++ unity_border_radius_update (window);
138++ }
139++}
140++
141++static void
142++on_window_scale_changed (GtkWidget *widget,
143++ GParamSpec *pspec,
144++ GtkWindow *window)
145++{
146++ unity_border_radius_update (window);
147++}
148++
149++static void
150++unity_border_radius_update_and_monitor (GtkWindow *window, GtkWidget *widget)
151++{
152++ if (!window->priv->use_unity_border_radius)
153++ return;
154++
155++ g_signal_connect (widget, "style-updated", G_CALLBACK (on_decoration_style_changed), window);
156++ unity_border_radius_update (window);
157++}
158++
159++static gboolean
160++unity_border_radius_is_supported (GtkWindow *window)
161++{
162++#ifdef GDK_WINDOWING_X11
163++ GdkScreen *screen = _gtk_window_get_screen (window);
164++
165++ if (GDK_IS_X11_SCREEN (screen))
166++ {
167++ GdkAtom unity_atom = gdk_atom_intern_static_string ("_UNITY_GTK_BORDER_RADIUS");
168++ return gdk_x11_screen_supports_net_wm_hint (screen, unity_atom);
169++ }
170++#endif
171++
172++ return FALSE;
173++}
174++
175++static void
176+ unset_titlebar (GtkWindow *window)
177+ {
178+ GtkWindowPrivate *priv = window->priv;
179+
180+ if (priv->titlebar != NULL)
181+- g_signal_handlers_disconnect_by_func (priv->titlebar,
182+- on_titlebar_title_notify,
183+- window);
184++ {
185++ g_signal_handlers_disconnect_by_func (priv->titlebar,
186++ on_titlebar_title_notify,
187++ window);
188++
189++ g_signal_handlers_disconnect_by_func (priv->titlebar,
190++ on_decoration_style_changed,
191++ window);
192++ }
193+
194+ if (priv->title_box != NULL)
195+ {
196+@@ -4043,13 +4169,16 @@ gtk_window_enable_csd (GtkWindow *window
197+ GdkVisual *visual;
198+
199+ /* We need a visual with alpha for client shadows */
200+- if (priv->use_client_shadow)
201++ if (priv->use_client_shadow || priv->use_unity_border_radius)
202+ {
203+ visual = gdk_screen_get_rgba_visual (gtk_widget_get_screen (widget));
204+ if (visual != NULL)
205+ gtk_widget_set_visual (widget, visual);
206+
207+ gtk_style_context_add_class (gtk_widget_get_style_context (widget), GTK_STYLE_CLASS_CSD);
208++
209++ if (priv->use_unity_border_radius)
210++ gtk_style_context_add_class (gtk_widget_get_style_context (widget), "unity-csd");
211+ }
212+ else
213+ {
214+@@ -4118,6 +4247,7 @@ gtk_window_set_titlebar (GtkWindow *wind
215+ }
216+
217+ priv->use_client_shadow = gtk_window_supports_client_shadow (window);
218++ priv->use_unity_border_radius = unity_border_radius_is_supported (window);
219+
220+ gtk_window_enable_csd (window);
221+ priv->title_box = titlebar;
222+@@ -4129,6 +4259,8 @@ gtk_window_set_titlebar (GtkWindow *wind
223+ on_titlebar_title_notify (GTK_HEADER_BAR (titlebar), NULL, window);
224+ }
225+
226++ unity_border_radius_update_and_monitor (window, titlebar);
227++
228+ gtk_style_context_add_class (gtk_widget_get_style_context (titlebar),
229+ GTK_STYLE_CLASS_TITLEBAR);
230+
231+@@ -5857,13 +5989,19 @@ create_decoration (GtkWidget *widget)
232+ GtkWindowPrivate *priv = window->priv;
233+
234+ priv->use_client_shadow = gtk_window_supports_client_shadow (window);
235+- if (!priv->use_client_shadow)
236++ priv->use_unity_border_radius = unity_border_radius_is_supported (window);
237++
238++ if (!priv->use_client_shadow && !priv->use_unity_border_radius)
239+ return;
240+
241+ gtk_window_enable_csd (window);
242+-
243+ if (priv->type == GTK_WINDOW_POPUP)
244+- return;
245++ {
246++ if (priv->csd_requested)
247++ unity_border_radius_update_and_monitor (window, widget);
248++
249++ return;
250++ }
251+
252+ if (priv->title_box == NULL)
253+ {
254+@@ -5871,6 +6009,8 @@ create_decoration (GtkWidget *widget)
255+ gtk_widget_set_parent (priv->titlebar, widget);
256+ gtk_widget_show_all (priv->titlebar);
257+ priv->title_box = priv->titlebar;
258++
259++ unity_border_radius_update_and_monitor (window, priv->title_box);
260+ }
261+
262+ update_window_buttons (window);
263+@@ -6547,6 +6687,9 @@ get_shadow_width (GtkWindow *window,
264+ if (!priv->decorated)
265+ return;
266+
267++ if (priv->client_decorated && priv->use_unity_border_radius)
268++ return;
269++
270+ if (!priv->client_decorated &&
271+ !(gtk_window_should_use_csd (window) &&
272+ gtk_window_supports_client_shadow (window)))
273+@@ -7141,7 +7284,7 @@ gtk_window_realize (GtkWidget *widget)
274+
275+ attributes_mask = GDK_WA_X | GDK_WA_Y | GDK_WA_VISUAL;
276+
277+- if (priv->client_decorated && priv->type == GTK_WINDOW_TOPLEVEL)
278++ if (priv->client_decorated && priv->type == GTK_WINDOW_TOPLEVEL && !priv->use_unity_border_radius)
279+ {
280+ const gchar *cursor[8] = {
281+ "nw-resize", "n-resize", "ne-resize",
282+@@ -7223,6 +7366,12 @@ gtk_window_realize (GtkWidget *widget)
283+ }
284+ #endif
285+
286++ if (priv->use_unity_border_radius)
287++ {
288++ g_signal_connect (window, "notify::scale-factor", G_CALLBACK (on_window_scale_changed), window);
289++ unity_border_radius_update (window);
290++ }
291++
292+ child_allocation.x = 0;
293+ child_allocation.y = 0;
294+ child_allocation.width = allocation.width;
295+@@ -10587,7 +10736,7 @@ gtk_window_set_screen (GtkWindow *window
296+ }
297+ g_object_notify_by_pspec (G_OBJECT (window), window_props[PROP_SCREEN]);
298+
299+- if (was_rgba && priv->use_client_shadow)
300++ if (was_rgba && (priv->use_client_shadow || priv->use_unity_border_radius))
301+ {
302+ GdkVisual *visual;
303+
304
305=== removed file 'debian/patches/unity_rbga_tooltips.patch'
306--- debian/patches/unity_rbga_tooltips.patch 2016-03-10 02:12:49 +0000
307+++ debian/patches/unity_rbga_tooltips.patch 1970-01-01 00:00:00 +0000
308@@ -1,55 +0,0 @@
309-Description: Ensure that tooltips are drawn on a rgba surface in Unity
310- This has to be removed once better unity/CSD integration has landed
311-Author: Marco Trevisan <marco.trevisan@canonical.com>
312-Bug-Ubuntu: https://bugs.launchpad.net/bugs/1508357
313-
314-Index: gtk+3.0-3.18.8/gtk/gtktooltip.c
315-===================================================================
316---- gtk+3.0-3.18.8.orig/gtk/gtktooltip.c
317-+++ gtk+3.0-3.18.8/gtk/gtktooltip.c
318-@@ -166,6 +166,30 @@ gtk_tooltip_class_init (GtkTooltipClass
319- quark_current_tooltip = g_quark_from_static_string ("gdk-display-current-tooltip");
320- }
321-
322-+static gboolean
323-+in_desktop (const gchar *name)
324-+{
325-+ const gchar *desktop_name_list;
326-+ gchar **names;
327-+ gboolean in_list = FALSE;
328-+ gint i;
329-+
330-+ desktop_name_list = g_getenv ("XDG_CURRENT_DESKTOP");
331-+ if (!desktop_name_list)
332-+ return FALSE;
333-+
334-+ names = g_strsplit (desktop_name_list, ":", -1);
335-+ for (i = 0; names[i] && !in_list; i++)
336-+ if (strcmp (names[i], name) == 0)
337-+ {
338-+ in_list = TRUE;
339-+ break;
340-+ }
341-+ g_strfreev (names);
342-+
343-+ return in_list;
344-+}
345-+
346- static void
347- gtk_tooltip_init (GtkTooltip *tooltip)
348- {
349-@@ -200,6 +224,14 @@ gtk_tooltip_init (GtkTooltip *tooltip)
350- context = gtk_widget_get_style_context (window);
351- gtk_style_context_add_class (context, GTK_STYLE_CLASS_TOOLTIP);
352-
353-+ if (in_desktop ("Unity") == 0)
354-+ {
355-+ GdkVisual *visual;
356-+ visual = gdk_screen_get_rgba_visual (gtk_widget_get_screen (window));
357-+ if (visual)
358-+ gtk_widget_set_visual (GTK_WIDGET (window), visual);
359-+ }
360-+
361- atk_obj = gtk_widget_get_accessible (window);
362- if (GTK_IS_ACCESSIBLE (atk_obj))
363- atk_object_set_role (atk_obj, ATK_ROLE_TOOL_TIP);

Subscribers

People subscribed via source and target branches