Re: [Vala] Pango.Attribute questions

[pHilipp Zabel <philipp zabel gmail com>]

Hi,

On Fri, Nov 6, 2009 at 4:53 PM, Abderrahim Kitouni <a kitouni gmail com> wrote:
> Hi,
>
> 2009/11/6 pHilipp Zabel <philipp zabel gmail com>:
> > Hi,
> >
> > I'm a bit confused about how Pango.Attribute is supposed to work,
> > probable because I don't understand ownership issues of [Compact]
> > classes properly.
> >
> > I want to set some Pango attributes on a Gtk.CellRendererText, so I tried this:
> >
> > 1:      Pango.AttrList attr_list = new Pango.AttrList ();
> > 2:      Pango.Attribute attr_color = Pango.attr_foreground_new (color.red,
> > color.green, color.blue);
> > 3:      Pango.Attribute attr_scale = Pango.attr_scale_new (Pango.Scale.SMALL);
> > 4:      attr_list.insert (attr_color);
> > 5:      attr_list.insert (attr_scale);
> > 6:      renderer.attributes = attr_list;
> >
> > Vala complains with an "error: duplicating Attribute instance, use
> > unowned variable or explicitly invoke copy method" in lines 2, 3, 4
> > and 5.
>
> This sounds like a bug in the vapi, there is no reason you should be
> duplicating instances here : if I understand correctly
> attr_forground_new and attr_scale_new are actually constructors of
> Pango.Attribute, so you should own the returned reference (and hence
> not need to copy it or use an unowned variable).

Yes, looking at the vapi, it seems to me that the unowned keywords for
pango_attr_*_new are wrong.

> I'm not sure about insert but unless it takes ownership of the passed
> attribute (i.e. you don't need to free it after you pass it), you
> don't need to copy it. If it does take ownership, another trick (not
> mentioned in the error message because it doesn't work in all
> situations is to use an '(owned)' cast, i.e. attr_list.insert ((owned)
> attr_color); this way, attr_list is now responsible for freeing
> attr_color, and your attr_color is set to null).

Yes, insert takes ownership.

[...]
> > 1:      attr_list = pango_attr_list_new ();
> > 2:      attr_color = pango_attr_foreground_new (color.red, color.green, color.blue);
> > 3:      attr_scale = pango_attr_scale_new (PANGO_SCALE_SMALL);
> > 4:      pango_attr_list_insert (attr_list, attr_color);
> > 5:      pango_attr_list_insert (attr_list, attr_scale);
> > 6:      g_object_set (renderer, "attributes", attr_list, NULL);
>
> so the problem is that attr_forground_new and attr_scale_new should
> not be returning unowned references (I think they should be
> constructors).
> (and you should use either owned cast or copy depending on whether you
> wan to use it later or not)

How would I go about to change the pango.vapi in the right way?

After applying the patch attached to bug
https://bugzilla.gnome.org/show_bug.cgi?id=600993, the correct code is
generated when I use (owned) for the inserts, like you suggest:

1:      Pango.AttrList attr_list = new Pango.AttrList ();
2:      Pango.Attribute attr_color = Pango.attr_foreground_new
(color.red, color.green, color.blue);
3:      Pango.Attribute attr_scale = Pango.attr_scale_new (Pango.Scale.SMALL);
4:      attr_list.insert ((owned) attr_color);
5:      attr_list.insert ((owned) attr_scale);
6:      renderer.attributes = attr_list;

There is another issue with this binding, this
Pango.attr_font_desc_new constructor is missing completely:
        public static Pango.Attribute attr_font_desc_new (Pango.FontDescription desc);

> HTH,

helped indeed, thanks
Philipp