Now that we save GEGL operation arguments ourselves (instead of relying on
GEGL-generated XML), we can serialize and deserialize GimpConfig arguments which
are used in various operations implemented within GIMP core code.
Additionally:
- Also support saving and loading all enum types.
- PROP_EFFECT_ARGUMENT renamed to PROP_FILTER_ARGUMENT for consistent naming.
- A bit more accurate handling on save and load errors with dedicated messages
to various issues.
- Use PROP_FLOAT_OPACITY instead of the obsolete 8-bit PROP_OPACITY (actually
32-bit but stored as 0-255 int).
- Fix a leaking string.
*It's possible to have one layer selected but edit the filter of another layer.
The FilterTool code was updated to use the filter's drawable if editing.
*Prevents editing if multiple layers are selected.
This is a current limitation of the filter tool itself, but it was causing issues with NDE.
*If an XCF with a filter from a third-party plug-in is loaded, the filter won't be
created if the user doesn't have it installed.
Note that GEGL still throws a lot of warnings about missing properties - not sure how to resolve that.
*Refresh drawable when finished editing a filter to fix issue with lingering preview splits
*Fix issue with loading more than one filter after XCF format changes
- Do not leak allocated return value of gegl_node_to_xml_full().
- When merging layer effects, use gimp_drawable_filter_commit(), making
sure we use the exact same code path as when applying layer effects
destructively from the start. This also ensures that filters are
properly removed from the filter stack (unlike
gimp_drawable_merge_filter()), which was the reason why the rendering
was wrong (hence getting the buffer without effects first, then
reapplying it after was only a workaround to an actual bug).
- When removing a filter, verify the object still exists before doing
anything with it. If this was the last reference, we don't want to
call functions on this object. In gimp_drawable_filter_commit(), we
set up a weak pointer. In gimp_drawable_filter_remove_filter() itself,
we save the pointer to the filter's drawable before actual removal (as
we don't want to dereference a freed object later on).
- export_merge_layer_effects() should merge filters recursively through
layer groups.
- clean up the XCF code:
* No need to wrap the effect pointers list into 2 zero offset. Only
have one zero offset to indicate the list end.
* Add the layer effect mask in the effect structure (not in the layer
structure), similar as for layer masks.
* Effect name and icon made as main data in the structure, not as
properties.
This patch implements an initial form of
non-destructive editing. Filters now stay active
instead of being immediately merged down.
A new column is added to the layer tree view, which
can be clicked to show a pop-over menu.
Filters can currently be hidden/shown, edited, reordered,
deleted, and merged down from this pop-over menu.
Currently, this works on layers and layer selections only.
Plenty of room for improvement!
Since now the name of arguments will become more important, over order, let's
name the first parameter "run-mode" even in cases when this is a dummy argument
(most often the case when a procedure always acts the same, whether interactive
or not). I keep the mention of the parameter being useless in the nick and blurb
strings, as it's useful information. But let's keep using our "standard" arg
name "run-mode" for this first argument.
The new parasite format cannot be loaded by old versions of GIMP. This means we
must bump the XCF version (even though technically we didn't really touch the
XCF format itself because text layers are stored in a hackish way, yet text
layers are just too important nowadays to not care).
Nevertheless if an old XCF with text layers was loaded and the text layers left
untouched, the old parasite will be saved back as-is. Therefore no need to bump
the XCF version in this specific case. Only when we created new text layers or
edited existing ones.
xcf_save_prop() starts with va_start(), hence must end with va_end(). Yet any of
the xcf_write*_check_error() macros could end this function abruptly. Therefore
I'm adding a "cleanup" argument to the macros, allowing to add any code
necessary to clean the current function before returning.
These are not used anymore anywhere in our codebase! I'm sure some issues still
exist in various places, yet we can now consider that the multi-item awareness
project is finally over! Wouhou! 🥳
One big question which remains is whether I want to get back to the old naming
of "active" items, rather than "selected" items. The main reason to change the
wording globally was to be able to easily find remnants of non-multi-item aware
code. Now that it's all gone, I could very simply revert to the old naming.
This is in particular a big question for the public API for plug-ins, as the
"active" wording has been used for decades litterally. The only difference now
with how it used to be is that we could have several active items at once.
For the same file, which was saving in ~50 secs in RLE (now ~12), and ~80 secs
in zlib. It now saves zlib-encoded XCF in ~35 secs on the same machine with 8
threads.
Contributed code was not wrong, far from it! But the whole memory management was
a bit on the hard-to-read side. For maintainability, I prefer some simpler code,
which contains a bit more allocation but it's reasonable and at least much less
indexes to play with.
Also instead of gegl_parallel_distribute(), then waiting for each set of batches
to entirely finish, I'm using a GThreadPool which I progressively fill with new
batch data. And finally I'm running the gegl_buffer_get() inside the threads (I
know that we have only reads on the buffer at this point; also GEGL has an
internal lock mechanism anyway).
These actually do not provide a huge additional speedup (compared to the initial
speedup by just going multi-threaded), most likely because the I/O are now the
bottleneck anyway (which is quite a good thing!).
- Coding style fixes.
- Removing gimp_gegl_num_processors unknown variable which didn't exist (coming
from another patch from suzu. See discussion in #4985).
- Removing the `cleanup()` attribute to keep our code as generic as possible
without specific compiler extensions. Just free memory manually as is expected
in C.
Testing on a 115MiB XCF file, saving went down (on my machine, as average on
saving a few times) from about 50 seconds to 15 seconds.
This first version is already pretty neat, though we can probably do even
better.
Instead of storing vectors as properties, they have their own structure, which
make them able to store and load all the usual and common properties of other
items. In other words, it makes XCF now able to store locks, color tags and
several selected paths.
Creates two new parasites to save the black point compensation status
and rendering intent simulation settings in GimpImage.
The parasites are saved and loaded as part of the image in the
.xcf file.
More safety checks for detecting broken xcf files, also based on examining
issue #8230.
After reading an offset where layer, channel, etc. data is stored, we
add a check to make sure that offset is not before where we read the
offset value. Because the data is always written after the offset that
points to it.
Improvements in loading broken xcf files, based on examining issue #8230.
Besides checking for a minimum width and height, GIMP also has a maximum
size we can and should check.
In the case of the image itself, we change invalid dimensions to a size of
1 in hope that the individual layers etc will have the correct size.
For layer, we will also try to go on, but for channel and layer mask, we
will give up.
GIMP could crash if the information regarding old path properties read
from XCF was incorrect. It did not check if xcf_old_path succeeded and
kept trying to load more paths even if the last one failed to load.
Instead we now stop loading paths as soon as that function fails.
In case we have a failure here we also try to skip to the next property
based on the size of the path property, in hopes that the only problem
was this property.
GIMP stopped trying to read the XCF as soon as an invalid parasite was
encountered. However, in this specific case only the parasite data is
invalid, while the rest of the image is not corrupt.
Instead of terminating when we see a corrupt parasite, we skip to the
offset after the parasite. This may still be corrupt, but we can handle
that correctly, see e.g. the XCF in bugzilla issue 685086, which was
the reason of some of the previous changes.
Additionally:
- We add some logging to make it easier to handle future issues in this
area.
- We add tests for a NULL parasite name, and for reading a different
amount of parasite data than we expected. In both cases we return
NULL instead of a parasite.
XCF 17 includes the new visibility locks and the ability to add position
and alpha locks on layer groups.
I am going to push the various commits implementing these different
features together which is why we gather them as a single XCF version.
Due to recent changes on master the linked layers concept doesn't exist
anymore. The conversion code checks whether all linked layers are valid.
However, text layers need special handling.
In the past updating the linked_layers wasn't needed when the layer pointer
of a text layer was changed. Now, that has changed because we parse that
list for valid layers. To fix this we update linked_layers in the same
way as already was done for selected_layers.
We now save and load layer and channel item sets. Only missing set types
are path ones, but the whole path item is just its own exception in the
XCF format, and adding support for it, while keeping compatibility with
older XCF seem like a small headache. I could do it, but I actually
wonder if it is worth it. Would people really need to store sets of
paths?
Also this commit finally gets rid of any remnant of the old item "link"
concept (I think), so we are getting close to merging the branch.
I cleaned many remaining places where the concept of linked item still
survived.
On loading an XCF file with linked items, we are now going to create a
named sets for the linked items, allowing people to easily select these
back if the relation was still needed.
We don't remove gimp_item_get_linked() yet and in particular, we don't
save stored items into XCF files. This will come in an upcoming change.
In all 3 cases:
- xcf_read_string allocates a new string into name.
- If an error is detected, we return early - name remains unused.
Therefore we add g_free (name) to these early returns to avoid a leak.
… and s/gimp_parasite_flags/gimp_parasite_get_flags/
It is better to have a consistent API and the fact is that most
getter/setter functions use the _get|set_ naming, but these didn't.
I wondered if this was such a great idea to rename these anyway because
even though we are breaking API in GIMP 3, is it the best idea to rename
when no functional change happened? After discussing with Wormnest, we
still agreed it was still better to go for consistency rather than
regret later (and be stuck with these names for many more years).
Also this fixes these warnings:
> [649/1205] Generating gimp-3.0.vapi with a custom command
> Gimp-3.0.gir:24162.7-24162.56: warning: Field `Gimp.Parasite.flags' conflicts with method of the same name
> Gimp-3.0.gir:24318.7-24318.52: warning: Field `Gimp.Parasite.name' conflicts with method of the same name
Allow guides with off-canvas position since we can now view and work
outside the canvas.
The guide deletion interaction does not change too much, except you
don't delete a guide because it's dropped off-canvas but off the display
viewport area instead.
The XCF format is technically unchanged as PROP_GUIDES was already
expecting an int32 coordinate (with max value at GIMP_MAX_IMAGE_SIZE,
way below uint32 max anyway). Yet our code and XCF docs was explicitly
asking to ignore negative coordinate guides, which makes a change in
behavior for the XCF parser, hence a new version XCF 15. Loading will
still ignore negative position guides (even also bigger than image
dimension guides now) for XCF 14 and below, but will now accept any
position for XCF 15 and above.
XMP metadata saved by GIMP 2.8.x or earlier can have duplicate tags
making the XMP data invalid. There's not much we can do without a
whole lot of processing and complicated code and even then no
guarantee we would catch everything.
Instead let's just try to improve the message to the user so they
will be more likely to understand what's going on.
Although I haven't been able to reproduce it, it is apparently
possible to get a Stack Overflow when loading xcf files with
presumably very large dimensions on Windows. From what
I'm reading Windows normally has a smaller stack size than
Linux, probably why it hasn't surfaced there.
Instead of allocating on the stack let's do a g_malloc0
combined with g_free.
This parameter is not even used because we check again the selected
drawables during the save code. Allowing to set to 0 is simpler, and
also it will allow to not have to allocate an object array for layers
when we want to avoid memory management (during crash handling).
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).
In xcf_load_image(), suspend size updates for layer groups before
adding their sublayers, to avoid unnecessary intermediary size
updates. Resume size updates in reverse order, so that nested
groups are updated before their ancestors, after all layers have
been loaded.
When dropping a corrupted layer while loading an XCF, make sure to
clear the corresponding XcfInfo fields that point to it, so that
it's not erroneously used later, as we now continue loading the
image even after corrupted layers are encountered.
See, for example, issue #4643, for a corrputed XCF file affected by
this.
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.
and call it gimp_pdb_set_proc_icon(). Change icon registration code in
libgimp/ and app/ so it's now possible to register icons for temporary
procedures.
Jehan