This started as yet another report of leak by Massimo. But really the
leak of the GdkPoint created by the function
gimp_display_shell_push_zoom_focus_pointer_pos() is not only when
delta_y is 0. There are a few code paths in gimp_display_shell_scale()
when we would not pop this point. One of them is for instance when
window resizing in multi-window mode is allowed. There might be more
(but the code is convoluted enough not to be 100% sure if these are
possible with our specific case).
This specific function was initially created only to be used for unit
testing code (commit 7e3898da09), but it ended up being also used
internally (commit 792cd581a2). Since I see that the test for which
this code was initially created even seem broken right now (the assert
part for position check is commented out!), I even wonder if we should
keep it. We could indeed instead just add optional start_x|y arguments
to gimp_display_shell_scale(), which would be much cleaner. But I leave
it for now.
Instead I just make sure we clean the created GdkPoint after calling
gimp_display_shell_scale(). Also I get rid of the GQueue. It is clear in
the code that we are not expecting queuing interaction of several
positions. Worse right now, we could end up in weird cases where the
pushed points are not used when they should, then could end up being
used later in totally unrelated interactions (this would make the shell
position jump here and there). So let's just make it a single point.
Finally adding some appropriate comments in parts which are still a bit
wrong.
This function is not only used as "response" callback to the merge
dialog, but also called directly with a NULL dialog in
image_merge_layers_last_vals_cmd_callback().
This commit fixes:
> (gimp-2.99:130330): Gtk-CRITICAL **: 22:17:59.774: gtk_widget_destroy: assertion 'GTK_IS_WIDGET (widget)' failed
Reported by Massimo.
A few issues existed in the code:
- When the layers selection is changed, make sure we remove all
duplicates (not only the first one). This should not be a problem
anyway because we also do this duplication cleanup elsewhere now, but
still…
- Fix gimp_image_layer_stack_cmp(): we were comparing a GList element to
the data of another, so we were actually never finding duplicates.
- Add gimp_image_clean_layer_stack() for internal layers stack
management/cleanup. It takes care of recursively making sure we don't
leave duplicates, and remove all empty lists.
- Now use this new cleanup function inside
gimp_image_remove_from_layer_stack() instead of doing some incomplete
and broken element removal. This was especially broken as we were
removing a GSList element from a list we were iterating on (so we were
dereferencing a now freed element). This last issue was reported by
Massimo, and this is how I found the more general failure in this
layers stack management.
This fixes a memory leak as reported by Massimo.
But even more, we fix the code logics, as the cleanup items were never
added to the `item_cleanups` list.
As noted in issue #6032 the error message when trying to open a pattern
file with dimension larger than supported could be a little clearer.
We do this by adding the maximum allowed dimensions to the error message.
For GIMP patterns we have maximum allowed dimensions which we check when
loading a pattern. However, we did not check this when saving a pattern.
See issue #6032.
This commit adds a check when saving a pattern and adds a descriptive
error to make clear why saving fails.
After the change that allows multiple layers to be selected exporting to
a pattern fails.
Patch this as suggested by Lloyd Konneker by doing the same as for brushes:
do not allow multiple layers, instead only send the first drawable.
On Windows loading metadata from images with non ASCII characters in their
path failed. Part of the fix is in gexiv2 that now converts the path from
utf-8 to utf-16 on Windows.
However we were still sending a localized path to gexiv2 where it was
expecting utf-8. This caused the conversion and thus loading of metadata
to fail. Fix is to remove the special handling for Windows and use the
utf-8 filename.
Due to differences between Windows and most other platforms Ghostscript
didn't correctly load files with special characters on Windows.
First we needed to make sure that the filenames we use are in utf-8
format and then tell gsapi that we use utf8 encoding.
When exporting to bmp using a script or plug-in we could not set the
parameters use-rle, write-color-space-info, and rgb-format when used
non-interactively.
This is discussed in issue #491. The patch there does not work for master,
so I came up with this.
When exporting a C source file with runtime length encoding, the
C-string's array size does not accomodate for the null byte. However,
GIMP accomodates for the NULL byte in it's NON-RLE export, suggesting
that this has been a mere oversight for RLE.
This can cause at the worst a compile-time error and at least a warning
from the compiler.
I somewhat bisected GEGL commits between 0.4.20 and 0.4.24 and found
that the one that introduced the env var GEGL_OPERATION_INIT_OUTPUT is
the first showing the problem.
Reviewer (Jehan) note: so it would be commit 6e9610e65c on GEGL repo.
This fix makes sense as it means that since this commit, the output
buffer could have random values. It's not a problem for any operation
where we fill every value, but I guess it's not the case for
"gimp:cage-coef-calc" which was likely relying on the old behavior of
being 0-initialized.
Reviewer comment (Jehan): we have used this patch successfully on our
installers since start of 2021 (see commit b4d665d of our gtk-osx fork)
and it really improved the situation. I only fixed minor coding style
stuff in the patch.
Looking at what it does, I guess it is not ideal long-term if related to
10-bit display (as I understand from the comment), which a graphics app
would want to support properly. But for now, this is better than
extra-slow display until we get macOS developers able to look at this
more in depth in the future (I don't think that our dependencies are
really ready yet for 10-bit display support anyway, though I may be
wrong).
Some other forums seem to say it comes from macOS invalidating now more
than it should (i.e. the whole area instead of only the changed area)
and this NSViewUsesAutomaticLayerBackingStores flag would disable this
behavior. It might be one of these reasons, the other or both. This is
anyway a good first start for future contributors.
Andre Klapper