MR !565 was only a partial implementation as it was assuming all cursors
had the same hot spot coordinates in the file, which is false more often
than not (since usually it's several sizes for the same image, hence
coordinates move). I should have realized this before merging.
With this new commit, we actually loads the hot spot coordinates per
cursor, stores them as per-layer parasites, then exports with per-cursor
coordinates.
Also it makes the procedure API use int32 array (should be int16 but we
removed the support, now I think it may have been a mistake) which shows
the ugliness of our array support once again with additional size args
per array (even if it's the same size). Also I realize that our support
of arrays with config object is not good. This is also something we'll
have to look at.
As explained in previous commits, the _peek_ call is advantageous
because:
- It is less bug-prone as we don't have to handle freeing the string. In
all the cases I changed, I even spotted at least 2 cases where we were
leaking a string (in file-mng, `temp_file_name` is never freed; and we
were also leaking in an error case of gfig).
- As a consequence of the previous point: simpler code with less lines.
- In local file cases, the _peek_ variant does not even need to allocate
an additional string.
- In other case, if we query several times the path, it is allocated
once and cached so it stays efficient.
- When possible, working on the GFile rather than on a path string may
be more robust. For instance I changed one g_unlink() into a
g_file_delete(). Actually most reading/writing should be done with the
GIO API when possible, but I didn't want to change too much code
logics on this commit.
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' "$@"
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.
This commit just changes our saving API (i.e. the GimpSaveProcedure
class) to take an array of drawables as argument instead of a single
drawable.
It actually doesn't matter much for exporting as the whole API seems
more or less bogus there and all formats plug-ins mostly care only
whether they will merge/flatten all visible layers (the selected ones
don't really matter) or if the format supports layers of some sort. It
may be worth later strengthening a bit this whole logics, and maybe
allow partial exports for instance.
As for saving, it was not even looking at the passed GimpDrawable either
and was simply re-querying the active layer anyway.
Note that I don't implement the multi-selection saving in XCF yet in
this commit. I only updated the API. The reason is that the current
commit won't be backportable to gimp-2-10 because it is an API break. On
the other hand, the code to save multi-selection can still be backported
even though the save() API will only pass a single drawable (as I said
anyway, this argument was mostly bogus until now, hence it doesn't
matter much for 2.10 logics).
This gives a big cleanup in the meson.build files of the plug-ins.
It's also quite a bit more maintainable, since anything that changes in
libgimp's dependencies, linkage, ... doesn't have to be copy-pasted into
each plug-in.
When cross-compiling for Windows, I had this warning:
> plug-ins/file-ico/ico-load.c:221:22: warning: format '%lu' expects
> argument of type 'long unsigned int', but argument 5 has type 'long
> long unsigned int' [-Wformat=]
I guess that on some platform sizeof may be a long long uint. Just cast
the result to long uint instead (I could have done the other way around,
but I doubt the struct type IcoFileEntry would ever get bigger than long
uint max!).
and in an attack of madness, changes almost all file plug-in
code to use GFile instead of filenames, which means passing
the GFile down to the bottom and get its filename at the very
end where it's actually needed.
Turn all ID param specs into object param specs (e.g. GimpParamImageID
becomes GimpParamImage) and convert between IDs and objects in
gimpgpparams.c directly above the the wire protocol, so all of app/,
libgimp/ and plug-ins/ can deal directly with objects down to the
lowest level and not care about IDs.
Use the actual object param specs for procedure arguments and return
values again instead of a plain g_param_spec_object() and bring back
the none_ok parameter.
This implies changing the PDB type checking functions to work on pure
integers instead of IDs (one can't check whether object creation is
possible if performing that check requires the object to already
exist).
For example gimp_foo_is_valid() becomes gimp_foo_id_is_valid() and is
not involved in automatic object creation magic at the protocol
level. Added wrappers which still say gimp_foo_is_valid() and take the
respective objects.
Adapted all code, and it all becomes nicer and less convoluted, even
the generated PDB wrappers in app/ and libgimp/.
We now have both variants, one returning a GList, and another
returning an array. Turns out that while a list is often nicer,
sometimes a random-access array really keeps the code much simpler.
Adapt all plug-ins, and clean up a bit (like use g_list_reverse() once
instead of iterating the list reversed).
It's just too weird to be public. Remove its properties from the wire
protocol and from pluginrc. Instead, have all GParamSpecs' flags on
the wire and in pluginrc, so we can use stuff like
GIMP_PARAM_NO_VALIDATE.
Port the remaining few places to GIMP_PROC_ARG_STRING().
I'm sure something is broken now wrt UTF-8 validation,
will add tighter checks in the next commit.
I am going to forbid plug-ins from being installed directly in the root
of the plug-ins/ directory. They will have to be installed in a
subdirectory named the same as the entry point binary.
This may seem useless for our core plug-ins which are nearly all
self-contained in single binaries, but this is actually a necessary
restriction to eliminate totally the DLL hell issue on Windows. Moving
core plug-ins in subfolders is only a necessary consequence for it.
It seems the current code simply forgot to break on indexed types and
therefore hit some g_return_*if_reached() code breaking the logics.
Looking further, I see some code taking care of indexed images and
converting them to RGB. And testing after adding breaks looks like it
works just fine.
So I am assuming this was just forgotten breaks indeed, and not on
purpose not allowing indexed images (if that were the intent though,
this is not how it should be done).
...in both the core and libgimp.
Images now know what the default mode for new layers is:
- NORMAL for empty images
- NORMAL for images with any non-legacy layer
- NORMAL_LEGAVY for images with only legacy layers
This changes behavior when layers are created from the UI, but *also*
when created by plug-ins (yes there is a compat issue here):
- Most (all?) single-layer file importers now create NORMAL layers
- Screenshot, Webpage etc also create NORMAL layers
Scripts that create images from scratch (logos etc) should not be
affected because they usually have NORMAL_LEGACY hardcoded.
3rd party plug-ins and scripts will also behave old-style unless they
get ported to gimp_image_get_default_new_layer_mode().
Try to sort all GIMP_ICON_* defines into FDO categories like in
https://specifications.freedesktop.org/icon-naming-spec/latest/ar01s04.html
Add defines for all icons we override, rename some icons to their FDO
standard names, and mark the ones we duplicate with a comment so we
don't forget to rename those to standard names in 3.0.
being exported to libgimp, and having a non-exported value, this is a
horrible mess like with GimpLayerMode, but at least the cruft value
names are deprecated now.
with proper value names. Mark most values as _BROKEN because they use
weird alpha compositing that has to die. Move GimpLayerModeEffects to
libgimpbase, deprecate it, and set it as compat enum for GimpLayerMode.
Add the GimpLayerModeEffects values as compat constants to script-fu
and pygimp.
...to cause a denial of service (crash) via an ICO file with an
InfoHeader containing a Height of zero
Add some error handling to ico-load.c and bail out on zero width or height
icons. Also some formatting cleanup.
It's deprecated in GTK+ 3.x, so we get rid of some deprecation
warnings. Also, it has always only been a utility function that hides
what is really going on in the widget hierarchy.
Previously, the mask would be transparent if the alpha was <50%.
However, this causes pixels to become black in some places in Windows
(notably, the taskbar on Windows 10). Therefore, always set the mask
to opaque if a pixel is partially or fully opaque.
Jehan