I find these options always a bit unwise when people have no idea how to change
their mind. Therefore adding a tooltip with the information (though maybe we
should even do it even more clearly and adding that in checkbox text too?).
For this to happen, I've updated the main question to be "Keep the Embedded
Working Space?" instead of "Convert to * Working Space?". Basically now we put
emphasis on keeping the image's color profile as being the proposed action.
Therefore "Keep" is the default answer, on the right.
Yet "Convert" stays on the right too. There is no "Cancel" button anymore
because it is not a question we can really escape even though "Esc" still works,
and it still corresponds to "Keep", same as Enter now. The reason is that "Keep"
is actually the non-destructive option. So if someone really doesn't want to
answer and make an explicit choice, we do it for them with the only
non-destructive choice possible.
Also I'm changing "Convert" mnemonic to 'c'. I guess that earlier devs might
have used 'o' because it is the usual mnemonic for "OK", so it was right when
the question was "Convert?". But since now the question is the opposite, there
is no reason to make it look like "OK". Let's just use the first letter, which
is easier to guess (and not taken by any other button here).
I've been hesitating because I don't think "Convert" was the right default
anymore with our anyRGB move. Until now, it was kinda implying that sRGB (when
no preferred color profile was set) was kinda the recommended space. It's not.
If a file already has another space, it's probably much better to keep it.
First because "Convert" is often a destructive change (even more if the target
is sRGB), so for people who don't necessarily understand all the ins and outs,
we were kinda promoting a destructive default behavior.
On the other hand (and that's why I was hesitating):
* "Keep" is also what happens when we hit "Esc" so some might complain that
there is no easy "Convert" key anymore (there is alt-c, but it's for sure not
as easy as Esc or Enter). Basically it makes the "Convert" option always a
much more explicit choice (yet it is likely for the best for people who care
about their colors).
* There might have been many people who would just use the default (Convert
until now) without thinking too much of what was asked, and that was working
good because they would just publish on the web or similar basic usage. If
they continue just accepting with Enter without reading and understanding what
is asked, now they could publish images with a color profile (and we know it
doesn't always work perfectly on the web).
In other words, it might be the best default for professional usage now, yet
it requires to understand a bit what you are doing.
It's really a hard call!
Thanks to a report by Stefan Cornelius, we became aware that the flame
plug-in does no checking for correct input when loading a pre-saved
settings file.
I reworked the parser to read one or more values based on the type of
token, making sure we also don't read past the end of our token buffer.
All int values have a min and max value set. If any unexpected input is
encountered, we will give a warning.
The flame plug-in can read stored settings from a file. The expected
input is that a ; signifies the end of input.
However, with user input we cannot depend on this to be true, so we need
to make sure that we do not read past the end of our input buffer.
To do so, we add a length check.
We were using the plug-in name with underscores, which is incorrect.
Since nobody ever complained about this, this doesn't seem to be used
very often.
We will use the const that defines the plug-in name instead.
I added this option in commit 38d6783299. It makes the script stateful, able to
remember all previously processed DLL, hence making the whole job much faster
and efficient.
Adds a toggle to let users load TIFF pages with FILETYPE_REDUCEDIMAGE
tags. The specs do not state that these are always thumbnails, and they
may have other uses. We assume that if only one page with this metadata
exists then it is a thumbnail; otherwise, the toggle option is
enabled on load.
This was the last remaining bit in #8124. Basically I needed to check how
localization of menu paths worked. I was thinking of maybe have 2 arguments to
gimp_procedure_add_menu_path(), one non-localized (for default menu paths) and
one localized by the plug-in (for custom menus). That would break all plug-ins,
but also looking at our code, it's complicated to do right.
Instead let's just keep current API and add an example in function docs. We'll
see how we can improve the API if the very hypothetical problem I am foreseeing
actually happens some day: say a word in English translates to e.g. "Filters" in
some other language, whereas English "Filters" translates to yet another term;
in such case, this new menu would still merge with the default /Filters/ menu
when localized in this language, so we'd have the weird situation where the
custom menu label would have passed through 2 translations somehow.
But let's see how it goes. If we really need, in the future, we can deprecate
gimp_procedure_add_menu_path() and add a gimp_procedure_add_menu_paths() with a
base_path and a custom_path, while the custom_path would be expected to be
already translated.
Fixes:
> free(): double free detected in tcache 2
The file is closed also after resource_load() runs. Anyway it's better to free
resource on the same level they were created, so I remove the fclose inside the
function.
For the same file, which was saving in ~50 secs in RLE (now ~12), and ~80 secs
in zlib. It now saves zlib-encoded XCF in ~35 secs on the same machine with 8
threads.
Contributed code was not wrong, far from it! But the whole memory management was
a bit on the hard-to-read side. For maintainability, I prefer some simpler code,
which contains a bit more allocation but it's reasonable and at least much less
indexes to play with.
Also instead of gegl_parallel_distribute(), then waiting for each set of batches
to entirely finish, I'm using a GThreadPool which I progressively fill with new
batch data. And finally I'm running the gegl_buffer_get() inside the threads (I
know that we have only reads on the buffer at this point; also GEGL has an
internal lock mechanism anyway).
These actually do not provide a huge additional speedup (compared to the initial
speedup by just going multi-threaded), most likely because the I/O are now the
bottleneck anyway (which is quite a good thing!).
- Coding style fixes.
- Removing gimp_gegl_num_processors unknown variable which didn't exist (coming
from another patch from suzu. See discussion in #4985).
- Removing the `cleanup()` attribute to keep our code as generic as possible
without specific compiler extensions. Just free memory manually as is expected
in C.
Testing on a 115MiB XCF file, saving went down (on my machine, as average on
saving a few times) from about 50 seconds to 15 seconds.
This first version is already pretty neat, though we can probably do even
better.
This makes these buttons respond to scroll wheels again, a regression encountered in GIMP 2.99.x.
Code location and fix found by Massimo Valentini.
Fixes issue #6929.
Instead of storing vectors as properties, they have their own structure, which
make them able to store and load all the usual and common properties of other
items. In other words, it makes XCF now able to store locks, color tags and
several selected paths.
It seems I forgot to fix some usage of core gimp_vectors_export*(),
which now take a list of paths (not a single path anymore since commit
9fc8260c7c), as these were used by PDB functions.
Missing functions were:
* gimp_image_get_selected_channels()
* gimp_image_get_selected_vectors()
* gimp_image_list_selected_channels()
* gimp_image_list_selected_vectors()
* gimp_image_set_selected_channels()
* gimp_image_set_selected_vectors()
* gimp_image_take_selected_channels()
* gimp_image_take_selected_vectors()
There are discussions of renaming GimpVectors to GimpPath, which would
also be consistent with the GUI and make the always-plural less akward
in API. We'll see. For now keeping named like this.