No vacancy here....

On Sunday 23 November 2003 13:56, David Odin wrote:

gimpmiscui.h also defines the gimp_plug_in_get_path() function which is only used by the FractalExplorer, gfig, and gflare plug-ins. This function doesn't look really useful, since it is only a wrapper for gimp_gimprc_query(), with some error-checking. I would vote to remove it. Anyway, even if we keep it, it should at least be moved to libgimp/gimppaths_pdb.c.

gimppaths_pdb.c is related to path (vector) objects. This gimp_plug_in_get_path seems to relate to filesystem paths. We better think of somewhere else for it to live.

Regards, JS
->

Hi,

David Odin writes:

Most of the functions of gimpmiscui.h are related to the GimpFixmePreview stuff. This part looks pretty useful to me, and the API is clean enough to be kept imho. The only bad point I see here is that this code use the deprecated widget GtkPreview, but this could be easily fixed by using some GtkDrawingArea/GdkRgb stuff.

I don't think using GtkPreview is a problem at all. It may be deprecated but actually it is a useful widget and noone so far could explain why it has been deprecated at all. Of course you can change the code to keep it's own buffer and write an expose event handler that draws it to a drawing area but you'd just be replication the code that is in GtkPreview already.

The problematic part about GimpFixmePreview is that it lives in the wrong library (it should probably be in libgimpwidgets) and that the function and struct names are not tolerable. Since it is likely that we will introduce a new preview widgets for 2.2, I'd like not to see this FixmePreview API in any libgimp library. I'd vote for the solution we've found for libgck.

gimpmiscui.h also defines the gimp_plug_in_get_path() function which is only used by the FractalExplorer, gfig, and gflare plug-ins. This function doesn't look really useful, since it is only a wrapper for gimp_gimprc_query(), with some error-checking. I would vote to remove it. Anyway, even if we keep it, it should at least be moved to libgimp/gimppaths_pdb.c.

I agree that it isn't generally useful. It has been outlined already why libgimp/gimppaths_pdb.c is not a good place. Let me add the note that gimppaths_pdb.c is an autogenerated PDW wrapper and that it's part of libgimp, not libgimpui.

The last function in gimpmiscui.h is gimp_parameter_settings_new(). It is currently used by many plug-ins to present the parameters in a consistant way. This one looks pretty useful to me, but should be moved to in a more appropriate place, such as libgimpwidgets/gimpwidgets.[ch]

This function may look useful but it is not acceptable for libgimpwidgets for the following reasons:

(1) It hardcodes a frame title to a string that we tried to remove from as many plug-ins as possible. "Parameter Settings" is really the worst title one could choose for an options frame. The function should have taken a title parameter.

(2) The function name is stupid, mainly for the reason given above.

And this is definitely the worst: (3) And this is definitely the worst: The function either creates a table or a frame depending on the parameters passed to it.

In my opinion, the change that introduced this function should be completely reverted. The function doesn't provide any useful funtionality and should be folded back into the plug-ins that use it.

Sven

Hi,

David Odin wrote:

Most of the functions of gimpmiscui.h are related to the GimpFixmePreview stuff. This part looks pretty useful to me, and the API is clean enough to be kept imho. The only bad point I see here is that this code use the deprecated widget GtkPreview, but this could be easily fixed by using some GtkDrawingArea/GdkRgb stuff.

What do you propose, exactly?

gimpmiscui.h also defines the gimp_plug_in_get_path() function which is only used by the FractalExplorer, gfig, and gflare plug-ins. This function doesn't look really useful, since it is only a wrapper for gimp_gimprc_query(), with some error-checking. I would vote to remove it. Anyway, even if we keep it, it should at least be moved to libgimp/gimppaths_pdb.c.

Removal sounds good.

All the functions in libgimp/gimpmisc.h are related to regions or pixels fetchers or iterators. I'll investigate more to see if they should be kept, moved in a better place, or removed.

A pixel iterator that transparently does tile management would be very useful, so some of these functions should probably stay around. The API needs some work, though - looks like most of the functions could be delegated to the plug-ins somewhat, once we kept a PixelFetcher and an Iterator (perhaps with a slightly modified API?).

I think that the GimpPixelFetcher API is pretty usable and transparently does tile stuff... perhaps we should be using this more consistently? For example, in plug-ins/common/edge.c the pixelfetcher is used only for the edge cases where the pixels aren't all in the same tile, which kind of defeats the purpose of transparent tile handling.

The current API just feels a little awkward, but I'm not sure how to improve it apart from perhaps changing the names of the functions ending with 2 ... perhaps just splitting out into gimppixelfetcher.[ch] and gimprgniterator.[ch], and refining the API would be enough here?

Cheers, Dave.