GNOME.org
 

Re: [Gimp-developer] PF_ENUM, SF_ENUM, dynamically defined enums for plugins

Hi all!

On Mon, Aug 15, 2022 at 12:25 AM Ofnuts via gimp-developer-list <
gimp-developer-list gnome org> 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 gnome org
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 gnome org
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
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]