In gimp-parallel, always flush the async-operations queue (by
executing all remaining operations on the caller thread) when
setting the async-pool thread count to 0 (as happens when setting
GEGL_THREADS=1, per the previous commit,) and not only when
shutting GIMP down. Otherwise, pending asynchronous operations
can "get lost" when setting GEGL_THREADS to 1.
Additionally, in gimp_gegl_init(), initialize gimp-parallel before
before connecting to GimpGeglConfig's "notify::num-processors"
signal, so that the number of async threads is set *before*
GEGL_THREADS, in order to avoid setting GEGL_THREADS to 1 while
async operations are still executing.
Also, allow setting the number of gimp-parallel-distribute threads
while a gimp-parallel-distribute function is running (which can
happen if gimp-parallel-distribute is used in an async operation,
as is the case for histogram calculation), by waiting for the
parallel-distribute function to finish before setting the number of
threads.
When GEGL_THREADS=1, concurrent access to the same buffer is not
safe, which can result in errors if asynchronous operations are
allowed to run in parallel to the main thread (see
https://gitlab.gnome.org/GNOME/gimp/issues/1721#note_265898.)
Disable parallel execution of asynchronous operations when
GEGL_THREADS=1 for now, to fix this. Ultimately, GEGL should be
able to remain thread-safe even when GEGL_THREADS=1. Note that we
want to execute asynchronous operations on a separate thread even
when GEGL_THREADS=1, since the goal here is mainly to avoid
blocking the main thread during their execution, rather than
speeding their execution up (in particular, it's benecifical to run
asynchronous operations in parallel even on a single-core machine,
while parallelizing GEGL operations generally isn't.)
To be used in the next commit.
I keep the GUI and core changes well separated in different commits so
that the core is easy to cherry-pick even though I will have to have
different GUI code.
GIMP_APP_VERSION does not include the micro version.
Also make version comparison with org.gimp.GIMP mandatory to force good
practice. This way, extension makers will have to advertize the version
of GIMP it works for, which will make a sane ecosystem of working
extensions only (hopefully!).
... (valgrind reports Invalid read)
Add gimp_babl_is_valid(), which takes a GimpImageBaseType and a
GimpPrecision, and determines whether the image-type/precision
combination is valid. Use this function to validate that loaded
XCFs use a valid type/precision combination, before trying to
create the image. Otherwise, we get a CRITICAL, and eventually a
segfault, when the combination is invalid.
Use the same function to validate the arguments of
gimp_image_new().
Introduce GIMP_PATTERN_MAX_SIZE (10000) and GIMP_PATTERN_MAX_NAME (256)
and validate pattern dimensions and pattern name length against them.
Add GIMP_BRUSH_MAX_NAME and validate that too.
Also make sure that the names are properly terminated, and some
cleanup.
In gimp_color_profile_new_from_icc_profile() and
gimp_image_validate_icc_profile(), don't raise a critical when
encountering an empty profile, but rather reject it gracefully with
an error.
... (Invalid read reported by valgrind)
In gimp_image_parasite_validate(), don't segfault when validating
a "gimp-comment" parasite of size 0 (i.e., whose data is a 0-byte
array, not an empty string), and just consider it invalid.
I realize that g_file_has_parent() is only looking for the direct
parent. I want to look for any ancester, so I loop through parents (with
a limit to avoid infinite looping with symlinks and such).
This is quite important since it will allow extensions to support only
some versions of GIMP (not trying to run a plug-in made for an
older/newer libgimp API for instance, or using new features, etc.).
It will also allow extensions to add dependencies to other extensions.
Extensions work for brushes, dynamics, MyPaint brushes, patterns,
gradients, palettes and tool presets.
More to come, but this is a first and working proof-of-concept.
Right now it only loads AppStream data, which is completely useless, yet
is a base of a managed extension system. Having proper metadata is what
will allow to actually know what is installed.
This is only the first draft.
Note that I am not adding the extension path into GimpCoreConfig on
purpose, since the point is not to have people manage their extension
directories manually anymore.
The extensions will be loaded from the build-time system path or the
config directory, and that's all.
What will probably be stored in the config though will be the remote
repositories URLs (allowing third-party extension repositories).
According to some bug reports, it seems that under some (unknown)
conditions we might save an empty custom gradient file on exit (for
equally unknown reasons). The only difference in the way we save
internal data files, such as the custom gradient, compared to
gimp_data_save(), is the fact that we currently don't explicitly
close the output stream, but rather only unref it.
The output stream should be implicitly closed (and hence flushed)
upon destruction, but maybe the unreffing is not enough to
guarantee that it's actually destroyed (maybe it spawns an extra
reference for some reason, who knows.) Anyway, let's just
explicitly close it, which also gives us a chance to catch and
report any errors occursing during flushing/closing (which,
altenatively, might be the culprit).
Additionally, a few more error-reporting improvements, to match
gimp_data_save().
Add an "async" field to the dashboard's "misc" group, showing the
number of async operations currently in the "running" state (i.e.,
all those GimpAsync objects for which gimp_async_finish[_full]() or
gimp_async_abort() haven't been called yet).
Preview generation for layer groups is more expensive than for
other types of drawables, mostly since we can't currently generate
layer-group previews asynchronously. Add a preferences option for
enabling layer-group previews separately from the rest of the
layer/channel previews; both of these options are enabled by
default. This can be desirable regardless of performance
considerations, since it makes layer groups easily distinguishable
from ordinary layers.
... which is an asynchronous version of
gimp_drawable_get_sub_preview().
We currently support async preview generation for drawables whose
buffer isn't backed by a GimpTileHandlerValidate tile handler
(i.e., anything other than group layers), since preview generation
fir such drawables may involve processing the corresponding graph,
which isn't thread-safe.
When the GIMP_NO_ASYNC_DRAWABLE_PREVIEWS environment variable is
defined, all drawable previews are synchronously generated.
Remove the "independent" parameter of gimp_parallel_run_async(),
and have the function always execute the passed callback in the
shared async thread-pool.
Add a new gimp_parallel_run_async_full() function, taking, in
addition to a callback and a data pointer:
- A priority value, controlling the priority of the callback in
the async thread-pool queue. 0 is the default priority (used
by gimp_parallel_run_async()), negative values have higher
priority, and positive values have lower priority.
- A destructor function for the data pointer. This function is
called to free the user data in case the async operation is
canceled before execution of the callback function begins, and
the operation is dropped from the queue and aborted without
executing the callback. Note that if the callback *is*
executed, the destructor is *not* used -- it's the callback's
responsibility to free/recycle the user data.
Add a separate gimp_parallel_run_async_independent() function,
taking the same parameters, and executing the passed callback in
an independent thread, rather than the thread pool. This function
doesn't take a priority value or a destructor (and there's no
corresponding "_full()" variant that does), since they're pointless
for independent threads.
Adapt the rest of the code to the changes.
and remove all other tool options parent setting/unsetting and
property copying code. Also select a tool at the end of
tool_manager_init() so it is in sync with what the tool options
manager does.
In gimp_data_factory_finalize(), wait on the factory's async set
after canceling it, and before continuing destruction. It's not
generally safe to just abandon an async op without waiting on it
-- this is a font-specific hack, due to the fact we can't actually
cancel font loading, and GimpFontFactory is prepared to handle
this.
Instead, in gimp_font_factory_finalize(), cancel and clear the
async set, so that GimpDataFactory doesn't actually wait for
loading to finish.
In gimp_font_factory_load_async_callback(), don't try to acess the
factory when the operation is canceled, since cancelation means the
factory is already dead. On the other hand, when the opeation
isn't canceled, make sure to thaw the container even when font
loading failed, so that we always match the freeze at the begining
of the operation.
GIMP_BRUSH_MAX_SIZE was already defined (as 10.000 pixels per dimension,
which is big for a brush) in gimpbrush.h. Let's just use this to
validate the size returned by the header.
The flag `free_selection_string` is used to track an array of strings
with some of them being static and others allocated. This should have
been an array of boolean but we can't change it because it is public API
(though it should really not have been!).
So let's just allocate every string of the `selection` array instead,
which makes the boolean flag useless now.
(cherry picked from commit b585201e5e)
We should not have essential signal connections (such as setting tool
options from brush properties) implemented in the tool options GUI
files, because they are not active until the options GUI is created.
Also, that magic is simply too hidden in the options GUI files.
Move the signal connections and the brush property copying code to
gimppaintoptions.c where is can also be done cleaner.
However, this must only be done for the main tool options instance
that is used for the GUI. Therefore, add a "gui_mode" boolean to
GimpToolOptions and set it to TRUE for all main tool options.
(this is ugly, but much less ugly and much less hidden than all the
places where code lives (like tool_manager.c) that can now be moved
into GimpToolOptions and its subclasses, and implemented cleanly
there).
Use gimp_input_data_stream_read_line_always(), instead of
g_input_data_stream_read_line(), in a bunch of places that don't
expect EOF. If we don't do that, the code assumes the GError
parameter is set by the function and returns an error indication,
causing the caller to segfault when it tries to access
error->message. Instead, we now process an empty line when EOF is
reached, which is caught by the normal parsing logic.
Additionally:
- Use gimp_ascii_strto[id]() when loading gradients, generated
brushes, and palettes, to improve error checking for invalid
numeric input.
- Improve gradient-segment endpoint consistency check.
- Allow loading palette files with 0 colors. They can be created
during the session, so we might as well successfully load them.
... which are similar to g_ascii_strtoll() (except that
gimp_ascii_strtoi returns a gint, and not a gint64), and
g_ascii_strtod(), however, they make error checking simpler, by
returning a boolean value, indicating whether the conversion was
successful (taking both conversion and range errors into account),
and return the actual value through a pointer.
... which is a drop-in replacement for
g_data_input_stream_read_line(), however, it always returns a non-
NULL value when there's no error. If the end-of-file is reached,
an empty string is returned.
gimp_layer_create_mask(): make sure we don't do a gamma conversion
when initializing the mask from a channel. This was probably not the
last place to need this fix.
Also get rid of a second switch(add_mask_type), must be some leftover
from long gone logic.
... with a color profile other than the gimp built-in.
Remove the separate alpha-channel copy in
gimp_image_color_profile_srgb_to_pixel(). We no longer need it,
since GimpColorTransform already takes care of that itself.
We used babl_process() to copy the alpha, which would also
transform the input color from R'G'B' to the output color space.
When the same buffer was used for both input and output, this call
would overwrite the input to the subsequent
gimp_color_transform_process_pixels() call; when the output color
space was different than R'G'B', this meant we'd pass the input to
gimp_color_transform_process_pixels() in the wrong color space,
producing wrong results. This was the case when converting the
foreground color for use with the smudge tool.
...after program restart
GimpContext was always supposed to keep the names of objects (brush,
pattern, font etc.) around even if these objects don't exist, for
cases like refreshing the data in a GimpDataFactory (which worked
fine), but also for deserializing the names of objects which don't
exist *yet* (delayed loading, no-data or whatever).
This commit fixes the delayed loading case (particularly affects fonts):
gimp_context_deserialize_property(): always keep the name of the
object around when it is not found, not only in the no-data case.
gimp_context_copy_property(): always copy the object *and* its name to
the dest context.
Add GimpConfig::duplicate() and ::copy() implementations which chain
up for duplicating/copying all properties and additionally copy all
object names to the new/dest context.
It seems it was simply forgotten. PROP_MASK_ALL is used at some very
central places, so this commit might fix a few subtle bugs, or
introduce new ones, everybody look for strange tool preset behavior
please :)
Never add an alpha channel in gimp_layer_real_transform(), it was only
added because our pre-GEGL transform code was shit. This is quite the
opposite of what the bug reporter asked for, but IMO the more correct
fix.
This will NOT go to 2.10 because it changes behavior.
