When a selection exists, we are copying then pasting the selection
contents. In particular, with multi-layer selection, it means pasting a
merged result of the selected layers (like a sample merged but limited
to selected layers).
Yet when no selection exists, with a single layer selected, a cut in
particular would remove the layer fully, then a paste would copy it
elsewhere (in the same image or even on a different image). This was
still working, but not with multiple layers. This is now fixed and we
can now copy/cut then paste several layers (without merge), which is
sometimes a very practical way to move layers (sometimes simpler than
drag'n drop, especially between images).
As a consequence, the PDB function gimp_edit_paste() now also returns an
array of layers (not a single layer).
Though the previous implementation worked fine on C plug-ins, I realized
it was problematic on bindings. In particular, the Python binding at
least was somehow freeing returned floating objects, unless assigned to
a variable.
For instance, the widget returned by the following code:
> dialog.get_color_widget('color', True, GimpUi.ColorAreaType.FLAT)
… was freed by the PyGObject binding when it was floating, even though
(transfer none) was set (hence telling the binding it should not free
the returned object). The workaround was to assign it to some variable,
even though I was not planning to use it.
Making sure all references are full fixes it.
GObject docs also notes:
> **Note**: Floating references are a C convenience API and should not
> be used in modern GObject code. Language bindings in particular find
> the concept highly problematic, as floating references are not
> identifiable through annotations, and neither are deviations from the
> floating reference behavior, like types that inherit from
> GInitiallyUnowned and still return a full reference from
> g_object_new().
… GimpProcedureDialog.
Technically I added:
- New gimp_prop_color_select_new() property widget to create a
GimpColorButton for a given GimpRGB property.
- gimp_procedure_dialog_get_widget() now supports a GimpRGB property and
will create a GimpColorArea by default.
- When the default doesn't suit you, a new function
gimp_procedure_dialog_get_color_widget() allows to create either a
GimpColorArea or a GimpColorButton (editable = TRUE), as well as
choose the area type (small or large checks, as well as flat area,
i.e. no alpha support).
Still the same problem as ever with the Python binding where we have a
hard time creating GParamSpec, hence we make them from object
properties.
See: https://gitlab.gnome.org/GNOME/pygobject/-/issues/227#note_570031
But then again, the Python binding way to create GObject properties does
not seem to give us a way to use our custom param types (or I didn't
find how). So when I create a property with Gimp.RGB type in Python, it
doesn't appear as a GIMP_PARAM_SPEC_RGB to our C code, but as a
G_PARAM_SPEC_BOXED. So my trick is to check the value type instead.
Note that I check the default value, but in reality it doesn't seem to
work much either. Better than no support at all anyway.
… with the updated API with one more argument (n_drawables + drawables
instead of single drawable).
This went unnoticed as most plug-ins use a config with named properties
instead of ordered GimpValueArray now.
New function to set some procedure dialog's widget sensitive, either
with a constant value, or by binding it to a boolean property (or its
inverse) of a config object.
The gimp_drawable_type() is an issue though as gimp_drawable_get_type()
is already defined as a common GObject API.
Though I'm actually wondering if GimpImageType is well called. Rather
than Type, shouldn't we go with ColorModel?
sed -i 's/\<gimp_drawable_bpp\>/gimp_drawable_get_bpp/g' "$@"
sed -i 's/\<gimp_drawable_width\>/gimp_drawable_get_width/g' "$@"
sed -i 's/\<gimp_drawable_height\>/gimp_drawable_get_height/g' "$@"
sed -i 's/\<gimp_drawable_offsets\>/gimp_drawable_get_offsets/g' "$@"
… the public API.
This was initially proposed by Niels De Graef in !101, and though I
still think this is much less practical for day-to-day development, it
is actually much nicer for the public part of the API. So let's use
these only in public libgimp* API only, not in core.
I actually already started to use these for some of the libgimpwidgets
classes and am now moving libgimp main classes to these macros.
* It doesn't expose the priv member (which is completely useless for
plug-in developers, only to be used for core development).
* It forces us to never have any variable members in the public API
(which we were doing fine so far in newest API, but it's nice to be
enforced with these macros).
* When using G_DECLARE_FINAL_TYPE in particular, it adds flexibility as
we can change the structure size and the members order as these are
not exposed. And if some day, we make the class derivable with some
signals to handle, only then will we expose the class with some
_gimp_reserved* padding (instead of from the start when none is
needed). Therefore we will allow for further extension far in the
future.
Moreover most of these libgimp classes were so far not using any private
values, so we were declaring a `priv` member with a bogus contents just
"in case we needed it in future" (as we couldn't change the struct
size). So even the easiness of having a priv member was very relative
for this public API so far (unlike in core code where we actually have
much more complex interactions and using priv data all the time).
s/gimp_image_base_type/gimp_image_get_base_type/
s/gimp_image_width/gimp_image_get_width/
s/gimp_image_height/gimp_image_get_height/
Sorry plug-in developers, more porting work! But really this seems like
the right thing to do in order not to get stuck with inconsistent naming
for many more years to come.
As the docs says, this was always allowed and would just imply we want a
dialog with all properties in order of declaration.
Yet this usage would output this warning:
> plug-ins/file-fli/fli-gimp.c:976:3: warning: not enough variable arguments to fit a sentinel [-Wformat=]
This commit take care of this warning.
There was at least a case when gimp_procedure_create_config() was called
twice, hence so was gimp_save_procedure_add_metadata() when a plug-in
was run.
It was happening when calling a procedure with less arguments than the
procedure had. In such case, gimp_procedure_run() would create a config
to fill it with defaults.
Fixes warnings such as:
> LibGimp-WARNING **: 01:29:57.044: Auxiliary argument with name 'save-exif' already exists on procedure 'file-png-save'
I always found the docs misleading because when it says "Returns the
list of layers contained in the specified image", I really read "all the
layers, at any level", except it doesn't. It only returns the root
layers and it is up to the plug-in developer to loop through these if
one needs to go deeper.
So let's make the function docs clearer.
We heavily rely on GError in libgimp to retrieve plug-in error messages.
In a lot of our code, we just use domain=0 for g_set_error*() functions
and alike, but this is actually forbidden and results in GLib warnings.
Some plug-ins instead create their own domain, other use G_FILE_ERROR
nearly everywhere, even in some cases where the choice is really
questionable. Since anyway this is mostly useful for passing the error
message through, it is much nicer to provide a generic domain
GIMP_PLUG_IN_ERROR, which can be used by all plug-ins when they don't
want to bother with the error domain.
… drawable array instead of a single drawable.
Instead of expecting a single drawable, GimpImageProcedure's run()
function will now have an array of drawable as parameter.
As a consequence, all existing plug-ins are broken again. I am going to
fix them in the next commit so that this change can be easily reviewed
and examined if needed later.
I only fix the Vala demo plug-in now (or rather, I just use the first
layer in the array for now) because otherwise the build fails.
The new function gimp_procedure_set_sensitivity_mask() allows plug-ins
to tell when a procedure should be marked as sensitive or not.
gimp_procedure_get_sensitivity_mask() retrieves this information.
Currently plug-ins are automatically marked as sensitive when an image
is present and a single drawable is selected. Nowadays, we can have
multiple selected layers so we should allow plug-ins to tell us if they
support working on multiple drawables. Actually we could even imagine
new plug-ins which would be made to work only on multiple drawables.
Oppositely, there are a lot of plug-ins which don't care at all if any
drawable is selected at all (so we should allow no drawable selected).
Finally why not even imagine plug-ins which don't care if no image is
shown? E.g. plug-ins to create new images or whatnot. This new API
allows our core to know all this and show procedure sensitivity
accordingly. By default, when the function is not called, the 1 image
with 1 drawable selected case is the default, allowing existing plug-ins
easier update.
Note: this only handles the sensitivity part right now. A plug-in which
would advertize working on several layer would still not work, because
the core won't allow sending several layers. It's coming in further
commits.
Since version 0.27.3 exiv2 has changed how it returns
comments for Exif.Photo.UserComment. We now get
the comment including the charset=Ascii value.
Let's remove anything that's not part of the actual
comment. To not complicate things we will only
handle charset=Ascii for now since I've never seen
any other charset used.
1. Convert xmp /Iptc4xmpExt tag parts to /iptcExt because
exiv2 fails when we try to use the default namespace.
2. Don't only set structs from a fixed list but find all
xmp array elements and check what the best struct
type is: bag or seq.
3. Work around a sorting issue in (g)exiv2 by using a natural
sorting algorithm ourselves.
4. Added some g_debug statements to make it easier to
determine the cause of issues.
This fixes the following warning (and 4 similar others):
> libgimp/gimpimagecombobox.c:133: Warning: GimpUi: "destroy" annotation needs one option, none given
Brought by commit df766d5443. It's my fault for not properly reviewing
the patch as I failed to notice the new warnings.
Similar to gimp_image_set_selected_layers() except that it takes a GList
(instead of a C array) and also it takes ownership of the list pointer.
This makes it much easier to use in some specific situations.
It isn't being used by any plug-in or any code in GIMP at all even.
Let's get rid of it while we can still break API, so we can cut down on
all the complexity of the gimp-param stuff a bit.
- Do not only check the step width, but also the time interval between 2
progress calls. No need to run a PDB call, then update the GUI every
millisecond or so. This would just unecessarily slow down the plug-in
for updates which the user won't ever see. From my tests, 20 updates
per second is plenty enough to have the progression look fluid. No
need for much more.
- Do not warn anymore on stderr when we drop progress updates. Even if
just on the unstable builds, such warning is wrong. First because it
depends on files and machines. Typically a lot of processing could set
their progress updates relatively to layers. Yet we currently consider
that 1/256 steps are too small. So what if you have more than 256
layers? This would make the same code print a warning on big files,
and none on small files.
The second reason is that we should not encourage plug-in developers
to have limited progression updates, but the opposite (progression
info makes for good feedback), neither should we expect them to
compute the step size or the time between updates. It's a much saner
approach to have them only take care about computing relevant update
steps while our API focuses on filtering these in order to avoid
overloading the GUI.
It makes for good progression feedback, sharp GUI while not taking all
CPU time on it, all this while making it easy on plug-in developers.
… but only when a menu label was set with
gimp_procedure_set_menu_label(). In such case, this menu label is used
as dialog title (with mnemonic underscore removed).
The `precision` parameter in particular had no min/max, which meant we
could provide a forbidden parameter (e.g. a negative precision) which
would cause a core CRITICAL. We must forbid illegal values from PDB side
(hence outputting a normal plug-in error message, not a core bug).
Also improving a bit the description of this parameter as I was
wondering what precision was needed exactly to get a stroke length. This
is the precision for determining whether a portion of the stroke is
"straight enough" or if we want to break it into smaller pieces until we
get a straight portion.
The term "Defaults" is not clear enough and looks like it may be
redundant with the "Factory Defaults" button. Let's try an alternative
"Save Settings" and "Load Saved Settings".
Also adding some tooltips.
And finally making the "Load Saved Settings" only sensitive if the "Save
Settings" button had been used at least once.
This is what the GNOME's HIG calls a "button menu" apparently. Adding
this arrow makes it clearer that it is not a finale action but an
intermediate one allowing you to see more choices.
See also report #6145 where this was raised among other things.
`g_object_notify()` actually takes a global lock to look up the property
by its name, which means there is a performance hit (albeit tiny) every
time this function is called. So let's encourage using
`g_object_notify_by_pspec()` instead.
Another nice advantage is that it's a bit safer at compile-time, since
now typos will at least be caught by the compiler (as the enum value has
to match).
I was trying to avoid too large dialogs as this `metadata` frame can
hold random plug-in settings. But let's go for always the same number as
columns as the max number of common settings (i.e. most often 3
columns).
Use gtk_widget_list_mnemonic_labels() to look for mnemonic of common GTK
widgets. Also warn when several mnemonic were set on a given widget
("wasting" keys when it seems we are always looking for available
mnemonics).
Also warn with core action IDs too when they miss a mnemonic.
Update `gimp_window_get_native_id()` a little to be more correct
(although it still won't work on Wayland).
Most important of all, we shouldn't assume that if a given GDK backend
is enabled at compile time, that this is also the one that is being
used. For example, on Linux, both `GDK_WINDOWING_X11` and
`GDK_WINDOWING_WAYLAND` can be set, but you still need to do a runtime
check if you're running under one WM or the the other.
A small cleanup is that we immediately check if a widget is realized by
checking if it's `GdkWindow` is NULL or not and return immediately
(since we need to check its type later on anyway).
Finally, we can remove `GDK_NATIVE_WINDOW_POINTER` as that is a GTK+ 2.0
construct, so it's dead code anyway.
Jehan