12:54 a.m.
On Fri, Aug 26, 2005 at 09:11:54AM +0200, Ralf Stephan wrote:
You wrote a while ago:
> ...
> Next is the main window work. This involves conversion of two code
> objects: SPView and SPDesktop. Each of those also have a variety of
> accessory classes to convert as well. This is the area I've been
> working on, and I'm trying to do it by first refactoring these from C to
> C++. If anyone's interested in working on this, I can give more details.
Yes, please. I assume ui/view/edit.* is the new class for SPDesktop,
so I guess what is next is completing Edit according to SPDesktop
and test it in the --new-gui environment. After that,
That's correct.
However, I found that straight up conversion of SPDesktop to ::Edit to
be difficult; I attempted to do this in one all-at-once conversion but
it quickly unravelled into a mess. I think the best approach is to do
the conversion a piece at a time. Start at an easy point and work your
way in.
The place I found to start was with converting SPView to C++. I've got
this about half done, and it'll be fairly obvious what's left to do.
After that, it may make sense to go ahead and knock out conversion of
the SPViewWidget class next, since that's not too complicated.
Since SPDesktop derives from SPView, having SPView in C++ is the first
step to convert SPDesktop. Next thing would be to start converting
SPDesktop into a C++ class. Next move the file(s) to their proper final
location. Finally rename SPDesktop to Inkscape::UI::View::Edit.
I found for the renaming, that it helped to create a typedef.
Note that there are some forward definitions of the class that get in
the way; I wasn't sure if it was better to remove them or to try to
refactor them. Ultimately, though, at some point you have to just start
going through the whole codebase and rename stuff. That sounds like a
lot of work, but actually for SPView I found I was able to do it over
the course of a long airplane flights. ;-)
> ...
> Fourth, there are several main window components such as the statusbar,
> context menu, and so on that have not received any attention so far. We
> have the gtk+ code for these items, and they just need to be converted
> to gtkmm. I suspect the best approach is to refactor them in place.
> The current statusbar code is pretty good, although it could use a few
> enhancements. The current context menu is okay but could use a lot more
> attention to make it more usable.
Do I have the order of things right?
It is likely I will take over single steps of the process *without*
taking over the whole thing officially, as I like to work on several
different micro-tasks at the same time, probably going the circle
documentation->bugfixing->refactoring->doc...
Thus, I will ask you from time to time 'what next?' until I have
the big picture.
Yes, I think you've got it. The process isn't totally linear; there's
several things that could be worked on independently. Let me know how
your progress goes; I expect to keep plugging away at it too.
Bryce
2:17 a.m.
New subject: patterns (was Re: Getting involved)
Having read a bit into code called by --new-gui, I noticed that
Inkscape::UI::View::Edit is rather the Gtkmm equivalent of SPDesktopWidget,
while SPDesktop is used by both the Gtk+ and Gtkmm application code,
and rightly so, as it no longer contains GUI specific code.
Since SPDesktopWidget and all other view widgets are now separated out
of the resp. view classes, it would be time to apply a pattern, I believe
it's called a Bridge by the gang of four:
View ViewWidget subclass ViewWidgetCImpl
+Desktop uses +DesktopWidget ===+===> +DesktopWidgetCImpl
+SVGView +SVGViewWidget | +SVGViewWidgetCImpl
|
(interface) | (real code, Gtk)
== |
(abstract base class) |
| ViewWidgetCXXImpl
+===> +DesktopWidgetCXXImpl
+SVGViewWidgetCXXImpl
(real code, Gtkmm)
What do you say?
Is the design sound?
Are there better names?
ralf
9:52 a.m.
New subject: patterns (was Re: Getting involved)
On Wed, Sep 07, 2005 at 11:17:40AM +0200, Ralf Stephan wrote:
Having read a bit into code called by --new-gui, I noticed that
Inkscape::UI::View::Edit is rather the Gtkmm equivalent of SPDesktopWidget,
while SPDesktop is used by both the Gtk+ and Gtkmm application code,
and rightly so, as it no longer contains GUI specific code.
Since SPDesktopWidget and all other view widgets are now separated out
of the resp. view classes, it would be time to apply a pattern, I believe
it's called a Bridge by the gang of four:
View ViewWidget subclass ViewWidgetCImpl
+Desktop uses +DesktopWidget ===+===> +DesktopWidgetCImpl
+SVGView +SVGViewWidget | +SVGViewWidgetCImpl
|
(interface) | (real code, Gtk)
== |
(abstract base class) |
| ViewWidgetCXXImpl
+===> +DesktopWidgetCXXImpl
+SVGViewWidgetCXXImpl
(real code, Gtkmm)
What do you say?
Is the design sound?
Are there better names?
This looks good. I tend to prefer the name 'Edit' over 'Desktop', but
otherwise this looks good. Do you think we would really need both C and
C++ implementations?
Bryce
2:33 a.m.
New subject: patterns
Bryce:
I tend to prefer the name 'Edit' over 'Desktop'
This statement is not consistent with your (early) implementation
in application/editor.*, where you have a SPDesktop* AND an Edit*.
OTOH, it supports my notion that some of your code in ui/view/edit.*
really belongs in EditWidget, as part of it it is Gtkmm specific.
mental:
It's almost always better to do a straightforward implementation
and get
it structured appropriately -- and _then_ factor out the appropriate
abstractions, than it is to introduce an extra abstraction anticipating
a future need.
Yes. That's the order of steps.
JonCruz:
So I'd probably summarize that as starting with abstraction is
good,
just as long as it's the proper abstraction. :-)
I fear there is no such thing. Things keep changing.
But thanks for your kind mail.
ralf
7:59 a.m.
New subject: patterns
On Sep 8, 2005, at 2:33 AM, Ralf Stephan wrote:
JonCruz:
> So I'd probably summarize that as starting with abstraction is good,
> just as long as it's the proper abstraction. :-)
>
I fear there is no such thing. Things keep changing.
But thanks for your kind mail.
Your fears are mainly unfounded.
:-)
I've been doing this at work for years. Sometimes it takes a little
to get into the swing of things when starting this with someone new,
but by the end of the design session, things usually fall together
and a better grasp of the problem is arrived at.
Sometimes it's a little hard to grasp without a live discussion, so
if you get on Jabber sometime later PDT I can go through some of what
I mean.
9:56 a.m.
New subject: patterns (was Re: Getting involved)
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
11:13 a.m.
New subject: patterns
> 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.
I wondered about the existing circularity. Some of the needed callbacks
would be only used by singular classes, and where the reference to the
widget itself is returned, it is used by Gtk-specific classes. In these
cases, Desktop is best not involved at all.
Bryce asked:
Do you think we would really need both C and C++ implementations?
Maybe not, although it would make switching easy. But replace C with
Mac or native Windows or ... So the abstraction is needed anyway.
Nearly 10 years ago, I created and maintained the yarec package that
had ncurses and Gtk UIs in one binary, so the idea is not new. However,
I don't propose full Gtk and Gtkmm inkscape in one binary!
Thanks for the feedback, to which I might write more later,
ralf
3:15 p.m.
New subject: patterns
On Wed, Sep 07, 2005 at 08:13:24PM +0200, Ralf Stephan wrote:
> > 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.
I wondered about the existing circularity. Some of the needed callbacks
would be only used by singular classes, and where the reference to the
widget itself is returned, it is used by Gtk-specific classes. In these
cases, Desktop is best not involved at all.
Bryce asked:
> Do you think we would really need both C and C++ implementations?
Maybe not, although it would make switching easy. But replace C with
Mac or native Windows or ... So the abstraction is needed anyway.
Nearly 10 years ago, I created and maintained the yarec package that
had ncurses and Gtk UIs in one binary, so the idea is not new. However,
I don't propose full Gtk and Gtkmm inkscape in one binary!
Ah, that makes sense. I didn't think about alternate widget set
implementations. It's entirely possible that one day someone would come
along with a cool, complete wxWidgets-based interface that we would like
to hook onto, and this level of abstraction would permit that.
Btw, there is a presentation in the docs directory of the codebase I did
that explains some of my ideas for the rearchitecting of the
View/Desktop code. You might find it worthwhile to take a look at it.
Bryce
5:39 p.m.
New subject: patterns
On Wed, 2005-09-07 at 15:15 -0700, Bryce Harrington wrote:
Ah, that makes sense. I didn't think about alternate widget set
implementations. It's entirely possible that one day someone would come
along with a cool, complete wxWidgets-based interface that we would like
to hook onto, and this level of abstraction would permit that.
I dunno, I kinda feel like we should YAGNI the extra level of
abstraction for now. At least until the gtkmm stuff has completely
solidified.
It's almost always better to do a straightforward implementation and get
it structured appropriately -- and _then_ factor out the appropriate
abstractions, than it is to introduce an extra abstraction anticipating
a future need.
While the future need might be realistic, generally an abstraction
created out of thin air (rather than being hoisted out of an existing
implementation) tends to fit poorly with the actual requirements when it
is finally needed.
-mental
9:12 p.m.
New subject: patterns
On Sep 7, 2005, at 5:39 PM, MenTaLguY wrote:
It's almost always better to do a straightforward implementation
and get
it structured appropriately -- and _then_ factor out the appropriate
abstractions, than it is to introduce an extra abstraction
anticipating
a future need.
At first I disagreed with this generalization, but then I realized
some details that differentiate what I was thinking of and how it
diverges.
* Adding extra abstraction for later -- more often not good.
* Starting with abstraction and implementing behind it.
The latter is what I'd suggest is good. However, the driving force
would be in analyzing the base concepts involved and proceeding from
there. That is, toss around all the ideas involved, and then boil
them down to their pure "thing-ness" that make those things what they
are. Then group up in nice modules, and abstract things at the
boundaries. Then go in and implement each module.
So it would be the case for abstracting at module boundaries rather
than abstracting for possible future need. Of course, when done
properly the former can make the latter much easier. Then again
abstracting at module boundaries helps reduce spaghetti linkage,
improve testability and clarify desgin. So that can go in as an
immediate payoff as compared to the "anticipating a future need"
guess at payoff.
So I'd probably summarize that as starting with abstraction is good,
just as long as it's the proper abstraction. :-)
6658
days inactive
6671
days old
inkscape-devel@lists.inkscape.org
9 comments
5 participants
participants (5)
-
unknown＠example.com -
Bryce Harrington -
Jon A. Cruz -
MenTaLguY -
Ralf Stephan