gimp_config_writer_printf () is used to print out a gsize variable, data_length.
gsize's actual size varies depending on platform, so using the format "%lu" throws
a warning on some platforms and switching to "%llu" throws a warning on others.
@Wormnest suggested ("%" G_GSIZE_FORMAT) as a universal format to resolve
this warning.
- file-psd, sig[4] and block_len variables are initialized to stop uninitialized warnings
- file-xpm, g_file_peek_path () is cast to (char *) to stop "discarded const" warnings
- gimpconfig-serialized.c, %llu is used instead of %lu to stop overflow warning
These will replace the int arguments used in place of enums. The problem of int
arguments used as list of choices is that it makes calling PDB functions very
opaque. This is especially bad when a list is long, so you constantly have to
refer to the documentation to understand what a series of numbers mean in
argument lists.
And the second issue is that plug-in developers have to manually maintain a list
of values both in the GUI and in the documentation string. This help text may
get out-of-sync, may end up with missing values or whatnot. Also if it is used
as tooltips, it makes for very weird tooltips in the graphical interface, with
an overlong technical list of int-values mapping which should ideally only be
made visible in the PDB procedure browser listing.
There are 3 fixes here:
1. First, searching for "\"GimpFont\"" to determine whether or not this is a new
format text parasite is not enough, because this string can be found in a
text layer with "GimpFont" only as contents. It will serialize as:
> (text "GimpFont")
Instead search for "(font \"GimpFont\"" which cannot be created as part of
the text contents because of the unprotected double quotes).
2. We must verify that strstr() did not return NULL when searching for markup
delimiters. It may happen if the parasite contents is invalid (we must always
assume it can be, since it's user data).
3. When deserialization of a text from parasite fails (e.g. because parasite
doesn't actually exist or its format is wrong), still make sure the GimpText
has a font (setting the standard one before deserializing). Otherwise GIMP
crashes down the line.
For this, I also had to fix gimp_config_deserialize_object(): the object type
name must be parsed even if an object was already set in a GObject property.
In GimpText, The font used to be stored as a string containing its name,
Now, it is stored as a GimpFont object, which makes more sense and makes
operations on fonts easier (such as serialization).
I add a new class method deserialize_create() to GimpConfigInterface which
returns the GimpConfig object per deserialization, instead of modifying an
existing bare object.
This matters for cases like our GimpResource (and later our GimpItem) classes
which are fully managed by libgimp and should be unique objects per actual
resource. It should even be possible to compare the pointer itself for identity.
That's why we need to let GimpResource create the object (in reality request it
to the infra and only ref it) through this new class method.
With this commit and the previous ones, all GimpResource are now properly stored
as plug-in settings (e.g. the "film" plug-in has a font setting which is now
properly remembered).
These identifiers are not portable (across various installations and therefore
not for XCF either), but at least they are reasonably identifying data on a same
installation (unlike GimpResource's int ID which is only valid within a single
session) which makes them very fine for plug-in settings storage.
When a data file disappears, we fallback to the context default data instead.
… fails.
This happens for plug-in settings storage. Serialization stops at first failed
property serialization, whereas we could store more. This feels like the last
settings feature is broken.
GLib has a specific type for byte arrays: `GBytes` (and it's underlying
GType `G_TYPE_BYTES`).
By using this type, we can avoid having a `GimpUint8Array` which is a
bit cumbersome to use for both the C API, as well as bindings. By using
`GBytes`, we allow other languages to pass on byte arrays as they are
used to, while the bindings will make sure to do the right thing.
In the end, it makes the API a little bit simpler for everyone, and
reduces confusion for people who are used to working with byte arrays
in other C/GLib based code (and not having 2 different types to denote
the same thing).
Related: https://gitlab.gnome.org/GNOME/gimp/-/issues/5919
Simplifies chooser widgets (e.g. GimpBrushSelect) by eliminating attributes (e.g. opacity) of chosen resource.
See #8745, but this commit fixes that by first refactoring the code.
Refactors GUI widgets (e.g. GimpBrushSelectButton and GimpBrushSelect etc.)
Refactor by "Extract class" GimpResourceSelectButton from GimpBrushSelectButton etc.
This moves common code into an inherited class (formerly called GimpSelectButton)
but the subclasses still exist.
The subclasses mainly just do drawing now.
Refactor by "Extract module" GimpResourceSelect from GimpBrushSelect etc.
Moves common code into one file, generic at runtime on type of GimpResource,
that is, the new code dispatches on type i.e. switch statements.
In the future, when core is changed some of that can be deleted.
The files gimpbrushselect.[c,h] etc. are deleted.
The module adapts the API from core to the API of callbacks to libgimp.
Note that core is running the resource chooser (select) widgets remotely.
Core is still calling back over the wire via PDB with more attributes
than necessary.
The new design gets the attributes from the resource themselves,
instead of receiving them from core callback.
The libgimp side adapts by discarding unneeded attributes.
In the future, core (running choosers for plugins) can be simplified also.
Fix gimp_prop_chooser_brush_new same as other resources.
Finish changes, and clean style.
Annotations
So procedures can declare args and GimpProcedureDialog show chooser
widgets
Fix so is no error dialog on id_is_valid for resources
Palette.pdb changes and testing
Memory mgt changes
Gradient pdb
Font and Pattern tests
Test brush, palette
Cleanup, remove generator
Rebase, edit docs, install test-dialog.py
Whitespace, and fix failed distcheck
Fix some clang-format, fix fail distcheck
Fix distcheck
Cleanup from review Jehan
RGB 0..255/0..100 and LCh/HSV settings are now remembered when closing
and reopening GIMP.
A few enums were explicitly cast to GimpColorSelectorChannel to clear
some compiler warnings.
Now that we bumped our meson requirement, meson is complaining about
several features now deprecated even in the minimum required meson
version:
s/meson.source_root/meson.project_source_root/ to fix:
> WARNING: Project targets '>=0.56.0' but uses feature deprecated since '0.56.0': meson.source_root. use meson.project_source_root() or meson.global_source_root() instead.
s/meson.build_root/meson.project_build_root/ to fix:
> WARNING: Project targets '>=0.56.0' but uses feature deprecated since '0.56.0': meson.build_root. use meson.project_build_root() or meson.global_build_root() instead.
Fixing using path() on xdg_email and python ExternalProgram variables:
> WARNING: Project targets '>=0.56.0' but uses feature deprecated since '0.55.0': ExternalProgram.path. use ExternalProgram.full_path() instead
s/get_pkgconfig_variable *(\([^)]*\))/get_variable(pkgconfig: \1)/ to
fix:
> WARNING: Project targets '>=0.56.0' but uses feature deprecated since '0.56.0': dependency.get_pkgconfig_variable. use dependency.get_variable(pkgconfig : ...) instead
Generated *enums.c now have an additional stamp no-op header include
(see last 2 commits). Sync this change into the autotools generation
scripts to prevent back and forth useless generation of these files each
time we switch from one build system to another.
They are nearly the same as initially, except that now they include an
intermediate stamp header which will be generated by the build system.
The only 2 enums which don't need these includes (and are not versioned)
are libgimp/gimpenums.c and libgimpthumb/gimpthumb-enums.c.
Our meson build system was not properly building the enums.c file,
because they are versionned.
I did a similar trick as what I did for the pdbgen, which is that I used
a wrapper script around the existing perl script, which sets proper
options and generate a stamp file in the end (which is considered by
meson as the actual custom target, not the C file since it is generated
in the source dir).
The most important part is that the stamp file is a generated header
source (not just a random text file) which is **included** by the
generated C file. This is what will force meson to regenerate the C file
if the header is updated, **then** build using this new version, not use
an outdated versionned version (which would make for hard to diagnose
bugs), through the indirection of the intermediate stamp header.
See #4201.
See also: https://github.com/mesonbuild/meson/issues/10196#issuecomment-1080742592
Fixing these 2 warnings in the CI which end up fatal:
WARNING: Invalid fragment for 'Gimp.Parasite': it should be struct
Serializes the object properties of @config to a [class@Parasite].
^~~~~~~~~~~~~~~~
WARNING: Invalid fragment for 'GLib.MainLoop': it should be struct
it has a GUI and is hanging around in a [class@GLib.MainLoop], it must call
^~~~~~~~~~~~~~~~~~~~~
GLib has a specific type of NULL-terminated string arrays:
`G_TYPE_STRV`, which is the `GType` of `char**` aka `GStrv`.
By using this type, we can avoid having a `GimpStringArray` which is a
bit cumbersome to use for both the C API, as well as bindings. By using
`GStrv`, we allow other languages to pass on string lists as they are
used to, while the bindings will make sure to do the right thing.
In the end, it makes the API a little bit simpler for everyone, and
reduces confusion for people who are used to working with string arrays
in other C/GLib based code (and not having 2 different types to denote
the same thing).
Related: https://gitlab.gnome.org/GNOME/gimp/-/issues/5919
… object arrays.
We were not supporting duplicating object pspec for no good reason. Well
maybe the reason was that libgimpconfig does not see these types which
are in libgimp. But then the trick is to compare by name.
As for object array, they are present as subtypes of GimpArray specs.
Yet most GimpParamSpec*Array-s are subclass of GimpParamSpecArray but
GimpParamSpecObjectArray are their own GParamSpecBoxed subclass (same as
the Gimp*Array-s are just typedef-s of GimpArray but GimpObjectArray is
its own boxed type).
So I had to move the object array test as its own case to fix support.
Finally do not ignore anymore any type in gimp_config_class_init(). When
we create a GimpConfig, we want to know when a type is not implemented
as parameter (and if it's a well known type, we need to implement it).
Since the parsing failure I was experiencing was normal (not a bug), and
since I was not seeing error being passed down to deserialize(), I
assume this assert was always wrong. I hadn't realize that the GError
object was in fact populated by deserialized through g_scanner_error()
calls which were setting the GError passed at creation of the scanner
through a handler.
So there was indeed a bug in one of the functions called by our
deserialize() implementation. There was one failure case where the error
was just reported with g_warning() instead of g_scanner_error().
Now that it's fixed, I brought back the asserts (previous commits)
because the error object is actually always well populated.
As discussed with mitch on IRC, these asserts make no sense. They can
happen if we fail to parse user-side data. Also currently deserialize()
does not even pass the GError down so we would always assert on failed
parsing.
What must be done instead if change the signature of deserialize() and
all its implementations, with a GError arg. Then this GError will
properly bubble up to the caller for appropriate handling.
… assert the existence of GError.
This is even worse as deserialize() method does not even take a GError
parameter anyway so this assert will always go off when a
deserialization failed (which happened in my case as I was working on a
plug-in API, hence gimp_procedure_config_load_last() actually failed to
load a previous version of the plug-in-settings when I changed a
procedure arg's type).
Just fail the deserialization normally and let the calling code handling
this case. Nevertheless it is kind of useful to bubble-up the error to
calling code, so I add a TODO in the interface header (hopefully to see
and improve this before we release GIMP 3.0).
Since it appeared with GLib 2.68.0, we could not change this until we
bumped the dependency which has only become possible a few days ago
(since Debian testing is our baseline for dependency bumps). Cf.
previous commit.
As this is a drop-in replacement (just a guint parameter changed to
gsize to avoid integer overflow), search-and-replace with:
> sed -i 's/g_memdup\>/g_memdup2/g' `grep -rIl 'g_memdup\>' *`
… followed by a few manual alignment tweaks when necessary.
This gets rid of the many deprecation warnings which we had lately when
building with a recent GLib version.
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.
Used for instance if a plug-in had a GimpParasite argument (or aux
argument), which might not seem too useful at first. But anyway we
propose the type, so let's support it properly.
And actually I am going to use it in the next commit as a trick to save
binary data in stored last values for a plug-in.
… 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.
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.
Negative int values were not correctly handled because value.v_int is unsigned
causing a conversion to a large positive value.
To fix this we cast it to gint64 first before making it negative.
Alx Sa