This was happening when trying to load a file before fonts were fully
loaded, which may happen when loading while starting GIMP (either from
CLI, or from a file browser, etc.), or simply just after start for
people with a lot of fonts, whose font loading may take a long time as
background task.
Note that I didn't manage to reproduce properly because from reports, it
seems like the problem appears where some fonts may be only partly
loaded so gimp_font_get_hash() fails to load the font and get a hash. I
never managed to trigger such a case and no reporters answered my call
for testing of debug builds.
As a consequence, I'll just assume that simply waiting for all fonts to
be loaded before starting to load images would work out.
Note that the crash was not happening when text layers were using the
older syntax (pre-XCF 19) of text layer data, since it was not storing
any font hash, which means that we were not trying to compare hashes. It
would also not crash if fonts were not fully loaded yet, but we didn't
have any weird intermediate state where fonts appeared in the list yet
their file were not hashable (cf. what I failed to reproduce, as
explained in previous paragraph). But both these cases were not ideal
either anyway because then we could load the XCF apparently OK… except
that the correct fonts would not be associated to the related text
layers (hence as soon as you start to edit said texts, the rendering
would break). So we should wait for fonts to be loaded, and that was a
bug even when you can't reproduce the crash.
When starting GIMP without loading an image, or simply when fonts are
loaded quickly enough, it won't make any difference. So it should not
make startup any slower for most common use cases and installations.
This is a complementary commit to my previous one. Now let's try to
detect cycling links when importing images as link layers. It's much
friendlier this way, with early errors (rather than let people load
links with cycles, then save a XCF, and finally get an error message at
next XCF load).
As of 443947c6, we no longer require passing
the selected drawables to the export plug-ins
(as each plug-in now grabs those from the
image itself).
Therefore, we can remove the restriction that
there must be a selected layer before you
can export or save an image.
Our build files were relying 'sysroot' to find gexiv2.h but this is
not possible with Apple Clang om which sysroot points to macOS SDK.
So, exotic environments like Homebrew were failing. Let's fix this.
… determine if a file is XCF.
This was kinda already working for files loaded through the GUI. Yet the
code in file_open_file_proc_is_import() was assuming a file loaded
through the file-compressor plug-in is necessarily XCF.
Even though it is the original use, the code in file-compressor is
actually generic and is able to load any supported format additionally
compressed with gz, bz2 or xz.
Furthermore, we will soon support zip-ped imaged, explicitly to support
the format .hgt.zip which is quite common (see !2483). So remove any
logic of a file loaded by file-compressor as meaning it's a compressed
XCF.
Moving the gimp_image_undo_free and gimp_image_clean_all
functions outside the link layer check prevents the image getting
dirty. Hence, closing a new linked layer image without changes
doesn't open a save changes dialog anymore.
… the whole canvas while keeping aspect ratio of the source image.
The previous dimensions were not entirely "out of the blue" since they
were taken from the file, but very often dimensions from vector images
are kinda bogus (apart from aspect ratio, they don't mean much) anyway.
Now we will just fill the canvas box by default.
When changing the source file of an existing link layer though (through
Link Layer properties), the dimensions will be within the current layer
bounding box.
Also I realized that we need to also store the link width/height in the
XCF because the buffer width/height may not be right (in case a
transform matrix was applied). This commit therefore breaks XCF file
with link layers made in the last 2 days. Since we have not made even a
dev release, it's probably fine to do this and not bump the XCF version.
Revert the logic of opening all files as link layers back into loading
their current content as normal layers.
Instead just add a new action dedicated to open images as link layers
and add it to the File menu.
Per UX discussions with Aryeom.
We now re-render a link (by loading its file) immediately upon setting
the new file, i.e. during the calls of gimp_link_new() or
gimp_link_set_file(). As part of this change:
* A GimpLink now stores a GeglBuffer. And this is changed each time a
file change happens (per the GFileMonitor). In particular that also
means that gimp_link_get_buffer() does not reload the image file at
each call for no reasons, and gimp_link_is_broken() does not have a
`recheck` argument either. This is much more efficient.
* These 2 functions also have a GimpProgress and GError now. We use this
among other things to pass on a specific GimpProgress object. In
particular, the image file dialogs now show correct loading
progression again.
* As a general rule, the code is less confusing as we don't have to
wonder whether a GimpLink is ready. We can assume it always is, from
now on.
Note that gimp_link_duplicate() does not trigger a reload of the image
file. Since we assume that the source GimpLink is supposed to be
up-to-date already, we just copy its buffer as-is as an optimization.
This is still mostly a test of workflow. It is based on the idea that
link layers are normal yet enhanced layers: eventually we should be able
to have some improved less-destructive scaling/rotation (even without
NDE transform effects); you can manually drop the link anytime anyway
(hence getting back to the good old fully destructive workflow); any
pixel editing automatically drops the link too.
Now this still raises quite a lot of questions:
* The link can be confusing to people used to the old way and they may
not realize that editing the original file separately would also
update the render in this file (which may not be what they wanted;
maybe they just wanted to grab a snapshot of this file at a given
time).
* You could also want to link XCF files.
* You could also want to link remote files (especially in a controlled
network environment).
* Linked images are currently taken as a whole; we definitely want layer
support to handle multi-layer image formats (so that you could link
only a single layer, or a collection of layers; do we want to be able
to edit visibility of linked layers separately too? Would be neat). So
how would we handle automatic linking when opening a file? Maybe we
just reproduce the layer structure as link layers (one link layer
monitoring only one layer from the linked file)?
Anyway this is work-in-progress, UX-wise.
This commit was edited after GIMP 3.0, now that we have dedicated
support for loading vector images with GimpVectorLoadProcedure.
- By default, when loaded as GimpLinkLayer, vector images are loaded at
a size so that they are exactly contained in the image.
- When scaled with Scale Layer dialog, link layers of type vector are
re-loaded from the source file to always stay perfectly crisp.
If somehow an image is not passed when opened from the command line, this avoids an infinite loop
as was seen in #13702
This still does not solve the issue of how a bad image was passed in in the first place.
Now that GimpParamSpecFile makes file validation depending on the action
set in the param spec, when trying to open a non-existing file (which
can happen through GUI, for instance when opening through the Document
History dockable or Open Recent menu), we had a quite obscure error
about a value "out of range" used "for argument 'file' (#2, type
GFile)", which is because core tried to set the GFile for a non-existing
file into the second parameter of the plug-in PDB. This is not a very
nice error for end-users.
The old error was much nicer as it used to say things like:
> Could not open '/path/not/existing.png' for reading: No such file or directory
But in fact, this string came from the plug-in, which means:
* first that the error can be different depending on the format
(inconsistency of error message);
* depending on bugs in plug-ins, it may just crash or return no error
messages;
* finally we were making a useless call to a load plug-in while we
should already know from core that it won't work.
The new message is something like:
> Error when getting information for file "/path/not/existing.png: No such file or directory
This error is generated by core, so it will always be consistently the
same, is understandable too and the plug-in won't even be called.
As a side related fix, I updated the GimpParamSpecFile validation to
only validate local files, just like we do in core. Remote files can
always be set (and we let plug-ins handle them), at least for the time
being.
… gimp_file_create_thumbnail().
One more case where "save" is misleading, and even more as it's not a
procedure where we control where an image is stored. We only say
basically "make a thumbnail which maps to this file according to my
platform's rules".
As a side fix, I also improved a bit the logic so that it allows @file
to be the exported or — as last fallback — the imported file.
And finally improve the function's docs.
Previously we used gimp_drawable_has_filters ()
to detect if a layer had any filters set.
However, this function only gets active
filters, leading to filters with their
visibility set to FALSE not being copied.
This patch checks the number of children
in the filter stack container instead to get
an accurate filter count.
- 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.
Adds code to copy any NDE filters when
importing an .xcf file as layers in an
existing project.
This also requires removing the check
for the layer being attached to an image,
which historically made sense because
filter effects were immediately merged
down. Now that filters can exist separately
the check is no longer required.
I'm moving the logic of choosing a correct default for width/height by adding an
"extract dimensions" callback in the procedure. The logic is that every vector
format out there should likely have metadata either for pixel dimensions or
physical dimensions, or at the very least for no-unit dimensions (ratio only).
Vector load procedures will have to implement only the extraction of such data
in a callback called by GIMP but not how to act upon them, so that we have a
common logic for all vector images.
I am implementing this callback first in the SVG plug-in, moving all the code
to extract dimensions (and improving it) in this callback.
Also I am deleting "file-svg-load-thumb" procedure. I could simply reimplement
it using the same code, but it looks to me like this is very useless for vector
formats to have a specific thumbnail procedure (unless it were to use very
specific metadata for faster result). This is vector data, just ask it directly
at the proper bounding box size.
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.
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.
When a plug-in enters a bogus file name such as "Untitled.xcd" as URI,
g_file_is_native() returns FALSE. So when saving such file, core code tries to
mount a remote volume and fails (of course), without even opening the file
dialog.
This change is an attempt to detect such cases beforehand and not even try to
save it (just open the file dialog directly; the file name is still useful as
default proposed file name, as this part is actually valid).
Actually remote files will have a valid URI anyway (something with a valid
scheme, such as https:// or whatever other scheme).
As side fix, I am adding a missing space to the error which I got.
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.
In file_open_image(), mount non-native files *before* looking up a
file-proc. Previously, we'd only mount the file after the initial
lookup, and fail to perform a second lookup if the mount succeeded,
leaving us with a NULL file-proc and a subsequent segfault.
Additionally, simplify the rest of the remote-file code-path.
Multi selection actually only really matter when "Merge within active
groups only" option is checked, in which case we are able to merge
layers within several layer groups simultaneously, and end up with
multi-selected merged layers.
Also not sure why both layers-merge-layers and image-merge-layers exist,
as they are exactly the same (exact same callback called when
activated).
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).
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.
It's an ancient concept from ancient times when we didn't have URIs
and only filenames (not to speak of GFile), and actually even from
before the ancient time before that ancient time when we first had
ones and zeros, and only had zeros.
... which invalidates the entire image. This replaces all calls to
gimp_image_invalidate() with the full canvas size, since the image
content can now be larger than the canvas.
GimpDisplay contains only the ID logic and the "gimp" and "config"
pointers, and lives in the core.
GimpDisplayImpl is a subclass and contains all the actual display
stuff. The subclass is only an implementation detail and doesn't
appear in any API.
Remove all hacks which pass displays as gpointer, GObject or
GimpObject through the core, or even lookup its type by name,
just use GimpDisplay.
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/.
