On Thu, 19 Feb 2004 16:00:08 +0100
Sven Neumann <[EMAIL PROTECTED]> wrote:

> Hi,
> Ernst Lippe <[EMAIL PROTECTED]> writes:
> > The decision if the plug-in "needs" the temporary drawable
> > is made by its designer and it is certainly not forced by
> > the preview. It is perfectly possible to draw a row of
> > pixels on the preview (indeed for a very long period that
> > has been the only way you could draw on the preview, support
> > for drawables is a recent addition). If a plug-in designer
> > writes a plug-in in such a way that it can only output to
> > a drawable, then it will always have to allocate a temporary
> > drawable (unless you want to mess around with the original
> > input drawable, which sound pretty horrible). 
> > But when the internal datastructures for the algorithm
> > can be used to construct a single row for the preview
> > there is no reason to use a temporary drawable.
> So you are seeing the GimpPreview as just a widget that plug-ins can
> draw on. 
Basically, yes.

> However our goal is to provide a way to add a preview to
> plug-ins basically w/o changing their code. 
It is an interesting idea, but when you look at the
requirements for the preview as they were discussed last
year (see http://refocus.sourceforge.com/preview/requirements.html)
it was not part of the list. 

If you would have proposed it as a requirement at that stage
I would have rejected it, because I simply think that it
is infeasible as it stands.

There are several point that you will virtually always have
to change when you want to add a preview to an existing
plug-in, and in some cases (some plug-ins have pretty messy
code) these changes have to be rather drastic.
A few points:
* How do you detect that parameters have been changed?
Users will expect that the preview gets updated when
they change some parameters. Many plug-ins do not
need to detect this at the moment because the values
only become relevant when the user presses the OK button.
When rewriting such a plug-in you will have to add
modification detection code, and you will also have
to modify the code that collects the current values
of the parameters and runs the underlying algorithm
with these parameters. Some plug-ins completely
mix their business logic with their GUI and in these
cases it might require quite a bit of work to fix this.
* Current plug-ins assume that they can write the
results back to the input drawable. So you will either
need to coerce to write back their results somewhere
else or you will have to mess around with the original
drawable (probably some hidden global "preview-mode" switch).
* Current plug-ins are not very flexible in the area
that they render. Some always generate the entire
drawable, and others generate the area from gimp_drawable_mask_bounds.
For the latter category you could of course think of a hack
that changes the output of gimp_drawable_mask_bounds based
on some hidden global "preview-mode" switch, but then
you will also have to consider the consequence of this
decision for all other places where this function may
be used, not to mention the fact that the whole idea
of global mode switches is inherently ugly.

So I don't think that should try to find some solution without
rewriting, but to find some framework that could easily fit a large
set of the current plug-ins.

> The change should be
> limited to the plug-in dialog and a few hooks here and there. 

Unless there have been some major changes to the GIMP plug-ins since I
last looked at them, I believe that this idea is not very realistic.

> Your
> preview widget could be part of this infrastructure since it provides
> a way to draw pixels to the screen, but in its current state, it
> certainly doesn't meet the goals I outlined above.

> > > One of the design requirements of the preview is to provide a
> > > convenient way for plug-ins to preview their results. Convenient means
> > > that it should be possible to reuse the existing pixel manipulations
> > > routines without or with small changes. Allocating temporary drawables
> > > in the core is in my opinion not a viable solution for this problem.
> > > 
> > > It should however be possible to keep the temporary drawables
> > > completely on the plug-in side. To achieve this, the GimpDrawable
> > > wrapper would have to know whether it needs to get the tiles from the
> > > core (via shared memory or over the pipe) or allocate them in the
> > > plug-in process. I think this change would be rather straight-forward.
> > 
> > But unless I am mistaken, this temporary drawable has only
> > a limited functionality, because you cannot use any of
> > the "core" operations on it.
> > So I am not really certain if it would be worth the effort.
> I don't see how core operations would be useful at all. The only use
> of the temporary drawable is to allow the preview to be rendered using
> the same GimpPixelRgn routines that are used to render the final result.

Uh, are you saying here that we can simply ignore those plug-ins that
call any of the core tool operations or other plug-ins?

> > Furthermore, this functionality would only be needed
> > for those plug-in algorithms where it is difficult
> > to compute an entire row of pixels, and actually
> > I cannot remember that I have ever seen one. 
> > So if you have some good examples, perhaps the problem
> > could also be solved by changing the interface of the
> > preview, e.g. adding support for drawing tiles on the
> > preview.
> Basically any plug-in that works on pixel rows is less than optimal
> and should be rewritten at some point.  Good plug-ins are supposed to
> work on tiles rather than rows. 

I am afraid that with this definition there are very few "good"

> If our preview widget encourages the
> plug-in author to work on a row-by-row basis, then we will never get
> plug-ins that operate on the pixels in the most efficient way.

Like I said, I don't object to adding a tile-based interface as well,
it is just that I have not yet seen any plug-in that needed it.

Now obviously you have a problem with having to allocate temporary
drawables in the GIMP core just for the sake of previewing.

But what is the problem? Is it just a performance issue?  As far as I
have seen, it is not a serious issue, I have a pretty slow computer
and the performance of the preview is quite acceptable. In most cases
the drawables are pretty small, much smaller than the real images.  In
general I am pretty skeptical about design decisions that sacrifice
simplicity for performance's sake, in the long term that is generally
a bad strategy (yes there are some cases where it is a correct
decision but you should always analyze them pretty carefully).

So what is the simplest solution for the plug-ins?

Most existing plug-ins will write their output only to their input
drawable. But for the preview we don't want to modify the original
drawable, but the plug-in must somehow be coerced to write its output
to something that the preview could read. If you really don't want to
change the plug-ins algorithm at all the only solution is to somehow
intercept the modifications to the original drawable and route them to
the plug-in. This appears to be a different solution than you
proposed, and I think that it would be very messy, you would have to
change the functions that operate on the drawable to check if they
should operate on the real drawable or that they should send their
result to some other structure that could be used with the plug-in.
Then the plug-in would have to somehow set a switch that is associated
with this drawable to indicate if it is in "preview-mode" or not. This
entire approach looks pretty messy, and it can't see how it could work
for plug-ins that invoke any of the core painting tools or other
plug-ins. So on the face of it this approach does not look very

Another approach is to rewrite each plug-in so that it can use an
output data-structure that is different from the input drawable.  The
most obvious solution is to give the plug-in a second output drawable
that could be different from the input drawable. For most plug-ins
that I have seen this is a fairly easy modification. I guess that you
have also been thinking along these lines. So here is were your
proposal comes in. My first objection against it, is that it appears
to break the existing semantics of a drawable because most operations
that are valid for a drawable are not valid for a temporary drawable.
Also this makes the existing code more complicated, the cache manager
must also be able to handle tiles that are never sent to the GIMP
core, and there must be some kind of error handling for unsupported
operations on these temporary drawables. I don't know how complicated
this would be but it certainly won't make the existing code more
simple. Another problem for which I don't see an easy solution is how
many tiles should be cached for the drawable. After all when you are
worried about performance you are probably not very happy when the
temporary drawable has the same number of tiles as the original
image. A possible solution is to allocate only a single fixed tile for
the entire temporary drawable, but in that case this solution is only
usable for plug-in algorithms that can compute individual tiles in a
single pass. I believe that such plug-ins are only a very small
fraction of the total set of existing plug-ins. So, unless you
consider it OK to keep all tiles for the temporary drawable in core,
it seems that there must be some additional function call to "commit"
a certain tile. At this moment I don't see an easy method that the
cache manager could use to determine this, a possible solution would
be to use gimp_tile_unref, but then you would have to be very careful
not to call this function inside the other pixel region routines,
e.g. during a gimp_pixel_rgn_get_row.

A slightly different solution for the temporary drawable problem would
be to define a new object that can handle both rendering to the tiles
of a drawable as well as rendering to some local set of
tiles. Essentially this object would have an interface that is very
similar to the pixel-region interface. I have implemented something
similar for my refocus plug-in and I consider it more elegant that
changing the existing semantics of a drawable. After all, in cases
where you could successfully use temporary drawable, you are only using
a very limited part of the drawable functionality, effectively you are
assuming that a drawable is simply a collection of tiles. Of course
the plug-ins must be rewritten so that they use the new function calls
instead of the old pixel region functions, but it seems that you could
easily achieve most modifications with a simple textual substitution.

Still I am not very convinced that it is really worth the
effort. There are only very few tile-based plug-ins for which
"temporary drawable" might be the best approach.  A large number of
the plug-ins generate data on a row basis and for these plug-ins it
might be both simpler and faster (no tile overhead) to simply write
the generated rows to the preview.

So you are completely correct when you say that preview plug-in
does not solve all of your plug-in problems. 

But I remain quite skeptical of your implicit suggestion that the overhead
due to using a real drawable is important.
When you look at the total sequence of events that is normally used
to generate a new content of the preview there are quite
a number of steps:
* X generates a mouse event. 
* The mouse event gets routed to GTK (involves at least one process swap,
I am not completely certain if the window manager is involved
as well).
* GTK processes the event and routes it to the preview.
* Preview allocates a new drawable (actually it could
reuse the previous drawable when it has the same size).
* Preview calls the plug-in to render a new image.
* Preview draws the image on the pixbuf.
* GTK draws the pixbuf on the screen via X.
So when you look at the whole process I really doubt
that the drawable is in general a serious bottle-neck.
Of course, I can be convinced otherwise if you could
show me some good timing measurements.

Ernst Lippe
Gimp-developer mailing list

Reply via email to