On 21 Feb 2001, Sven Neumann wrote: > Nathan C Summers <rockwlrs@xxxxxxxxxx> writes: > > > Problem: Many tools instruct the core to destroy themselves on certain > > kinds of state changes, such as a change of image or display. While some > > tools are quite good at handling these changes, others are quite unstable > > psychologically and commit hara-kari for the smallest reasons. > > This problem should go away as soon as all tools are proper objects > with init and finalize functions, destroy signals etc. That lessens but does not solve the problem that the code is more complicated because of the destruction of tools due to state changes. > > This is inefficient. It complicates the tool handling code, causes a lot of > > unneccesary frees, mallocs and initialzations, and seems to me to be a lot > > like "Windows must restart for these changes to take effect." On the > > other hand, multiple instances of a tool should exist for diffent input > > devices so that they can be in different states. > > I do not think object destruction and creation every now and then causes > significant overhead. It creates no significant overhead in interactive mode, true, but think about scripts. Imagine a script that loops like this: tool A plugin X tool B plugin Y In the current system, everything in the tool is recreated every time through the loop, resulting in a lot of redundant operations. PDB functions are handled rather inelegantly currently and should be a first class part of the tool system. I'm thinking about various ways of doing that right now. > Tools should be created when needed and destroyed > when the last refcount drops. I see no advantage in keeping refcounts on > unused tool objects. Of course not. The question is when to create and when is a tool not needed. > > Proposed Solution: Tools should just deal with having thier state > > changed. We can introduce a function tool_manager_get_tool that takes a > > tool class and input device and returns the correct tool (creating it on > > the fly if needed). The toolbox would just call that function and set the > > current tool on that device to that. active_tool should just go away. > > (so should iterating over the list of registered tools, perhaps) > > IMO the GimpToolInfo object Mitch just introduced to resurrect the tool > system is the right way to go. GimpToolInfo objects stay around so Gimp > knows what tools are available, can display icons, etc., the real GimpTool > only exists when needed. I really like what Mitch has done with the tool code. He read my mind when it came to renaming the gimp_tool_emit_* functions to more standard names. The GimpToolInfo objects are elegant. Mitch: if you haven't yet written the code to get rid of the items marked as needing to go away in the GimpTool class structure, let me know so that I can write it and you can spend your time on less trivial code. I also wonder what happened to standard_control_func. I know it was misnamed, but it contained code that all of the current tools use and was fit for inheritance. In general, it should be ran before the tool's specialized control code. > > Problem: Some tools, such as iscissors, keep around a lot of cached data > > generated from the image they are attached to. Changing the image they > > are working on clears this cache. This can be slow when working on > > multiple images or layers. > > Is there really a noticeable slowdown? Yes. Maybe not on gigahertz machines, but it sure is noticable on my old machine, especially on big images. One of the big advantages to not destroying a tool after another is selected is that caches can be maintained longer. > > Proposed Solution: a generic object, ToolCache, from which the specific > > kind of cache would be derived. A virtual function, compute_cache, would > > compute the value to be cached. > > > > For efficiency reasons, the cache may either be generated on-the-fly when > > its values are requested, or whenever the target changes. However, if the > > cache is not accessed after a certain number of changes it automagically > > switches to on-the-fly mode to conserve CPU cycles. <snip> > > Problem: The number of ToolCaches should be kept down to a reasonable > > level. > > > > Proposed Solution: have a maximum number a tool can have. Make it > > configurable, perhaps on a per-tool basis. > > This sounds overly complicated and I'm not convinced that there's a reason > to introduce such a complex system. I might be wrong and simply not seeing > your point here. I'm partly anticipating tool plugins, where the latencies would be bigger. But my point is that lots of redundant calculations are never a good thing. After thinking about it for a bit I've decided that the immediate refreshing may not be such a great idea, and that it should just redo the calculations on-the-fly if neccessary. It also optimizes cases where you may or may not be accessing information. We may still want to implement the immediate recalculations for plug-ins, since latency is greater there. (Think scripts, especially.) Some profiling would be in order. I've decided to rename the object to GimpCache because it's not just useful for tools. > > P. S. The new implementations of DrawCore (now called DrawingTool) and > > PaintCore (now called PaintingTool) will be commited as soon as I finish > > the game of freeciv I'm playing. > > All Gimp objects start with the word Gimp, so it should be GimpDrawingTool > and GimpPaintingTool. It is. I just think of the Gimp part as a namespace. Besides, after typing GimpPaintingTool way too many times (yes, I found the macro command in my editor...) I'm kind of sick of typing that big, long name. ;) (btw, why the "ing" ??). Because I figured that people would confuse GimpPaintTool with the paintbrush. > If I remember correctly, the > DrawCore is that ugly thing we use to draw on the display, so it should > probably totally go away and be implemented on top of the yet to be written > GimpDisplay. There isn't much code to it. But it will have to stay until GimpDisplay gets written, as you can't meaningfully write code for an API that doesn't exist. ;) > GimpPaintingTool is meant as the base class all paint tools derive from?! > I think it would help if you could send more info about your plans for > the Gimp's tool system. OK, here is the class hierarchy (some tools have been left out for brevity) GtkObject | GimpObject | GimpTool | \ | GimpTextTool | \ | GimpDynamicTextTool? | GimpDrawingTool | | \ | | GimpColorPicker | | | GimpPaintingTool | \ | GimpPaintbrush | GimpTransformingTool | | GimpRotate GimpPerspective (the TranformingTools have names common enough that they probably should have a Tool appended to them.) The Transform tool also has the problem that it's really a front for the other transform tools. But neither problem is unsolvable. I'd like all the tools that don't take a specific point as input to be turned into something else. My definition of a tool is something that you click on the image with. They should also use the image view directly (as should plugins). For example, by color select would count as a tool, but contrast/brightness wouldn't. It will take some effort to do, of course, but it's worth it in simplicity and user understanding. (Why do some things that work like filters not respond to repeat last filter?, etc.) Something to think about. Rockwalrus
