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!
Renaming the temporary function gimp_scale_entry_new2() into
gimp_scale_entry_new() now that the original code is entirely gone. This
is now a fully-fledged widget with a nice and proper introspectable API.
I got rid of gimp_scale_entry_set_sensitive(), as we can now use the
generic gtk_widget_set_sensitive(), and I ported the 2 plug-ins using
this function.
I also created gimp_scale_entry_set_value() to set the value (nicer than
setting object properties).
Orientation is now handled by core code, just next to profile conversion
handling.
One of the first consequence is that we don't need to have a non-GUI
version gimp_image_metadata_load_finish_batch() in libgimp, next to a
GUI version of the gimp_image_metadata_load_finish() function in
libgimpui. This makes for simpler API.
Also a plug-in which wishes to get access to the rotation dialog
provided by GIMP without loading ligimpui/GTK+ (for whatever reason)
will still have the feature.
The main advantage is that the "Don't ask me again" feature is now
handled by a settings in `Preferences > Image Import & Export` as the
"Metadata rotation policy". Until now it was saved as a global parasite,
which made it virtually non-editable once you checked it once (no easy
way to edit parasites except by scripts). So say you refused the
rotation once while checking "Don't ask again", and GIMP will forever
discard the rotation metadata without giving you a sane way to change
your mind. Of course, I could have passed the settings to plug-ins
through the PDB, but I find it a lot better to simply handle such
settings core-side.
The dialog code is basically the same as an app/dialogs/ as it was in
libgimp, with the minor improvement that it now takes the scale ratio
into account (basically the maximum thumbnail size will be bigger on
higher density displays).
Only downside of the move to the core is that this rotation dialog is
raised only when you open an image from the core, not as a PDB call. So
a plug-in which makes say a "file-jpeg-load" PDB call, even in
INTERACTIVE run mode, won't have rotation processed. Note that this was
already the same for embedded color profile conversion. This can be
wanted or not. Anyway some additional libgimp calls might be of interest
to explicitly call the core dialogs.
Our currently registered JPEG magics was using possible fields at the
start of the file (JFIF, Exif) in 7th byte. A JPEG provided in #5584 was
starting with yet another field (apparently some Photoshop-specific
field "Photoshop 3.0.8BIM") thus magics detection failed (and since
extension was wrong, the file was sent to another plug-in, hence loading
failed).
Looking further, it looks like the 3 first bytes are actually always
common to all JPEG and are the right magics/signature for a JPEG. These
3 bytes don't seem to be used by other formats as well according to my
research, which means we should not have false positives either. So
let's switch to this magics 0XFFD8FF at byte 0.
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.
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.
jpeg-save.c:56: warning: "DEFAULT_QUALITY" redefined
56 | #define DEFAULT_QUALITY 90.0
|
In file included from C:/msys64/mingw64/x86_64-w64-mingw32/include/windows.h:71,
from C:/msys64/mingw64/x86_64-w64-mingw32/include/rpc.h:16,
from C:/msys64/mingw64/x86_64-w64-mingw32/include/objbase.h:7,
from C:/msys64/mingw64/x86_64-w64-mingw32/include/shlwapi.h:16,
from C:/msys64/mingw64/include/jmorecfg.h:19,
from C:/msys64/mingw64/include/jpeglib.h:31,
from jpeg-save.c:33:
C:/msys64/mingw64/x86_64-w64-mingw32/include/wingdi.h:1140: note: this is the location of the previous definition
1140 | #define DEFAULT_QUALITY 0
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).
And always pass URIs to all file procedures, the ones what didn't
register as "handles remove" will only ever get local file:// URIs.
Change all file plug-ins (also legacy ones) to expect URIs instead
of filenames, and convert to local paths in the plug-in.
The wire protocol should now be almost 100% clean of non-UTF-8 strings.
Basically this commit makes sure that all return values that are marked
as "Returns:" also have a `(nullable)` annotation if it is mentioned on
the same line that NULL can also be returned.
This will prevent a few problems in GObject-introspection.
Use the drawable's space for the load/save formats to avoid unwanted
color conversions. Also cleaned up the loading code a bit, it was
doing weird duplicated stuff for the preview and !preview cases.
... in JPEG export.
Same as the WebP export, which is quite similar (8-bit max format), when
no profile was explicitly set, we want to convert any data from storage
format to non-linear (unlike when exporting high bit depth formats, such
as TIFF).
We only make an exception for 8-bit linear. This commit adds this
exception.
GIMP should not convert assigned profile to sRGB just because we stored
as linear on the XCF. In other words, we should not look at the image
precision to decide whether to export as linear (previously only 8-bit
linear images), but at the profile TRC. There are basically 3 cases:
(1) We don't save a profile, then convert to sRGB whatever the source
precision (because readers would assume sRGB for a no-profile jpeg).
(2) We save the default profiles: convert to sRGB because it's usually
a better choice for 8-bit formats and even working at 32-bit float
*linear* doesn't mean you want to export as 8-bit int *linear*. As the
image creator made no explicit export choice, we make an acceptable
default one.
(3) We save an explicitly assigned profile: keep the profile TRC, don't
convert!
Note that this apparently won't work perfectly right now, as GIMP
replaces the original TRC with the linear default TRC when converting to
linear. So the expected TRC is lost in such case when you have not
explicitly reset the correct profile. Yet this is on GIMP side and this
part of the issue should be fixed with the space invasion merge. For
now, this is how the plug-in should work.
This is based on my late discussion with Ell. Please everyone, and Ell
especially, review! :-)
(cherry picked from commit c5f7bac2ba)
... linear itself AND if we export the profile.
In most cases we want to save 8-bit image formats (here JPEG) as
non-linear, even though the work image may have been linear itself (yet
with higher bit depth). The reasons are shadow posterization on low bit
depth, and the fact that JPEG compression was designed for perceptually
uniform RGB and introduces shadow artifacts with linear RGB (see #1070,
message by Elle Stone). The only exception is when the creator was
working explicitly on 8-bit linear (not higher bit depth) AND if we
export the profile (otherwise most loaders around assume sRGB). In such
a case, let's consider the creator knows what one is doing and keep the
exported image linear.
Similar logics is already used in PNG exporter (though a bit of a
variant since PNG supports 16-bit so it is instead: 8-bit linear without
profile is promoted to 16-bit non-linear, and kept 8-bit linear with
profile).
... produces jpeg in linear color space.
The problem was that we were anyway always exporting to non-linear while
attaching a linear profile. A first approach would have been to export
in linear instead when the work image is linear, as proposed by mitch in
a first version of the patch. Yet as Elle Stone notes, it is not a great
idea to export 8-bit images as linear.
Instead let's continue to always export as non-linear, as we were
already doing. Yet when we also save a profile, and this one was
originally linear, let's convert it to sRGB TRC before exporting.
Add flag GIMP_METADATA_SAVE_COLOR_PROFILE to GimpMetadataSaveFlags and
initialize it from gimp_export_color_profile() in
gimp_image_metadata_save_prepare().
Adapt all plug-ins to use the bit from the suggested export flags and
pass the actually used value back to
gimp_image_metadata_save_finish().
This changes no behavior at all but creates hooks on the libgimp side
that are called with the context of an image before and after the
actual export, which might become useful later. Also, consistency
is good even though the color profile is not strictly "metadata".
...export dialogs
Move most stuff out of the "Advanced" expander, only nerdish encoding
options are left there.
Issue #701: Add a "Save color profile" toggle which honors the default
value configured in preferences and always saves the profile when
enabled.
This code was originally written by Thomas G. Lane and Todd Newman,
and proposed for inclusion in the Independent JPEG Group's software.
However, that fell through when Tom left the IJG. It was not written by
Marti Maria - Little cms only had a copy of that code.
This code has recently been merged into libjpeg-turbo, so it seems
reasonable to now consider libjpeg-turbo as the canonical source of
these files. However, since the GIMP carries the original version of
the code proposed for IJG's JPEG, and doesn't contain any of the more
recent and minor tweaks made during its inclusion in libjpeg-turbo,
only the original copyright holders (ie., Thomas G. Lane and Todd
Newman) are mentioned.
The relevant license text was lifted from the Independent JPEG Group's
software, similar to the way it's done in Chromium and other users of
this code.
https://gitlab.gnome.org/GNOME/gimp/issues/2119
All babl formats now have a space equivalent to a color profile,
determining the format's primaries and TRCs. This commit makes GIMP
aware of this.
libgimp:
- enum GimpPrecision: rename GAMMA values to NON_LINEAR and keep GAMMA
as deprecated aliases, add PERCEPTUAL values so we now have LINEAR,
NON_LINEAR and PERCPTUAL for each encoding, matching the babl
encoding variants RGB, R'G'B' and R~G~B~.
- gimp_color_transform_can_gegl_copy() now returns TRUE if both
profiles can return a babl space, increasing the amount of fast babl
color conversions significantly.
- TODO: no solution yet for getting libgimp drawable proxy buffers in
the right format with space.
plug-ins:
- follow the GimpPrecision change.
- TODO: everything else unchanged and partly broken or sub-optimal,
like setting a new image's color profile too late.
app:
- add enum GimpTRCType { LINEAR, NON_LINEAR, PERCEPTUAL } as
replacement for all "linear" booleans.
- change gimp-babl functions to take babl spaces and GimpTRCType
parameters and support all sorts of new perceptual ~ formats.
- a lot of places changed in the early days of goat invasion didn't
take advantage of gimp-babl utility functions and constructed
formats manually. They all needed revisiting and many now use much
simpler code calling gimp-babl API.
- change gimp_babl_format_get_color_profile() to really extract a
newly allocated color profile from the format, and add
gimp_babl_get_builtin_color_profile() which does the same as
gimp_babl_format_get_color_profile() did before. Visited all callers
to decide whether they are looking for the format's actual profile,
or for one of the builtin profiles, simplifying code that only needs
builtin profiles.
- drawables have a new get_space_api(), get_linear() is now get_trc().
- images now have a "layer space" and an API to get it,
gimp_image_get_layer_format() returns formats in that space.
- an image's layer space is created from the image's color profile,
change gimpimage-color-profile to deal with that correctly
- change many babl_format() calls to babl_format_with_space() and take
the space from passed formats or drawables
- add function gimp_layer_fix_format_space() which replaces the
layer's buffer with one that has the image's layer format, but
doesn't change pixel values
- use gimp_layer_fix_format_space() to make sure layers loaded from
XCF and created by plug-ins have the right space when added to the
image, because it's impossible to always assign the right space upon
layer creation
- "assign color profile" and "discard color profile" now require use
of gimp_layer_fix_format_space() too because the profile is now
embedded in all formats via the space. Add
gimp_image_assign_color_profile() which does all that and call it
instead of a simple gimp_image_set_color_profile(), also from the
PDB set-color-profile functions, which are essentially "assign" and
"discard" calls.
- generally, make sure a new image's color profile is set before
adding layers to it, gimp_image_set_color_profile() is more than
before considered know-what-you-are-doing API.
- take special precaution in all places that call
gimp_drawable_convert_type(), we now must pass a new_profile from
all callers that convert layers within the same image (such as
image_convert_type, image_convert_precision), because the layer's
new space can't be determined from the image's layer format during
the call.
- change all "linear" properties to "trc", in all config objects like
for levels and curves, in the histogram, in the widgets. This results
in some GUI that now has three choices instead of two.
TODO: we might want to reduce that back to two later.
- keep "linear" boolean properties around as compat if needed for file
pasring, but always convert the parsed parsed boolean to
GimpTRCType.
- TODO: the image's "enable color management" switch is currently
broken, will fix that in another 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.
Jehan