Merge lp:~muktupavels/compiz/gwd-theme-implement-update-border-extents into lp:compiz/0.9.12

[Alberts Muktupāvels]

GWDTheme: implement update_border_extents.

GWDTheme: implement update_border_extents.

[lp:~muktupavels/compiz/gwd-theme-implement-update-border-extents]

4027. By Alberts Muktupāvels on 2016-05-19

Add space.

4028. By Alberts Muktupāvels on 2016-05-19

Simplify code.

[Alberts Muktupāvels (muktupavels)]

Sam, did you look at this?

[lp:~muktupavels/compiz/gwd-theme-implement-update-border-extents]

4029. By Alberts Muktupāvels on 2016-05-19

Merge changes from lp:~albertsmuktupavels/compiz/gwd-theme-implement-get-shadow.

4030. By Alberts Muktupāvels on 2016-05-19

Revert re-styling in wnck.c.

[Alberts Muktupāvels (muktupavels)]

1) "As I mentioned in another review - it may be better to have a mechanism in place to update this property when the metacity theme changes."

When metacity theme changes new GWDTheme will be created. Also in metacity 3.20 there will be no meta_theme_get_current.

2) Assign and declare... Is that required? I find code more readable this way. Also existing code first declare varible and only then assign.

[Sam Spilsbury (smspillaz)]

> 1) "As I mentioned in another review - it may be better to have a mechanism in
> place to update this property when the metacity theme changes."
>
> When metacity theme changes new GWDTheme will be created. Also in metacity
> 3.20 there will be no meta_theme_get_current.

Ah okay, I've found it - g_set_object(&gwd_theme, g_object_new ...) in an older review.

Might be good to add a comment in the constructor there about how a GWDTheme exists only for a single MetaTheme and is destroyed and re-created on theme changes, since that threw me off initially.

>
> 2) Assign and declare... Is that required? I find code more readable this way.
> Also existing code first declare varible and only then assign.

The existing code does use the C89 style, though there's really no reason for it to, since we're not building on Windows ;-). C89 comes with a lot of serious disadvantages and switching to assign-and-declare has lots of advantages:

1. Eliminates a whole class of bugs where unassigned (and thus garbage) variables are used.
2. Keeps the declaration as close as possible to first use, so you can easily see where a particular variable becomes relevant.
3. Prevents readers from having to jump up and down in order to determine variable types (though admittedly this is more a symptom of excessively long functions. The C89 style just exacerbates the problem).

I don't mind if further refactors to clean all that up come in later revisions - its just low hanging fruit and something worth doing whilst the code is being touched.

[lp:~muktupavels/compiz/gwd-theme-implement-update-border-extents]

4031. By Alberts Muktupāvels on 2016-05-20

Add comment.

4032. By Alberts Muktupāvels on 2016-05-20

Assign and declare.

[Alberts Muktupāvels (muktupavels)]

Added comment and fixed few "assign and declare" things. Can this branch we considered ready?

[Sam Spilsbury (smspillaz)]

Looks good to me :)

review: Approve

=== modified file 'gtk/window-decorator/cairo.c'
--- gtk/window-decorator/cairo.c	2016-02-29 13:41:59 +0000
+++ gtk/window-decorator/cairo.c	2016-05-20 10:29:03 +0000
@@ -954,17 +954,3 @@
 {
     return 1.0f;
 }
-
-void
-update_border_extents (decor_frame_t *frame)
-{
-    frame = gwd_decor_frame_ref (frame);
-
-    gwd_cairo_window_decoration_get_extents (&frame->win_extents,
-					     &frame->max_win_extents);
-
-    frame->titlebar_height = frame->max_titlebar_height =
-	    (frame->text_height < 17) ? 17 : frame->text_height;
-
-    gwd_decor_frame_unref (frame);
-}

=== modified file 'gtk/window-decorator/frames.c'
--- gtk/window-decorator/frames.c	2016-05-20 10:29:02 +0000
+++ gtk/window-decorator/frames.c	2016-05-20 10:29:03 +0000
@@ -280,9 +280,8 @@
 
     frame_update_titlebar_font (frame);
 
-    if (strcmp (frame->type, "switcher") != 0 &&
-	strcmp (frame->type, "bare") != 0)
-	(*theme_update_border_extents) (frame);
+    if (strcmp (frame->type, "switcher") != 0 && strcmp (frame->type, "bare") != 0)
+        gwd_theme_update_border_extents (gwd_theme, frame);
 
     gwd_theme_get_shadow (gwd_theme, frame, &active_o, TRUE);
     gwd_theme_get_shadow (gwd_theme, frame, &inactive_o, FALSE);

=== modified file 'gtk/window-decorator/gtk-window-decorator.c'
--- gtk/window-decorator/gtk-window-decorator.c	2016-05-20 10:29:02 +0000
+++ gtk/window-decorator/gtk-window-decorator.c	2016-05-20 10:29:03 +0000
@@ -173,7 +173,6 @@
 
         theme_draw_window_decoration = meta_draw_window_decoration;
         theme_calc_decoration_size = meta_calc_decoration_size;
