GNOME Bugzilla – Bug 751677
per-widget font options
Last modified: 2015-07-01 14:37:59 UTC
gnome-builder has a need for overriding font options for certain widgets. The api should look like this: GDK_AVAILABLE_IN_3_18 void gtk_widget_set_font_options (GdkWidget *widget, const cairo_font_options_t *options); GDK_AVAILABLE_IN_3_!8 const cairo_font_options_t *gtk_widget_get_font_options (GtkWidget *widget); And it just needs to be hooked into gtkwidget.c:update_pango_context
Created attachment 306427 [details] [review] Add gtk_widget_set_font_options and gtk_widget_get_font_options
Review of attachment 306427 [details] [review]: Thanks for working on this ::: gtk/gtkwidget.c @@ +10383,3 @@ + cairo_font_options_t *options_copy = NULL; + + g_return_if_fail (GTK_IS_WIDGET (widget)); if (options == widget->priv->font_options) return; @@ -10354,5 +10356,34 @@ - if (screen) - { - pango_cairo_context_set_font_options (context, ... 2 more ... + if (widget->priv->font_options) + { + pango_cairo_context_set_font_options (context, ... 31 more ... We need to use C89 comments: /* foo */ @@ +10398,3 @@ + if (widget->priv->font_options != NULL) + cairo_font_options_destroy(widget->priv->font_options); + widget->priv->font_options = options_copy; I think we are missing a free of the options when the object is freed. Probably best to put this in dispose or finalize callbacks.
Review of attachment 306427 [details] [review]: Thanks for the patch! Only minor changes needed. One thing that is missing here is that you need to destroy the font_options in finalize. ::: gtk/gtkwidget.c @@ +10375,3 @@ + * Sets the #cairo_font_options_t used for Pango rendering. When not set, + * the defaults font options for the #GdkScreen will be used. + * You need to add a Since: 3.18 at the end of the doc comment here. @@ +10388,3 @@ + { + // On malloc failure this returns a copy of a dummy no-op font options + options_copy = cairo_font_options_copy(options); GTK+ coding style has a space before ( in function calls. That needs fixing throughout @@ +10398,3 @@ + if (widget->priv->font_options != NULL) + cairo_font_options_destroy(widget->priv->font_options); + widget->priv->font_options = options_copy; Looking at the corresponding code in gdk_screen_set_font_options, I don't think we have to handle oom here. I would just do this the same way we do in that function: if (priv->font_options != options) { if (priv->font_options) cairo_font_options_destroy (priv->font_options); if (options) priv->font_options = cairo_font_options_copy (options); else priv->font_options = NULL; @@ +10410,3 @@ + * + * Returns: (transfer none): the #cairo_font_options_t or %NULL if not set + * Since: 3.18 here as well
Created attachment 306442 [details] [review] Add gtk_widget_set_font_options and gtk_widget_get_font_options
Review of attachment 306442 [details] [review]: Looks good now, thanks!
Created attachment 306446 [details] [review] Add gtk_widget_set_font_options and gtk_widget_get_font_options
Review of attachment 306446 [details] [review]: ok
Attachment 306446 [details] pushed as 15c73a2 - Add gtk_widget_set_font_options and gtk_widget_get_font_options