I'm still very unclear why exactly but it would seem that just queuing
the redraw with an idle function is not enough. At least on Windows,
Jacob was having cases where opening an image would get stuck unless the
mouse was moved (causing draw events most likely).
So let's use a timeout function instead. Probably no need to queue the
idle followed by the timeout function as we had before commit
4fee04b839. Instead just directly queue a draw if relevant, then run the
timeout at regular interval (marching ants speed).
This way, we would queue a lot less canvas region unnecessary redraws.
We still remake the time-before-last-draw check in the draw() signal
handling before we want to update the marching ants index for draw
events coming for other reasons (canvas updates, moving/zooming on
canvas, exposition changes, etc.).
… displayed.
We should use the dimensions from the GimpDisplayShell not the the
GimpCanvas. Indeed the canvas is shorter when rulers are visible, hence
the selection next to the extreme sides (bottom and right sides of the
canvas) was not drawn.
As suggested in a comment (itself coming from an IRC discussion), we
should not use gdk_window_(begin|end)_draw_frame() functions as this
works on X, but not on Wayland anymore. Instead draw directly during
draw() call of the shell widget, and force it to happen regularly, to
update the marching ants, via gtk_widget_queue_draw_region().
This is tested and works on Wayland. Please everyone, test thoroughly to
make sure it works well in all situations, and also that we don't get
any unexpected slowdowns.
Since the symptoms are very similar, it is highly possible that it also
fixes the issue #5952 too, for selection not showing on macOS since Big
Sur 11 (maybe they changed the same way as Wayland did). Unfortunately I
can't check this myself. Please test, whoever has access to a macOS Big
Sur and can build GIMP!
Simply it should free only GimpProgressDialog as these would be
dedicated dialog (with no meaning once progression is done), and leave
alone other GimpProgress widgets. In any case, it should not output
CRITICAL errors on these.
Fixing the following CRITICAL:
> GIMP-CRITICAL: gui_free_progress: assertion 'GIMP_IS_PROGRESS_DIALOG (progress)' failed
When dropping an image on the toolbox.
… waitpid().
My use case was when testing gimp_brush_select_new(). When doing tests,
if I let the brush dialog opened while letting the plug-in exit cleanly
otherwise, it somehow locked the process (I have stared so long at the
code and still don't understand why).
gimp_plug_in_close() would wait for it forever and the only way out was
to kill GIMP completely. I guess we could try to run waitpid() with
WNOHANG (and finally force-kill the child if it really won't exit) but
it made the whole logics extra complicated.
The logics with g_child_watch_add_full() is that we don't stop waiting
for the child and just set a callback to finalize what needs to be. Now
the worst case scenario would be to leave zombie processes dangling
around, but it's better than freezing GIMP.
Finally as a weird side effect, doing this change even unblocked the
process with an unfinished brush selector so we don't even have a zombie
anymore (at least for this specific case). All good in the end!
Last side effect: it can speed up a tiny bit the plug-in close as we
don't wait for processes anymore, which could be more visible at first
startups (when we reload all the plug-ins). Though there is nothing
scientific to my numbers, after multiple "first startups", it seems it
went down from about 3.5 to 3.2 seconds on my specific machine. Nothing
extra fancy, but we take what we can (and speeding up the startup was
never the goal of this change anyway). It doesn't affect Windows which
has its own logics to handle process termination and I preferred not to
touch it.
There are 2 parts for this fix:
- First expect the GimpPdbDialog to possibly disappear while
gimp_pdb_dialog_run_callback() is running. This can indeed happen as
this core dialog is tied to a PDB call. If the calling processus
crashes (which may happen, and has to be expected for third-party
plug-ins), then this dialog may just end up closing at anytime (signal
"plug-in-closed" from the plug-in manager which implies a
GTK_RESPONSE_CLOSE, hence dialog destruction).
To account for this, we check the dialog availability with a weak
pointer and returns the info to the caller as well.
- Don't connect to "value-changed" on the spacing adjustment because
when a crash happened, I had cases when the adjustment was finalized
while being set (crash in GTK code). This one is a bit harder to
explain (I had to look long at backtraces) but having a proper signal
"spacing-changed" on the GimpBrushFactoryView is actually much cleaner
code than relying on a public object anyway and it fixes this crash.
Note: this fix is related to my previous commit. When running
gimp_brush_select_new() from Python code, the plug-in crashed when
trying to run the callback, which also resulted into core crash (and
this part is obviously not acceptable at all).
When a selection exists, we are copying then pasting the selection
contents. In particular, with multi-layer selection, it means pasting a
merged result of the selected layers (like a sample merged but limited
to selected layers).
Yet when no selection exists, with a single layer selected, a cut in
particular would remove the layer fully, then a paste would copy it
elsewhere (in the same image or even on a different image). This was
still working, but not with multiple layers. This is now fixed and we
can now copy/cut then paste several layers (without merge), which is
sometimes a very practical way to move layers (sometimes simpler than
drag'n drop, especially between images).
As a consequence, the PDB function gimp_edit_paste() now also returns an
array of layers (not a single layer).
Removing useless condition, add a g_return_if_fail() assertion for the
only (impossible unless bug) case which we don't expect. Also set
default mindist to G_MAXLONG instead of a magic number (which was ok now
but might become a problem if some day colormap allowed more than 16-bit
per channel colors).
Finally break when we reach a distance of 0 since we won't get lower
anyway, so better stop early.
Thanks to Rafał Mikrut and Øyvind Kolås for code commenting.
We cannot just compare the drawable format with the model-type specs of
the color model. We need to include the space now.
In my case, some random screenshot converted to gray then indexed would
assert because the format is "Y' u8-space-gray-sRGB" (or for layers with
alpha: "Y'A u8-space-gray-sRGB"), hence indexed conversion failed and
ended up dark.
With my previous commit, I improved the search action display and search
algorithm (which was returning wrong results), but we had lost showing
the non-sensitive reason in menu item tooltips. This fixes it, by
actually appending the reason, but only in the end, on the GtkWidget
tooltip (not in the action's tooltip itself).
In some cases, in particular for actions generated from plug-in
procedure right now, we were displaying the reason of the insensitivity
(typically right now, only the drawable type is cited). This was done by
appending the reason to the tooltip, separated by 2 newlines, which
resulted in extra ugly design, no nice way to style this info directly
(with pango for instance if the widget display allows it, or on a
separate info widget in a possible future, or whatnot).
Also it would mean that the action search could match a disabled action
by mistake if a search word happens to be in the reason message.
This improves the situation with the following changes:
* gimp_action_set_sensitive() now takes an optional reason string to set
the reason message.
* Same for gimp_action_group_set_action_sensitive().
* gimp_action_get_sensitive() returns an optional reason string.
* gimp_procedure_get_sensitive()'s tooltip return value now becomes a
reason (it won't contain anymore the tooltip and the reason
concatenated, only the reason for separate processing).
… single drawable run() function.
All Scheme scripts use a single drawable and I am not so sure where to
change this (or rather I hope someone will handle this rather than I).
So let's not output a warning which would result into a stacktrace,
blocking the GUI for a second or 2 and displaying an annoying popup each
time. Let's just output to stderr for now until we get into a better
state.
which will be shown when selected template pixel density don't
match the image pixel density.
Then user can either keep current image ppi and scale the
template or set image ppi to match the template ppi.
Density selector displayed only when template and image ppi
differs and template unit is matches one of real world units
(e.g. inch, mm, etc).
Closes: GNOME/gimp#1140
This should not be necessary and I could not find the bug in GIMP. It
might be in GTK. In any case, after hiding, then showing back the export
dialog, the file name is right but not the output folder (apparently
revert to current working directory?).
With this, we force it to be the same folder as we left the dialog.
The gimp_drawable_type() is an issue though as gimp_drawable_get_type()
is already defined as a common GObject API.
Though I'm actually wondering if GimpImageType is well called. Rather
than Type, shouldn't we go with ColorModel?
sed -i 's/\<gimp_drawable_bpp\>/gimp_drawable_get_bpp/g' "$@"
sed -i 's/\<gimp_drawable_width\>/gimp_drawable_get_width/g' "$@"
sed -i 's/\<gimp_drawable_height\>/gimp_drawable_get_height/g' "$@"
sed -i 's/\<gimp_drawable_offsets\>/gimp_drawable_get_offsets/g' "$@"
s/gimp_image_base_type/gimp_image_get_base_type/
s/gimp_image_width/gimp_image_get_width/
s/gimp_image_height/gimp_image_get_height/
Sorry plug-in developers, more porting work! But really this seems like
the right thing to do in order not to get stuck with inconsistent naming
for many more years to come.
… and s/gimp_parasite_flags/gimp_parasite_get_flags/
It is better to have a consistent API and the fact is that most
getter/setter functions use the _get|set_ naming, but these didn't.
I wondered if this was such a great idea to rename these anyway because
even though we are breaking API in GIMP 3, is it the best idea to rename
when no functional change happened? After discussing with Wormnest, we
still agreed it was still better to go for consistency rather than
regret later (and be stuck with these names for many more years).
Also this fixes these warnings:
> [649/1205] Generating gimp-3.0.vapi with a custom command
> Gimp-3.0.gir:24162.7-24162.56: warning: Field `Gimp.Parasite.flags' conflicts with method of the same name
> Gimp-3.0.gir:24318.7-24318.52: warning: Field `Gimp.Parasite.name' conflicts with method of the same name
Removing this check makes the treatment of LittleCMS consistent with
all the other dependencies checked in the same file, which only check
that the runtime version is at least the required version.
As long as we were compiled against LittleCMS >= 2.8, and are now
running against a version that has at least the same symbols, it doesn't
necessarily matter whether the version we are running against is the
same one we were compiled against.
Distributions like Debian and Ubuntu track the versions in which
individual symbols were introduced, which allows runtime dependencies
to be weakened when no newer symbols are actually used; this is
practically necessary when working with very large numbers of packages,
to avoid a new version of a dependency library unnecessarily blocking
upgrade of dependent packages. However, this doesn't work if dependent
packages add their own checks that bypass this mechanism.
Signed-off-by: Simon McVittie <smcv@debian.org>
LittleCMS 2.12.0 defines LCMS_VERSION as 2120. We want to print that
as 2.12.0, not 2.2.0.
Resolves: https://gitlab.gnome.org/GNOME/gimp/-/issues/6505
Signed-off-by: Simon McVittie <smcv@debian.org>
I always found the docs misleading because when it says "Returns the
list of layers contained in the specified image", I really read "all the
layers, at any level", except it doesn't. It only returns the root
layers and it is up to the plug-in developer to loop through these if
one needs to go deeper.
So let's make the function docs clearer.
… drawable array instead of a single drawable.
Instead of expecting a single drawable, GimpImageProcedure's run()
function will now have an array of drawable as parameter.
As a consequence, all existing plug-ins are broken again. I am going to
fix them in the next commit so that this change can be easily reviewed
and examined if needed later.
I only fix the Vala demo plug-in now (or rather, I just use the first
layer in the array for now) because otherwise the build fails.
The new function gimp_procedure_set_sensitivity_mask() allows plug-ins
to tell when a procedure should be marked as sensitive or not.
gimp_procedure_get_sensitivity_mask() retrieves this information.
Currently plug-ins are automatically marked as sensitive when an image
is present and a single drawable is selected. Nowadays, we can have
multiple selected layers so we should allow plug-ins to tell us if they
support working on multiple drawables. Actually we could even imagine
new plug-ins which would be made to work only on multiple drawables.
Oppositely, there are a lot of plug-ins which don't care at all if any
drawable is selected at all (so we should allow no drawable selected).
Finally why not even imagine plug-ins which don't care if no image is
shown? E.g. plug-ins to create new images or whatnot. This new API
allows our core to know all this and show procedure sensitivity
accordingly. By default, when the function is not called, the 1 image
with 1 drawable selected case is the default, allowing existing plug-ins
easier update.
Note: this only handles the sensitivity part right now. A plug-in which
would advertize working on several layer would still not work, because
the core won't allow sending several layers. It's coming in further
commits.
See the comments in MR!424.
Basically realpath relies on false assumptions (probably ones which used
to be true when the API got created) on the max size of a path. Actually
nowadays paths can be much bigger than what the macro advertizes or can
even be unbounded.
The Linux version of realpath() allows the second parameter to be NULL,
in which case it would allocate the buffer, exactly for this reason
(written in the BUGS section on the man). Unfortunately this behavior is
not standardized in POSIX and the man from Apple I found does not
indicate it will do this.
So let's use g_canonicalize_filename() instead, which seems to do the
right thing. Similarly use g_strdup_printf instead of g_snprintf().
Crop tool used stale color / pattern values when performed crop with
'Allow growing' option. Its context was not notified when fg/bg/pattern
value was changed.
Closes: #4103
Was caused by widget tool fixed_center_x and fixed_center_y coordinates set to
coordinates of mouse click instead of rectangle center after converting channel
selection bbox to rectangle.
Now rectangle fixed_center_x and fixed_center_y coordinates are always updated
when tool widget x1, x2, y1, or y2 coordinates are updated.
Closes#6487
It isn't being used by any plug-in or any code in GIMP at all even.
Let's get rid of it while we can still break API, so we can cut down on
all the complexity of the gimp-param stuff a bit.
Allow guides with off-canvas position since we can now view and work
outside the canvas.
The guide deletion interaction does not change too much, except you
don't delete a guide because it's dropped off-canvas but off the display
viewport area instead.
The XCF format is technically unchanged as PROP_GUIDES was already
expecting an int32 coordinate (with max value at GIMP_MAX_IMAGE_SIZE,
way below uint32 max anyway). Yet our code and XCF docs was explicitly
asking to ignore negative coordinate guides, which makes a change in
behavior for the XCF parser, hence a new version XCF 15. Loading will
still ignore negative position guides (even also bigger than image
dimension guides now) for XCF 14 and below, but will now accept any
position for XCF 15 and above.
Jehan