Re: goocanvasmm: Item::get_items_at API

[Murray Cumming <murrayc murrayc com>]

On Tue, 2008-03-11 at 17:19 +0100, Armin Burgmeier wrote:
> I believe the current implementation of Item::get_items_at is wrong:
> 
> #m4 _CONVERSION(`GList*',`Glib::ListHandle< Glib::RefPtr<Item> >',`
> $2($3, Glib::OWNERSHIP_NONE)')
> 
> _WRAP_METHOD(Glib::ListHandle< Glib::RefPtr<Item> > get_items_at(double
> x, double y, const Cairo::RefPtr<Cairo::Context>& context, bool
> is_pointer_event, bool parent_is_visible, Glib::ListHandle<
> Glib::RefPtr<Item> >& found_items), goo_canvas_item_get_items_at)
> 
> Since the ownership of the returned ListHandle is set to NONE, the
> actual GList* will be leaked. However, if we set shallow ownership, then
> we get memory corruption because in the C API, the returned GList* is
> meant to be the same as the one passed in, with perhaps some items added
> to the front and thus returning a new list head.

Can we copy the GList* to avoid that problem?

> If I get it right, then the ownership of the found_items ListHandle
> needs to be set to none, and a new ListHandle with shallow ownership
> needs to be returned. We can't do this though, probably, because the
> ownership of the ListHandle is private.
> 
> I think the C++ way to handle this is to use an insert iterator such as
> 
> template<typename InsertIter>
> InsertIter get_items_at(double x, double y, const
> Cairo::RefPtr<Cairo::Context>& context, bool is_pointer_event, bool
> parent_is_visible, InsertIter iter);
> 
> to be used like this:
> 
> std::vector<Glib::RefPtr<Item> > items;
> some_item->get_items_at(..., std::inserter(items, items.end()));
> 
> This is also more powerful because items can be inserted anywhere
> instead of just at the beginning. However, I don't think other *mm
> projects do anything similar, which is why I am asking for opinions
> first.
> 
> The corresponding vfunc still needs special consideration then, since
> virtual functions cannot be templatized. Probably it is enough to just
> pass a specific container, such as std::vector, since the only code
> calling it is goocanvasmm itself anyway.
> 
> Any opinions? Do you think it is OK to change it this way, or do you
> have other ideas to tackle this?
> 
> Armin
> 
> _______________________________________________
> gtkmm-list mailing list
> gtkmm-list gnome org
> http://mail.gnome.org/mailman/listinfo/gtkmm-list
-- 
Murray Cumming
murrayc murrayc com
www.murrayc.com
www.openismus.com