-        theme_update_border_extents = meta_update_border_extents;
         theme_get_event_window_position = meta_get_event_window_position;
         theme_get_button_position = meta_get_button_position;
         theme_get_title_scale = meta_get_title_scale;
@@ -184,7 +183,6 @@
 
         theme_draw_window_decoration = draw_window_decoration;
         theme_calc_decoration_size = calc_decoration_size;
-        theme_update_border_extents = update_border_extents;
         theme_get_event_window_position = get_event_window_position;
         theme_get_button_position = get_button_position;
         theme_get_title_scale = get_title_scale;
@@ -194,7 +192,6 @@
 
     theme_draw_window_decoration = draw_window_decoration;
     theme_calc_decoration_size = calc_decoration_size;
-    theme_update_border_extents = update_border_extents;
     theme_get_event_window_position = get_event_window_position;
     theme_get_button_position = get_button_position;
     theme_get_title_scale = get_title_scale;

=== modified file 'gtk/window-decorator/gtk-window-decorator.h'
--- gtk/window-decorator/gtk-window-decorator.h	2016-05-20 10:29:02 +0000
+++ gtk/window-decorator/gtk-window-decorator.h	2016-05-20 10:29:03 +0000
@@ -314,7 +314,6 @@
 					     int     text_width,
 					     int     *width,
 					     int     *height);
-void     (*theme_update_border_extents)     (decor_frame_t *frame);
 void     (*theme_get_event_window_position) (decor_t *d,
 
 					     gint    i,
@@ -565,9 +564,6 @@
 		      gint    *width,
 		      gint    *height);
 
-void
-update_border_extents ();
-
 gboolean
 get_button_position (decor_t *d,
 		     gint    i,
@@ -656,9 +652,6 @@
 meta_get_title_scale (decor_frame_t *);
 
 void
-meta_update_border_extents ();
-
-void
 meta_update_button_layout (const char *value);
 
 #endif

=== modified file 'gtk/window-decorator/gwd-theme-cairo.c'
--- gtk/window-decorator/gwd-theme-cairo.c	2016-05-20 10:29:02 +0000
+++ gtk/window-decorator/gwd-theme-cairo.c	2016-05-20 10:29:03 +0000
@@ -18,6 +18,8 @@
  */
 
 #include "config.h"
+#include "gtk-window-decorator.h"
+#include "gwd-cairo-window-decoration-util.h"
 #include "gwd-theme-cairo.h"
 
 struct _GWDThemeCairo
@@ -28,8 +30,26 @@
 G_DEFINE_TYPE (GWDThemeCairo, gwd_theme_cairo, GWD_TYPE_THEME)
 
 static void
+gwd_theme_cairo_update_border_extents (GWDTheme      *theme,
+                                       decor_frame_t *frame)
+{
+    frame = gwd_decor_frame_ref (frame);
+
+    gwd_cairo_window_decoration_get_extents (&frame->win_extents,
+                                             &frame->max_win_extents);
+
+    frame->titlebar_height = frame->max_titlebar_height =
+        (frame->text_height < 17) ? 17 : frame->text_height;
+
+    gwd_decor_frame_unref (frame);
+}
+
+static void
 gwd_theme_cairo_class_init (GWDThemeCairoClass *cairo_class)
 {
+    GWDThemeClass *theme_class = GWD_THEME_CLASS (cairo_class);
+
+    theme_class->update_border_extents = gwd_theme_cairo_update_border_extents;
 }
 
 static void

=== modified file 'gtk/window-decorator/gwd-theme-metacity.c'
--- gtk/window-decorator/gwd-theme-metacity.c	2016-05-20 10:29:02 +0000
+++ gtk/window-decorator/gwd-theme-metacity.c	2016-05-20 10:29:03 +0000
@@ -18,18 +18,93 @@
  */
 
 #include "config.h"
+
+#include <metacity-private/theme.h>
+
+#include "gtk-window-decorator.h"
 #include "gwd-theme-metacity.h"
 
 struct _GWDThemeMetacity
 {
-    GObject parent;
+    GObject    parent;
+
+    MetaTheme *theme;
 };
 
 G_DEFINE_TYPE (GWDThemeMetacity, gwd_theme_metacity, GWD_TYPE_THEME)
 
+static GObject *
+gwd_theme_metacity_constructor (GType                  type,
+                                guint                  n_properties,
+                                GObjectConstructParam *properties)
+{
+    GObject *object;
+    GWDThemeMetacity *metacity;
+
+    object = G_OBJECT_CLASS (gwd_theme_metacity_parent_class)->constructor (type, n_properties, properties);
+    metacity = GWD_THEME_METACITY (object);
+
+    /* Always valid and current MetaTheme! On theme change new GWDThemeMetacity
+     * object will be created and old one destroyed. If Metacity theme is not
+     * valid (gwd_metacity_window_decoration_update_meta_theme returns FALSE)
+     * then GWDThemeCairo will be created.
+     */
+    metacity->theme = meta_theme_get_current ();
+
+    return object;
+}
+
+static void
+gwd_theme_metacity_update_border_extents (GWDTheme      *theme,
+                                          decor_frame_t *frame)
+{
+    GWDThemeMetacity *metacity = GWD_THEME_METACITY (theme);
+    GdkScreen *screen = gtk_widget_get_screen (frame->style_window_rgba);
+    MetaStyleInfo *style_info = meta_theme_create_style_info (screen, NULL);
+    MetaFrameType frame_type;
+    MetaFrameBorders borders;
+
+    gwd_decor_frame_ref (frame);
+
+    frame_type = meta_frame_type_from_string (frame->type);
+    if (!(frame_type < META_FRAME_TYPE_LAST))
+        frame_type = META_FRAME_TYPE_NORMAL;
+
+    meta_theme_get_frame_borders (metacity->theme, style_info, frame_type,
+                                  frame->text_height, 0, &borders);
+
+    frame->win_extents.top = frame->win_extents.top;
+    frame->win_extents.bottom = borders.visible.bottom;
+    frame->win_extents.left = borders.visible.left;
+    frame->win_extents.right = borders.visible.right;
+
+    frame->titlebar_height = borders.visible.top - frame->win_extents.top;
+
+    meta_theme_get_frame_borders (metacity->theme, style_info, frame_type,
+                                  frame->text_height, META_FRAME_MAXIMIZED,
+                                  &borders);
+
+    frame->max_win_extents.top = frame->win_extents.top;
+    frame->max_win_extents.bottom = borders.visible.bottom;
+    frame->max_win_extents.left = borders.visible.left;
+    frame->max_win_extents.right = borders.visible.right;
+
+    frame->max_titlebar_height = borders.visible.top - frame->max_win_extents.top;
+
+    meta_style_info_unref (style_info);
+
+    gwd_decor_frame_unref (frame);
+}
+
 static void
 gwd_theme_metacity_class_init (GWDThemeMetacityClass *metacity_class)
 {
+    GObjectClass *object_class = G_OBJECT_CLASS (metacity_class);
+    GWDThemeClass *theme_class = GWD_THEME_CLASS (metacity_class);
+
+    object_class->constructor = gwd_theme_metacity_constructor;
+
+    theme_class->update_border_extents = gwd_theme_metacity_update_border_extents;
 }
 
 static void

=== modified file 'gtk/window-decorator/metacity.c'
--- gtk/window-decorator/metacity.c	2016-03-17 17:47:36 +0000
+++ gtk/window-decorator/metacity.c	2016-05-20 10:29:03 +0000
@@ -1491,63 +1491,4 @@
     meta_button_layout = new_layout;
 }
 
