- Fix annotations for gimp_export_options_get_image() to make it
actually introspectable with the GimpImage being both input and
output. Even though the logic doesn't change much (the input image may
be overriden or not), it doesn't matter for introspection because
images are handled centrally by libgimp and therefore must not be
freed. Actually deleting the image from the central list of images
though remains a manual action depending on code logic, not some
automatic action to be handled by binding engines.
- Add G_GNUC_WARN_UNUSED_RESULT to gimp_export_options_get_image()
because ignoring the returned value is rarely a good idea (as you
usually want to delete the image).
- Remove gimp_export_options_new(): we don't need this constructor
because at this point, the best is to tell plug-in developers to just
pass NULL everywhere. This leaves us free to create a more useful
default constructor if needed, in the future. Main description for
GimpExportOptions has also been updated to say this.
- Add a data_destroy callback for the user data passed in
gimp_export_procedure_set_capabilities().
- Fixing annotations of 'export_options' object from pdb/pdb.pl: input
args would actually be (nullable) and would not transfer ownership
(calling code must still free the object). Return value's ownership on
the other hand is fully transfered.
- Add C and Python unit testing for GimpExportOptions and
gimp_export_options_get_image() in particular.
- Fix or improve various details.
Note that I have also considered for a long time changing the signature
of gimp_export_options_get_image() to return a boolean indicating
whether `image` had been replaced (hence needed deletion) or not. This
also meant getting rid of the GimpExportReturn enum. Right now it would
work because there are no third case, but I was considering the future
possibility that for instance we got some impossible conversion for some
future capability. I'm not sure it would ever happen; and for sure, this
is not desirable because it implies an export failure a bit late in the
workflow. But just in case, let's keep the enum return value. It does
not even make the using code that much more complicated (well just a
value comparison instead of a simple boolean test).
This patch creates a GimpExportOptions class in both
libgimpbase and in libgimp. Currently it is a mostly empty
object, but it will be added to after 3.0 to allow for
additional export options (like resizing on export while
leaving the original image intact)
libgimp/gimpexport.c was removed, and most of its content
was copied into libgimp/gimpexportoptions.c. gimp_export_image ()
was replaced with gimp_export_options_get_image () in all
export plug-ins.
GimpExportProcedure has a new function to set the default
image capabilities for each plug-in on creation. It also sets up
a new callback function, which allows the options to respond to
user setting changes (such as toggling 'Save as Animation' in the
GIF or WEBP Plug-in).
This fixes all our GObject Introspection issues with GimpUnit which was
both an enum and an int-derived type of user-defined units *completing*
the enum values. GIR clearly didn't like this!
Now GimpUnit is a proper class and units are unique objects, allowing to
compare them with an identity test (i.e. `unit == gimp_unit_pixel ()`
tells us if unit is the pixel unit or not), which makes it easy to use,
just like with int, yet adding also methods, making for nicer
introspected API.
As an aside, this also fixes#10738, by having all the built-in units
retrievable even if libgimpbase had not been properly initialized with
gimp_base_init().
I haven't checked in details how GIR works to introspect, but it looks
like it loads the library to inspect and runs functions, hence
triggering some CRITICALS because virtual methods (supposed to be
initialized with gimp_base_init() run by libgimp) are not set. This new
code won't trigger any critical because the vtable method are now not
necessary, at least for all built-in units.
Note that GimpUnit is still in libgimpbase. It could have been moved to
libgimp in order to avoid any virtual method table (since we need to
keep core and libgimp side's units in sync, PDB is required), but too
many libgimpwidgets widgets were already using GimpUnit. And technically
most of GimpUnit logic doesn't require PDB (only the creation/sync
part). This is one of the reasons why user-created GimpUnit list is
handled and stored differently from other types of objects.
Globally this simplifies the code a lot too and we don't need separate
implementations of various utils for core and libgimp, which means less
prone to errors.
...in non-interactive cases.
gimp_export_image () handles various
tasks like rasterizing NDE filters. It only
runs in interactive cases however, so if the
users calls gimp-file-save the filters are
not exported.
Since Jehan removed the hidden dialogue
in 0dc9ff7c, we can now safely call
gimp_export_image () in all cases to make
image export more consistent. This step is
also preparation for setting up the new
API with GimpExportOptions.
With the new API introduced int d1c4457f,
we next need to port all plug-ins using
the argument macros to functions.
This will allow us to remove the macros
as part of the 3.0 API clean-up.
Port all plug-ins to retrieve the layers
directly from the image rather than
having them passed in. This resolves some
issues with introspection and sets the
foundation for future API work.
Though it's not visible and could happily wait for after GIMP 3 release, this
was annoying when grepping. Just did a quick cleanup.
I also removed gimprc.common which is a forgotten remnant from the autotools
build.
Resolves#10932
Since GIMP distinguishes between saving
XCF and exporting image like PNG,
we should change the PDB to show
export rather than save in the function
calls.
If we leave a space between the macro name and opening parenthese for argument
lists, the args are not considered macro args (which will be discovered when
using it). I experienced this issue while testing code on some plug-in
yesterday, so thought I might as well fix all these broken macros for casting to
the specific GimpPlugIn subclass, so that we won't have a next time.
The gimp_procedure_run() already existed, though it was with an ordered
GimpValueArray array of arguments. Its usage feels redundant to the series of
gimp_pdb_run_procedure*() functions (which is confusing), but
gimp_procedure_run() was actually a bit more generic, because it does not
necessarily calls GimpProcedure-s through the PDB! For instance, it can runs a
local GimpProcedure, such as the case of one procedure which would want to call
another procedure in the same plug-in, but without having to go through PDB. Of
course, for local code, you may as well run relevant functions directly, yet it
makes sense that if one of the redundant-looking function is removed, it should
be the more specific one. Also gimp_procedure_run() feels a lot simpler and
logical, API wise.
A main difference in usage is that now, plug-in developers have to first
explicitly look up the GimpPdbProcedure with gimp_pdb_lookup_procedure() when
they wish to call PDB procedures on the wire. This was done anyway in the
gimp_pdb_run_procedure*() code, now it's explicit (rather than calling by name
directly).
Concretely:
* gimp_pdb_run_procedure(), gimp_pdb_run_procedure_config() and
gimp_pdb_run_procedure_valist() are removed.
* gimp_procedure_run() API is modified to use a variable args list instead of a
GimpValueArray.
* gimp_procedure_run_config() and gimp_procedure_run_valist() are added.
* gimp_procedure_run_config() in particular will be the one used in bindings
which don't have variable args support through a (rename-to
gimp_procedure_run) annotation.
Passing (name, type, value) triplets is actually useless because we can get the
type information from the procedure/config anyway. That only adds one more
verification to do. Let's just change the function so that we pass (name, value)
couples instead, pretty much like in `g_object_set()`.
As far as plug-in API is concerned, at least the calling API, order of arguments
when calling PDB procedures doesn't matter anymore.
Order still matters for creating procedures with standard arguments (for
instance, "run-mode" is first, then image, or file, drawables or whatnot,
depending on the subtype of procedure), but not for calling with libgimp.
Concretely in this commit:
- gimp_pdb_run_procedure_argv() was removed as it's intrinsically order-based.
- gimp_pdb_run_procedure() and gimp_pdb_run_procedure_valist() stay but their
semantic changes. Instead of an ordered list of (type, value) couple, it's now
an unordered list of (name, type, value) triplets. This way, you can also
ignore as many args as you want if you intend to keep them default. For
instance, say you have a procedure with 20 args and you only want to change
the last one and keep the 19 first with default values: while you used to have
to write down all 20 args annoyingly, now you can just list the only arg you
care about.
There are 2 important consequences here:
1. Calling PDB procedures becomes much more semantic, which means scripts with
PDB calls are simpler (smaller list of arguments) and easier to read (when
you had 5 int arguments in a row, you couldn't know what they refer to,
except by always checking the PDB source; now you'll have associated names,
such as "width", "height" and so on) hence maintain.
2. We will have the ability to add arguments and even order the new arguments in
middle of existing arguments without breaking compatibility. The only thing
which will matter will be that default values of new arguments will have to
behave like when the arg didn't exist. This way, existing scripts will not be
broken. This will avoid us having to always create variants of PDB procedure
(like original "file-bla-save", then variant "file-bla-save-2" and so on)
each time we add arguments.
Note: gimp_pdb_run_procedure_array() was not removed yet because it's currently
used by the PDB. To be followed.
… than a GimpValueArray.
Similar to other GimpProcedure, move to using a config object. A difference is
that thumbnail procedures are always run non-interactively.
Also fixing WMF load thumbnail procedure: the dimension computation was wrong
when the image was wider than tall.
This is not the main reason for the specific output in #9994. These ones are
more probably because of similar usage in GTK (which updated its own calls to
g_file_info_get_is_hidden|backup() in version 3.24.38). But we should likely
also update the various calls we have to use the generic
g_file_info_get_attribute_*() variants.
To be fair, it is unclear to me when we can be sure that an attribute is set.
For instance, when we call g_file_enumerate_children() or g_file_query_info()
with specific attributes, docs say that it is still possible for these
attributes to not be set. So I assume it means we should never use direct
accessor functions.
The only exception is that I didn't remove usage of g_file_info_get_name(),
since its docs says:
> * Gets a display name for a file. This is guaranteed to always be set.
Even though it also says just after:
> * It is an error to call this if the #GFileInfo does not contain
> * %G_FILE_ATTRIBUTE_STANDARD_DISPLAY_NAME.
Which is very contradictory. But assuming that this error warning was
over-zealous documentation, I kept the direct accessors since they are supposed
to be slightly more optimized (still according to in-code documentation) so
let's priorize them when we know they are set for sure.
This adds the PSD metadata plug-in procedure call to the JPEG
plug-in, as part of implementing issue #7549.
Also implements the import half of issue #1842.
JPEGs only store image-level metadata like paths.
This adds the PSD metadata plug-in procedure call to the TIFF
plug-ins, as part of implementing issue #7549.
Also implements the import part of issue #2921.
TIFFs can have both image and layer-level metadata.
The load_paths() function was removed, as the PSD plug-in should
handle this now.
- This is unneeded in all import procedures. See previous commit. Note though
that this is not because of a change in previous commit. This was already
useless previously. The file set with this PDB function was overridden by the
core anyway (i.e. even before the previous commits).
In app/file/file-import.c:file_import_image(), the imported file is correctly
set (so there is no need to set it from plug-in, which anyway libgimp's
gimp_image_set_file() was not doing) and the XCF file is reset to NULL
(rendering the call to gimp_image_set_file() in a GimpLoadProcedure useless).
- Similarly, this is a useless call in export procedures because
app/file/file-save.c:file_save() overrides such call too. I could only see one
such case for JPEG export, which was quite useless.
- Finally in other types of plug-ins, setting a non-XCF file extension was
interfering with the save feature (similarly to commit e6e73e14c7). I only
fixed the screenshot implementations doing such a thing.
- I left a few usages which will have to be looked at more in details later.
On Windows fopen () is limited to the current codepage,
GLib's g_fopen () instead accepts full UTF-8 by calling
_wfopen () internally (or a similar wide-char CRT routine).
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
CMYK profile is now stored in GimpImage on load
(rather than being discarded) and it's used for export rather than
the default simulation profile stored in Preferences
This is the consequence of previous commit. Plug-ins' label and
documentation are now localized before sending these data to GIMP core.
In other words, we replace N_() macros with basic gettext calls.
Hence avoiding the stderr messages. These are going to be localized with
centrally installed catalogs "gimp*-std-plugins", "gimp*-script-fu" and
"gimp*-python".
We now handle core plug-in localizations differently and in particular,
with kind of a reverse logic:
- We don't consider "gimp*-std-plugins" to be the default catalog
anymore. It made sense in the old world where we would consider the
core plug-ins to be the most important and numerous ones. But we want
to push a world where people are even more encouraged to develop their
own plug-ins. These won't use the standard catalog anymore (because
there are nearly no reasons that the strings are the same, it's only a
confusing logic). So let's explicitly set the standard catalogs with
DEFINE_STD_SET_I18N macro (which maps to a different catalog for
script-fu plug-ins).
- Doing something similar for Python plug-ins which have again their own
catalog.
- Getting rid of the INIT_I18N macro since now all the locale domain
binding is done automatically by libgimp when using the set_i18n()
method infrastructure.
We already had import support through littleCMS. We now use fully
babl/GEGL which makes our code more straightforward and identical,
whichever the input format.
The export support is totally new. It comes with a checkbox to propose
selecting CMYK export and a label displaying the CMYK profile which will
be used.
Now this whole implementation has a few drawbacks so far, but it will be
a good first sample for future CMYK-related improvements to come:
* The export profile I am using is what we call the "simulation
profile" from the GimpColorConfig. This corresponds to the default
"Soft-proofing" profile as set in Preferences. In particular, this is
not the actual soft-proofing profile for this image which might have
been changed through the View menu because this information is
currently and unfortunately unavailable to plug-ins. It is not the
"Preferred CMYK Profile" either, as set in Preferences.
TODOS:
- We really need to straighten the soft-proof profile core concept by
storing it in the image and making it visible to plug-in.
- Another interesting improvement could be to create a
GimpColorProfile procedure argument which would be mapped to a color
profile chooser widget, allowing people to choose profiles in
plug-ins. For an export plug-in in particular, it could allow to
select a profile different from the soft-proof one at export time.
* When we export, if no profile is choosen, babl will use a naive
profile. It would be nice to store this naive profile into the JPEG if
the "Save color profile" option is checked (same as we store a generic
sRGB profile when no RGB profile is set).
* When we import, we just import the image as sRGB. Since CMYK gamuts
are not necessarily within sRGB (some part of the spectrum is usually
well within, but other well outside), other than the basic conversion
accuracy issue, we may lose colors. It would be much nicer to be able
to select an output RGB profile. Optionally if we could create a RGB
color space which is made to contain the whole input CMYK profile
color space, without explicit choice step, it would be nice too.
* I am using babl's "cmyk" format, not the expected "CMYK" format.
"cmyk" is meant to be an inverted CMYK where 0.0 is full ink coverage
and 1.0 none. Nevertheless when loading the resulting JPEG in other
software (editors or viewers alike), the normal CMYK would always
display inverted colors and the inverted cmyk would look fine.
Finally I found a docs from libjpeg-turbo library, explaining that
Photoshop was wrongly inverting CMYK color data while it should not.
This text dates back from 1994, looking at the commit date which
introduced this paragraph. In the 28 years since then, could this
color inversion have become the de-facto standard for JPEG because one
of the main editor would just output all its JPEG files this way?
See: dfc63d42ee/libjpeg.txt (L1425-L1438)
While doing this cleanup, I found at least several other string leaks
in: file-compressor, file-gegl, file-pdf-save, file-raw-data, file-xwd,
jpeg-load, psd-save…
So it's quite worth it!
Note: in file-pdf-save, there is a global variable file_name which seems
to be happily leaked without caring (didn't look in details, but looks
so). I didn't fix this one which will require a bit more in-depth logics
care.
Commit aba721ae68 fixed its intended bug but brought a new: each time
the preview was updated, a new display was created. This fixes this new
bug. Hopefully it's good now!
The port had a slight error, because in gimp-2-10, the display_ID
actually had 3 states: 0 when gimp_export_image() kept the original
image to which we just add a preview layer, -1 when it created a new
image which we wanted to put in its own display, and the display ID
itself when created.
With the new API where display variable is an object, we can only have 2
cases. So I create an additional variable separate_display to make the
distinction.
In PNG, JPEG and TIFF export plug-ins which use the new API, use our new
function to set widget sensitivity.
Note that part of it is similar to commit 6a2910bd3b on `gimp-2-10`,
making "Save GeoTIFF data" checkbox insensitive when there are no such
data available (this feature was late on the main branch as we rushed
for 2.10.24 release).
Similarly to the previous commit, it is not only about using the new
API. I also make sure we do not assume that parasite data is
nul-terminated. In many places, we were just assuming so because these
were supposed to be parasite our code set. Yet these are data input
contained in files which can be wrong for any reason (corrupted file,
bugs, other scripts/plug-ins editing these parasites…). So instead of
assuming string parasites are always correctly formatted, I make sure
they are nul-terminated by passing them through g_strndup() when
necessary.
Not sure why we didn't see the crash earlier and it suddenly shows up
now. Anyway we must delete the exported image **after** calling
gimp_procedure_config_end_export() on it, obviously.
… GimpSaveProcedureDialog.
See issue #6092 and the discussion with Jacob. Basically we are trying
to improve the metadata situation with more edit abilities and
awareness, while in the same time having the export dialogs less of a
mess (the "Comment" input in particular will most likely move to the
metadata editor itself; I left it for now, until the move is done).
The "(edit)" link will basically just run "plug-in-metadata-editor".
Also as a side note: I realized that gimp_pdb_run_procedure() runs
procedures synchronously and wait for a result, which is fine for quick
non-interactive plug-ins, but freezes the calling process otherwise.
Actually even when we want synchronous result, we should allow for GUI
events to be processed (otherwise the OS just thinks the calling export
plug-in is a zombie and proposes to kill it). This API should probably
be improved (and an alternative async version added as well).
This format name is a public facing name for a file format, such as
"PNG", "JPEG", or "C-source". Since it is public facing, the function
recommends to localize it too.
This is an optional name, yet is made mandatory if you want to use
GimpSaveProcedureDialog because it will be used for the dialog title
(ensuring that all support format have a similar export dialog title).
Following this change, gimp_save_procedure_dialog_new() does not ask for
a title anymore (if anyone absolutely wants to set a custom title,
setting the "title" property on the dialog is always possible anyway,
but a generic and consistent title should be set as a default).
Also updating the 3 plug-ins which were already using the now-changed
API.
The various generic metadata options did not have mnemonic in the base
language (US English).
Also add or fix metadata in file-png|jpeg|tiff so that every option has
a unique mnemonic.
This is nearly 600 lines less for basically the same logics! Removed
code is in particular all the GUI code is favor of the new GUI
generation.
I also cleaned a lot of stuff, removing many global variables or ugly
pieces of code. I also removed a lot of redundant code of things which
are now generic, such as handling of "gimp-comment" parasite (this is
now handled by GimpSaveProcedure and GimpImageMetadata) as well as
saving previous run's values (this is also handled generically).
Note that Advanced Options used to be in an expander. For now I chose to
get them immediately visible (still in their own "Advanced Options"
section, but it's now a normal frame, not an expander hidden by default)
since lately we got some input that many "advanced options" in various
dialogs should not be hidden away. So let's try like this for now (even
though it packs quite a lot of options in the same dialog!).
I thoroughly tested, yet that were so many changes that bugs may have
sneaked in. Please anyone, test JPEG export!
Jehan