GNOME Bugzilla – Bug 636654
Fix build with GTK+3 style-context branch
Last modified: 2018-05-22 14:30:11 UTC
When building with 2.91.6 (after style-context branch was merged) we get errors like: empathy-account-widget.c: In function ‘account_widget_set_entry_highlighting’: empathy-account-widget.c:209: error: implicit declaration of function ‘gtk_widget_get_style’ empathy-account-widget.c:209: error: assignment makes pointer from integer without a cast empathy-account-widget.c:224: error: implicit declaration of function ‘gtk_widget_modify_base’ I'll try to whip up a patch.
Created attachment 175973 [details] [review] Patch for account_widget_set_entry_highlighting() One patch of several needed.
Review of attachment 175973 [details] [review]: ::: libempathy-gtk/empathy-account-widget.c @@ +204,3 @@ + const GdkRGBA white = { 1.0, 1.0, 1.0, 1.0 }; + GdkRGBA color; + GtkStyleContext *context; Move these into the block where they are used. @@ +212,3 @@ if (highlight) { + gtk_style_context_get_background_color (context, GTK_STATE_SELECTED, &color); This should use a GtkStateFlags, not the old GtkStateType. @@ +221,3 @@ + color.red = (color.red + white.red) / 2; + color.green = (color.green + white.green) / 2; + color.blue = (color.blue + white.blue) / 2; This particular whitening appears many times throughout Empathy, perhaps we should abstract it into a utility function.
Here are some more fixes (this branch includes Stefw's unreviewed work): http://git.collabora.co.uk/?p=user/danni/empathy.git;a=shortlog;h=refs/heads/style-context So far everything I've done is untested. Wondering how to convert the old GTK_STATE_NORMAL to GtkStateFlags. Currently using a flags of 0.
Bah bug #636500 was already opened and I had a WIP branch as well: http://git.collabora.co.uk/?p=user/cassidy/empathy;a=shortlog;h=refs/heads/gtk-style-636500 I let you guys finish this.
*** Bug 636500 has been marked as a duplicate of this bug. ***
Review of attachment 175973 [details] [review]: ::: libempathy-gtk/empathy-account-widget.c @@ +221,3 @@ + color.red = (color.red + white.red) / 2; + color.green = (color.green + white.green) / 2; + color.blue = (color.blue + white.blue) / 2; I don't think GdkColor and GdkRGBA uses the same unit to represent colors, so I'm not sure this is correct.
(In reply to comment #6) > I don't think GdkColor and GdkRGBA uses the same unit to represent colors, so > I'm not sure this is correct. Hasn't GdkColor been changed to a GdkRGBA here?
(In reply to comment #6) > I don't think GdkColor and GdkRGBA uses the same unit to represent colors, so > I'm not sure this is correct. Do you mean they use a different gamma curve? If so then I can see why the results would be different. But if not, then I think this new code should work, as we just want to go half way to white.
Not sure, I didn't check in details, I just noticed that GdkColor.read is a guint16 while GdkRGBA.red is a gdouble. But best if probably just to test. :)
Hmmm, I wonder how we're going to fix the gtk_rc_parse_style() usage in empathy_chat_window_class_init(). We would probably add our own GtkCSSProvider (or other GtkStyleProvider). But since we don't yet know how the common GTK themes will add the focus padding (or if they will at all), then we don't really know how to override it. Will they use border or padding, and so on.. In addition in empathy-theme-manager.c on_style_set_cb, if we want all those different colors we're going to have to access them through different widget paths, or use the defined colors. I'm not sure that every theme would define the same color names though. Anyway, at this point I'm kind of out of my depth, so I'll hand this back over to you Guillaume. I thought I'd try to help out, but didn't get very far :)
Created attachment 176456 [details] [review] http://git.collabora.co.uk/?p=user/cassidy/empathy;a=shortlog;h=refs/heads/gtk-style-636654 here is a branch using new API for which I'm pretty sure of the right way to do libempathy-gtk/empathy-cell-renderer-text.c | 17 ++++++++++++----- libempathy-gtk/empathy-chat-text-view.c | 2 +- libempathy-gtk/empathy-status-preset-dialog.c | 8 ++++---- libempathy-gtk/empathy-theme-boxes.c | 10 +++++----- libempathy-gtk/empathy-video-widget.c | 10 ++++------ 5 files changed, 26 insertions(+), 21 deletions(-)
I opened bug #637298 about the missing API to access theme colors.
Created attachment 176462 [details] [review] New version; http://git.collabora.co.uk/?p=user/cassidy/empathy;a=shortlog;h=refs/heads/gtk-style-636654 libempathy-gtk/empathy-account-widget.c | 25 +++++++++++++------------ libempathy-gtk/empathy-cell-renderer-text.c | 17 ++++++++++++----- libempathy-gtk/empathy-chat-text-view.c | 2 +- libempathy-gtk/empathy-contact-list-view.c | 19 +++++++++---------- libempathy-gtk/empathy-individual-view.c | 21 ++++++++++----------- libempathy-gtk/empathy-persona-view.c | 21 ++++++++++----------- libempathy-gtk/empathy-status-preset-dialog.c | 8 ++++---- libempathy-gtk/empathy-theme-boxes.c | 10 +++++----- libempathy-gtk/empathy-ui-utils.c | 10 ++++++++++ libempathy-gtk/empathy-ui-utils.h | 3 +++ libempathy-gtk/empathy-video-widget.c | 10 ++++------ 11 files changed, 81 insertions(+), 65 deletions(-)
Comment on attachment 176462 [details] [review] New version; http://git.collabora.co.uk/?p=user/cassidy/empathy;a=shortlog;h=refs/heads/gtk-style-636654 > if (!selected) { >- GdkColor color; >+ GdkRGBA color; > >- color = style->text_aa[GTK_STATE_NORMAL]; >+ gtk_style_context_get_color (context, 0, &color); > > attr_color = pango_attr_foreground_new (color.red, color.green, color.blue); > attr_color->start_index = attr_size->start_index; This looks wrong. GdkColor.red (et al) is between 0 and 65535, while GdkRGBA.red is between 0.0 and 1.0. The pango_attr_foreground_new() call (and everything using color) is taking wrong values. > static void >diff --git a/libempathy-gtk/empathy-persona-view.c b/libempathy-gtk/empathy-persona-view.c >index adfe6be..02a9ff5 100644 >--- a/libempathy-gtk/empathy-persona-view.c >+++ b/libempathy-gtk/empathy-persona-view.c >@@ -236,29 +236,28 @@ cell_set_background (EmpathyPersonaView *self, > GtkCellRenderer *cell, > gboolean is_active) > { >- GdkColor color; >- GtkStyle *style; >- >- style = gtk_widget_get_style (GTK_WIDGET (self)); >- > if (is_active) > { >- color = style->bg[GTK_STATE_SELECTED]; >+ GdkRGBA color; >+ GtkStyleContext *style; >+ >+ style = gtk_widget_get_style_context (GTK_WIDGET (self)); >+ >+ gtk_style_context_get_background_color (style, GTK_STATE_FLAG_SELECTED, >+ &color); > > /* Here we take the current theme colour and add it to > * the colour for white and average the two. This > * gives a colour which is inline with the theme but > * slightly whiter. > */ >- color.red = (color.red + (style->white).red) / 2; >- color.green = (color.green + (style->white).green) / 2; >- color.blue = (color.blue + (style->white).blue) / 2; >+ empathy_make_color_whiter (&color); > >- g_object_set (cell, "cell-background-gdk", &color, NULL); >+ g_object_set (cell, "cell-background-rgba", &color, NULL); > } > else > { >- g_object_set (cell, "cell-background-gdk", NULL, NULL); >+ g_object_set (cell, "cell-background-rgba", NULL, NULL); > } > } This whole construct is repeated all over the place, you may want to factorize it too? Like a empathy_widget_set_background_color(widget, is_active). Though since different widgets have different background property names, it may not be such a good idea. >diff --git a/libempathy-gtk/empathy-theme-boxes.c b/libempathy-gtk/empathy-theme-boxes.c >index bf97f3f..058fb48 100644 >--- a/libempathy-gtk/empathy-theme-boxes.c >+++ b/libempathy-gtk/empathy-theme-boxes.c >@@ -289,12 +289,12 @@ theme_boxes_maybe_append_header (EmpathyThemeBoxes *theme, > tag = gtk_text_tag_table_lookup (table, EMPATHY_THEME_BOXES_TAG_HEADER); > g_object_get (tag, "foreground-set", &color_set, NULL); > if (color_set) { >- GdkColor *color; >+ GdkRGBA color; > >- g_object_get (tag, "foreground-gdk", &color, NULL); >- gtk_widget_modify_fg (label1, GTK_STATE_NORMAL, color); >- gtk_widget_modify_fg (label2, GTK_STATE_NORMAL, color); >- gdk_color_free (color); >+ g_object_get (tag, "foreground-rgba", &color, NULL); GtkTextTag doesn't have a foreground-rgba property. >+void >+empathy_make_color_whiter (GdkRGBA *color) >+{ >+ const GdkRGBA white = { 1.0, 1.0, 1.0, 1.0 }; >+ >+ color->red = (color->red + (white).red) / 2; >+ color->green = (color->green + (white).green) / 2; >+ color->blue = (color->blue + (white).blue) / 2; >+} The paranthesis around white can go away. Everything else looks good.
You also need to bump the GTK+ dependency to 2.91.6
(In reply to comment #14) > > if (!selected) { > >- GdkColor color; > >+ GdkRGBA color; > > > >- color = style->text_aa[GTK_STATE_NORMAL]; > >+ gtk_style_context_get_color (context, 0, &color); > > > > attr_color = pango_attr_foreground_new (color.red, color.green, color.blue); > > attr_color->start_index = attr_size->start_index; > > This looks wrong. GdkColor.red (et al) is between 0 and 65535, while > GdkRGBA.red is between 0.0 and 1.0. The pango_attr_foreground_new() call (and > everything using color) is taking wrong values. You're right, nice catch. I removed this patch from now, the GTK+ guys are going to add some new API for this. > > static void > >diff --git a/libempathy-gtk/empathy-persona-view.c b/libempathy-gtk/empathy-persona-view.c > >index adfe6be..02a9ff5 100644 > >--- a/libempathy-gtk/empathy-persona-view.c > >+++ b/libempathy-gtk/empathy-persona-view.c > >@@ -236,29 +236,28 @@ cell_set_background (EmpathyPersonaView *self, > > GtkCellRenderer *cell, > > gboolean is_active) > > { > >- GdkColor color; > >- GtkStyle *style; > >- > >- style = gtk_widget_get_style (GTK_WIDGET (self)); > >- > > if (is_active) > > { > >- color = style->bg[GTK_STATE_SELECTED]; > >+ GdkRGBA color; > >+ GtkStyleContext *style; > >+ > >+ style = gtk_widget_get_style_context (GTK_WIDGET (self)); > >+ > >+ gtk_style_context_get_background_color (style, GTK_STATE_FLAG_SELECTED, > >+ &color); > > > > /* Here we take the current theme colour and add it to > > * the colour for white and average the two. This > > * gives a colour which is inline with the theme but > > * slightly whiter. > > */ > >- color.red = (color.red + (style->white).red) / 2; > >- color.green = (color.green + (style->white).green) / 2; > >- color.blue = (color.blue + (style->white).blue) / 2; > >+ empathy_make_color_whiter (&color); > > > >- g_object_set (cell, "cell-background-gdk", &color, NULL); > >+ g_object_set (cell, "cell-background-rgba", &color, NULL); > > } > > else > > { > >- g_object_set (cell, "cell-background-gdk", NULL, NULL); > >+ g_object_set (cell, "cell-background-rgba", NULL, NULL); > > } > > } > > This whole construct is repeated all over the place, you may want to factorize > it too? Like a empathy_widget_set_background_color(widget, is_active). Though > since different widgets have different background property names, it may not be > such a good idea. I think it's good as it. Most of the -view code is pretty similar but hopefully most of the views should go away at some point (by continuing refactoring contact/folks code). > >diff --git a/libempathy-gtk/empathy-theme-boxes.c b/libempathy-gtk/empathy-theme-boxes.c > >index bf97f3f..058fb48 100644 > >--- a/libempathy-gtk/empathy-theme-boxes.c > >+++ b/libempathy-gtk/empathy-theme-boxes.c > >@@ -289,12 +289,12 @@ theme_boxes_maybe_append_header (EmpathyThemeBoxes *theme, > > tag = gtk_text_tag_table_lookup (table, EMPATHY_THEME_BOXES_TAG_HEADER); > > g_object_get (tag, "foreground-set", &color_set, NULL); > > if (color_set) { > >- GdkColor *color; > >+ GdkRGBA color; > > > >- g_object_get (tag, "foreground-gdk", &color, NULL); > >- gtk_widget_modify_fg (label1, GTK_STATE_NORMAL, color); > >- gtk_widget_modify_fg (label2, GTK_STATE_NORMAL, color); > >- gdk_color_free (color); > >+ g_object_get (tag, "foreground-rgba", &color, NULL); > > GtkTextTag doesn't have a foreground-rgba property. Right :\ I asked to GTK+ devs, a new property will be added soon. I removed the commit for now. > >+void > >+empathy_make_color_whiter (GdkRGBA *color) > >+{ > >+ const GdkRGBA white = { 1.0, 1.0, 1.0, 1.0 }; > >+ > >+ color->red = (color->red + (white).red) / 2; > >+ color->green = (color->green + (white).green) / 2; > >+ color->blue = (color->blue + (white).blue) / 2; > >+} > > The paranthesis around white can go away. removed.
(In reply to comment #15) > You also need to bump the GTK+ dependency to 2.91.6 Indeed; done.
r+ Should we leave this opened for the two commits you've removed until we have the missing APIs and we can push them?
Thanks merged. Yeah, let's keep the bug open, my original branch didn't fix everything anyway.
The fedora-64 buildbot is failing with the following error now. empathy-cell-renderer-expander.c: In function 'empathy_cell_renderer_expander_render': empathy-cell-renderer-expander.c:339:2: error: implicit declaration of function 'gtk_paint_expander' empathy-cell-renderer-expander.c:339:2: error: implicit declaration of function 'gtk_widget_get_style'
Yeah, as said we didn't port everything yet. If you re-enable deprecated API it should build fine.
We should revert http://git.gnome.org/browse/empathy/commit/?id=218a0c003b0528423164b1670daf33ec360acd20 once this bug has been fixed.
I'm working on this
Created attachment 177947 [details] [review] Stop using some deprecated symbols http://git.collabora.co.uk/?p=user/pochu/empathy.git;a=shortlog;h=refs/heads/gtk-3-fixes This fixes more gtk 3 deprecated symbols (some GtkStyleContext and some others). There are still 3 more things to fix, I'll try to look at them later.
Review of attachment 177947 [details] [review]: Looks good. ::: libempathy-gtk/empathy-password-dialog.c @@ +191,3 @@ if (priv->grabbing) { + gdk_device_ungrab (gdk_event_get_device (event), You don't check here if get_device() returns NULL; is that ok ?
Comment on attachment 177947 [details] [review] Stop using some deprecated symbols 11:36 < pochu> cassidy: I don't really know about _get_device(). the doc says it can return NULL, and I'm checking for that in the _grab case, so I'm doing that too on the _ungrab one now, OK to push now?
Sure go for it.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/empathy/issues/308.