Re: PF_ENUM, SF_ENUM, dynamically defined enums for plugins

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

 



Hi all!

On Mon, Aug 15, 2022 at 12:25 AM Ofnuts via gimp-developer-list <
gimp-developer-list@xxxxxxxxx> wrote:

> I'm not convinced this needs to be an enum. The basic problem it to
> show  a list of strings, and return the index of the selected one.  The
> contents of the list can be quite dynamic. As far as I can tell, in 2.10
> they aren't cached in pluginrc. In 3.0, the query/init methods of the
> plugin suggest that things can be dynamic and a list of URLs/devices
> could be created during registration (this could also change if the user
> changes the UI language, and only the plugin would know how to retrieve
> the translations...).  How the returned integer is used is left to the
> imagination of the author.
>

I discussed the topic of enum-type arguments in the MR opened by Lloyd:
https://gitlab.gnome.org/GNOME/gimp/-/merge_requests/709

Feel free to read the discussion, though I'll also try to re-explain it
here.

As a first disclaimer, I will discuss this with C code because I am
personally not interested into script-fu-only specifics, neither into any
language-specifics. This includes C itself (i.e. I don't care about
solutions which work only on C either), but C is our base language, which
is why it's our generic language here.
We used to have many bindings in many languages, most of them (but
script-fu) are now dead (development-wise) because they focused on logic
specific to the language (which is great and for sure has a lot of
advantages, don't get me wrong, but also means a lot more continuous
maintenance).

>
> So, IMHO, this is more like a variant of regular integer, instead of
> showing a slider you display  a list of labels.
>

This is actually the current state of things currently in GIMP 2.99/3.0.

When we want a list of options (a.k.a. enum-like type) as argument, we
declare it as an int argument. And we list the values in the docs string:

      GIMP_PROC_ARG_INT (procedure, "preset",
                         _("Source _type"),
                         _("WebP encoder preset (Default=0, Picture=1,
Photo=2, Drawing=3, "
                           "Icon=4, Text=5)"),
                         0, 5, WEBP_PRESET_DEFAULT,
                         G_PARAM_READWRITE);

On API/plug-in/script programmer side, it's ugly because it means that
people rely on meaningless integers and always have to refer to docs to
re-read existing code. Also it means if we forget to update the docs, the
possible values might be incomplete. Worse, for some procedure, the number
of options is so huge, we currently don't list them in the string docs.
This is the case of "pixel-format" in plug-ins/common/file-raw-data.c which
has 19 possible values. So I didn't even bother writing the docs, though
it's also because I was planning for better system to declare list of
options.

On GUI side though, we don't have such a problem because we have the API to
easily map int values to individual string items:

  /* Create the combobox containing the presets */
  store = gimp_int_store_new ("Default", WEBP_PRESET_DEFAULT,
                              "Picture", WEBP_PRESET_PICTURE,
                              "Photo",   WEBP_PRESET_PHOTO,
                              "Drawing", WEBP_PRESET_DRAWING,
                              "Icon",    WEBP_PRESET_ICON,
                              "Text",    WEBP_PRESET_TEXT,
                              NULL);
  gimp_procedure_dialog_get_int_combo (GIMP_PROCEDURE_DIALOG (dialog),
                                       "preset", GIMP_INT_STORE (store));

So my initial idea was to move this "store" list (but it will have to be
another type, not depending on GTK) into the declaration part of plug-ins.
This way, at least the problem of documenting the values and even managing
errors (non-supported values) is no more.
Then we would not have to maintain a huge docs for the arg (but smaller doc
strings for each values) and we can generate better docs for such
arguments. Though we would still have to deal with integer on caller side.
It would be better with semantic values.

An alternative could be to use string arguments (also associated to a
limited given list of allowed values on callee side), this would bring
semantic, using for instance "picture" as arg rather than whatever int
value is WEBP_PRESET_PICTURE.

In any case, the best would be to have real enum types declared and usable
on caller side too, hence using the same type on both side, and that brings
some semantic to the API. It seemed to me that it was not possible to
generate enum types dynamically, but Lloyd seems to think it is thanks to
GTypeModule. So let's see how it goes. :-)

