In the threshold and levels tools, use gimp_wait() to wait for
histogram calculation to complete before applying auto-adjustment,
so that a message is displayed in the meantime. Allow the
operation to be canceled, in which case we simply abort the auto-
adjustment, but let the histogram calculation continue.
Use gimp_fonts_wait(), added in the previous commit, to wait for
fonts to finish loading before operations that depend on font
availability. In particular, this includes font- and text-related
PDB functions, and text-layer rendering.
... which waits for font-loading to complete, using gimp_wait() to
show a message while waiting.
Note that we don't currently allow the wait to be canceled, since
it may have unpredictable effects on plug-ins, but the interface is
such that the wait might not complete, so calling code should be
prepared for that.
Add a GimpGui::wait() virtual function, and a corresponding
gimp_wait() function. The function takes an object implementing
the GimpWaitable interface, and a printf-style message, and waits
for the object to become ready, displaying the message as
indication in the meantime. The default implementation simply
prints the message to STDERR.
Implement the function in gui-vtable, using the busy-dialog plug-
in added in the previous commit, to display the message in a
dialog. Additionally, if the object implements the GimpCancelable
interface, provide a "cancel" button in the dialog, which, when
pressed, causes gimp_cancelable_cancel() to be called on the
object. Note that the function keeps waiting on the object even
after requesting cancelation; GimpTriviallyCancelableWaitable can
be used to stop the wait once cancelation has been requested.
Pass the current icon theme directory to plug-ins through the
config message, and add a gimp_icon_theme_dir() libgimp function
for retrieving it. Note that we already have a similar
gimp_icon_get_theme_dir() PDB function, which we keep around, since
it can be used to dynamically query for the current icon dir,
unlike the former, and since it returns a dynamically-allocated
string, while the rest of the config-related functions return
statically allocated strings.
Use the new function, instead of gimp_get_icon_theme_dir(), in
gimp_ui_init(). This allows gimp_ui_init() to run without making
any PDB calls. Consequently, this allows us to start plug-ins that
call gimp_ui_init() without entering the main loop in the main app.
We're going to add a plug-in that displays an interactive dialog
while the main app is blocking waiting for an operation to
complete, and we need to be able to start the plug-in without
entering the main loop, to avoid the possibility of arbitrary code
being executed during the wait.
Bump the protocol version.
While fonts are loading, show a GimpBusyBox with an appropriate
message above the text tool options, and make the options
themselves insensitive, and in font views, such as in the fonts
dialog (through gimp_container_editor_bind_to_async_set(), added in
the previous commit).
... which takes a GimpAsyncSet and a message, and shows a
GimpBusyBox with that message, instead of the container view, while
the async set is nonempty.
We're going to use this for font-loading indication in font views,
such as in the fonts dialog.
We already avoid rendering text layers while fonts are loading,
showing an appropriate message, but this will soon be replaced with
waiting for the fonts to finish loading.
Instead, don't allow the text tool to start at all while the fonts
are loading (showing an appropriate tool message, and a BAD cursor
modifier), and halt the tool when reloading fonts if it's already
started.
Replace the boolean fonts_loading member of Gimp with
fonts_async_set, which is a GimpAsyncSet object. This allows us
to easily respond to the completion of font loading and reloading,
as will be done in the next commits.
Additionally, move the call to FcConfigSetCurrent(), used to
activate the loaded font configuration, from the async thread to
the main thread, just to be on the safe side, and avoid calling
FcInitReinitialize() in gimp_fonts_reset() if font loading is still
in progress, which is unsafe.
GimpAsyncSet represents a dynamic set of running GimpAsync objects.
The objects are automatically removed from the set once they're
synced.
GimpAsyncSet implements the GimpWaitable and GimpCancelable
interfaces, allowing the entire set to be waited-on or canceled.
Additionally, GimpAsyncSet provides an "empty" property, which
indicates whether the set is empty or not. This allows responding
to the completion of all the GimpAsync objects through the set's
"notify::empty" signal, or drive UI changes through property
bindings.
GimpTriviallyCancelableWaitable is a proxy object for another
GimpWaitable object, implementing both the GimpWaitable interface
and the GimpCancelable interface. While waiting on the proxy
simply waits on the underlying waitable, canceling the proxy
doesn't affect the underlying waitable, even if it implements
the GimpCancelable interface as well, but rather causes subsequent
wait operations on the proxy to successfully complete immediately.
This essentially causes cancelation to abort only the wait, rather
than the underlying operation.
GimpUncancelableWaitable is a simple proxy object for another
GimpWaitable object, implementing only the GimpWaitable interface.
Its main purpose is to mask away the cancelability of an object
implementing both GimpWaitable and GimpCancelable.
In gimp_parallel_run_async(), lower the priority of threads
executing independent async operations. Independent operations
are generally potentially long-standing background tasks, which we
don't want to bog down the rest of the program.
This is currently only implemented on Linux and Windows.
The GIMP version string and the backtrace both look completely wrong
when pasted as-is on gitlab. Somehow all linefeed are gone.
This can be fixed by surrounding these as blocks (triple backticks),
using markdown syntax. Of course now the debug content is not for our
tracker only since packagers are encouraged to replace with their own
tracker URL, but this small markdown syntax is simple enough that it
should not break formatting on other platforms (as far as I know).
(cherry picked from commit d4ff504735)
In gimp_parallel_run_async(), when aborting a GimpAsync operation
in reponse to its "cancel" signal, properly clean up internal data
attached to the object, to avoid use-after-free if the signal is
emitted again.
(cherry picked from commit 3fa4c01bcf)
Add a boolean "independent" parameter to gimp_parallel_run_async().
When FALSE, the passed function is run in the shared async thread
pool; when TRUE, the passed function is run in an independent
thread.
Generally, async operations should run in the async pool, however,
it might be desirable to run long-standing operations, especially
ones that can't be canceled, in independent threads. This avoids
stalling quicker operations, and shutdown.
Adapt the rest of the code for the change. In particular,
initialize the font cache in an independent thread.
(cherry picked from commit ad8add6808)
In gimp_parallel_run_async(), connect to the returned GimpAsync's
"cancel" signal, and abort the operation in response if it's still
enqueued, i.e., if its execution hasn't started yet.
(cherry picked from commit 3958ffbe50)
Have GimpAsync implement the GimpWaitable and GimpCancelable
interfaces, added in the previous two commits, instead of providing
its own public version of the corresponding functions.
Add gimp_async_cancel_and_wait() as a convenience function for both
canceling an async operation, and waiting for it to complete.
Adapt the rest of the code to the change.
(cherry picked from commit e2c56ef407)
Fonts should not be blocking startup as this provides a very bad
experience when people have a lot of fonts. This was experienced even
more on Windows where loading time are often excessively long.
We were already running font loading in a thread, yet were still
blocking startup (thread was only so that the loading status GUI could
get updated as a feedback). Now we will only start loading and proceed
directly to next steps.
While fonts are not loaded, the text tool will not be usable, yet all
other activities can be performed.
(cherry picked from commit 2484dec7d5)
Improve the formalism of a GimpAsync object being "sycned"
(previously referred to as the main thread being "synced" with the
async thread), by both providing a gimp_async_is_synced() function,
separate from gimp_async_is_stopped(), and by improving the type
and function descriptions.
Make sure all previously added callbacks have been called after a
call to gimp_async_wait[_until](), even if these functions are
called from within a callback.
(cherry picked from commit a901c3c1f4)
Replace the custom threading code with a call to
gimp_parallel_run_async(). This simplifies the code, while
maintaining the current (blocking) behavior. In the future, we
might build upon this to actually load the fonts in the background.
(cherry picked from commit 3e92e7a449)
... which is similar to gimp_async_wait(), taking an 'end_time'
parameter, controlling how long to wait for the async operation to
complete.
(cherry picked from commit 68c57548fd)
The default stack size for new threads on MacOS is 512 KiB, making
our 512 KiB limit for stack-allocated buffers in
gimp_operation_layer_mode_real_process() too high. Lower it to
256 KiB.
(cherry picked from commit 367399e5c0)
In gimp_drawable_gradient(), pass the undo description ("Gradient")
to gimp_gegl_apply_operation(), so that its displayed as the
progress text while rendering the gradient, even when applying a
shaped gradient, in which case we end the progress after
calculating the distance map, causing the progress text be NULL
during rendering unless explicitly set.
(cherry picked from commit deee2f14f5)
... which is a drop-in replacement for gegl_buffer_copy(),
parallelizing the copy operation when the source and destination
formats are different, requiring a conversion.
(cherry picked from commit fded25b38c)
and remove all clipping hacks for drawing the canvas background, turns
out they never worked and we were relying on the pattern set on the
window, gah! This optimizes away one entire step of drawing of image
size, for each expose...
The zoom focus discussion on IRC suggests that everybody is annoyed
about centering behavior (or lack thereof), so here is a way to
explicitly center the image witout zooming.
... _even at low zoom levels_
Pass GIMP_ZOOM_FOCUS_POINTER to gimp_display_shell_scale() when
wheel-scrolling, and change the scaling code to really honor
GIMP_ZOOM_FOCUS_POINTER and not apply magic image centering.
This keep the same point centered under the mouse for wheel-scrolling
and the zoom tool (== when the zooming is really triggered at a
certain mouse position).
Implement GimpPickable::get_pixel_average(), added in the previous
commit, in GimpDrawable, GimpImage, and GimpProjection, using
gimp_gegl_average_color(), added in the commit before last. This
is significantly faster than the default implementation.
... which calculates the average color of the pickable over a given
area. Use this function in gimp_pickable_pick_color() when
sample_average is TRUE, and provide a default implementation which
calculates the average color using GimpPickable::get_pixel_at(), as
gimp_pickable_pick_color() did before.
The default implementation is rather slow; classes that implement
the GimpPickable interface can provide a faster specialized version
(see the next commit).
GimpDeviceInfo is a GimpToolPreset now, and using the user context
would cause the device info selected in the dialog to be set globally
as tool preset. Use a temporary context instead, the container view
just needs it because.
gimp_airbrush_stamp(), which is called during the airbrush timeout
to periodically stamp the airbrush, assumes the saved symmetry's
origin is the same as during the original call to
gimp_airbrush_paint(). However, since commit
bc09c71818, we clear the symmetry's
origin at the end of gimp_paint_core_paint(), resulting in an empty
symmetry with a NULL origin during gimp_airbrush_stamp(). As a
result, no dab is painted, and we segfault if there's active
dynamics.
Fix this by saving the symmetry's origin coords during
gimp_airbrush_paint(), and restoring them in gimp_airbrush_stamp().
We rely on GimpDeviceInfo's tool-options being non-NULL, so make sure
this is the case and fix NULL options with the current tool's options
after loading devicerc.
GimpDeviceInfo is the only way to store per-device settings like
color, brush etc. It used to be derived from GimpContext and therefore
limited to the context's properties, causing everything else (all
tool-individual options) to be lost on device change.
Derive it from GimpToolPreset instead, so it's capable of storing
arbitrary tool options.
Adapt things to the new class hierarchy and add a bunch of signal
handlers that make sure the active device's GimpDeviceInfo is updated
properly when the tool changes. Also change device switching
accordingly.
Change GimpDeviceStatus to only show the stuff that is relevant to
each device's tool.
And various small changes to make things work properly...
...like gaussian blur in indexed mode
In GimpDrawable's source node, after the filter stack, insert a node
that converts the pixels back to the drawable's format if the drawable
is indexed.
...in weird ways
In gimp_rectangle_tool_button_press(), when the press triggers a
GimpEditSelectionTool operation (like moving a floating selection),
make sure we don't call COMMIT on a zero-size rectangle, because that
would get special treatment by commit(). Instead, CANCEL a zero-size
rectangle so stuff behaves as if it has never been there.
In gimp_pickable_contiguous_region_by_seed(), use gegl_buffer_get()
to sample the mask buffer, instead of using a sampler. The sampler
is created at the beginning of the operation, and is subsequently
used after modifying the mask buffer, which should be avoided,
since the sampler may return outdated cached data.
We get a lot of packaging bugs for third-party builds in our tracker,
especially since our debug tool now opens directly our bug tracker.
It would be preferred if third-party packagers were to make a first
filtering of bugs and only reported once they knew for sure that the bug
is in core code, and not in packaging.
Our configure script will now propose a --with-bug-report-url option
allowing to set a different web address. This address will be the one
opening in the debug dialog.
... instead of gegl_buffer_sample()
GEGL commit 26f13cbfe9aaaa8c176162e54fdbb8af6876538e got rid of the
per-buffer cached samplers, making gegl_buffer_sample() much more
expensive, suitable primarily for one-off samples.
Use a sampler object instead.
That was an obvious faux ami with French.
Status changed to "press Enter to refine", with the requirement of
staying not too long (this is a status message, not documentation), and
that also announces what will be the next step.
In gimp_histogram_editor_validate(), clear the update timeout, so
that if we validate the histogram before the timeout (in
particular, if editor->histogram itself is clear) we don't end up
unnecessarily re-validating it at the timeout.
... which provides a set of higher-level lock-free atomic
operations.
Currently, the only two operations are
gimp_atomic_slist_push_head() and gimp_atomic_slist_pop_head(),
which atomically push/pop the first element of a GSList.
Code cleanup.
Improve function descriptions.
Improve function precondition checks.
Make sure the main thread is properly synced with the async thread
before calling the completion callbacks in the idle function.
Guarantee that all callbacks are called in FIFO order, even
callbacks added during the execution of other callbacks.
It is not obvious that you have to hit Enter to validate the first step
of Foreground Selection tool, i.e. the rough free selection. Adding this
information in status message.
Since Enter would only work once the free selection is closed or that
you have at least 3 points (then Enter closes the selection for you), I
also add gimp_free_select_tool_get_n_points() so that we can know how
many points were created from the Foreground Select tool, and only
update the status message when relevant.
In GimpHistogramView, connect the view to the bg-histogram's
"notify" signal, and update the view in response. Previously, we
only updated the view in response to the main histogram's "notify"
signal.
The curves tool only uses the bg-histogram, and after the switch to
asynchronous histogram calculation, in commit
49382e53d5, it failed to update the
histogram view after the calculation was complete.
In GimpCurvesTool, calculate the histogram of the target drawable
asynchronously, rather than synchronously, to avoid blocking the UI
while the histogram is calculated.
In GimpLevelsTool, calculate the histogram of the target drawable
asynchronously, rather than synchronously, to avoid blocking the UI
while the histogram is calculated.
In GimpThresholdTool, calculate the histogram of the target
drawable asynchronously, rather than synchronously, to avoid
blocking the UI while the histogram is calculated.
Additionally, fix the initial "low" threshold value for > 8bpc
images (127/255 => 0.5).
In GimpHistogramEditor, calculate the histogram of the active
drawable asynchronously, rather than synchronously, to avoid
blocking the UI (and the execution of PDB calls) while the
histogram is calculated. For large images, this can notably
improve responsiveness, as well as plug-in execution speed, while
the histogram dialog is mapped.
... which is similar to gimp_drawable_calculate_histogram(),
calculating the histogram asynchronously on a separate thread.
Note that when calculating the histogram of the drawable's source
node, the node is rendered synchronously on the main thread (if
necessary), and the histogram of the result is calculated
asynchronously on a separate thread.
... which runs a user-provided function asynchronously, returning a
corresponding GimpAsync object. This can be used to execute code
off the main thread, using the GimpAsync object to synchronize as
necessary.
Note that while the code allows for running multiple asynchronous
functions in parallel in a thread pool, we currently limit the pool
to a single thread, queueing overlapping async function, since we
have no use for parallel asynchronous functions at the moment.
GimpAsync represents an asynchronous task, running without blocking
the main thread. It provides functions that allow waiting for
completion, queueing completion callbacks, and requesting
calcelation. See the code for more details.
The interface and the implementation are intentionally limited for
the time being, to keep things simple, and are geared toward
asynchronous tasks executed on a separate thread (see the next
commit). The public interface is relatively general, however, and
we may extend the implementation to support other kinds of tasks at
some point, who knows...
To avoid an infinite loop, gimp_item_linked_is_locked() was temporarily
unlinking items before calling gimp_item_is_position_locked(). This
worked only based on gimp_item_real_is_position_locked() code which
called gimp_item_linked_is_locked() only when it was linked. It did not
take into account the fact that it was an abstract method which could
have other implementations. In particular the group layer implementation
would call in turn gimp_item_is_position_locked() on each child layer.
Basically temporarily unsetting the link was anyway a very ugly hack.
The point is simply that we are only interested by the value of the
`lock_position` flag for this item, without further "intelligence". For
this, use gimp_item_get_lock_position() instead.
Displays can be opened multiple times, which caused the device manager
to try to add their devices multiple times (which gets prevented with
warnings), and then remove the devices prematurely when the
multiple-opened display gets closed the first time (which is even
worse).
Add a simple hash that keeps track of how often displays are open, and
only add/remove their devices on first open and last close.
This actually happened with gtk-inspector on the gtk3-port branch, but
there is no reason this can't also happen in stable.
... directory in the configuration folder.
This subfolder should have been created already. Just in case it is not
(for any reason), just recreate it rather than popping up an error.
In the gradient-tool editor, when responding to an endpoint size-
entry "changed" signal, make sure there is a selected handle, and
bail otherwise. We can receive a "changed" signal without a
selected handle, if the endpoint is deselected while a change to
the size-entry's value is in progress, causing the the tool GUI to
be unmapped, and the changed value to be committed.
When in anti-erase mode, always return TRUE, since it's only
effective when the drawable has an alpha channel.
Also, remove an unnecessary #include in gimp_ink_tool.c.
... closing one of them crashes GIMP
GimpSymmetry keeps a strong reference to the drawable passed to
gimp_symmetry_set_origin() until the next call to set_origin(), or
until it's destroyed. This can unnecessarily extend the lifetime
of the drawable. In particular, it can extend the lifetime of a
floating selection past the destruction of the image mask, during
image destruction, which results in a NULL mask pointer in
gimp_layer_invalidate_boundary(), called when detaching the
floating selection as part of its destructor.
Add gimp_symmetry_clear_origin(), which clears the origin set using
gimp_symmetry_set_origin(), dropping the reference to the drawable,
and call it in gimp_paint_core_paint() after painting. This avoids
extending the lifetime of the drawable, and fixes the bug.
In gimp_drawable_calculate_histogram(), when including the
drawable's filters in the histogram (i.e., when calculating the
histogram of the drawable's source node, rather than its buffer),
if the drawable is a projectable, call
gimp_projectable_{begin,end}_render() before/after calculating the
histogram, respectively. This is necessary for pass-through
groups, whose {begin,end}_render() functions disconnect/reconnect
the group's backdrop from/to the group's layer stack. If we fail
to do this, the backdrop is erroneously included in the histogram.
Note that currently layer groups can't have any filters applied to
them, so we never run into this situation, but once we have non-
destructive editing we probably will.
Add a GimpPaintTool::is_alpha_only() virtual function, which
subclasses can override to indicate whether painting only affects
the alpha channel (assuming FALSE by default). Override the
function in Gimp{PaintBrush,Ink,Clone}Tool, returning TRUE when the
current paint mode only affects the alpha (as per
gimp_layer_mode_is_alpha_only(); see the previous commit,) and in
GimpEraserTool, returning TRUE when the target drawable has an
alpha channel.
When the function returns TRUE, and the target drawable doesn't
have an alpha channel, or the alpha channel is locked, show a BAD
cursor modifier, and raise an appropriate warning when attempting
to paint.
... which determines if a layer mode's blend function only affects
the alpha, maintaining the backdrop's color. This is currently
true only for ERASE, SPLIT, and ANTI_ERASE modes.
When the operation doesn't have an input pad, composite it over the
input using gimp:normal, instead of gegl:over, for consistency with
the rest of our compositing code (although the result should be the
same).
... drifts/sharpens when applying additional gradients
In GimpDrawableFilter, don't use gegl:over for operations without
an input pad, since this only works if the filter's mode is
REPLACE. If the filter's mode is different, in particular, if it's
NORMAL, we end up compositing the operation's output against the
input twice: once in gegl:over, and again in the filter's
applicator. Notably, this happens for the gradient tool.
Instead, revert commit 5b80d3d3be
(with some additions to avoid constructing unnecessary nodes when
the operation has no input) and simply change the applicator's mode
to NORMAL if the oepration doesn't have an input, and the filter's
mode is REPLACE.
so plug-ins cannot thaw what they haven't frozen, and the code
can clean up frozen stuff left behind by crashed or broken plug-ins.
Also redo the cleanup code so it only keeps track of the undo group
counts and freeze counts of what *this* GimpPlugInProcFrame did
itself. That should make it even stricter against broken code that
could mess up internals.
Blend them on top of the source buffer using gegl:over like
GimpDrawableFilter does interactively. Fixes "Repeat Last" and
probably some other stuff for source OPs.
gimp_histogram_view_draw_spike(): we were shifting the bg_histogram to
the right by accidentially offsetting the 'i' variable, use a local
variable for the loop instead.
These procedures freeze/thaw the corresponding containers of the
image, allowing plug-ins that perform many changes affecting any of
these containers to suppress updates to the corresponding dialogs,
significantly improving performance.
... while the container is frozen
In GimpContainerView, do nothing in response to a
gimp_container_view_{select,activate,context}_item() call while the
view's container is frozen. While the container is frozen the view
is empty, and these functions can segfault.
It is not used anywhere anymore and can be replaced by the more powerful
gimp_stack_trace_print() (which can also allocate a string containing
the backtrace, hence is a proper replacement call).
g_alloca() is not very advisable, especially when it might be used to
allocate a big chunk of memory at once. It is better to allocate dynamic
memory with malloc(), or in particular with g_try_malloc() which won't
abort the program on failure.
This might be slightly slower (one of the advantages of memory on the
stack, though not even an absolute truth) but probably not by much, if
at all, and it's better to be safe anyway.
Still looking for possible crash case during gimp_font_list_load_names()
(cf. bug 795650). g_utf8_validate() segfaults if called with NULL.
Though looking at pango_font_description_to_string() implementation, it
doesn't look like it would ever return NULL. Yet it is worth
double-checking. We don't load font every second anyway.
Add more assertion checks (similar to commit d094ab7e56).
This is still not a fix per-se, but at least would make the code a bit
more robust. In particular FcObjectSetDestroy() could crash if somehow
FcObjectSetBuild() had returned a NULL pointer. And obviously
dereferencing a NULL fontset would crash as well.
Now if any of these happened, no fonts would be loaded. But at least
GIMP would not crash.
After the switch of random-seed properties from INT to UINT, their
upper bound results in a negative value when converted to a
gint32, causing a CRITICAL in the call to g_random_int_range().
Use g_random_double_range() instead, which has enough precision to
accurately represent all values in the range, and round the result.
Remove the log handlers registered in errors_init(), in
errors_exit(), and call errors_exit() before destroying the Gimp
instance, since the log handlers depend on it. This avoids
segfaulting if a message is logged after destroying the Gimp
instance.
In GimpTextTool, when starting the editor in response to a button
press, always set the text tool's image first, so that the style
editor picks the correct resolution for the text-size entry.
Currently the image is only set when clicking inside the active
drawable's bounds.
Some crash happens inside this function (cf. bug 795650) on Windows.
Because of very inaccurate trace on this OS, it is hard to determine the
exact issue. Let's at least add some basic check on function parameters.
This won't fix any core issue, but may make things a bit more robust and
bugs easier to detect.
As proposed on IRC. This will allow people to debug their fonts (for
instance when there are permission issues or whatnot) by knowing the
list of problematic fonts in an error dialog at startup (and not only on
terminal).
This is for Windows where we apparently need to call
g_win32_locale_filename_from_utf8() before feeding the path to
FcConfigAppFontAddFile().
Unfortunately as we discovered earlier (cf. commit ba06a0fe86), this
can sometimes return NULL. So we absolutely need to check for the path
not being NULL first.
This GError will either specify the font which failed to load (if
unique) or a vague message about fonts which fail to load.
Unfortunately right now, this message also goes to terminal because the
GUI is not ready yet.
Do not use FcConfigAppFontAddDir() because it seems to be half-loading
some fonts, even when they are not actually readable. On the other hand,
FcConfigAppFontAddFile() properly fails and does not leave the font list
in unstable state.
It doesn't look like it is actually much of a problem, but anyway it's
better to have non-absolute paths in config files when possible.
Thanks to jtojnar on IRC for reporting these (and MyPaint brush paths
from my previous commit).
Absolute paths not to be used in $XDG_CONFIG/GIMP/{version}/tags.xml.
These are actually only an identifier, and not used as a path at all
anyway. Moreover MyPaint brushes even generate checksum (which allows
to remap the GimpData appropriately even if the paths are changing).
But anyway it's better not to have absolute paths when we can prevent
so.
gimp_rectangle_select_tool_cursor_update(): always set a cursor and
cursor modifier even if no GimpToolRectangle widget exists, so we are
not stuck with the last set cursor after committing or halting the
tool.
Symmetric conical gradients only span half a revolution (unlike
assymetric ones, which span an entire revolution), and therefore
require only half the cache size.
Add a crop node to the GimpDrawableFilter graph, applied after the
filter's output, cropping the output to the filter area (the same
area used for the input crop node). If we fail to do this, filters
whose op's bounding box is bigger than the input region can affect
areas outside the drawable, when the filter is rendered as part of
the image graph (in contrast to being comitted). This is
particularly relevant to source ops, that may have an infinite
bounding box.
We probably didn't notice this until now, since before the recent
GimpProjection update-area changes, only the drawable's area would
get invalidated in response to changes in the filter, so regions
outside the drawable wouldn't normally get rendered. However, this
could still have been triggered by causing regions outside the
drawable to be invalidated by other means.
Add gimp_drawable_gradient_adjust_coords(), which adjusts the
gradient segment coords according to the gradient type, so that, in
cases where the gradient span is unrelated to the segment length,
the gradient cache (in GimpOperationGradient) is big enough not to
produce banding. Use the new function in gimp_drawable_gradient()
and in the gradient tool, instead of duplicating the logic.
Move the shapreburst coordinate-adjustment logic to the new
function, and add appropriate logic for conical gradients.
Remove the code that avoids using the gradient cache for conical
gradients from GimpOperationGradient.
Undo the part of commit fa9a4108c3
that moved cache generation from process() to prepare(). prepare()
is called after each property change, in order to calculate the
op's bounding box for invalidation. Since we only need the cache
for actual processing, generating it in process() avoids that
overhead.
In the various types of fill operations, and in fade operations,
use the paint composite-mode of the current paint mode, which is
the composite mode we use during painting, instead of AUTO, which
results in the default mode we use for layer compositing. This
effectively means that filling using any non-legacy, non-
subtractive mode can paint over transparent areas, rather than
being limited to nontransparent areas.
Invalidate the gradient cache while setting relevant properties,
and validate it, reconstructing if necessary, during prepare(),
rather than process(), to avoid the need to use a mutex.
Make sure the cache has at least two elements, corresponding to
the initial and final colors of the gradient, since both colors
might be needed, and to avoid division by zero.
Avoid using a cache if its necessary size is too big, or if the
gradient type is conical, since the necessary cache size for
conical gradients is unrelated to the gradient line length.
Improve index rounding during cache lookup.
Lots of indentation fixes.
When merging a pass-through group, change its mode to NORMAL first,
to avoid a critical when duplicating the group as a regular layer.
Preserve the group's blend/composite space/mode while changing its
mode (note that only the composite space currently matters, since
the other parmaeters are immutable for pass-through groups.)
In gimp_text_tool_connect(), when auto-removing empty text layers upon
text layer or tool change, make sure we don't try to remove a layer
that has already been removed, which can happen if this function is
reached *because* somehow the layer was removed externally (e.g. by
the user in the layers dialog).
We were only able to translate selections and layers (bot not channels
and paths) via the PDB, this new procedure fixes that. Deprecation of
old API and some more transform consistency to follow...
it used to be "text" which is almost always too extreme; use "text_aa"
instead which is half way between "text" and "base" and always gives a
reasonable contrast.
... it doesn't exist.
The tmp/ dir in the config folder should already be created by GIMP, but
just in case it is not there, try and create it, since all code calling
these assumes that it exists.
... raises a CRITICAL
gimp_item_{start,end}_move() currently serves two different
purposes: It is used by GimpLayer to suspend/resume mask resizing
of the layer's ancestors; this is necessary whenever an operation
on a layer might affect the size of its ancestors. It is also used
by GimpGroupLayer to suspend/resume its own mask resizing; this, on
the other hand, is only necessary before applying one of the
transformation functions to the group, so that mask modification is
handled by GimpLayer. In other words, the effects of
gimp_item_{start,end}_move() on group layers are only necessary in
a subset of the cases in which these functions are used.
While in itself this isn't a problem, it does cause issues when
removing a group layer: gimp_image_remove_layer() calls
gimp_item_start_move() before removing the layer, and
gimp_item_end_move() afterwards. While the former function is
called while the layer is still attached to the image, the latter
function is called after the layer is no longer attached. Since
GimpGroupLayer pushes an undo step in response to these calls, only
the call to start_move() results in an undo step, while the call to
end_move() doesn't, resulting in an unbalanced
GIMP_UNDO_GROUP_LAYER_START_MOVE undo step on the stack. This
causes problems when undoing the operation.
Add gimp_item_{start,end}_transform() functions, and corresponding
GimpItem::{start,end}_transform() virtual functions, which are more
specialized versions of gimp_item_{start,end}_move(), which should
be used instead of the former before/after transforming an item; in
other cases, such as when removing ot reordering an item,
gimp_item_{start,end}_move() should still be used. The default
implementation of GimpItem::{start,end}_transform() calls
gimp_item_{start,end}_move(), respectively, so subclasses that
override these functions don't have to do that themselves.
In GimpGroupLayer, override GimpItem::{start,end}_transform(),
instead of GimpItem::{start,end}_move(), for the same purpose of
suspending mask resize. This avoids these functions from being
called when removing a layer group, fixing the bug.
In gimp_image_reorder_item(), call gimp_item_start/end_move()
before/after reordering the item (and use an undo group, so that
the resulting undo actions are grouped together with the reordering
undo action,) so that if the item is a child of a group layer, and
reordering moves it out of the group in a way that causes the
group's mask to be resized, the mask will be properly restored when
undoing the operation.
After Alexandre Prokoudine's insistent demand! :-)
I am still not sure how wise this is, since this should be really
considered a "developer-only" option. Basically these tools are really
too buggy and unstable and we should not shine too much light on these.
The counter-argument is that doing so will favor the bitrot.
Well ok. At least let's add a big warning message at the top of the
Playground page, to make it very clear (if that were not already the
case) that basically this is not to be considered a secret feature, but
really more a "we are looking for contributors" option.
In the gimp-channel-combine-masks PDB wrapper, only push an undo step
if the modified channel is attached to an image. It's a completely
reasonable use case to combine unattached channels.
... when using default aspect ratio
The call to
gimp_{crop,rectangle_select}_tool_update_option_defaults() when
starting the crop/rectangle-select tools seems to no longer be
necessary, while it overrides user modification to the default
ratio's landscape/portrait orinetation (which only really matters
for the crop tool, since the rectangle-select tools' default ratio
is 1:1). Fix this by simply removing the call.
The gimppaintcore-loops functions perform very little actual
computational work (in case of do_layer_blend(), at least for
simple blend modes), which makes the cost of buffer iteration, and
memory bandwidth, nonnegligible factors. Since these functions are
usually called in succession, acessing the same region of the same
buffers, using the same foramts, coalescing them into a single
function, which performs all the necessary processing in a single
step, can improve performance when these functions are the
bottleneck.
Add a gimp_paint_core_loops_process() function, which does just
that: it takes a set of algorithms to run, and a set of parameters,
and performs all of them in one go. The individual functions are
kept for convenience, but are merely wrappers around
gimp_paint_core_loops_process().
Be warned: the implementation uses unholy C++ from outer space, in
order to make this (sort of) managable. See the comments for more
details.
Commit ddfc7715cb changed the
airbrush periodic stamp behavior, so that instead of using the main
brush, it issued a full MOTION event, potentially using a different
brush when using a GIH brush.
Fix this, by renaming the "timeout" signal of GimpAirbrush to
"stamp", and by adding a new gimp_airbrush_stamp() function, which
should be used for painting the periodic airbrush dab in response,
instead of calling gimp_paint_core_paint() directly, and which
calls gimp_airbrush_paint() instead, as the old code did.
In order to call this function from the paint thread, we replace
the various gimp_paint_tool_paint_core_foo() functions, introduced
in the above commit, with a generic gimp_paint_tool_paint_push()
function, which takes a callback (and a data pointer) to run on the
paint thread, and queues it for execution (when not using the paint
thread, the function is called directly from the calling thread.)
...(Pixel, Position and Size, Alpha Channel) are enabled.
gimp_edit_selection_tool_translate(): add checks for locks and bail
out with a message if the item is NULL or locked.
gimp_move_tool_button_press(): refactor NULL and lock checks to look
more like the new code added above, and also check "lock-content" of
channels and masks: moving them changes their pixels.
GimpAirbrush currently performs painting and flushes the image on
its own during the airbrush timeout. This is unsafe w.r.t. the
paint thread, since the timeout is run on the main thread, while
paint commands should run on the paint thread.
Add a "timeout" signal to GimpAirbrush, and simply emit this signal
during the airbrush timeout, rather than actually painting.
Connect to this signal in GimpAirbrushTool, and use
gimppaintool-paint to perform the actual painting, in a thread-safe
manner (see the previous commit.)
We'd like subclasses of GimpPaintTool to be able to issue paint
commands to the tool's paint-core (in particular, see the next
commit.) Since paint commands should be executed on the paint
thread, the subclasses must not call the paint-core functions
directly, but should rather let gimppainttool-paint issue the
commands on their behalf.
Reorgainze gimppainttool-paint to make it usable for this purpose
by subclasses. In particular, add
gimp_paint_tool_paint_core_paint() and
gimp_paint_tool_paint_core_interpolate(), which call the
corresponding paint-core functions on the paint thread.
Additionally, rename the {start,end,flush}_paint() virtual
functions of GimpPaintTool to paint_{start,end,flush}(), and rename
gimp_paint_tool_is_painting() to gimp_paint_tool_paint_is_active(),
so that all the gimppainttool-paint-related stuff are grouped under
the same namespace.
Don't unconditionally overwrite all the proc's description, author
etc. Instead, try to preserve them and append the "Deprecated" notes
to the help texts and generated comments.
Only affects one procedure because we killed the meta info of all
other deprecated procs so far, but now we don't have to do that any
longer.
Add new PDB group "drawable_edit" which has all procedures from the
"edit" group which are not cut/copy/paste.
The new group's procedures don't have opacity, paint_mode
etc. arguments but take them from the context instead. Unlike the old
gimp-edit-fill, gimp-drawable-edit-fill now uses the context's opacity
and paint_mode.
The new gimp-drawable-edit-gradient-fill procedure uses even more
context properties which are also newly added with this commit
(gradient_color_space, gradient_repeat_mode, gradient_reverse).
And some cleanup in context.pdb.
This is still WIP, nothing in the edit group is depcreated yet.
Rename gimpblendtool{.c,-editor.{c,h}} to
gimpgradientool{.c,-editor.{c,h}}. Note that this commit only
renames the files; the actual changes are done in the next commit,
so that git doesn't consider them new files.
Rename gimpdrawable-blend.h to gimpdrawable-gradient.h. Note that
this commit only renames the file; the actual changes are done in
the next commit, so that git doesn't consider this a new file.
Partially revert commits 4f2e078ccb
and b0beb0197a, since the changes
they introduced to some of the renamed files were big enough for
git to consider them entirely new files, hence we lost their
history. The next few commits fix this.
This commit also partially or entirely undoes followup commits
5f6dfc7617,
c3f98cccbd,
6b0f5136e0,
and 3736bfd189, which will be
restored by the next few commits as well.
The upper text will be centered on the top quarter of the bottom quarter
of the splash image, whereas the bottom text will be centered on the
bottom quarter of the bottom quarter of the splash (unless the splash is
too small, in which case the double of the layout pixel extents will be
used). Basically don't use absolute pixel values anymore for
positionning. This should all be done relatively since there are
nowadays all kind of display size (and positionning the text 6 pixels to
the bottom, as it was done, may be ok on low density displays, yet will
look ugly on high density screens).
Also write this down in the splash requirements in the release howto so
that splash designers are aware that the bottom quarter of their image
will have to be adapted for printing text.
In GimpBrushTool, remember the settings used for the last cached
brush boundary, and avoid creating a new copy if the settings
didn't change. This should lower the overhead of
gimp_brush_tool_flush_paint() when not using dynamics.
Using CIE Lab yields gradients that more closely resemble the perceptual
gradients but without the gamma based blending problems of linear-RGB / CIE
XYZ.
Commit f5cb1fed85, which performed
brush outline generation in GimpPaintTool in synchrony with the
paint thread, wasn't enough, since GimpSourceTool could still call
gimp_brush_tool_create_outline() directly during its
GimpDrawTool::draw() method, leading to the same race condition
when executed concurrently with the paint thread.
Partially revert the above commit, so that outline generation is
handled as before, as far as GimpPaintTool is concenered. Instead,
add GimpPaintTool::{start,end,flush}_paint() virtual functions; the
first two are called when starting/ending painting using the paint
thread, while the third is called during the display-update
timeout, while the main thread and the paint thread are
synchronized. This allows subclasses to perform non-thread-safe
actions while the threads are synchronized.
Override these functions in GimpBrushTool, and cache the brush
boundary in the flush() function. Use the cached boundary in
gimp_brush_tool_create_outline() while painting, to avoid the above
race condition, both when this function is called through
GimpPaintTool, and through GimpSourceTool.
Commit b279c2d217 was breaking a specific use case, which I oversaw:
when space bar activates the move tool, you may want to release the
space bar while mouse button is pressed, and expect to still be able to
move the layer/selection/guide, but releasing space was stopping the
move immediately. The move tool must only be deactivated when both space
and button 1 are released, and the move itself must continue as long as
button 1 is pressed (when started while space was pressed).
As a nice side effect of this commit, panning and canvas rotation are
also improved since now they can be continued while releasing space
(respectively shift-space) if mouse button 1 was pressed, and up until
the mouse button is released. Pressing space again, then releasing the
mouse, back and forth, also work as expected (i.e. move tool stay
activated though the move stops; and panning or rotation continue).
Of course now we don't get anymore panning/rotation stuck while neither
space nor mouse buttons are pressed (which was the original bug). At
least one of these need to stay pressed for panning/rotation/move to
stay activated. And initial activation is obviously always through
(shift-)space only.
This includes migrating properly any custom shortcut (menurc), as well
as a few strings in tool-presets/, and finally "gimp-blend-tool" in
contextrc and devicerc.
File toolrc also has some occurrences, but we are already skipping it
anyway, same as whatever is under tool-options/.
Hopefully I missed nothing.
First WIP commit, adds:
- enum GimpGradientBlendColorSpace { RGB_PERCEPTUAL, RGB_LINEAR }
- linear blending mode for gradient segments
- tool options GUI for the blend and paint tools which use gradients
Don't flush the source pickable in gimp_source_core_motion(), as
this introduces a race condition between the paint thread and the
display-update timeout, when the source pickable is the image
containing the target drawable.
Flushing the source pickable shouldn't be currently necessary, and
either way, should happen during initialization.
Most expected behavior in normal transform is to see the preview,
whereas you usually don't want to see it in corrective mode. In 2.8
actually, it seems like it was not even possible to see the image
preview in corrective mode.
So let's set "show-preview" to these defaults when "direction" property
is updated. It is still possible to change it manually for any specific
use cases (i.e. you can hide the preview in normal transform, and
oppositely you can show it in corrective transform), but at least now
defaults are sane.
... or during rotation.
If checked before rotation, it works as expected, i.e. one sees only the
original or the rotated image.
While rotation is in progress: if unchecked, one sees neither the
original nor the image preview; if checked, one sees both original and
rotated preview.
Let's make the behavior consistent and only show exactly one version at
all time.
Invert zoom adjustment using the mouse or the keyboard when in
inverse mode. Take zoom factor into account when panning using the
keyboard. Fix motion cancelation.
... edges for MyPaint brush.
Adding the concept of "stateful" symmetry when a tool needs to make sure
of corresponding stroke numbers and orders while painting (i.e. stroke N
at time T+1 is the continuation of stroke N at time T). This is the case
for the MyPaint brushes and the ink tool.
GimpToolGyroscope is a tool widget providing canvas interaction for
3D rotation. The widget doesn't preset a UI, but rather
facilitates panning, rotation, and zoom, by dragging the canvas, or
using the keyboard.
Rotation is expressed using yaw/pitch/roll angles (performed in
this order). A zoom factor can be specified, which affects the
magnitude of the rotation per distance traveled. The widget can
operate in inverse mode, performing an inverse transformation.
Make sure a channel -> selection -> channel roundtrip never does any
gamma conversion.
In gimp_channel_duplicate(), make sure a created channel has the
right format, and the right data. Fixes selection -> channel.
When switching off quick mask, call gimp_item_to_selection() instead
if gimp_selection_load(), the latter was implementing a shortcut which
is now wrong.
Remove gimp_selection_load() which is now unused.
Unrelated: also remove gimp_selection_save(), it was an obvious
3-liner used only twice.
In GimpPaintTool, brush outline generation took place during
gimp_paint_tool_draw() even while painting. This function is run
concurrently with the paint thread. When using dynamics, this
introduced a race conidition between updating the brush mask in the
paint thread, and updating the brush boundary in the main thread.
Move brush outline generation during painting to
gimppainttool-paint.c, and perform it in the display-update
timeout, while the main thread and the paint thread are
synchronized.
After last commit, all paint tools work correctly with a separate
paint thread, so we can remove the option for specific paint tools
to opt out. Particularly, GimpMybrushTool now uses a separate
paint thread too.
Note that the separate paint thread can still be disabled through
the GIMP_NO_PAINT_THREAD environment variable.
Reorganize/clean up gimppainttool-paint. In particular, move all
paint-core interaction during painting to gimppainttool-paint.c, so
that we can have more control over what's going on; specifically,
enter the drawable into paint mode *before* starting the paint
core, so that it picks up the correct buffer. This fixes painting
with the paint thread using GimpApplicator, and enables us to use
the paint thread with GimpMybrushTool.
This function is unsafe during signal handling. And in any case, when
printing to stderr, I don't think we need to convert to UTF-8. Quite the
contrary, the system encoding may be more appropriate.
One additional fix for the gimp-channel-combine-masks procedure,
it needs both the combined *and* combined-to buffer in
gimp_gegl_mask_combine_buffer() to be treated specially.
Storing selections and layer masks as linear grayscale, but channels
as whatever-the-layers-are caused severe problems in images with
gamma-corrected layers: when combining channels with the selection,
they would go thorugh a gamma conversion before being combined, giving
unexpected results.
This commit changes all channels to always be linear, except in 8-bit
images, where they continue to be "Y' u8", for compatibility with old
XCF files, and because linear 8-bit can't really be used in
compositing (channels can be visible too).
To fix channel -> selection combinations also for these images, add a
small hack to gimp_gegl_mask_combine_buffer() which makes sure the
to-be-combined channel's pixels are always read as-is, without any
gamma conversion. After changing channels to linear, this makes no
difference except in the 8-bit images where we need this hack.
Add gimppainttool-paint.[ch], which takes care of painting during
motion events in GimpPaintTool. Perform the actual painting in a
separate thread, so that display updates, which can have a
significant synchronization overhead, don't stall painting.
Allow specific paint tools to opt-out of a separate paint thread,
and avoid it in GimpMybrushTool, since it doesn't seem to work.
The separate paint thread can be explicitly disabled by setting the
GIMP_NO_PAINT_THREAD environment variable.
gimp_drawable_start/end_paint() are used to enter/exit paint mode
for a given drawable. While the drawable is in paint mode,
gimp_drawable_get_buffer() returns a copy of the real drawable's
buffer, referred to as the paint buffer, so that modifications to
the returned buffer don't immediately affect the projection, and
calls to gimp_drawable_update() queue the updated region, instead
of emitting an "update" signal.
gimp_drawable_flush_paint() can be called while the drawable is in
paint mode, in order to copy the updated region of the paint buffer
back to the drawable's real buffer, and to emit "update" signals
for the queued region.
We use these functions in the next commit, to move painting to a
separate thread in the paint tools.
Align rectangles added to the display paint area, in
gimp_display_paint_area(), to a coarse grid, to reduce the
complexity of ther overall area. This is similar to commit
49285463e6, however the alignment
happens in display space, instead of image space.
Last commit caused -xobjective-c to be passed during linking on
Mac, causing object files to be treated as source files. Add a
-xnone flag to AM_LDFLAGS, canceling the effect of -xobjective-c.
Additinally, add a -xobjective-c++ flag to AM_CXXFLAGS, so that we
can use Objective-C in C++ files on Mac, if we ever need to.
On Mac, pass -xobjective-c to the compiler through AM_CFLAGS, not
AM_CPPFLAGS, so that it's only used for C sources, and not C++
sources. In the latter case, it clashes with the -std=... flag,
spewing an error. Thanks, Partha :)
Statusbar progress updates that update the progress bar are
relatively expensive, slowing down operations that report progress
frequently. Only update the progress bar if a certain amount of
time has passed since the last update, to counter that.
In gimp_gegl_convert_color_profile(), when src/dest_rect is NULL,
use the extents of src/dest_buffer, instead of passing a NULL area
to gimp_parallel_distribute_area(), which results in a CRITICAL.
Additionally, only report progress on the main thread.
gimp-templates.c:143:15: warning: ‘strncpy’ output truncated before terminating nul copying 3 bytes from a string of the same length [-Wstringop-truncation]
strncpy (dpi, "ppi", 3);
^~~~~~~~~~~~~~~~~~~~~~~
Need to check if we must override PixbufStyle's draw_layout() after
each theme change, not only at the beginning of themes_init(), so it
works also when the the pixbuf engine was not already loaded at
startup.
Commit cb239e60f6 introduced
artifacts when using the smudge tool with multithreads. Fix this
(caused by a wrong offset passed to an iterator) plus indentation
fixes.
Move gimp_brush_core_{subsample,pressurize,solidify}_mask() to a
separate gimpbrushcore-loops.cc file, so that they can be C++-ified
independently from the rest of the code. The next commit
parallelizes them.
The next commit is going to parallelize gimpbrush-transform using
the gimp_parallel_distribute_foo() functions. To spare us a lot of
boilerplate code, we're going to use C++ lambdas as callback
arguments to these functions.
This commit does the initial conversion of gimpbrush-transform.c to
C++, renaming it to gimpbrush-transform.cc. We do this in a
separate commit, so that the changes are small enough for git to
register this as a renamed file, rather than a new file, so that we
don't lose the file's history.
Add gimp-parallel.[cc,h], which provides a set of parallel
algorithms.
These currently include:
- gimp_parallel_distribute(): Calls a callback function in
parallel on multiple threads, passing it the current thread
index, and the total number of threads. Allows specifying the
maximal number of threads used.
- gimp_parallel_distribute_range(): Splits a range of integers
between multiple threads, passing the sub-range to a callback
function. Allows specifying the minimal sub-range size.
- gimp_parallel_distribute_area(): Splits a rectangular area
between multiple threads, passing the sub-area to a callback
function. Allows specifying the minimal sub-area.
The callback function is passed using an appropriately-typed
function pointer, and a user-data pointer. Additionally, when used
in a C++ file, each of the above functions has an overloaded
template version, taking the callback through a generic parameter,
without a user-data pointer, which allows using function objects.
Similar to commit 845eb522b6, I had a CRITICAL which happened on a
device_changed, triggering gimp_display_shell_update_focus(), this time
in focus in.
Remove the connect_after() hack from GimpImageWindow again and instead
add gimp_display_shell_canvas_realize_after() and restore the configured
ruler visibility there. Should work for all cases now.
Commit fd6d4931c8 accidentally
introduced a bug that caused Wilber's eyes to misbehave. This
commit is an attempt to fix this issue. Unfortunately, it seems
like the bug can still be triggered through a certain sequence of
actions...
Add "ellipsize" property to GimpColorFrame and set it to
PANGO_ELLIPSIZE_END in the the pointer information dockable.
Better cut off long numbers than make them expand the dock.
Use the recently-added gegl_tile_handler_damage_rect() function
during GimpTileHandlerValidate invalidation, instead of manually
voiding the tile pyramid. This function avoids voiding mipmapped
tiles entirely when only a subarea of the tile needs to be redrawn.
See GEGL commit 3210f4ffc3c569a2acd9483811cb141070112bc6.
gimp_image_window_constructed(): connect_after to the notebook's
"switch-page" signal so gimp_display_shell_appearance_update() is
called after gimp_display_shell_canvas_realize(). Just another hack
to fix the hack...
If variables are edited directly, in some cases, the GUI and the
symmetries may end up out-of-sync. The variable can only be edited
through property setting.
This value is actually used as a special value when removing a guide to
get rid of a symmetry, in particular it is set in the guides' callback
gimp_mirror_guide_removed_cb() for "removed" signal.
If not setting to 0, when adding back a symmetry, it starts with weird
low or high values near to border (whereas when set to 0, it is reset to
default position afterwards).
... parasite.
Not sure if that should be considered a bug. On one hand, it should
because that's core code. On the other hand, symmetry saving currently
uses parasites, which is a feature which can also be used by people
randomly. So "theoretically", there could be any data in a parasite and
we should not assume specific format.
Anyway still keep the error message but just print to standard error
output instead. Also print a bit more details (parasite name and
contents) as it would help for debugging when such a case were to occur.
Care for future changes of symmetries, by adding a version property. For
now, all symmetries are at version 0, and no real check is done. If any
symmetry bumps its settings in the future, it will have to override also
update_version() to change settings properly if necessary, according to
any new behavior.
Add GimpGuiConfig::filter-tool-use-last-settings wchich defaults to FALSE.
Honor the new option in gimp_gegl_procedure_execute_async() and add
it to prefs -> dialog defaults.
Add the color in gimp_smudge_paint(INIT) like GimpPaintbrush does.
Achieve this by calling gimp_palettes_add_color_history() directly,
not by inheriting GimpPaintbrush because GimpPaintbrush and GimpSmudge seem to share few common features.
We are not doing any write operation on this mask data so copying all
the data just to read it and unreffing it in the end is only a cost on
performance.
See also bug 694917.
When adding a rectangle to a projection's update area, align the
rectangle to a coarse grid, to reduce the complexity of the overall
area. We currently align the rectangle to a 32x32 grid, which
seems to be a good tradeoff between the overhead of processing a
complex area, and the overhead of processing a large area.
This keeps the same rectangle packing behaviour, so to behave exactly as
before for what concerns batching the updates, but should be lighter
when looping to find the first good rectangle to use.
In rtree_insert(), some conditions in the if tests are implied by
previous conditions. And therefore the 2 successive for loops are
actually identical.
In rtree_node_insert(), it is wrong/harmful to insert zero sized
rectangles in the tree because they can never be selected and just make
the list longer. So rtree_node_create() should just return NULL when w
or h are 0.
See bug 694917, comments 51 to 61.
because a simple platform dependency in gimpcoreconfig.c doesn't do
the trick (there can be both manual builds and bundled builds on
windows and macos). Use an AC_DEFINE() instead.
Also, make sure the system gimprc and its manpage are generated with
the correct values.
Some gimprc properties' default values depend on the machine where
"make dist" in run. We had an ugly hack in place to force
(num-processors 1) in the installed system gimprc and its manpage, but
were still leaking "tile-cache-size" and "mypaint-brush-path".
The files are generated by the hidden options --dump-gimprc-system
and --dump-gimprc-manpage which exist only for this purpose.
In gimpconfig-dump.c, special case the three properties in
dump_gimprc_system() and dump_gimprc_manpage() to output constant
default values for "num-processors" and "tile-cache-size" and
output @mypaint_brushes_dir@ in "mypaint-brush-path" which can
be replaced at configure time.
Also introduce etc/gimprc.in so @mypaint_brushes_dir@ can actually be
substituted for the installed system gimprc.
Otherwise the brushes won't be found when bundling on macOS or with the
Windows installer. Build-time path from configuration is still used on
other platforms.
The debug menu is currently not included in stable versions.
Include the menu unconditionally, but hide it, and its associated
actions, by default in stable versions. Allow enabling the menu
using a new --show-debug-menu command-line option, in the same vein
as --show-playground.
Turn the g_warning() into a g_printerr().
Duplicate accelerators should not trigger a WARNING, because they come
from a config file and there is nothing that can be fixed in the code
to prevent that.
when setting a custom parse error with g_scanner_error() we *must*
return G_TOKEN_NONE as expected token, or general GimpConfig error
handling will try to overwrite our error with its generic "unexpected
token" message, which triggers a warning.
As suggested by massimo in bug #694917, move unconditional creation/destruction
of a wrapper GeglBuffer object from top-level scope of the function to the
single conditional scope where it is used.
In gimp_image_convert_precision(), don't overwrite the 'progress'
parameter with the object queue, since we need to call
gimp_progress_end() on it at the end of the process.
In gimp_operation_buffer_source_validate_process(), align the ROI
to the tile grid *before* intersecting it with the validate-
handler's dirty region. This is necessary since, even though
subsequent operations will only read data within the ROI, the
entire tiles containing the ROI will be fetched, resulting in an
area potentially greater than the ROI. We need to validate this
area in advance, or else it will be validated as part of the
subsequent operations, which can lead into the same deadlock we're
trying to prevent.
Apparently former 2/3 value was too big according to some.
This makes the splash take now at most a quarter of the screen area. I
really don't think that's too much anymore.
In gimp_image_resize_with_layers(), calculate the set of resized
layers before changing the image size, so that we correctly
identify image-sized layers w.r.t. the old image size. (Fixes
commit 139a23451ddc588c91610f67daa799afc2f89080.)
Since error handling is based on preferences, the config needs to be
loaded first. Otherwise the gimp->config object does not exist yet and
we get a bunch of "'G_IS_OBJECT (object)' failed" assertion which
recurse in error handling when trying to get the "debug-policy"
property.
Just init the error handling later. It means it won't handle early
loading code, but that is not much of an issue.
In gimp_brush_core_color_area_with_pixmap(), use the native area
format when painting the brush, instead of always going through
"RGBA float", and create the pixmap -> area fish only once, instead
of once per scanrow.
In gimp_brush_core_paint_line_pixmap_mask(), avoid modulus
calculation at each pixel.
See bug #694917.
Add gimp_item_scale_by_factors_with_origin(), which is an extension
of gimp_item_scale_by_factors(), taking the input/output points of
origin for the transformation (both of which are (0, 0) in the case
of gimp_item_scale_by_factors()). Implement
gimp_item_scale_by_factors() in terms of the new function, and Use
the new function when scaling group layers, instead of manually
calculating the children boundaries, so that the behavior is
uniform across whole-image scaling and group-layer scaling.
The new function rounds all four edges of the boundary to the
image-global pixel grid, instead of only rounding the top/left
edges to the global grid, and the bottom/right edges to the item-
local grid. This preserves layer-adjacency when scaling.
Use GimpObjectQueue, added in the previous commit, in various
instances where we perform an action on a set of objects. This
improves progress reporting, by using a single progress for the
entire operation, rather than reporting the progress of each object
individually, and by taking the relative cost of each object into
account, instead of assuming a uniform cost for all objects.
In particular, this affects the various whole-image operations
(i.e., transformations and color conversions), operations on linked
items, and operations on layer groups. This also affects layers
with masks, whose progress is now reported together instead of
individually.
Additionally, this commit fixes erroneous group-layer mask cropping
during undo when resizing the image, by properly calling
{start,end}_move() on all the resized layers before starting the
operation, and when scaling the image, by only scaling top-level
layers, and letting group layers scale their children themselves.
GimpObjectQueue implements a queue of GimpObjects. It derives from
GimpSubProgress, and hence can be used as a GimpProgress object.
It keeps track of the total memsize of the objects that were
pushed-to and popped-from the queue, and uses these numbers to set
the corresponding subrange of the progress object when an object is
popped.
This provides an easy way to perform an operation on a set of
objects, correctly reporting progress based on the relative sizes
of the objects, which is assumed to be a good estimate of the
relative cost of processing each object.
Make the parent GimpProgress object of a GimpSubProgress instance
settable through a property during construction, so that we can use
it as a base class.
In gimp_group_layer_{start,end}_move(), push corresponding undo
steps, which perform the opposite operation during undo, and make
sure that mask-cropping is frozen during group-layer move
operations.
This fixed erroneous group-layer mask cropping when undoing/redoing
a group-layer move operation multiple times.
...should really use last values
When creating a layer or channel "from last values", really use the
values last set be the user in the respective dialogs. In particular,
don't use properties of the active layer or channel. I have no idea
what we were thinking when adding that obscure logic.
When pasting in place over a layer group or a content-locked item,
change the paste type to NEW_LAYER_IN_PLACE, rather than NEW_LAYER,
so that the new layer is still pasted in the right location.
Additionally, avoid showing the "Pasted as new layer because ..."
message when pasting over a layer group or a content-locked item,
when the paste type is NEW_LAYER[_IN_PLACE] to begin with.
This reverts commit 554347e0ff.
For some weird reason, this fixed the `make check` but broke the `make
distcheck`. I am lost. Better revert, and now distcheck works great.
I'm not sure how useful is this test if we have to just constantly tweak
the sessionrc for it to pass. But well… here it is.
Now make check fully passes.
Not using %d in the singular form of English does not prevent other
languages to use %d in any form they wish to. This will still work and
will still be replaced by the relevant number of images.
So I revert commit a0724783d8 because it is just prettier (in English)
to write "An image" rather than "1 image", but this does not mean you
have to do the same in other languages! Adding a comment so that
translators know about it.
Also directly modify the msgid in the Polish and Russian translations
which already translated this string, so that the translations does not
end up unnecessarily fuzzy.
Fix a CRITICAL when calling gimp_layer_mode_get_format() with an
AUTO composite space and a NULL preferred format, which is valid:
it means the layer mode is composite-space agnostic (as DISSOLVE
is), and that there's no preferred format.
A NULL preferred format can occur during
gimp_operation_layer_mode_prepare() if the layer's mode node is not
yet attached anything through its "input" or "aux" pads, which is
the case during the call to gimp_layer_update_mode_node() while
constructing the layer's node in gimp_layer_get_node().
Add signal GimpTextBuffer::color-applied which is emitted when text is
inserted or when color is applied to a span of text.
In GimpTextTool, connect to the signal and update the global color
history.
Unrelated: rename gimp_text_tag_get_color() to get_fg_color() and add
boolean return values to get_fg_color() and get_fg_color() which
indicates if a color is set on the tag at all. This ended up unneeded
in the fix but is an improvement regardless.
Since commit d916fedf92, GIMP has had the hidden feature to salvage
images (if possible) during a crash into a backup folder. This commit
finishes the feature by opening a dialog proposing to try and recover
the salvaged images.
This is not perfect yet since it doesn't "remember" the XCF path (in
case it was a previously saved image). The images open as new unsaved
and dirty images, but directly from the contents at crash time. For now,
it is up to people to figure out what they correspond to, if relevant.
In gimp_gegl_apply_scale(), use a CLAMP abyss policy for the scale
op, to avoid leaking transparency into the image when scaling
drawables.
Note that this (intentionally) only affects whole-image/layer
scaling, and not scaling done using any of the transform tools.
In gimp_gegl_apply_{border,grow,shrink,flood}(), which are used
by the corresponding channel functions, pass crop_input = TRUE to
gimp_gegl_apply_operation(), to clip the input to the output rect.
These operations process the entire input in one go, regardless of
the requested output region; however, the channel functions
calculate the output region according to the known channel bounds,
hence clipping the input to these bounds doesn't affect discard any
information, while avoiding unnecessary work. In particular, this
makes the corresponding operations on small selections in big images
much faster.
Add a crop_input parameter to gimp_gegl_apply_[cached_]operation().
When TRUE, the functions crop the op's input to the destination
rect. This is particularly useful for functions that process the
entire input in one go (by means of get_cached_region()). See the
next commit.
Pass crop_input = FALSE at all call sites for now, to keep the
current behavior.
After many discussions, it has been decided to export the metadata by
default since it seems to be what many people would expect and they
would consider they "lost" metadata (especially if they overwrite their
original). I don't entirely agree since privacy (particularly if you are
not aware of metadata and information they may contain) is also an issue
but not many seem to agree with me.
So here it is! All metadata now exported as a default!
The debug dialog is awesome and extremely helpful, but I realize it may
be a better default experience on *stable* to raise it only in case of
crashes. CRITICAL are bad and should be fixed, but sometimes their
consequence is actually not visible except for this dialog, and people
on stable builds may prefer not to see this dialog. Also we will likely
get a lot of duplicates for the same bugs once everybody has this by
default, which will be very annoying to attend to, unless we had
automation (which we don't right now).
The option is still available in preferences anyway so people really
interested in helping can activate the option for CRITICAL and even
WARNING anyway, even on stable releases.
Focus change events were expecting the current tool control to be
inactive, which was the case most of the time. Yet with stylus, the
canvas was sometimes receiving GDK_BUTTON_PRESS events before
GDK_FOCUS_CHANGE. In particular the canvas was receiving a button press
before the focus out, then button release and focus in. Therefore by the
time the focus out event happens, the tool control is active, which
broke a few calls.
Therefore I add a few checks and returns immediately when
gimp_tool_control_is_active() return TRUE, especially since we also run
gimp_display_shell_update_focus() calls after a button press anyway so
the state should already be consistent.
Add a gimp_filter_tool_reset_widget() function, which resets the
tool widget associated with the filter's controller -- i.e., it
resets those properties of the widget that aren't controlled by the
op's properties to some "default" state. For most controller types
this is a NOP; for transform-grid controllers, we move the pivot
back to the center of the drawable, w.r.t. the current transform.
Call gimp_filter_tool_reset_widget() after resetting or reloading
the tool's config.
...only remembers horizontal radius, duplicates it for vertical
Keep a list of the GUI's chain buttons around. When changing the
entire config object like on reset or selecting saved settings, unlik
them all after remembering their "active" state, and after changing
the settings activate the ones that were active before, but only if
the values they link are still the same.
... (gimp-context-set-paint-method...) is called.
GimpContext initialized with standard paint info at constructed() time
to ensure there is always a paint_info even if none were set manually.
This property is currently only used for gimp_edit_blend() to control
how are computed distances. In the future, it could be used for more
functions making use of "gegl:distance-transform" operation, or even for
other algorithms, if relevant.
This new property obviously comes with 2 new PDB calls:
gimp_context_get_distance_metric() & gimp_context_set_distance_metric()
... scrolling in progress.
In particular, this could happen while panning with mouse middle click
and hitting space. This space should simply be ignored.
The bug was affecting actually both canvas rotation and panning when
done with space key. If the first mouse button was also clicked, then
released after the space key, we ended up in some stuck action. It could
only be unstuck by hitting/releasing space again.
I am actually unsure that this was not originally done on purpose,
especially since the code has these 2 status variables space_pressed and
space_release_pending, but really apart from looking at this code, the
behavior just looks very buggy and impracticable.
The new behavior is to just stop the canvas panning/rotation as soon as
space is released (which is also how it is documented in our manual, and
how everyone seems to use the feature). I only kept the variable
space_release_pending, which I use as was used space_pressed before.
Avoid redrawing GimpCanvasProgress items upon
GimpProgress::set_value() if a minimal amount of time hasn't passed
since the last call. This notably improves performance of
frequently-updated GimpCanvasProgress items.
... when gradient_type >= GIMP_GRADIENT_SHAPEBURST_ANGULAR.
Our current GUI code for the Blend tool options disables the "Repeat"
widget with any of the shaped and spiral gradient types, which means (I
assume) no repeat mode is allowed on these gradients. Nevertheless it
was possible to change the repeat mode in another gradient type, then
switch to one of these and get the repeat processed even though the GUI
shows insensitive.
I could simply reset the repeat mode to GIMP_REPEAT_NONE when setting
one of these gradient types, but I think you'd want the repeat to stay
at its value (being insensitive is enough to mean whatever value is set
is not taken into account). So instead, I just unsync this specific
property when appropriate.
Note also I don't do this in the gimp:blend operation code because I am
not sure this specific behavior is meant to be a generic blend behavior
or just relative to the tool (render of the shaped gradients is barely
different with a repeat, but there is still a difference).
It seems old blend tool (from GIMP 2.8) was using manhattan distance,
whereas the new one uses euclidean. I guess there must be use cases for
both. In any case, it is a good idea to simply propose the option since
the property exists in the "gegl:distance-transform" operation.
See also bug 781621.
Our composite modes don't correspond directly to the Porter-Duff
operators after which they're named, and these names aren't too
descriptive anyway.
Rename the composite modes as follows:
Source Over => Union
Source Atop => Clip to Backdrop
Destination Atop => Clip to Layer
Source In => Intersection
Update relevant code, including UI text, enumerator names, function
names, and action names.
When starting the tool with one of the gradient types for which the
repeat option should be deactivated, it is not. Run the handler function
once at GUI creation.
Also compare the gradient type with an enum value, which makes the test
clearer than using an int.
PDB function gimp_edit_blend() was based on "gimp:shapeburst" operation
whereas the rest of GIMP (in particular, the Blend tool) used
"gegl:distance-transform" which is much faster.
Setting the operation to "manhattan" metric ensures that it still
renders the same way as in 2.8 while being a lot faster.
There was still a problem regarding as how it renders differently from
the Blend tool, but it turns out that the Blend tool is the one
rendering differently from how it used to in 2.8. We should discuss
adding the "metric" property in the tool options.
This operation is currently broken on multi-thread. So disable
multi-threading, at least temporarily (if not forever since apparently
we can get similar output with "gegl:distance-transform", but much
faster and nicer). See bug 781621.
Move the button-highlight update to its own function, and call it
when the active image changes, as well as when its floating
selection changes.
Call the floating-selection-changed signal handler when the active
image changes, so that we correctly update its row's attributes.
We disallow creating a new layer from a floating selection when its
associated drawable is a channel, so there's no point in
highlighting the "new layer" button in this case. Note that the
"layers-new" action remains sensitive, showing an error message if
activated. Not sure if it's a good thing or not, but whatever.
... when there's a floating selection
Layer-dialog interaction is restricted while the image has a
floating selection, which often causes confusion. Highlight the
three layers-dialog buttons that "finalize" a floating selection --
the "new layer" button, the "anchor layer" button, and the "delete
layer" button -- to indicate that these buttons are used to finish
the paste operation. The "new" and "anchor" buttons use a green
highlight color, while the "delete" button uses a red highlight
color.
In gimp_plug_in_open(), use gimp_spawn_set_cloexec() to prevent the
parent's end of the read/write pipes from being inherited by the
spawned plug-in, instead of passing the corresponding file
descriptors to the plug-in as command-line arguments, and having
gimp_main() close them.
Adding new command-line arguments to plug-ins is problematic, since
their ability to handle them depends on their protocol version,
which is only communicated after the plug-in is spawned.
Regardless, this is much simpler.
In gimp_plug_in_open(), use gimp_spawn_async(), added in the
previous commit, instead of g_spawn_async(). See the previous
commit for the rationale.
Since gimp_spawn_async() doesn't provide a mechanism to perform any
cleanup in the child before exec()ing, move the closing of the
parent's end of the read/write pipes from the app to the plug-in's
gimp_main(), passing the relevant file descriptors to the plug-in
through argv.
gimp_spawn_async() is similar to, but more limited than,
g_spawn_sync(). Unlike the latter, gimp_spawn_async() uses
vfork(), instead of fork(), when possible.
On Linux, a process that uses large amounts of memory (as GIMP may)
can hang during a fork() if overcommitting is enabled, and there's
not enough memroy. Using vfork() avoids that, since it doesn't
duplicate the parent's address space.
In case of error in gimp_prop_eval_parse_reference(), we were obviously
freeing the string which had just been allocated by g_strdup(), not the
pointer to this string.
Thanks to Massimo for raising this issue.
gegl:recursive-transform applies a transformation recursively to
an image. The custom GUI allows controlling the transformation
matrix using a transform-grid controller, added in the previous
commit.
... which allows ops to create a transform-grid widget, similar to
the unified-transform tool, which can be used to control a
transformation matrix.
Implement the transform-grid controller in GimpFilterTool.
The bug is very hard to reproduce, probably because it requires specific
timing conditions but this looks like this commit would prevent it.
Apparently the signal handler gimp_container_view_name_changed() may
have been run while the container view (set as user data) was most
likely already finalized, hence leaving an invalid dangling pointer.
Let's just make sure we disconnect this handler (and another) when we
finalize the container view and its private data.
... current aspect ratio
In gimp_{rectangle_select,crop}_tool_start(), move
GimpToolRectangle signal connection to the end of the function. In
particular, connect the signals *after* the call to
gimp_{rectangle_select,crop}_tool_update_option_defaults(), since
it may result in an emission of a "change-complete" signal, whose
handler calls the function again with ignore_pending == FALSE, which
would override the ratio set with ignore_pending == TRUE.
Having to sync the "valid" flag with the presence of a histogram is
error-prone (cf. previous commit).
Instead gimp_histogram_editor_validate() return value will just depend
on the presence of the histogram. And "valid" becomes "recompute", i.e.
a flag to request for "recomputation" of the histogram.
When the histogram is freed, we need to set valid to FALSE, in order to
force recreation as soon as needed. Otherwise we may hit some race
condition of trying to work with a NULL histogram. For instance this
happened when starting painting fast enough after switching the active
image.
I had to shuffle a bit the order of initialization since we were
creating a folder for the crash logs, as well as one for the backups a
bit too early. So now I move errors_init() after configuration folder
creation/migration, and I create both these folders in this function
(especially since gimp_init_signal_handlers() is run even earlier).
For this later reason, I also check for backtrace_file and backup_path
being allocated in gimp_eek() since it is also possible for signals
happening before errors_init(). In such a case, we will simply bypass
the GUI error handler (terminal error handler still possible) and the
backup (anyway there is nothing to backup at this point).
I could also try to create these 2 directories at the last second, when
needed. But since we are trying to do the strict minimum during crash
handling, it is better to do whatever can be done earlier.
Don't choke when calling gimp_tool_rectangle_set_constraint() while
there's no active image, or while the active image has no active
layer, which can happen when updating the default aspect ratio of
the crop tool. This would previously result in CRITICALs.
Additionally, use weak pointers for the crop tool's current_image
and current_layer members, to avoid potential dangling pointers.
While not currently necessary, this makes the code less dependent
on the exact order of events.
The current sorting logic of actions in the history is essentially
linear, so that when an action is activated it moves up one place
in the history. This has the undesirable effect that actions take
very long to climb up the history list, as well as that actions at
the top of the list can change their relative order too frequently.
Improve the sorting logic, such that items climb up the list
faster, while top items retain their relative position longer. See
the comment at the top of the diff for the actual logic.
A hidden feature of the action search dialog, is that actions can
be matched based on their label's initials. E.g., "gb" will match
"Gaussian blur". While very convenient, this feature is currently
limited to two-letter initialisms.
Extend initialism-based search, by matching arbitrarily-long
initialisms, and by allowing partial matches (with lesser
priority.)
The undo/redo actions' label changes based on context, and may
interfere with the labels of more relevant, but less frequent,
actions.
For example, after applying filter Foo, the label of edit-undo
becomes "Undo Foo", so searching for "Foo" results in both
edit-undo, and the action referring to the filter, with edit-undo
most likely appearing at the top of the list due to its frequency.
Excluding the undo/redo actions from the history is a simple, if
suboptimal, way to fix this.
... and rename gimp_action_history_excluded_action() to
gimp_action_history_is_excluded_action().
is_blacklisted_action() determines whether an action should be
excluded from *both* the history and the search results, while
is_excluded_action() determines if an action should be excluded
only from the history. This eliminates some redundancy across
gimpaction-history and action-search-dialog.
... current aspect ratio
When updating the default aspect ratio of a widget-less crop tool,
construct a temporary GimpToolRectangle widget, so that we can use
it to call gimp_tool_rectangle_constraint_size_set() and pick the
correct ratio, instead of just bailing.
When halting the crop tool, update the default aspect ratio, which
now does the right thing, as per the above.
Update the default aspect ratio upon changes to the active layer of
the current image, and to the size of the active layer, which
affect the default aspect ratio when "current layer only" is
toggled.
... current aspect ratio
When starting the rectangle-select (and ellipse-select) tools,
properly reset their default aspect ratio to 1:1. This fixes an
issue where the tool would use the aspect ratio of the last
rectangle when there's no user-overriden aspect ratio specified.
This restores the 2.8 behavior, except for the fact that the aspect
ratio resets to 1:1 when the tool is commited (if there's no user-
overriden ratio), rather than keeping the aspect ratio of the last
rectangle (i.e. "Current"); in 2.8 this only happend when halting.
The current behavior seems more consistent anyway.
Set the scale of the GimpRectangleOptions highlight-opacity
spinscale to 100, so that the spinscale's range is 0-100, instead
of 0-1, like the rest of our opacity spinscales.
gegl:color-to-alpha-plus was merged back to gegl:color-to-alpha in
GEGL, so we can change the custom propgui constructor to refer to
gegl:color-to-alpha.
Remove the threshold-range compression toggle and logic, since the
corresponding property was removed in GEGL.
Revert commit 24fcabc1ca, which
allowed passing a NULL buffer to gimp_drawable_set_buffer[_full](),
leaving the drawable without a buffer in a semi-functional state --
this is too risky.
Instead, have gimp_drawable_steal_buffer() assign an empty 1x1
buffer to the stolen-from drawable, rather than leaving it without
a buffer at all.
... g_find_program_in_path() instead of a test run.
I knew there was a `which` equivalency in glib but could no find it
anymore. I finally found it thanks to a comment by Rishi. :-)
Remove the impromptu boundary invalidation when converting a
channel to a selection mask in xcf_load_channel_props(), as this
happens implicitly when stealing the channel's buffer, since
commit 38d4aa8121.
When constructing a text layer from an existing layer in
gimp_text_layer_from_layer(), steal the old layer's buffer using
the new gimp_drawable_steal_buffer(), instead of using simple
pointer assignment, as the latter is generaly unsafe (even though
it should currently work).
Since commit d0ae244fe8, which
connects GimpChannel instances to their buffer's "changed" signal,
the XCF loading code that steals a channel's buffer when converting
it to a selection mask (which happens when loading an XCF with a
saved selection mask) is unsafe, since fails to perform the
necessary cleanup and setup of the buffer in the old and new
channel objects, respectively.
Perform the buffer transfer using the new
gimp_drawable_steal_buffer(), which does the same thing in a safe
manner.
... which transfers a buffer from one drawable to another in a safe
manner, leaving the source drawable empty.
There are already two ad-hoc instances where we steal a drawable's
buffer through simple pointer assignment, however, this avoids
performing potentially necessary cleanup and setup. In particular,
since commit d0ae244fe8 this causes
actual errors. The next two commits replace those instances with
calls to gimp_drawable_steal_buffer().
... which clears the drawable's buffer, performing any necessary
cleanup, without setting a new buffer. While the drawable has no
buffer, it can only be used in a very limited way, in particular,
it may be destroyed, and it may be assigned a new buffer.
This is used by the next commit to implement
gimp_drawable_steal_buffer(), which transfers a buffer from one
drawable to another in a safe manner, leaving the source drawable
empty.
Current code was redirecting WARNING and CRITICAL errors to normal
messaging when the debugging was deactivated (in Preferences). But if
you deactivate these on purpose, then it means you don't want to get
annoyed by small pop-ups either.
This commit makes them directly displayed in terminal, as they used to
before, when debugging is deactivated.
As previously explained, this was the next and logical step after
debugging. At the very end, just before exiting the process, let's
attempt to save all unsaved (i.e. "dirty") images. Of course we try to
do so as backup files in the config directory (once again, this would
be better under $XDG_CACHE_HOME/GIMP/ though) because we must not touch
the originals.
Currently we only have some automatic saving, but we don't warn yet that
backups were made. Also we don't keep track of the original paths for
later recovery hints. Proposed recovery would be worth being done at
next start of GIMP when we detect files in the backup directory (with a
typical "What should we do with these?" dialog).
Also it is to be noted that it is not a 100%-sure system. While testing
various test cases, I had many cases where the images were successfully
saved, but others when the backup failed (in particular when playing
with double freeing). I'm not sure if this is because of some memory
allocation during XCF saving or some other issue which could be improved
later (hopefully).
... on perceptual gamma image
When constructing the paint core's paint buffer, in GimpBrushCore
and GimpInk, use the drawable's format as the preferred format in
the call to gimp_layer_mode_get_format(), instead of NULL.
Subsequently, use the paint buffer's format, instead of the source
buffer's format, as the preferred iterator format in
do_layer_blend(), since the iterator format must match the paint
buffer format.
It seems gegl_buffer_sample() crashes with the first parameter being
NULL so let's just test its value first. This will output a huge
quantity of CRITICALs in this edge case but that's much better than a
crash. :-)
See also bug 793371 where this bug is being processed.
When checking the Cage Transform operation code, I saw some ifdef meant
to be switched to when bug 645810 is fixed. It also says in a comment:
> /* When Gegl bug #645810 will be solved,
> this should be a good optimisation */
Checking said bug, it appears it has been fixed since 2012!
I also fixed a bit the parameters in gegl_buffer_sample() call since it
seems the function signature has changed quite a bit.
Since a few commits, I don't generate the traces anymore in errors.c but
delay this to gui-message.c and rely on the message severity to decide
whether or not generating traces.
Unfortunately none of the current severities are properly describing
this new type of messages. Even GIMP_MESSAGE_ERROR is used everywhere in
our code NOT for actual programming bug, but often for data errors
(which are not bugs but proper messages and should obviously not prompt
a debug trace).
This reverts commit 94c6bb4603, because
unlike what the bug title suggests, Smudge apparently *should* use
perceptual RGB, so now everything works as before.
Setting push_undo to FALSE in these functions is premature -- let
this stuff be handled at the actual point where the undo is
pushed, which might correspond to a different item than the one for
which gimp_item_{start,end}_move() was called.
In particular, when removing a layer from the image,
gimp_item_end_move() is called (with push_undo == TRUE) on the
layer after it's been removed, but we still need the appropriate
undo enrties to be pushed for the affected group layers.
... a selection.
The crash was the result of an unmatched gimp_item_end_move() call,
which is an error. Add the matching gimp_item_start_move() call
when starting to drag a selection in GimpEditSelectionTool. Revert
last commit, so that unmatched gimp_layer_end_move() calls are not
silently ignored, and add a check instead.
... a selection.
The regression appeared with commit 10c125c627.
gimp_layer_end_move() may sometimes run even while a symmetric
gimp_layer_start_move() had not run. For instance this happens when
releasing the mouse button after dragging a ctrl-alt-click created
floating layer.
Therefore let's check that layer->move_stack is not NULL before
dereferencing it.
Slight back step from commit 34fe992f44. I don't keep track anymore of
the number of errors inside GimpCriticalDialog. The problem is that GTK+
calls must happen in the main thread, and errors in another thread will
be delayed into the main thread through gdk_threads_add_idle_full().
This makes any backtrace generated as a consequence of a threaded error
useless (in particular any error happening in GEGL since we always
process these as multi-threaded, whether they are or not).
Instead I now keep track of the number of errors in gui-message.c, which
still allows to reset the counters when the unique debug dialog is
closed. Therefore I can now generate backtraces conditionally to the
error counters inside the problematic thread (and right when the error
happened), without any GTK+ call.
This finally makes GEGL backtraces useful in the debug dialog! :-)
It is not always very useful since GEGL makes heavy use of threads, and
therefore a backtrace of the main thread for an error on another thread
is mostly useless. But that's a start. I am still improving.
I was holding on non-GIMP messages until now because we don't have as
much control on them, and for some errors, they may be huge. For
instance, the bug told by Massimo in bug 792787, comment 22, generates
hundreds of thousands (and even millions for big enough polygons) of
errors. But I can now allow these to pass since previous commit when I
now only display a few errors, and then redirect remaining errors to
stderr.
Also get rid of gimp_third_party_message_log_func() and instead make
gimp_message_log_func() handle correcly non-GIMP messages by keeping
their domain.
We don't want an infinite number of traces because it takes some time to
get. Until now I was keeping track of traces in app/errors.c, but that
was very sucky because then I was limiting traces per session. Instead
save them as a variable of a GimpCriticalDialog instance. Therefore only
generate the traces for WARNING/CRITICAL at the last second, when
calling the dialog.
When too many traces are displayed, just fallback to just add error
messages only. But then even errors without traces can be time-consuming
(if you have dozens of thousands of errors in a few seconds, as I had
the other day, updating the dialog for all of them would just freeze the
whole application for a long time).
So also keep track of errors as well and as last fallback, just send the
remaining errors to the stderr.
which is just a #define to g_assert for now, but can now easily be
turned into something that does some nicer debugging using our new
stack trace infrastructure. This commit also reverts all constructed()
functions to use assert again.
When removing a GimpContainerTreeStore item, make sure that editing
of corresponding tree-view rows is canceled first, by emitting a
"row-changed" signal. Otherwise, we can run into trouble when
removing an item that is being edited.
A tool commit can be triggered in various cases, and the tool manager
relies on gimp_tool_has_display() to decide whether to run a tool
action. This function does much more than just checking GimpTool's
display. It also checks status_displays, and for a transform tool in
particular, it checks GimpDrawTool's display. This may be right for
other tools (I have no idea), so I can't just change this function.
Anyway we have to assume it is not a programming error if a transform
tool gets a COMMIT action while display is NULL (i.e. tool is halted).
When this happens, let's simply ignore.
This fixes the edge case raised by Ell, in comment 2 of bug 793150: when
an image has no layer, transform tools can't work and display is NULL.
But it still outputs status messages and therefore status_displays is
not empty. So the tool manager will still run a COMMIT action, which is
not an error. We only have to discard such COMMIT silently.
Improving previous commit. Rather than calling:
> GIMP_TOOL_GET_CLASS (tool)->control (tool,
GIMP_TOOL_ACTION_HALT,
display)
> gimp_tool_clear_status()
... in the COMMIT action, which is basically what the HALT action does,
simply pass through from one case to the other. It also adds the call to
gimp_tool_control_halt() which is most likely right anyway since we are
halting the tool. This also makes the code consistent and any future
changes to HALT case will be directly enabled after COMMIT.
... 'GIMP_IS_DISPLAY (display)' failed.
This may happen when committing first a transform tool, then switching
to another tool. In this case, the tool manager will attempt to commit
again because gimp_tool_has_display() returns TRUE since status displays
were not cleared. Unfortunately transform tools don't handle very well
trying to commit when it was already done (hence both GimpTool and
GimpDrawTool displays are NULL).
The proposed solution is to clear the statuses after committing.
Completing previous commit, the next switch was not raising any error,
but I believe the new "in place" variants of paste as floating selection
also have to be processed for mask removal.
In a switch(), not all paste type were listed (the new "In Place"
versions in particular were missing), therefore we were hitting a
g_return_val_if_reached() error.
Have GimpChannel connect to the drawable buffer's "changed" signal,
so that we can invalidate the channel's boundary whenever the
buffer contents change. Currently, the calls to
gimp_drawable_invalidate_boundary() dispersed throughout the code
are not enough.
Moreover, invalidate both the boundary and the bounds in
gimp_channel_invalidate_boundary(), since both are necessary when
the buffer changes.
In gimp_layer_start_move(), keep the set of ancestors for which for
which we suspended mask cropping, so that we can resume mask
cropping for the same groups in gimp_layer_end_move(). This is
necessary, since gimp_image_remove_layer() calls gimp_item
start_move() before removing the layer from the layer tree, and
gimp_item_end_move() after removing the layer from the layer tree,
at which point the layer has no ancestors.
Using g_get_user_data_dir() is maybe right on Win32 (for this roaming
vs. local directory logics), but not on Unix-like systems, where we end
up trying to write in the data directory (usually not even supposed to
be writable by applications).
Also while at it, I replace g_get_prgname() by PACKAGE_NAME. It turns
out that this function returns NULL, maybe because of the init order.
Actually ideally, this file should rather go under:
$XDG_CACHE_HOME/GIMP/<version>/
But last we discussed this, it was decided that files should not spread
too much (though I still disagree!).
If the backtrace() API is available, it should always be possible to
debug. Still, display a message whether or not gdb or lldb are present,
as preferred debugging solutions (much better traces).
Replacing the boolean property "generate-backtrace" by an enum
"debug-policy". This property allows one to choose whether to debug
WARNING, CRITICAL and FATAL (crashes), or CRITICAL and FATAL only, or
only FATAL, or finally nothing.
By default, a stable release will debug CRITICAL and crashes, and
unstable builds will start debugging at WARNINGs.
The reason for the settings is that if you stumble upon a reccurring bug
in your workflow (and this bug is not major enough for data corruption,
and "you can live with it"), you still have to wait for a new release.
At some point, you may want to disable getting a debug dialog, at least
temporarily. Oppositely, even when using a stable build, you may want to
obtain debug info for lesser issues, even WARNINGs, if you wish to help
the GIMP project.
It can be argued though whether the value GIMP_DEBUG_POLICY_NEVER is
really useful. There is nothing to gain from refusing debugging info
when the software crashed anyway. But I could still imagine that someone
is not interested in helping at all. It's sad but not like we are going
to force people to report. Let's just allow disabling the whole
debugging system.
... stacktrace into a file on non-Win32 systems.
This has a few advantages:
- First, we don't need to duplicate stacktrace code inside the
independent gimp-debug-tool (I even noticed that the version in the
tool was gdb-only and not updated for lldb fallback; proof that code
duplication is evil!). Instead, even on a crash, we can create the
stacktrace from the main binary and simply pass it as a file.
- Secondly, that allows to fallback to the backtrace() API even for
crashes (this was not possible if the backtrace was done from a
completely different process). That's nice because this makes that we
will always get backtraces in Linux (even though backtrace() API is
not as nice as gdb/lldb, it's better than nothing).
- Finally this makes the code smaller (i.e. easier to maintain), more
consistent and similar on all platforms.
... our own implementation.
Though the GUI stacktrace is better for most (because it is visible even
when not run in a terminal), the CLI options are quite useful too and
may still be preferred by some, in particular developers. So it may as
well be benefiting from the better implementation. Glib traces are quite
weak even though they also use gdb and debug info are present (often,
even though I had these traces, I had to run gdb separately; now it
won't be necessary in many cases). My traces include more information.
Note that I didn't implement gimp_print_stack_trace() from previous
gimp_get_stack_trace() because I cannot allocate a string after some
types of crash (e.g. segmentation faults). So instead,
gimp_print_stack_trace() now take care optionally of both cases: either
allocating a string, or directly pipe to a file descriptor.
We avoid resizing the mask as a result of changes in the group's
bounding box while the group is being moved (i.e., translated,
rotated, etc.), so that GimpLayer transforms the original mask,
rather than a cropped mask. We still need to crop the mask after
we're finished moving the group, however. This commit takes care
of that.
Override GimpItem::resize(), instead of GimpLayer::resize(), when
resizing a group layer, so that GimpLayer doesn't try to resize the
mask. Instead, the let the usual mask resizing logic in
GimpGroupLayer handle that.
Also, make sure that the mask is properly restored upon undo when
group resizing is suspended outside of any mask-suspension block,
which can happen when resizing the group.
Add layer-mask support for group layers. Group-layer masks work
similarly to ordinary-layer masks, with the following
considerations:
The group's mask size is the same as group's size (i.e., the
bounding box of its children) at all times. When the group's size
changes, the mask is cropped to the new size -- areas of the mask
that fall outside of the new bounds are discarded and their data is
lost (sans undo), and newly added areas are filled with black (and
hence are transparent by default).
The new gimp_group_layer_{suspend,resume}_mask() functions can be
used to modify this behavior. Between the outermost pair of
suspend/resume calls, the old mask data is remembered, and is used
to fill the newly added areas while cropping the mask when the
group is resized. We override GimpItem::{start,end}_move() for
GimpLayer, to call these functions (suspend() in start_move(), and
resume() in end_move()) for each of the layer's ancestors.
As a result, while moving a layer, or a set of layers, atomically,
such as while dragging with the move tool, or moving linked layers,
the ancestors' mask data is not lost, and is only discarded at the
end of the operation.
This commit also takes care of properly handling undo for group-
layer mask crops, properly invalidating the image when the group
layer's mask is shown, and enabling the mask actions for group
layers (obviously :).
Add gimp_item_{start,end}_move(), and corresponding
GimpItem::{start,end}_move() virtual functions, which should be
called before/after "moving" the item (i.e., translating, scaling,
resizing, flipping, rotating, or transforming the item). Moves
performed between the outermost pair of start/end calls are treated
atomically.
What exactly does "treated atomically" entail depends on the
subclasses -- GimpItem doesn't provide a default implementation for
these functions, so the current commit doesn't change any behavior.
The next commit, which adds layer-mask support for group layers,
uses the functions to avoid cropping the mask too early while a
child is moving.
GimpItem calls {start,end}_move() in the various "move" functions
(gimp_item_{translate,scale,...}(), before performing the actual
operation. Additionally we call the functions in the
gimp_image_item_list_foo() functions, for each participating item,
so that the items are moved as a unit. We call the functions in
the various gimp_image_remove_foo() functions, since removing an
item may affect the size of its ancestors, and is therefore akin to
moving. We also call the functions in GimpEditSelectionTool, so
that the move tool moves items atomically while dragging.
This avoids warnings when the handle positions the handle-transform
tool result in a matrix that transforms the TransformGrid's handles
(which are all invisible) to coordinates that are outside of the
corresponding properties' range.
The result of applying a perspective-transform to a Bezier curve is
only an approximation. When the curve is highly nonlinear, the
result may diverge significantly from the real transformed curve.
Subdivide the curve as necessary in gimp_transform_bezier_coords()
to counter that. Adjust gimp_bezier_stroke_transform()
accordingly.
I was directed by Massimo to some bug which was repeatedly generating
dozens of thousands of GEGL WARNINGs and that was completely taking over
the GUI if redirected to GimpErrorDialog. Currently GEGL warnings are
not redirected there, but the problem is still there, and we don't want
GIMP warnings to freeze the whole GUI either.
So only increment the repeat variable upon gimp_message_box_repeat() and
delay actual GUI update to a later low-priority idle function.
Override GimpStroke::transform() for GimpBezierStroke, using
gimp_transform_bezier_coords() to transform the stroke's segments,
so that clipping done properly.
The next commit is going to perform clipping when transforming
Bezier strokes. When parts of the stroke get clipped, the result
consists of multiple strokes.
Adapt gimp_stroke_transform() in preparation, to allow for the
transformation to result in multiple strokes, by adding a GQueue*
parameter that receives the transformed strokes.
For convenience, we allow passing NULL as the argument, in which
case the current behavior is maintained. However, NULL should only
be passed when clipping is known to be unnecessary.
Adapt the rest of the code for the change.
Note that this technically affects public API: existing stroke
object IDs now become invalid after transforming their containing
vectors object. However, this is unlikely to affect code in
practice.
... which transforms a single cubic Bezier segment, performing
clipping by the near plane, to avoid erroneously transforming parts
of the curve behind the camera.
... which is the same as gimp_transform_polygon(), but using
GimpCoords for the vertices, instead of GimpVector2.
Specify when the input and output arguments may alias, in the
description of gimp_transform_polygon().
Fix indentation, typos, style. Use array parameters for the
control points, instead of using individual by-value parameters.
Use GArray* for the results, instead of GArray**. Verify
arguments.
Adapt the rest of the code to the changes.
filters-commands.c always needs an image and a drawable, so use
return_if_no_drawable(), not return_if_no_display().
Also fix the sensitivity of the shadows-highlights actions, which made
this bug triggerable at all.
When the resulting matrix transforms all input points behind the
camera, negate the matrix, instead of failing, which results in a
matrix that transforms the input points to the corresponding points
in front of the camera. This avoids rejecting certain valid
transforms as invalid, in the generic transform tools (unified,
perspective, and handle transform).
Make the number of input and output points explicit in the
function's signature, and add comments.
I actually think the buttons were translatable, but the strings were
simply not auto-extracted with previous code (which is nearly the same
in the end). Should be better now.
The reason is that this file is now included for a binary in tools/ as
well (the debug binary) and tools/ contents needs to be built before
app/. Even using BUILT_SOURCES in the Makefile under app/ is not enough.
Anyway it makes sense that this file should be under the root of the
repository since that describes the status of the source repository. So
let's move it up one folder.
It is nice because when available (Linux only?), it is a lot faster than
using a dedicated debugger such as GDB or LLDB, and also it allows to
always have a backtrace, even when no debuggers are installed.
Unfortunately the output is a lot less detailed, with no file paths, no
line numbers (even when debug symbols are there), no local values
printout, etc. It's pretty bare, with function names and the stack
levels. This is why it is not given priority, and GDB and LLDB are still
preferred when available.
Since commit 9fdf35550b, I removed the GIMP_APP_GLUE_COMPILATION check
because we need to have the whole versioning info from the new debug
widget. It just makes sense to go further and just make this a proper
internal API to get version information.
... so that the transformed boundary is properly clipped.
Adjust the boundary-size algorithms to operate on arbitrary
polygons.
Avoid using gimp_matrix3_will_explode() in
gimp_drawable_transform_buffer_affine() and falling back to
cropping the result, and avoid setting the "clip-to-input" property
of gegl:transform. Neither of those in needed anymore.
This effectively reverts the app/ part of commit
768d06614f. The next commit revets
the libgimpmath/ part.
Add a "clip" property to GimpCavnasTransformGuides. When set, use
gimp_transform_polygon() for transforming the guides and the
bounding box, so that they're properly clipped.
Add a corresponding "clip-guides" property to
GimpToolTransformGrid, and set it to TRUE in GimpToolHandleGrid, so
that the handle-transform tool's guides are clipped properly.
gimp_transform_polygon() transforms an (open or closed) polygon by
a GimpMatrix3, performing clipping to the near plane, to avoid
erroneously transforming points behind the camera.
This is obviously not possible anymore to do this manually so this step
is bogus in a crash case. We keep this step for other (non-fatal)
errors. We may add an automatic "attempt" to save in a backup file
later, which may not work depending on how bad the crash is (which is
why it will be done in a backup file, we don't want to corrupt saved
files).
SIGABRT is definitely a fatal error, at least in GIMP context. It is
used by g_assert() and more generally by abort().
Actually I am a bit unsure about the difference of gimp_terminate() and
gimp_fatal_error(). The former mostly depends on whether we used
--debug-handlers or not, which reads "Enable non-fatal debugging signal
handlers". But the way we handle them, the list of signals handled by
gimp_terminate() seem to always end up fatal as well, anyway. So either
we should *really* make them non-fatal (I could imagine that SIGTERM or
SIGINT indeed could be better handled for instance), or we should just
get rid of this terminate/fatal_error differentiation which seems
totally artificial and non-existing in the current code.
AFAIK this means on all platforms but Win32 and macOS which would rather
need relative path and therefore cannot make use of build-time
LIBEXECDIR. Anyway on these platforms, leaving the binary in BINDIR is
not likely to "pollute" too much as it would on Linux or BSD where
people often use terminal.
It was previously untested, hence as expected needed fixes. First I add
our own exception handler using Win32 API SetUnhandledExceptionFilter().
Second, I reorder things so that ExcHndlInit() is run after this setter,
since they will be executed as a FILO and we need backtraces to be
generated before our separate GUI runs. Last I run the backtrace GUI as
async. No need to keep the main GIMP waiting since the traces have
already been generated into a separate file.
Also replace gtk_show_uri() by the implementation taken straight from
our web-browser plug-in, since apparently gtk_show_uri() doesn't work in
Windows (and probably not macOS either since I see we have a separate
implementation for this platform as well). I would like to be able to
use the PDB but can't because this code needs to be usable both within
the main process and into a separate tool process. Ideally, this should
just be a utils function which could be included without a problem.
* Type pid_t is not cross-platform. Just use int instead, and convert it
to respective type on each platform.
* Get rid of several useless include which should have been removed a
few commits ago, when I reimplemented the backtrace function.
* Better handle the various macros in gimp_eek() (between G_OS_WIN32,
HAVE_EXCHNDL and GIMP_CONSOLE_COMPILATION, but also no_interface and
generate_backtrace options, that was a bit messy).
* Make gimpdebug now always built, whatever the platform.
The feature already exists in our code and produces backtraces upon a
crash into a file. The only difference is that we are now getting the
file contents and showing it in our new debug dialog, so that it works
similarly on all platform (and therefore making the debug info visible
to people, otherwise they would never report, even though the data is
generated).
The difference with gdb/lldb is that it doesn't allow backtraces at
random points (for debugging non-fatal yet bad errors). Also the API has
just 2 functions and in particular an ExcHndlInit() but no way to unload
the feature. So we don't need the debugging page in Preferences because
the switch option would not work. On Windows, the feature will be
decided at build time only.
Last point: the code is untested on Windows so far. I assume it would
work, but there is at least one point I am unsure of: will ExcHndl have
already generated the backtrace file when gimpdebug runs? If not, I will
have to let gimp die first to be able to get the backtrace.
This is just a bit more consistent with existing code. Also build the
gimpdebug tool only when GIMP_CONSOLE_COMPILATION is not set and run
when --no-interface CLI option is not set since it is a GUI tool.
This will determine whether to output backtrace in a GUI and is disabled
by default on stable, and activated in dev builds. It is a bit redundant
with --stack-trace-mode option CLI and will take priority when enabled
since most people would run GIMP with a graphical interface anyway.
This was a bit harder since even though we handle fatal signals,
allowing us to do any last action before GIMP crashes, it seems more
memory allocation is not allowed at this time. So creating a dialog or
simply getting the return output of gdb into the main process is not
allowed. What I do instead is running a separate program (gimpdebug)
which will take care of creating the new dialog and running a debugger.
I still use GimpCriticalDialog code from this separate binary, while I
continue to use this widget also within GIMP for non-fatal errors. The
reason why we still want to use it within GIMP is that we can bundle
several non-fatal errors and backtrace this way (fatal errors don't
return anyway) and it's easier to do so when created from the main
process.
Don't use g_on_error_stack_trace() from glib anymore. It is
over-complicated, using gdb in interactive mode and running command
writing in the pipe input. Sometimes it even gets stuck and never
return. This is useless since gdb even has a batch mode, to just run
commands and exit directly. I just use this.
GIMP will now try to get a backtrace (on Unix machines only for now,
using g_on_error_stack_trace(); for Windows, we will likely have to look
into DrMinGW).
This is now applied to CRITICAL errors only, which usually means major
bugs but are currently mostly hidden unless you run GIMP in terminal. We
limit to 3 backtraces, because many CRITICAL typically get into domino
effect and cause more CRITICALs (for instance when a g_return*_if_fail()
returns too early).
the angle arc and the small helper line were displayed on opposite
sides of the first point. Now they are on the same side, just like for
all other angles.
This reverts 2069496af3 for
gimpmeasuretool.c, we can't use gimp_display_shell_get_line_status()
here because the angle to show is *not* the angle as shown in the
paint tools (between x1,y1 and x2,y2), it is determined by a possible
third point.
Also, the info window and the statusbar were using different
coordintates to detemine the line length. This would have been easily
fixable, but the wrong angle wasn't.
Derive GimpUnifiedTransformTool, GimpPerspectiveTool, and
GimpHandleTransformTool from GimpGenericTransformTool, eliminating
their common logic.
Remove gimp_transform_matrix_handles(), which is no longer used.
... gimp_transform_matrix_generic()
Replace the separate x/y coordinate arrays of GimpHandleGrid with
GimpVector2 arrays, and use gimp_transform_matrix_generic(),
instead of gimp_transform_matrix_handles(), when calculating the
matrix. When the resulting matrix is invalid, hide the guides.
... which can be used to control the guides' visibility.
Will be used by the next commit, to hide the guides in
GimpToolHandleGrid when the tranformation is invalid.
A subclass of GimpTransformTool, to be used as a base class for
transform tools that calculate their matrix based on 4 pairs of
input/output points, and that display the transform matrix as their
GUI (this includes the unified, perspective, and handle transform
tools; the next commit ports them over to
GimpGenericTransformTool).
When the resulting matrix of the input/output mapping sends any of
the output points to, or past, infinity, GimpGenericTransformTool
sets GimpTransformTool::transform_valid to FALSE, and displays an
appropriate message in the tool GUI, instead of showing the matrix.
... which specifies whether the transform matrix is valid.
Subclasses can then set this member to indicate that the current
transformation is invalid (which can happen in the unified,
perspective, and handle transform tools). When the transform is
invalid, GimpTransformTool doesn't apply it upon committing, and
returns an error instead.
This commit doesn't set the transform_valid member in any of the
subclasses, so there's no effective change. The next commit adds a
GimpGenericTransformTool subclass, that will use the new member.
When picking a row whose leading coefficient is nonzero, use fuzzy
comparison against an epsilon, instead of exact comparison against
zero, to minimize the effect of numerical errors.
... on top-level layers.
There was even a comment for this, but I missed this when I moved some
code to the top of the function in commit b9577a783d. Now moving this
call up as well. This appeared to be more of a problem when merging
layers without a GUI (script-fu). I'm guessing the GUI calls this anyway
before.
Replace all g_assert_not_reached() in app/core/ by g_return_if_reached()
or g_return_val_if_reached(). GIMP may handle a lot of creative work,
sometimes unsaved for hours. We should not just crash on purpose.
g_assert*() could theoretically be turned off on a glib build, but this
is nearly never done, and is not a solution either (actually it is
probably even worse because the broken code would just continue on a
forbidden path). It is much better to return with a warning on such
forbidden code paths, allowing someone to report a bug without
experiencing a crash and data loss.
For now, I only took care of g_assert_not_reached() inside app/core.
More g_assert*() code should be replaced.
Note: assert are acceptable in plug-ins though, but not in the main
executable, unless absolutely necessary (something happening so bad that
crash is better than continuing).
Don't use g_assert(). Instead use g_return_val_if_fail().
This commit therefore does not fix the actual bug, but at least it does
not crash. GIMP simply outputs a warning upon trying to merge down a
hidden layer. The actual fix will follow later.
The CPU group monitors GIMP's CPU usage, and can measure the amount
of time the CPU has been active, which can be used to get a rough
estimate of the execution time of certain operations.
Be warned: while the CPU group is available on *nix and Windows, it
has only been tested on Linux.
Allow controlling the gauge/history visibility, and the history
interpolation method, of individual values.
Improve redraw elision when some values are hidden.
...if "Show rulers" is disabled
Add HACK to gimp_display_shell_canvas_realize() that makes sure the
rulers are always mapped once for each new GimpDisplayShell. This
seems to magically fix all the crashes.
Before you get too exceited -- no, this commit doesn't integrate
transform previews into the image graph :) We still use a
separate canvas-item overlay, just like before, but instead of
using an impromptu implementation to render the preview, we use
gegl:transform. We properly adjust the matrix passed to the op
according to the display scale, so that we still render only as
much as needed.
This is both notably faster than the current code, and, for
perspective transforms, more accurate.
Whereas we would only scale *down* big pixel images, we should both
scale up or down vector images since such format is made for display at
any size. This way, a vector splash screen is always displayed at ideal
size, whatever your display size.
Current code was using the dimension of the screen as a max size. That
is really too big. 2/3 of the screen size is an acceptable size being
both well visible and not overwhelming.
Also the current code was cropping, not rescaling, the splash image,
which is obviously not an acceptable solution because on a very small
displays, we would end up with ununderstandable piece of a bigger image.
This new code will allow to ship big size default splash image(s), and
display it in an acceptable size on both low and high density displays.
We indeed got a feedback from someone with a 4K display who was saying
the current dev splash screen was tiny on one's display.
Of course, custom splash can still be at any size; but from now on, we
will need for the *default upstream splash image(s)* to be of huge
dimension in order to show up well everywhere (at least Full HD splash,
which is half of a 4K UHD screen).
Animated splash images are still not resized though and will show up at
their default dimension. This means we cannot ship animated splash
screens as a default for the time being (they can still be installed as
custom splash).
The four remaining "classic" color tools (Brightness-Contrast, Curves,
Levels and Threshold) are in fact just special UIs for otherwise
completely normal filter ops.
Add normal filter actions for them and invoke them like all
other filters, which makes them show up in the filter history
automatically.
The only small hack needed is to special case them in
gimp_gegl_procedure_execute_async() so the right tools are created
instead of the default GimpOperationTool. Also, blacklist the
automatically generated tools actions from action search and the
shortcut editor.
gimp_action_history_init(): when deserializing the action history,
check each action against gimp_action_history_excluded_action() so
when we decide to exclude an action, it doesn't come back as history
zombie from disk.
... be used effectively.
We have had display pixel density detection for quite some time, but I
guess the step I set to switch to the "Huge" icon size (48px for the
toolbox icons) was too high at 300 PPI. Someone with a screen of about
280PPI reported the icons to be far too small on GIMP 2.9.8.
It seems that 240PPI is already even considered as XDPI already. Let's
just set our new "Huge" step at the 250PPI intermediate.
In any case, it seems GIMP 2.10 will have problems with even denser
displays, but the way GTK+2 handles icon sizes with a GTK_ICON_SIZE_*
enum is quite limited. That is quite a problem considering screens
getting denser in pixels nowadays. Hopefully GTK+ 3 will improve the
situation.
...on the list dialogs on the Input text area
gimp_container_editor_constructed(): connect the container
view's "select-item" with G_CONNECT_AFTER because the signal
is G_SIGNAL_RUN_LAST. Some quick greps didn't find anything that
would be affected, except fixing this bug. Found by Massimo.
...instead of center
The scale tool implicitly uses GimpToolTransformGrid's "pivot-x" and
"pivot-y" properties, so they need to be properly initialized and
updated to be at the grid's center.
Also add a tool options toggle "Around center".
The attached patch avoids CRITICALs when executing Debug actions
with no images open.
1) do not run projection benchmark unless there is an image
2) memsize of pluginmanager member was incorrectly computed using
gimp_object functions for 2 GObjects
This reverts commit 36258a671a.
This commit was making the rotated canvas rendering quite horrible to
the point that I think it would make the canvas rotation feature barely
usable. See bug 759287, comments 13 to 18.
I think we will need to find other ways to accelerate rendering.
Compromise on quality is possible, but I think that in this case, this
was more than just a compromise. It was more like completely abandonning
quality. We could even see the lines "spiking" while you were rotating!
Like your drawing was alive!
When the active modifier mode is GIMP_TOOL_ACTIVE_MODIFIERS_SAME (the
tool does not want to distinguish between modifier states depending
on whether or not the first mouse button down), we need to make sure
these states are in sync in GimpTool's bookkeeping, and we must not
generate synthetic modifier releases when the mouse button is
released.
Looks like we should use L*, C*, and h° to stay consistent (in
particular Lab L* and LCH L* are the same value), and also to allow for
future implementation of variants of LCH.
Value "h°" was the hardest to choose since we sometimes see just "h",
sometimes with a star, and other times with a degree. Even reference
documents display the 3 versions in the same documents! I just went with
a 2-letter version with degree, as seen on Wikipedia, and to align
better with other 2-letter LCh values. That's pretty arbitrary and can
be changed if another shortened name is considered better.
Elle Stone says (cf. bug 791484, comment 9):
> there are several variants of "Lab" out there, with the most commonly
> used (and the version GIMP currently uses) being the 1976 version,
> which uses asterisks to differentiate it from the earlier "Hunter"
> version. So yes, asterisks are technically correct.
Better use the most conventional naming. And as a side effect, it makes
differentiating Lab a* and Alpha shortened names more obvious, while not
making them that much bigger (2 characters instead of one for "a*").
To mark them as different strings with a context, otherwise "B" for Blue
and "B" for Lab b* cannot be translated separately (for instance).
See commit 7ac7b9519f and previous commit.
These labels were shortened but it's difficult for translators to know
what they are, especially when same shortened labels are common to
different concepts.
See commit 7ac7b9519f.
... removed by commit 0f9da165e0, and
improved by this commit.
Our foo-light modes aren't really prepared to handle out-of-range
input. Make sure that in-range input doesn't result in out-of-
range output.
Add a safe_div() function to gimpoperationlayermode-blend.c, and
use it in the relevant blend funcs, instead of plain division.
This function clamps the quotient to some reasonable range, to
avoid infinities, and maps epsilon/... to 0, to avoid NaN. The
latter part results in similar qualitative results to the
corresponding legacy modes, when calculating 0/0.
Oh blasphemy! The Wilber logo in the toolbox can now be disabled
directly via the Preferences dialog (on the Toolbox page).
The logo is staying enabled by default though. Long live Wilber!
...and present linear RGB Histograms
This is step one: implement the feature at all (without new defaults
or proper GUI, cough).
Add boolean "linear" properties to GimpOperationPointFilter,
GimpCurvesConfig and GimpLevelsConfig.
In the filter, simply set the input/output formats to linear in
prepare().
In the curves and levels tools, add "Linear" toggles from hell,
like in the histogram dockable, and make sure things work right
wrt changing and resetting the property, switching from levels
to curves, and picking colors.
The result currently changes when switching a non-nop curves/levels
between perceptual and linear, because adjusting the parameters
between the spaces is not implemented yet.
It appears that GTK+/GNOME don't have an icon with
"help-browser" ID anymore, but we have exactly the icon
that is needed for two menu entries.
Also, the menu entry for "Search and Run..." was using
a system icon for finding stuff, which looked wrong when
used with a symbolic theme.
We could come up with something overly clever like
a dedicated CLI/terminal icon, but people would expect
a magnifier instead (judging by what Blender does)
and that's what we already use for the Zoom tool.
Hence the lazy fix.
In legacy layer modes that may produce out-of-range output given
in-range input, clamp the result after blending and before
compositing, instead of after compositing, to avoid producing
different results than 2.8 in certain cases.
Improve the disabling/enabling of extended input events for the
canvas during enter/leave-notify events, in particular, so that
enter-notify events that are a result of pointer ungrabbing don't
erroneously reeanble extended input events.
Something about the unqueueing and requeueing of the entire event
queue during motion compression fries GTK's brain w.r.t. extended
input events. Instead, have gimp_display_shell_compress_motion()
return the first non-compressed event to the caller, making it
responsible for dispatching it after handling the motion event.
Add "color-profile-path" to GimpDialogConfig to remember the last-used
path in any profile chooser dialog.
Whenever a GimpColorProfileChooserDialog is created, call a new
gimpwidgets-utils helper function that connects to the dialog's "show"
and "response" signals and makes sure "color-profile-path" is set on
the dialog if it doesn't have a current folder already, and sets the
property back to the config object when a profile was actually chosen
from disk.
Fix the crashes from the third zip:
- forgot to guard the other writing place in the RLE decoder
- one byte after the buffer is still one byte too much
- protect against seeking to bogus offsets
Add "clamp-input" (which clamps the input values to [0..1])
and "clamp-output" (which clips the final result to [0..1]),
properties, parameters and GUI to:
- GimpLevelsConfig
- GimpOperationLevels
- The levels tool dialog
- The gimp_drawable_levels() PDB API
The old deprecated gimp_levels() PDB API now sets both clamping
options to TRUE which restores the 2.8 behavior.
Also reorder some stuff in GimpLevelsConfig and elsewhere so the
levels parameters are always in the same order.
Add brush dimension/depth/compression sanity checks for v6 brushes,
and make sure we don't overrun the RLE decoder's destination buffer.
This properly rejects all brushes from the second zip in the bug.
...outside area of Crop Tool -> Highlight option
Add "highlight-opacity" property and turn the controlling GUI into an
expanding toggle that reveals an opacity slider.
...to allow more space for the channel values
to allow the Pointer, Sample Points, and Color Picker Information
dialogs to be narrower and still show the channel value, without the
channel value running over the top of the channel identifier.
Mitch: did even more and also split the coordinates display to two
lines, because on large images the widget's width was flickering
or the labels were overwriting each other.
...in brushes user directory
Consider 8bim section size unsigned, to avoid seeking backward when a
malicious brush includes an 8bim section unknown to GIMP.
This avoids the possibility to start an infinite loop on GIMP start.
Found just a water drop in the ocean, GIMP is still not secure.
Mitch: Added more sanity checks on the Abr's width/height/bytes so now
all brushes in the zip attached to the bug are properly rejected
instead of crashing GIMP.
This ensures that MyPaint default brushes are installed, otherwise this
makes the MyPaint brush tool quite useless and confusing unless MyPaint
is installed (which was making MyPaint a de-facto dependency of GIMP
until this commit!). Also we won't have to guess anymore the right path
for these brushs. The path will be known at compile time.
If the user enters a value in the shear tool dialog, that value must
be honored. Always set the shear direction to the edited axis and
reset the other axis to 0.
Use a GimpSpinScale widget instead of scale entry cruft, it handles
the model vs. view factor of 100.0 correctly. Also modernize the GUI
without using a table.
The C language only promotes data values up to (un)signed int,
which is 32 bit, if no larger data type is used within the
calculation. Having a multiplication of two gint variables,
even if the expected target variable is of type gsize (64 bit),
leads to a possible integer overflow.
This bug can be triggered in gimp_temp_buf_new, which is used
to allocate memory for given supplied dimensions and bytes per
pixel. If triggered, less memory than needed is allocated and
therefore allows out of boundary accesses, either resulting in
possible code execution or information leakage.
While at it, make sure that the supplied format can actually be
resolved to a bytes per pixel value. If not, return NULL.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Replace gimp_parameter_*() utility functions with equivalent
gimp_properties_*() functions which deal with separate arrays of names
and values, and use g_object_new_with_properties() instead of
g_object_newv().
After commit 8029508fbe, we always
render the image in chunks that are at most
GIMP_DISPLAY_RENDER_BUF_WIDTH x GIMP_DISPLAY_RENDER_BUF_HEIGHT,
even when the window's scale factor is > 1.
This reverts commit 40bc5307dc.
It's not *exactly* the same. The floating selection can belong to a
channel or layer mask. Also, this belongs into the GUI layer, not the
core.
Anchoring a floating layer is basically equivalent to merge down. This
is already what we do in other merging actions (flatten image and merge
visible layers).
Refactor GimpDashboard to autogenerate the UI based on a
description of the different variables, fields, and groups.
Allow individual groups to be expanded/collapsed, and individual
fields to be enabled/disabled. Save the relevant state in the
dashboard's aux-info.
Add fields for the new GeglStats properties, as per GEGL commit
25c39ce6c9bb618f06ac96d118e624be66464d74. The new fields are not
enabled by default.
Add "reset" action, to clear the history, and reset cumulative
data.
Current migration code would still return TRUE for success if the copy
of a regular file failed for any reason.
Also getting rid of some weird block and making sure we free dirname
only when it has been assigned a value.
When recursively browsing folders, there is always the risk of infinite
recursivity, in particular with symbolic links which can create loops.
Let's just assume that we don't have any data over 5 levels of
directories to avoid a security risk.
paintbrush.pgm and paintbrush01.pgm are binary identical. If they were
brushes we ship as data, that would be more annoying because they could
be used in scripts or in various other places. But here it looks they
are used for gimpressionist plug-in only, which means they are probably
not used in some random script. All it takes is probably only to
properly migrate gimpressionist presets.
Presets using "paintbrush.pgm" will be migrated to use
"paintbrush01.pgm" instead.
In particular, I noticed that some data can be on 2 levels (or more?).
For instance gimpressionist presets were not migrated from 2.8 to 2.9
because of this.
Override get_invalidated_by_change() with the same logic as
get_required_for_output(), to avoid unnecessary invalidation.
Avoid format conversion when input and aux have the same format.
Add pass-through fast path when the ROI is completely inside/
outside the cropped rectangle.
Use bulk memcpy(), instead of per-pixel test-and-copy, in
process().
This avoids unnecessarily processing regions of the input and/or
aux nodes that will get cropped out. In particular, this avoids
processing cropped-out regions when using the filter tool in split-
view mode.
Since commit a427213fb8, the special glib value "help" does not work
anymore as a helper (listing the list of available flags).
This means that to use GIMP_DEBUG environment variable, one has to
either know them all by heart or check the app/gimp-log.c file.
This commit still leaves "help" as a normal flag for GIMP_LOG_HELP
domain in GIMP, but creates instead "list-all" as the new helper value.
Moreover as a fallback, setting a random value to GIMP_DEBUG with no
valid flags in it will also output the list of available flags. This
way, one doesn't even have to remember a specific string to obtain the
list.
Replace the "lock brush size to zoom" paint option with a "lock
brush to view" option, which links the entire brush transform to
the view transform, so that the brush remains invariant in display
space under scaling, rotation, and reflection.
Add support for reflecting brushes as part of their transformation.
The reflection is performed as the last step of the transformation,
across the vertical axis.
The option to reflect the brush is not exposed in the UI, or
through the PDB, but is intended to be used for linking the brush
transformation to the view transformation, in the next commit.
When you move an endpoint in the Blend Tool, angle and distance
information are especially important, in case you want to draw a
gradient with specific values.
Currently Blend tool only shows the vector coordinates whose usefulness
is a bit of a question. Now it will also show distance (in current shell
unit) and angle!
Add an offset_angle parameter to gimp_constrain_line(), which
offsets the radial lines by a given angle.
Add gimpdisplayshell-utils.[ch], with two new functions:
- gimp_display_shell_get_constrained_line_offset_angle():
Returns the offset angle to be passed to
gimp_constrain_line(), in order to constrain line angles in
display space, according to the shell's rotation angle and
flip mode.
- gimp_display_shell_constrain_line(): A convenience function
which calls gimp_constrain_line() with the said offset angle.
Use the new functions in all instances where we constrain line
angles, so that angles are constrained in display space, rather
than image space.
The only exception is GimpEditSelectionTool, which keeps
constraining angles in image space, since it's not entirely obvious
that we want to constrain angles of dragged layers/selections in
display space.
The file formats GBR and PAT contain names which are supposed to be
NUL-terminated within the files. If no such terminating NUL byte
exists, the parsers of GBR and PAT trigger an out of boundary read
during utf-8 conversion.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
Add gimp_display_shell_[un]transform_with_scale(), which are
similar to gimp_display_shell_[un]transform(), however, they
transform the bounding box to/from uniformly-scaled image space,
given the scale factor as a parameter. These functions are more
accurate than using gimp_display_shell_[un]transform() and applying
the scaling separately, in particular, when the scale matches the
(horizontal or vertical) display scale.
Use these functions in gimp_display_shell_draw_image(), to avoid an
off-by-one error when transforming screen-space chunks to image-
space chunks, which leads to the symptoms described in the bug.
Fix another potential off-by-one error affecting non-uniformly
scaled images, and window scale factors other than 1.
The dashboard dockable shows the current GEGL cache and swap sizes,
and their recent history. It has options to control the update
rate and history duration of the data, and an option to warn (by
raising/blinking the dialog) when the swap size approaches its
limit.
GimpMeter visualizes a set of values that change over time. It
consists of a gauge, showing the most-recent sampled values, a
history graph, showing a plot of the values over time, and an LED,
which can be used as a boolean indicator for some condition.
GimpMeter is used in the dashboard dockable, added in the next
commit.
Replace the GIMP_BOUNDARY_HALF_WAY macro by two others : one for perceptual and
one for linear gamma.
Use the GIMP_BOUNDARY_HALF_WAY_LINEAR to compute channels and floating selection
boundaries.
It never belonged inside "tools". Also rename its "pdb" subdirectory
to "groups". This had to happen before 2.10 so cherry-picking between
branches doesn't become a nightmare in the future.
It turns out we already have the support since it uses the same axis as
the "wheel", used by other devices (for instance the Airbrush pen of
Wacom has a wheel reporting on this same axis).
We can't do any fancy support for this right now, but at least we can
clarify a bit the dynamics naming so that people understands it can be
used for both wheel and rotation input.
Fix the various "Wheel" dynamics strings into "Wheel/Rotation".
Use CAIRO_FILTER_FAST when painting the xfer surface to the
screen. This notably improves performance when the canvas is
rotated, at the cost of lower filtering quality.
Based on a patch by Massimo.
Move the entire image-space/screen-space transformation logic from
gimp_display_shell_render() to gimp_display_shell_draw_image(), so
that the former works entirely in image space, and do the chunking
and clipping in screen-space, making sure that image-space chunks
are never larger than
GIMP_DISPLAY_RENDER_BUF_WIDTH x GIMP_DISPLAY_RENDER_BUF_HEIGHT,
even when the window's scale factor is greater than 1.
Add a GIMP_BRICK_WALL environment variable, which, when set, shows
the screen-space chunk bounds.
In gimp_layer_new(), set opacity and mode using the setter
functions, instead of modifying the members directly, so that all
the necessary side effects take place.
Add gimp_item_get_merged_color_tag(), which returns the color tag
of the nearest ancestor (including the current item) that has a
color tag other than NONE. Use this function in GimpItemTreeView,
instead of gimp_item_get_color_tag(), to set the cell color of
items, so that item's with a NONE color tag inherit the color of
their parent. Add a boolean "inherited" parameter to
gimp_get_color_tag_color(), which indicates if the color tag is the
item's actual color tag, or an inherited color tag, and modify the
returned color accordingly, so that inherited colors are less
saturated/lighter than non-inherited ones.
The free select tool now commits on double click inside a closed
polygon, which caused the foreground select tool to switch modes in
the middle of a click, breaking both its own and its parent class'
state.
Fixed by detecting whether the commit was done by double click and
delaying the mode switch until after the parent class button release
code is done.
Unrelated: Don't call both COMMIT and HALT, the generic tool mechanism
does that automatically now, forgot to port this file.
Keep multi-threading disable on the cage transform operation. It is now
fixed but is a lot much slower than when single-threaded, making it
painful to use on bigger images.
So let's reenable later if we can improve this. See bug 787663.
Override GimpLayer::get_effective_mode() in GimpGroupLayer, to
perform strength-reduction of pass-through groups to normal groups
under certain conditions (see gimp_group_layer_get_effective_mode()
for the logic.)
The main motivation for this is the fact that Photoshop uses pass-
through mode as the default mode for groups, resulting in many PSDs
using pass-through groups generously and unnecessarily. Since
pass-through groups are more expensive that normal groups, reducing
them to normal groups when possible can make a big difference.
Note that, while the results of the strength-reduced composition
are theoretically equivalent, there may be small differences in
practice due to numerical errors, especially when using low
precision. This is unlikely to be an issue, but, just in case,
allow disabling this optimization using the
GIMP_NO_PASS_THROUGH_STRENGTH_REDUCTION environment variable.
gimp_layer_get_effective_mode() returns the actual layer mode,
blend space, comosite space, and composite mode used for the
layer's mode node, allowing them to be different from the values of
the corresponding layer properties. The aim is to allow us to
replace expensive layer configurations with cheaper but equivalent
ones transparently. This is used in the next commit to replace
pass-through groups with normal groups under certain conditions.
The effective values are computed by the new
GimpLayer::get_effective_mode() virtual function. The default
implementation provided by GimpLayer returns the corresponding
layer properties as-is (replaceing AUTO with concrete values).
Subclasses can override this function, providing more
sophisticated logic.
When attaching a layer as a floating selection to a drawable,
unbind its visiblility from its activeness, as per the previous
commit, and use its visibility to control the activeness of the
drawable's floating selection filter.
Properly update the drawable when the floating selection's
visibility and excludes-backdrop properties change.
Add an "active" property to GimpFilter, which replaces its
"visible" property. The new property assumes the lower-level role
"visible" had -- controlling whether the filter has any effect as
part of its parent filter-stack.
Add a "visible" property to GimpItem, separate from the "active"
property, which assumes the higher-level role "visible" had --
controlling whether the item is considered "visible", as per the
GUI. By default, the item's "visible" property is bound to the
filter's "active" property, so that changes in visibility directly
affect the filter's "activeness"; this binding can be controlled
using the new gimp_item_bind_visible_to_active() function.
This distinction is currently necessary for floating selections.
Floating selection layers must not be active in their parent stack,
regardless of their visibility, in particular, so that their mode
node doesn't hide the entire backdrop when their composite mode
excludes the backdrop (i.e., when it's dst-atop or src-in).
Instead, their visibility should affect the activeness of the
floating-selection filter of the drawable they're attached to.
This is handled by the next commit.
Currently, when a GimpFilter's visibility changes, we rely on its
various visibility-changed signal handlers to rewire the filter
node's graph to reflect the change. This has two main
disadvantages:
- There's no easy, generic way to toggle a filter's effect,
especially one that is not subclassed, since GimpFilter only
takes care of the case where visibility becomes FALSE, and does
nothing by itself when it becomes TRUE again.
- While GimpDrawable does handle the visibility => TRUE case, it
doesn't disconnect the filter's input from its mode and
(potentially) source nodes when it becomes invisible. As a
result, while none of the drawable's graph is processed as part
of the composition when not visible, the mode and source nodes
do get invalidated when the filter's input is invalidated, such
as while painting on a layer below the drawable. This is
particularly bad for pass-through groups, since their source
node can be an arbitrarily complex graph, whose invlidation
incurs a nontrivial overhead.
Instead, don't touch the filter's node at all when visibility
changes, and rather have GimpFilterStack remove it from the graph
entirely when the filter becomes invisible, and re-add it once it
becomes visible again. This solves both of the above problems, as
well as simplifies the code.
When merging a drawable filter, we call
gimp_gegl_apply_cached_operation() on a node that's part of the
drawable's filter stack graph. The function rewires the node's
input, and doesn't restore its original input connection before
returning, leaving the graph in an inconsistent state. Currently,
this doesn't matter, since we remove the filter right after that,
but the next commit expects the filter stack graph to remain
consistent.
Remember the original source node of "operation" in
gimp_gegl_apply_cached_operation(), and restore it upon exit, to
fix that.
Since commit ff59aebbe8, blur-gauss plug-in and the associated
"plug-in-gauss" action don't exist anymore. Migrate any custom shortcut
to "filters-gaussian-blur", which is the expected replacement.
See also bug 775931.
Current logics of dealing with duplicate accelerators was just to delete
one randomly. This works ok in most case since we can't be in the head
of people and can't know which one they want to keep (yet we can't keep
both because that makes it very complicated to reset the shortcut
appropriately by hand/GUI, without a tedious work of researching which
other action uses the same shortcut. See commit 2a232398c4).
There is still some cases where we can be a bit more clever than random
deletion: when one of the accelerator is mapped to a non-existing
action. In this case, let's delete this accelerator in priority. Not
only the chances of this being the expected result are higher; but even
worse, there is anyway no GUI way to delete the accelerator for the
non-existing action! Thus you could try and reset your existing action's
shortcut as many times as you want in the GUI, it would always end up
deleted at next startup!
Note that if the non-existing action's shortcut has no duplicate, it
won't be deleted. This ensure that you could uninstall a plugin, then
reinstall it later and still have your custom shortcuts saved in the
meantime. A shortcut of a non-existing action will *only* be cleaned out
if it is redundant with the shortcut of an existing action.
Use gimp:buffer-source-validate, introduced in the previous commit,
for the source node of GimpDrawables. This avoids threading issues
with layer groups, or any other drawables that may use a validating
buffer, by making sure the buffer is validated before any
succeeding operations, and hence the associated graph is processed
on the same thread as the parent composition.
Restore multithreaded processing in GimpOperationLayerMode.
gimp:buffer-source-validate is a drop-in replacement for
gegl:buffer-source, however, if the attached buffer has a
validating tile-handler, it makes sure the required region is
validated during process(). This avoids a situation in which
validation happens in different worker threads at the same time
during the processing of a succeeding operation; since validation
is protected by the buffer's tile-storage mutex, this can result in
either a deadlock (currently), or an effective fallback to single-
threaded processing.
When switching a group layer from pass-through mode to a non-pass-
through mode, flush the projection synchrnously, to make sure that
the projection's buffer gets properly invalidated at that point,
and can be subsequently used as a source for the rest of the
composition.
Add a boolean 'pass_through' member to GimpGroupLayerPrivate, which
indicates if the group is a pass-through group, and use it instead
of checking the group's mode, so that we can detect that the group
used to be a pass-through group when the mode changes, and to
simplify the rest of the code.
Set the priority of group-layer projections according to the group
layer's depth, so that top-level groups have a priority of 1
(compared to a priority of 0 for the image projection), and nested
groups have a priority one greater than their parent (in other
words, shallower groups have higher priority than deeper groups,
all of which have lower priority than the image.)
This makes pass-through groups much faster, in particular.
... which control the priority of the projection's idle source.
The projection's priority is specified relatively to
GIMP_PRIORITY_PROJECTION_IDLE (i.e., a priority of 1 results in an
idle source with priority GIMP_PRIORITY_PROJECTION_IDLE + 1, etc.)
Add GimpViewable::ancestry-changed signal, which is emitted when
the viewable's ancestry changes, i.e., when its parent, or the
parent of one of its ancestors, changes.
Add gimp_viewable_get_depth() function, which returns the
viewable's depth in the hierarchy, i.e., its ancestor count.
...is a regression in common cases
Commit the free select tool on double click inside the polygon.
Done by implementing GimpCanvasItem::hit() in GimpCanvasPolygon, using
ugly code.
Massimo is worried that it could unload the module (maybe in some
specific cases?), which could indeed happen when the g_type_class_ref()
just before was the first call to the class (hence it's the only ref).
So let's just unref() in the exit() function instead.
Not sure if g_type_class_ref() can actually return NULL here, but let's
add a check, just in case.
Also unref() after since we ref-ed it ourselves.
Finally reorganize a bit to keep the private functions together and
named appropriately, clean some tabs and add a comment to remind of
further check/cleanup once we port to GTK+3.
Comment by Jehan after review:
"Quick and dirty hack" but a working one. Since the bug will likely
disappear with the GTK+3 port (to be verified) which uses another theme
system, let's just do it this way.
The value descriptions of GimpGradientColor,
GimpGradientSegmentColor, and GimpGradientSegmentType enums appear
in the on-canvas gradient editor UI, as combo-box items in the tool
GUI overlay. Since we want to keep the overlay as small as
possible, we previously used abbreviations for these descriptions
(e.g., "FG (t)", instead of "Foreground (transparent)").
Replace the abbreviated descriptions with unabbreviated ones, and
move the abbreviations to the "abbrev" parameter. This way we get
the abbreviated version in the combo-box, and the full version in
the combo-box's menu.
Update the dprod production of generated enum files to include
abbreviated value descriptions, as per the previous commits.
Add a comment for translators above the abbreviated descriptions,
specifying the full description they abbreviate.
Temporarily disable multithreading for GimpOperationLayerMode, to
avoid the deadlock. The environment variable
GIMP_MULTITHREADED_COMPOSITING can be set to reenable it, for the
sake of debugging.
This reverts commit 4bd118ec8a.
The mutex introduced by the above commit should no longer be
necessary, after GEGL commit
8b034c437b0162b26f85eb80867914977ac3cf57.
When the group's offset changes, update the item's offset *after*
updating the group's offset node, so that the item's offset nodes
and the group's offset node are in sync when the corresponding
"notify" signals are emitted.
Same as for the color tags issue, short labels look much better in
menus. On the other hand, the longer description needs to be as a
tooltip, otherwise there is not enough information in the action search
to distinguish one action purpose from another.
It is possible to trigger a heap overflow while opening a malicious
pattern due to integer overflows.
The validation is adopted from plugin-parser. It also takes a proper
cast to gsize to avoid integer overflow in size calculation.
This reverts commit 189a474502.
As Mitch notes, this does not look that good in the menus. As for the
action search, since the tooltip is still shown below, the shortness and
duplication of the action labels make it less a problem.
...after exporting the image
Call gimp_image_name_changed() in both gimp_image_clean_all() and
gimp_image_export_clean_all() so we clear the cached displayed URI in
all cases, even if this means we're emitting "name-changed"
redundantly some times.
When a single blend-tool action adds and removes the same gradient
stop, restore the original gradient, rather than actually adding
and removing the stop, so that the affected midpoint returns to its
original state at the beginning of the action, rather than being
reset (and, consequently, so that the redo stack isn't lost.)
... from the undo stack
When a blend-tool edit action modifies the gradient, do a deep
comparison of the original gradient against the current gradient,
to test if anything changed, instead of just assuming that
something did change.
Current labels were very uninformative while tooltips contained what
should have been the labels. Just switch these.
Also replace GIMP_ICON_CLOSE by GIMP_ICON_EDIT_CLEAR for the various
*-color-tag-none actions. As a comment was reminding next to these
icons, the close icon was abused. The edit-clear icon on the other hand
is quite relevant.
Looking at most action labels, it seems the "Title Case" mixed-case
style has to be applied. Fix the few labels I found which were not
following this case style.
Also no need to have a tooltip when it is basically the same as the
label.
There were 4 actions displaying as "Visible" only: channels-visible,
drawable-visible, layers-visible and vectors-visible. This was not very
useful to differentiate them (for instance in action search). Just make
clearer labels.
Now add also flip information in the status bar so that one knows that
the canvas is flipped horizontally and/or vertically. Especially if you
often flip and rotate the canvas (or if you did it by mistake), at some
point, it may become confusing to remember whether this is the case. Now
it will be possible to check in a single glimpse at the status bar.
Similarly to what I previously did for the rotation information, hitting
the flip icons in status will allow to unflip easily without having to
go in menus or remember all shortcuts.
These information will be visible only when the canvas is flipped or
rotated.
Both view-rotate-other and view-zoom-other had for label "Othe_r...".
This is quite vague in particular when in out-of-menu contexts (i.e. the
action search).
In GimpCanvasTransformPreview, use the transform matrix to
determine if we're doing a perspective transform, rather than
relying on a separate property, so that we don't use the slow
perspective path unnecessarily.
Consequently, remove the does_perspective member of
GimpTransformTool, since it's no longer used.
Add "In Place" variants for all sorts of pasting:
- extend the GimpPasteType enum with IN_PLACE values
- add the needed actions and menu items
- merge the action callbacks into one, taking an enum value as parameter
- refactor the pasting code in gimp-edit.c into smaller functions
We probably have too menu items in the "Edit" menu now, needs to be
sorted out.
In gimp_group_layer_update_size(), when the layer's bounds have
changed, update the group layer's offset before the call to
gimp_pickable_flush() when reallocating the projection. Otherwise,
if the group layer's graph isn't constructed yet, it will get
constructed during the call to gimp_pickable_flush(), however,
gimp_group_layer_get_graph() will pick up the old coordinates for
the offset node.
... upon NaN values
Make the histogram bin calculation NaN-safe, by mapping NaNs to 0.
Ideally, NaNs should probably not be counted at all, but since we
already count negative values as 0, and > 1 values as 1, we might
as well not pessimize performance over it, at least until we add
support for unbounded histograms.
At the same time, improve rounding in the bin calculation, so that
the result is more accurate.
Return FALSE from gimp_display_shell_has_filter() when there are
filters, but they're all disabled, to avoid unnecessary extra
color conversions during rendering.
... leading to a crash
Add gimp_data_is_copyable() and gimp_data_is_dulicatable().
Use gimp_data_is_duplicatable() when setting the sensitivity of the
various "foo-duplicate" actions, instead of inspecting the object's
GimpDataClass::duplicate pointer directly, since this is no longer
an indication of whether a GimpData object is duplicatable or not
(since commit 33de4d5530).
When copying a generated brush, copy its "spacing" property, in
addition to the other properties, which hasn't been previously
copied by ::duplicate().
Finish up commit 17583ff04a, which
ported GimpGradient from ::duplicate() to ::copy(), by doing the
same for the rest of the GimpData subclasses that implement
::duplicate().
We still keep GimpData's ::duplicate() virtual function around,
even though it now points to the default implementation (which uses
::copy()) for all subclasses, since ::copy() is stronger than
::duplicate(), and we might want to have certain GimpData types
that are duplicatable, but not copyable.
When we have display filters, break the color profile transform in
two: first, convert from the image profile to sRGB, then apply the
filters, then convert from sRGB to the monitor profile.
When a display filter's configure() function returns NULL, use a
propgui for the filter, instead of not showing a widget at all, to
spare filters the need to manually construct a configuration gui.
When processing display filters, shift the filter buffer to the
top-left corner of the render area, and pass the actual render
area, instead of an area whose top-left coords are (0, 0), to the
display filter. This allows for position-dependent display
filters.
When applying a relative adjustment to a spin scale, don't wrap the
pointer around the corresponding screen edge if the spin scale's
value is already minimal/maximal.
While applying a relative spin scale adjusment (i.e., when dragging
from the lower half of the spin scale), wrap the pointer around the
screen edges (of the current monitor), so that the maximal possible
adjustment amount isn't artifically limited by the screen geometry.
This is especially useful for spin scales in dockables, since
dockables are normally placed near the edge of the screen.
When the mouse is released, move the pointer back to its initial
position (at the beginning of the drag), to allow for subsequent
adjustments.
Unfortunately, moving the pointer programatically isn't supported
on all envrionments (Wayland, Xephyr, ...), and worse yet,
detecting that the pointer failed to move is tricky, so we have to
resort to an ungly hack to maintain the current behavior in this
case. Gah :P
Small fix to last commit: make the name entry editable when the
data is renamable, even if it's not otherwise writable (completely
hypothetical for now.)
