Hi Rock,
sorry I did not comment on this earlier, I was quite busy since
returning from Guadec.
As mentioned in an earlier mail from Sven, we are not at all happy
with the introducion of plug-in tools and the ongoing exposure of the
internal object structure outside the core.
While I have no doubt that it's implementable in a nice and working
way, I think it is the wrong way to go. Tools are an internal
implementation detail that is closely related to ugly things like
GdkEvents and fast response to user input.
We should not add further tools, but drastically reduce their number
by separating their ui from the core. Then we make the factored out
core code (their actual functionality) accessible via both the PDB and
a module API. This is close to be finished for the paint tools. In the
end, there will be *one* paint tool with a variety of paint_core
implementations, each of them optionally featuring it's own tool
options.
The same holds true for the ImageMap tools, they will probably all
collapse into one single tool which simply uses the image_map modules
which are compiled in or loaded.
The tool itself is an ugly piece of code which is particularly hard to
maintain in in't current state, so I really want to keep it an
internal detail. Nobody should be forced to subclass GimpFooTool just
to get new functionality in. The way to go IMHO is subclassing
GimpFooCore (e.g. GimpPaintCore) to get new functionality and simply
let it handle by the single GimpPaintTool.
So why should a plug-in author subclass GimpPaintTool if the gimp core
does not even subclass it itself. (note it does currently, but all
paint tool subclasses in current cvs just handle different tool
options).
Why whould somebody want to deal with
GimpToolControl *gimp_tool_control_new (gboolean scroll_lock,
gboolean auto_snap_to,
gboolean preserve,
gboolean handle_empty_image,
gboolean perfectmouse,
/* are all these necessary? */
GdkCursorType cursor,
GimpToolCursorType tool_cursor,
GimpCursorModifier cursor_modifier,
GdkCursorType toggle_cursor,
GimpToolCursorType toggle_tool_cursor,
GimpCursorModifier toggle_cursor_modifier);
where all parameters are internal implementation details (read: hacks).
About 50% of these paramaters were added to get rid of nasty bugs
related to cursor updating and the whole tool system needs much more
attention before it actually works nicely. We might even want to add
more hacks to GimpTool and friends. We might want to change the way
the tools interact with the display and GdkEvents and we may want to
do this in a stable version if it is needed to fix bugs. This is
nothing we want to expose to a library.
Moreover, libgimpproxy does no good thing to our internal object
system. Exposing internal stuff to a library automatically submits it
to binary compatibility issuees, which is definitely not what we
want. While it's easy to stay compatible to a reduced module API like
for paint and image_map methods, it's a pain to do so for a whole
object system, it's inheritance tree, it's properties, signals and
whatever.
For instance, you moved GimpObject (including overly ugly crap like
it's "disconnect" signal) to libgimpproxy. Once tool plug-ins want to
be more than just a proof-of-concept, they will need access to
GimpDrawable, GimpImage and so on objects, submitting them all to
"don't change me" ABI issues.
I vote for creating a branch in CVS and reverting the tool plug-in
stuff in the HEAD branch, so we can finally get rid of the tool
overkill instead of adding the possibility to add new ones.
ciao,
--mitch