-void
-meta_update_border_extents (decor_frame_t *frame)
-{
-    MetaTheme *theme;
-    GdkScreen *screen;
-    MetaStyleInfo *style_info;
-    MetaFrameBorders borders;
-    MetaFrameType frame_type;
-    gint top_height;
-    gint bottom_height;
-    gint left_width;
-    gint right_width;
-
-    gwd_decor_frame_ref (frame);
-
-    frame_type = meta_frame_type_from_string (frame->type);
-    if (!(frame_type < META_FRAME_TYPE_LAST))
-        frame_type = META_FRAME_TYPE_NORMAL;
-
-    theme = meta_theme_get_current ();
-
-    screen = gtk_widget_get_screen (frame->style_window_rgba);
-    style_info = meta_theme_create_style_info (screen, NULL);
-
-    meta_theme_get_frame_borders (theme, style_info, frame_type, frame->text_height,
-                                  0, &borders);
-
-    top_height = borders.visible.top;
-    bottom_height = borders.visible.bottom;
-    left_width = borders.visible.left;
-    right_width = borders.visible.right;
-
-    frame->win_extents.top    = frame->win_extents.top;
-    frame->win_extents.bottom = bottom_height;
-    frame->win_extents.left   = left_width;
-    frame->win_extents.right  = right_width;
-
-    frame->titlebar_height = top_height - frame->win_extents.top;
-
-    meta_theme_get_frame_borders (theme, style_info, frame_type, frame->text_height,
-                                  META_FRAME_MAXIMIZED, &borders);
-
-    top_height = borders.visible.top;
-    bottom_height = borders.visible.bottom;
-    left_width = borders.visible.left;
-    right_width = borders.visible.right;
-
-    frame->max_win_extents.top    = frame->win_extents.top;
-    frame->max_win_extents.bottom = bottom_height;
-    frame->max_win_extents.left   = left_width;
-    frame->max_win_extents.right  = right_width;
-
-    frame->max_titlebar_height = top_height - frame->max_win_extents.top;
-
-    meta_style_info_unref (style_info);
-
-    gwd_decor_frame_unref (frame);
-}
-
 #endif

=== modified file 'gtk/window-decorator/wnck.c'
--- gtk/window-decorator/wnck.c	2016-03-17 17:47:36 +0000
+++ gtk/window-decorator/wnck.c	2016-05-20 10:29:03 +0000
@@ -169,7 +169,7 @@
 {
     decor_frame_t *frame = (decor_frame_t *) value;
 
-    (*theme_update_border_extents) (frame);
+    gwd_theme_update_border_extents (gwd_theme, frame);
 }
 
 void