Quoting Ralf Stephan <ralf@...748...>:
View ViewWidget +Desktop uses +DesktopWidget +SVGView +SVGViewWidget
(interface) == (abstract base class)
This feels weird to me. Ideally (I think) Desktop/SVGView should not need to know anything about the widgets attached to them -- they are basically "model" classes.
So the knowledge would be more one-way, where the widgets know about the model class, but not vice-versa.
Where the model needs to tell the widget something, it should be handled through callbacks that the widgets themselves register.
-- Placement New
Incidentally, you should no longer use placement new to initialize members of classes which have been converted to C++:
new (&_activate_signal) sigc::signal<void>();
The compiler takes care of that stuff automatically. We were only doing that in GObject-derived classes because GObject initialization bypasses the C++ compiler's normal mechanisms.
-- GC::Anchored
Also, since SPDesktop is GC::Managed now, it's a good idea to stop anchoring GC::Anchored objects from it. Failing to do so can result in memory leaks, particularly if there are reference cycles.
(SPDesktop and Inkscape::Selection do form a reference cycle...)
Under normal circumstances this means simply neither anchoring nor releasing such objects. So you would remove this bit from SPDesktop::~SPDesktop:
if (selection) { Inkscape::GC::release(selection); selection = NULL; }
But, since you were responsible for creating the object, you do still need to release it for it for it to be freeable. You can do that at creation:
selection = Inkscape::GC::release(new Inkscape::Selection(this));
This is safe because GC::release means "let the garbage collector decide", rather than "free this object", as it would for pure refcounting. The garbage collector can see that it is still referenced by a local variable/return value.
-mental