Re: [Gimp-developer] About libgimp/gimpmisc* files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

David Odin <David@xxxxxxxxxxx> 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

[Index of Archives]     [Video For Linux]     [Photo]     [Yosemite News]     [gtk]     [GIMP for Windows]     [KDE]     [GEGL]     [Gimp's Home]     [Gimp on GUI]     [Gimp on Windows]     [Steve's Art]

  Powered by Linux