On Tue, 2008-03-11 at 12:56 -0500, Jonathon Jongsma wrote: > On 3/11/08, Armin Burgmeier <[EMAIL PROTECTED]> 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. > > > > 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 > > I haven't had time to read this proposal thoroughly yet, but I just > wanted to mention that when I was wrapping this function originally, I > thought it felt like a pretty poor API at the C level. So maybe it > would be best to try to improve the API at the C level before > attempting workarounds in the wrapper. That said, I'll try to read > your proposal more thoroughly when I get a little more time and offer > comments as well.
The C API is the same as the g_list_* API. g_list_prepend, g_list_sort etc. behave the same way, so it's probably convenient for C programmers this way. Armin _______________________________________________ gtkmm-list mailing list [email protected] http://mail.gnome.org/mailman/listinfo/gtkmm-list