Jehan


> On 08/08/2022 14:42, Lloyd Konneker via gimp-developer-list wrote:
> > This is a continuation of a thread on this list:
> >
> https://mail.gnome.org/archives/gimp-developer-list/2022-July/msg00016.html
> .
> > The thread diverged to a discussion about future PF_OPTION implementation
> > in GIMP 3.
> >
> > Here are my preliminary thoughts.
> >
> > A PF_OPTION, in the old PyGimp, or a SF_OPTION, in ScriptFu,
> > is a declaration of a dynamic enum type.
> > The enum is created by the plugin.
> > Only the plugin knows the enum.
> > GIMP has no knowledge of the enum, until at runtime the plugin informs
> it.
> >
> > Suppose that for PF_OPTION we can implement so that a GIMP plugin
> > dynamically defines an enum type
> > (in the GType system)
> > Then when registering an arg of the plugin declared using PF_OPTION,
> > the plugin uses a g_param_spec_enum, passing the GType of the dynamic
> enum
> > type.
> > Then GIMP can use that g_param_spec to add a  property to the
> GimpProcedure
> > for the plugin,
> > and to create a GimpConfig for that property,
> > and show a dialog with a combo-box widget for the enum type.
> > All the latter stuff seems to work already.
> > That is, PF_ENUM and SF_ENUM seem to work,
> > for a plugin to declare an argument of a GEnum subtype that GIMP has
> > defined.
> > The problem is to implement the dynamic creation of an enum type.
> >
> > GLib appears to be able to create enum types dynamically.
> > Its complicated and arcane, not something I understand.
> > So what I suggest may not be possible.
> >
> > If you grep the GIMP repo, you find G_DEFINE_DYNAMIC_TYPE used in the
> > modules directory.
> > That is a GLib macro that seems pertinent, it seems to hide many of the
> > arcane details.
> > Anyway, there seems to be some history of dynamically defined types in
> GIMP.
> > The word "modules" is a GLib term for dynamically loaded code.
> >
> > One problem is that GLib documentation often uses the word "static" for
> the
> > array of GEnumValues
> > that are in a GEnum type.
> > I don't think that  means the key value pairs in the enum must be defined
> > as static vars in some C code.
> > I think you can still have an interpreted plugin that allocates an array
> of
> > key value pairs and creates a dynamic GEnum type from it.
> >
> > Another issue that concerns me is the "wire".
> > If a plugin creates a dynamic GEnum subtype,
> > does the type have to cross the wire, or only values of the GEnum?
> > I hope I am worrying too much.
> >
> > As Ofnuts pointed out, PF_ENUM is heavily used.
> > It  deserves some attention.
> > In the thread reference above, Jehan has said he has an idea for
> > implementation and realizes it needs attention.
> > Here I am offering an alternative high-level view of what the technical
> > problem is.
> >
> > If the technical problem is to create a dynamic GType,
> > then a pragmatic problem is that few developers deeply understand GType.
> > (Its a slog for me.)
> > Maybe Neils could look at it, or maybe we could get some mentorship from
> > GNOME?
> > _______________________________________________
> > gimp-developer-list mailing list
> > List address:gimp-developer-list@xxxxxxxxx
> > List membership:
> https://mail.gnome.org/mailman/listinfo/gimp-developer-list
> > List archives:https://mail.gnome.org/archives/gimp-developer-list
>
> _______________________________________________
> gimp-developer-list mailing list
> List address:    gimp-developer-list@xxxxxxxxx
> List membership:
> https://mail.gnome.org/mailman/listinfo/gimp-developer-list
> List archives:   https://mail.gnome.org/archives/gimp-developer-list
>


-- 
ZeMarmot open animation film
http://film.zemarmot.net
Liberapay: https://liberapay.com/ZeMarmot/
Patreon: https://patreon.com/zemarmot
Tipeee: https://www.tipeee.com/zemarmot
_______________________________________________
gimp-developer-list mailing list
List address:    gimp-developer-list@xxxxxxxxx
List membership: https://mail.gnome.org/mailman/listinfo/gimp-developer-list
List archives:   https://mail.gnome.org/archives/gimp-developer-list



[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