Handling of PSD merged images with alpha channel but where merged
transparency is not used is complicated.
We check the alpha_names resource for the presence of 'Transparency'. If
that is present, assume we have an alpha channel.
In case of a CMYKA PSD with layers the merged image actually is RGB with
the first three channels representing RGB and the Alpha in the fifth
channel. In this case we move the alpha data to the forth channel so
we can handle it as RGBA.
PSD images using CMYK as color mode and without layers didn't load
correctly (wrong colors).
When a PSD does not have any layers, CMYK color mode was not converted to
RGB.
Note that PSD color images that do have layers (e.g. CMYK and LAB) store
the merged image as RGB!
To do all this we added a conversion for CMYK PSD with 0 layers and added
code to correctly determine whether an alpha channel exists.
This also fixes the case where loading the merged image of a 16 bit per
channel RGBA psd loads with the alpha channel opaque.
Our psd plug-in checks for valid layer mask sizes. Apparently a psd
layer mask where the rendered flag is set can have invalid dimensions
0, 0, 0, -1.
We will skip the sanity checks for rendered layers and at the same time
move the debug statements up to have relevant debug info available even
if our sanity checks stop further processing.
libheif uses rav1e encoder by default to encode AVIF. However,
performance of AOM encoder got improved significantly, so
I think it is wiser to give preference to AOM, when both
AOM and Rav1e encoders are available.
Name of the used encoder is printed during export,
It can be: aom, rav1e, x265
Detected issues fixed here:
- Use GString and g_string_append since otherwise we need to add g_free
after every g_strconcat.
- No error checking.
- We need to g_free value_utf.
Not detected by coverity:
- Wrong quotes around utf-8.
- Remove unused includes.
Several resource leaks here were detected by coverity:
- Not freeing value_utf.
- Strings returned by g_strconcat should be g_free'd. To handle the string
concatenations easier we use GString and g_string_append instead.
Also introduce function get_list_elements to reduce the number of
repetetive elements.
While we were at it, also remove unused includes.
When writing metadata in the metadata-editor it did not check for empty
values. In combination with gexiv2 0.14.0 (which doesn't check in this
specific case for an empty list of gchar **) and trying to set a non
repeatable Iptc tag (Iptc.Application2.Headline) using
gexiv2_metadata_set_tag_multiple we get a crash.
This fix checks first for a non empty value before trying to save the
metadata tag.
Since the code is very similar also implemented this check for the Xmp
tags in addition to the Iptc and simplified some duplicate code.
See issue #7408 where this is causing a crash when using gexiv 0.14.0
in combination with trying to save it as a multiple tag with an empty list.
According to https://exiv2.org/iptc.html the tag Iptc.Application2.Headline
is not repeatable so we will set mode to "single.
In addition to that I added comments to explain the differences in the
meaning of "single" and "multi" between the list of Xmp tags and Iptc tags.
I had completely forgotten about Coverity Scan but today received an
email with some new scan results. This was one of the warnings that we
had a possible resource leak.
In truth, this is mostly a false positive since gimp_export_image() will
flatten the image. Yet why not be thorough…
Just like COLOR targa MAPPED (indexed) images with a size of 32 per
index should also have alphaBits set to 8 or else the images get
loaded incorrectly.
Loading of the merged image of a PSD was broken as mentioned by Massimo
in MR !476.
Instead of reverting the commit as suggested, we check first if loading
a merged image is requested. In case of an error we add a debug
message and then remove the error without stopping on it.
… multiple drawables as parameter.
Previous commit 7bb892f3 was "making it work" by making the API
inconsistent and also only using the first drawable, which is making the
logics meaningless.
Instead accept multiple drawables, and export only the selected drawable
(when alone) or the merged-down image containing only the selected
drawables (when many).
Note that in current implementation, this is not useful from GUI calls
because the fully merged image is always exported when run interactively
or with last vals (i.e. from the GUI) because gimp_export_image()
flattens the image. So this change would only work when called
non-interactively from other plug-ins. In such a case, multi-layer
images do no longer return an error and whatever items are selected
would change the export result.
See also #7370 for a discussion about how to handle the selected items
during export (because currently the `drawables` parameter of
GimpSaveProcedure's run function is clearly a mostly bogus parameter).
We currently cannot call gimp_pdb_run_procedure() for procedures
containing arrays because this C-type doesn't contain the size
information (which is in a second parameter but since the rule is not
hard-coded, our API can't know this).
See issue #7369.
strlcpy() only copies the specified number of bytes, but it also iterates over
the input string in order to return its length. We only populate the first
6 bytes of buf - the rest is uninitialised - hence strlcpy() will iterate
past the 3 bytes that we're trying to compare, and will only return after
reading an effectively random number of bytes (which could well be beyond
the end of buf). Since we ignore strlcpy()'s return value, program flow
is generally not affected (unless we hit a segfault I guess, which is unlikely
because we do 0-initialize some of our stack variables, meaning strlcpy() is
unlikely to go far). However, this iteration is wasteful, and triggers
complaints from code sanitizers.
Therefore we remove the strlcpy,() and switch to doing a strncmp() on just the
3 bytes that we care about (similar to what is done for the "GIF" check above).
This change has the nice benefit of avoiding an unneeded copy.
An example implementation of strlcpy can be found in glib, here's the line
where it iterates until the first NULL byte:
f763f2b7cb/glib/gstrfuncs.c (L1484)
See also ASAN output from when this was discovered below:
ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffc776e03d0 at pc 0x7f909b615ba2 bp 0x7ffc776e0370 sp 0x7ffc776e0368
READ of size 1 at 0x7ffc776e03d0 thread T0
#0 0x7f909b615ba1 in g_strlcpy /home/ahunt/git/glib/_build/../glib/gstrfuncs.c:1484:14
#1 0x55dc5c in load_image /home/ahunt/git/gimp/plug-ins/common/file-gif-load.c:419:3
#2 0x561f20 in LLVMFuzzerTestOneInput /home/ahunt/git/gimp/plug-ins/common/file-gif-load_fuzzer.c:79:17
#3 0x460e44 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:599:15
#4 0x46034a in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:505:3
#5 0x462067 in fuzzer::Fuzzer::MutateAndTestOne() /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:745:19
#6 0x462bf5 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:883:5
#7 0x450ea6 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:906:6
#8 0x47ae82 in main /home/abuild/rpmbuild/BUILD/llvm-12.0.0.src/build/../projects/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
#9 0x7f909a564349 in __libc_start_main (/lib64/libc.so.6+0x24349)
#10 0x424259 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120
Address 0x7ffc776e03d0 is located in stack of thread T0 at offset 48 in frame
#0 0x55d89f in load_image /home/ahunt/git/gimp/plug-ins/common/file-gif-load.c:375
This frame has 6 object(s):
[32, 48) 'buf' (line 378) <== Memory access at offset 48 overflows this variable
[64, 65) 'c' (line 379)
[80, 848) 'localColorMap' (line 380)
[976, 980) 'grayScale' (line 381)
[992, 996) 'version' (line 385)
[1008, 1016) 'image' (line 386)
HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork
(longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow /home/ahunt/git/glib/_build/../glib/gstrfuncs.c:1484:14 in g_strlcpy
Shadow bytes around the buggy address:
0x10000eed4020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10000eed4030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10000eed4040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10000eed4050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10000eed4060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x10000eed4070: 00 00 00 00 f1 f1 f1 f1 00 00[f2]f2 01 f2 00 00
0x10000eed4080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10000eed4090: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10000eed40a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10000eed40b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x10000eed40c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user: f7
Container overflow: fc
Array cookie: ac
Intra object redzone: bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone: cb
Shadow gap: cc
==15243==ABORTING
This is untested on my side, because the bug only happens on native
builds with meson (our CI has cross-builds with meson and native builds
with autotools and I only do cross-builds locally) but I think/hope it
will work.
Basically we were using .full_path() because these rc files were also
used as input of some configure_file() calls which doesn't like custom
target objects as input (it wants strings or file objects). Yet a bug
in meson didn't like the colon used in native Windows full paths ('C:'
and such) when used in windows.compile_resources(). This has been fixed
by Luca Bacci in: https://github.com/mesonbuild/meson/pull/9368
Yet we just cannot depend on very early meson (or worse dev meson code).
On the other hand, if the input is a custom_tgt object, it uses the
object ID which we give as first parameter of custom_target() so we know
it's appropriately named without colons (such as 'gimp_plugins_rc').
Thus we should not bump into this issue again.
For the few usage in configure_file(), I just add a .full_path() only
when needed at call time.
Last but not least, I replace the bogus `meson --version` call by a
`python3 -c 'exit()'` as advised by Eli Schwartz:
2afa019c70 (note_1284951)
The reason is that it is apparently possible (or will be when some
reimplementation of meson will be done) that the `meson` executable
itself does not exist. On the other hand, `python3` should always be
there, as a mandatory dependency of the build tool.
In order to use an appropriate `python3`, I made the
pythonmod.find_installation() check required in our build (which should
not be a problem since it's a meson requirement as well), even when the
-Dpython option is false (this one depends on other requirements too
anyway, such as version and pygobject). This way I can call this meson
variable of discovered python in my bogus call, instead of calling a
(potentially different) python from PATH environment.
These warnings only happens on debug output, so it's not huge bugs, yet
it's always better not to have warnings at build time.
Most are printf format type warnings, and one is because of uncomplete
switch() cases.
While doing this cleanup, I found at least several other string leaks
in: file-compressor, file-gegl, file-pdf-save, file-raw-data, file-xwd,
jpeg-load, psd-save…
So it's quite worth it!
Note: in file-pdf-save, there is a global variable file_name which seems
to be happily leaked without caring (didn't look in details, but looks
so). I didn't fix this one which will require a bit more in-depth logics
care.
As explained in previous commits, the _peek_ call is advantageous
because:
- It is less bug-prone as we don't have to handle freeing the string. In
all the cases I changed, I even spotted at least 2 cases where we were
leaking a string (in file-mng, `temp_file_name` is never freed; and we
were also leaking in an error case of gfig).
- As a consequence of the previous point: simpler code with less lines.
- In local file cases, the _peek_ variant does not even need to allocate
an additional string.
- In other case, if we query several times the path, it is allocated
once and cached so it stays efficient.
- When possible, working on the GFile rather than on a path string may
be more robust. For instance I changed one g_unlink() into a
g_file_delete(). Actually most reading/writing should be done with the
GIO API when possible, but I didn't want to change too much code
logics on this commit.
- use g_file_peek_path() instead of g_file_get_path() in various places.
Less prone to memory management bugs (not forgetting to free in all
cases). Also it is often more efficient, especially for local files
where the _peek_ variant does not even need to allocate any memory as
path is already hanging around (and local files are the most common
use cases, I believe!).
- Also gets rid of one GFile creation with g_file_new_for_path() from
the path returned from another GFile! We can just use the same GFile
as it is not freed and perfectly usable.
Freeing the path immediately could lead to a free-after-use error as we
used it for an error message when the file pointer failed to be created,
as reported by Massimo (thanks again!).
Using g_file_peek_path() has also other advantages, such as being less
error-prone, but also possibly more efficient. First looking at the
implementation, for local files, we already have the path around, so no
additional memory allocation even needs to happen. As for the generic
code path, it would still allocate a new string, yet cache it and reuse
it when needed later. This makes the _peek_ call much better for quick
peek-use-discard usage.
- .gitignore and gimprc.common updated by manually running ./mkgen.pl.
- Coding style fixes in file-jpegxl:
* function names go to the line by themselves (return type on previous
line).
* avoid some overlong lines by breaking lines between parameters.
* fix some alignments.
XMP array tags of type BAG and SEQ can have multiple values, each of
which is on a separate line. However, we were reading and saving it as
just one value.
We change this by setting each line as a separate value with
gexiv2_metadata_set_tag_multiple.
In addition to that we found that setting the type of tag struct with
gexiv2_metadata_set_xmp_tag_struct caused arrays of multiple values per
tag to be set incorrectly: the last value in the list got added multiple
times. I'm not sure if we are using this function incorrectly, or that
there is a bug in gexiv2 or exiv2. Anyway, since it seems that all tags
and values I tested work without calling this function, let's just omit
this call.
XMP array tags of type BAG and SEQ can have multiple values, each of these
values needs to be on a separate line to be correctly recognized as a
different value in the tag array. We were incorrectly loading all values
on one line separated by a comma.
For those tags that have equivalent IPTC tags we were also comparing just
the one XMP value with the whole, possibly multiple lines, of the same
IPTC, which could cause a failure to recognize identical tags.
We changed this to now have each value in a tag array on a separate line
by adding \n between values.
Each IPTC equivalent tag value is now compared to each value in the XMP
tag array and only added when a different value is found.
XMP tags that have multiple values (usually of type XMpBag or XmpSeq) were
shown on one line. Which, especially for long text values, could make it
difficult to see the separate values for that tag.
Let's do the same as we do for IPTC tags that can occur multiple times and
show each value on a separate line.
In the metadata-viewer refactor the code that adds multiple values per tag
to the list store into a separate function.
Currently used for IPTC tags only but the intention is to reuse it for
XMP tags with multiple values.
If you first open a multi page TIFF, exiv2 generates EXIF tags for the
first 3 pages in the form of Exif.Image.<tags>, Exif.Image2.<tags>,
Exif.Image3.<tags>. When exporting with EXIF saving enabled, exiv2 thinks
it needs to save TIFF pages for the EXIF metadata of the second and third
page too. Those pages saved by exiv2 contain only metadata no real image
data and give warnings when loading.
The EXIF tags read from page 2 and 3 are only the basic image specs. We
don't use those, nor do we add tags to it. Until we support handling of
metadata for those pages and exiv2 has better support for all TIFF pages
beyond the first few, I see no reason to save this information.
So for now lets just delete all tags that start with Exif.Image followed
by a number.
On Windows there is a long standing issue in its File Explorer. When
GIMP exported as a multi page (multi-layer) TIFF with thumbnail saving
enabled, then the Windows thumbnailer apparently gets stuck and does not
close the TIFF file when browsing a folder. Because those files are not
closed it is impossible to delete them. This issue has been reported
many times over the years to Microsoft without any reaction.
Investigation showed us that this lock up only happens when we save the
thumbnail as a subifd. To resolve this issue we change this and now let
exiv2 handle our thumbnail saving, just as for other image formats. For
TIFF this means the thumbnail is saved as the second page of a TIFF.
Previous commits have made sure that it is now easier to recognize a TIFF
page as a thumbnail and to make sure we don't load thumbnail TIFF pages.
Since saving as a subifd is what made TIFF thumbnail saving different
from other formats, this commit consists of only removal of code.
TIFF image pages can specify what type of image that page represents.
If the page is marked as FILETYPE_REDUCEDIMAGE we will consider it to be
a thumbnail and filter that out of the list of pages that can be selected
to be loaded.
In addition to that we will try to recognize certain pages as thumbnail
that don't have the subfiletype tag set.
We will consider it a thumbnail if:
- It's the second page
- PhotometricInterpretation is YCbCr
- Compression is old style jpeg
- First page uses a different compression or PhotometricInterpretation
If these conditions are true the page will also be filtered out.
We could consider having an option whether to filter out thumbnail pages.
Since in the situation up until now we also don't load thumbnails, I think
this should be considered as a separate feature.
Since it appeared with GLib 2.68.0, we could not change this until we
bumped the dependency which has only become possible a few days ago
(since Debian testing is our baseline for dependency bumps). Cf.
previous commit.
As this is a drop-in replacement (just a guint parameter changed to
gsize to avoid integer overflow), search-and-replace with:
> sed -i 's/g_memdup\>/g_memdup2/g' `grep -rIl 'g_memdup\>' *`
… followed by a few manual alignment tweaks when necessary.
This gets rid of the many deprecation warnings which we had lately when
building with a recent GLib version.
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.
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.
For 1-bit BMP export we did not show the save dialog, making it
impossible to change settings interactively.
We enable the save dialog for 1-bit too, making sure to disable RLE, which
is not available for 1-bit.
In addition, also set the RLE config option to false for all cases where
it can't be used.
… GimpUi.ProcedureDialog.new()
The old way is still working and acceptable as far as I am concerned.
Just switching to the recommended (by upstream) syntax to show it works
as expected and as a good example of code.
Our plug-in was not correctly handling DICOM images that use big endian
transfer syntax, which is deprecated.
We add support for that here, add a few g_debug statements to make future
debugging easier, and also return an error when we encounter a transfer
syntax that we can't handle instead of blindly continuing.
… source colors
Reviewer's (Jehan) note: a git blame shows this is a clear bug
introduced in commit 9d19bf2a3e, which was a cleanup patch, and the
BOUNDS() macro was not doing exactly the same thing as the code before
the change.
Added an option for exporting thumbnail in WebP Export dialogbox.
Additionally, introduced a function gimp_procedure_dialog_fill_expander.
The function is similar to gimp_procedure_dialog_fill_frame but allows
adding GtkExpander instead of GtkFrame.
I must have not been awake yet when I pushed the
fix for #6116 because it has two problems:
- Updating cur_progress caused by not editing the
for loop after a copy/paste, found thanks to
Stanislav Grinkov.
- Not using the correct drawable to determine the
drawable_type which I noticed while fixing the
above issue.
These issues are not present in the backported
version for 2.10.
GIF export was not taking into account that there can be
both grayscale layers with and without alpha in an image.
We make sure that the GEGL buffer knows the type of
drawable for the current layer.
We were loading 16-bit per sample SGI images as 8-bit
integer images even though we have had support for
16-bit integer for a long time now.
Changed to use 16-bit integer encoding now. Saw
this request a while ago on the gimp user mailing list.
Instead of only checking for the layer resources Lr16 and Lr32 make it
a loop and use the generic get_layer_resource_header to read all
resources.
We still need to figure out at a later time how to best handle these
resources.
For PSB images certain resources have 64-bit lengths.
Let's handle this transparently by adding a psd_version
parameter and depending on that and the type of
resource we decide if the length to read is 32 or 64-bit.
We also return the total header size. This way the
calling function can use that to determine the
remaining length.
Because we needed to check the signature when
loading the header we remove that check from
the load_layer_resource function since that
would be redundant.
Apparently layer height or width can be negative for certain
PSD layers that have the irrelevant flag set.
We move those checks down until after we have read that
flag and only validate if the irrelevant flag is not set.
Since the 1-bit conversion debug statements causes a huge amount
of info which can cause slowdowns when loading do it only when
explicitly setting it to IFDBG(4).
In certain places in a PSB file we need to read 64-bit
lengths where in a PSD we read a 32-bit length.
To make handling this easier we add a function that
determines if we need to read 4 or 8 bytes and
also handles converting the value from big endian.
Even though the current specs say max is 56 there
are sample files with more channels. Since there is
no reason why we can't handle more let's increase
the maximum to 99 for now.
Not taking a non zero length into consideration caused us to read from the
wrong location in certain cases and then fail loading.
Always use the specified length to go to the correct location. This is not
a guarantee that we will be able to correctly load the image but at least
we won't read wrong data from the wrong location.
Commit aba721ae68 fixed its intended bug but brought a new: each time
the preview was updated, a new display was created. This fixes this new
bug. Hopefully it's good now!
in Gimp and Photoshop- "Too many channels in layer: 4907"
To be better able to handle 64 bit offsets we switch to
using GInputStream/GOutputStream and its related
functions instead of using FILE functions like fseek.
In addition we changed several offsets and sizes to
64 bit instead of 32 bit to correctly handle large
image sizes and offsets.
I should probably have split this out in two parts
(the move to GInput/OutputStream, and the
changing to 64 bit variables) but don't feel like
it's worth the trouble at this point.
Exporting to psd could result in very large files which often failed to
load in both GIMP and PS when a group layer with a layer mask
was present.
Reported on our IRC channel with a sample xcf which made it
possible to figure out the problem.
The port had a slight error, because in gimp-2-10, the display_ID
actually had 3 states: 0 when gimp_export_image() kept the original
image to which we just add a preview layer, -1 when it created a new
image which we wanted to put in its own display, and the display ID
itself when created.
With the new API where display variable is an object, we can only have 2
cases. So I create an additional variable separate_display to make the
distinction.
So far all dds images were loaded as 8 bit per channel which makes
sense for most but not all dds formats.
This commit implements loading in 16 bit for "L16" - a 16 bit
luminance channel.
In addition to that we improve security a bit by not assuming
that L16 is the only left over case but instead explicitly
checking the correct value of rmask for L16.
For other cases we now set an error with enough details
to identify the type of DDS image that needs extra
handling.
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).
Let's increase the default size of the metadata viewer a bit.
Ideally we would add some logic to determine the width of
the first column since that one currently depends on the
length of the longest tag. However for now this change
will be enough for most cases.
The round corners script was assuming that the drawable always
was a layer, which is incorrect when a layer mask is selected.
We fix this by always requesting the active layer instead of
active drawable.
We also enable this script for RGB and GRAY with alpha
channel. Although it might not always make sense to
use this script when an alpha channel is present, there
can be use cases where it is perfectly fine.
So let the designer decide if this script is suited or not
instead of GIMP making that judgement.
This fixes the following error when building for Windows:
> error: too many arguments to function ‘mkdir’
On Windows, mkdir() is deprecated, it is an alias for _mkdir() which
doesn't have a mode parameter. The GLib version duplicates the POSIX
signature and is cross-platform (mode is simply ignored on Windows).
Closes: GNOME/gimp#541
Reviewer note: thanks to Stanislav Grinkov for cleaning up and reworking
a bit the patch, such as renaming the procedure to dir-make, as per
Kevin Cozens' review.
Looking at the documentation it is indeed red that should go
in the lowest bits and blue in the highest bits so just
reverse our code for red and blue.
We also update the version of our GIMP DDS plug-in,
this way we can catch and correct RGB10A2 images
written by older versions of our plug-in and correct
them.
Fixes:
> Calling error for procedure 'gimp-brush-get-pixels':
> Brush 'Circle (03)' not found
The data/brushes/Circle-3.vbr brush doesn't exist since commit
7aa12175a4 in 2012 as far as I can see.
- Now using multi-drawable run() signature, but sensitive only with a
single drawable.
- width|height() calls are now get_width|height().
- PDB call to 'plug-in-sel2path' now expects a drawable array as well.
Since we renamed a bunch of functions for consistency (the like of
gimp_image_width() to gimp_image_get_width() and others), most Scheme
scripts ended up broken. This is a simple bash fix with `sed` to at
least take care of these simple renaming cases.
Many scripts are still broken for other reasons after our API evolution
(to be continued).
The following changes were made:
- run() signature now moved to multi-drawables.
- sensitivity mask now enables the plug-in when one or several drawables
are selected. When several, a fog layer will be created over each
selected layer.
- Mnemonics added to properties.
- Color property finally used now that our API has support for boxed
parameters with GimpRGB value type.
- Use a config object instead of ordered GimpValueArray and use the
begin|en_run() API to store values between runs.
- Add a graphical interface generated with the new GimpProcedureDialog
API.
We did port it to the new API but we are doubting it is useful at all.
It might have been, historically, when foreground extract algorithm was
first introduced, but probably not anymore, even more as the algorithm
is now implemented mostly on GEGL side anyway. So if any benchmark were
to be done, it should probably be added as GEGL tests.
Also as we are planning to update even more the API lately, it feels
really like a waste of time to also fix this plug-in while we currently
believe it to be useless.
Still we move it to the gimp-data-extras repository for being able to
easily bring it back in case we missed some use.
This is the same as what we did for #5357
but in the reverse situation. The reason
for not doing the same at that time was
because I wasn't sure if that was valid
in all cases.
Looking at the documentation it does seem
to be the right solution and is working for
the supplied example image.
In PNG, JPEG and TIFF export plug-ins which use the new API, use our new
function to set widget sensitivity.
Note that part of it is similar to commit 6a2910bd3b on `gimp-2-10`,
making "Save GeoTIFF data" checkbox insensitive when there are no such
data available (this feature was late on the main branch as we rushed
for 2.10.24 release).
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.
Also ".sun" is a possible (and common) file name extension for Sun
Raster images, according to various sources and samples I found (these
samples with .sun extension also opened fine in GIMP, so it's not just a
subvariant which we may not handle or something of the sort). This one
is not so important though as we also register magic bytes for detection
(which is the proper way), but it can still be useful, mostly for
exporting (as we will direct to the SunRaster plug-in if someone tried
to export a file with .sun extension, since no other file format uses
this extension AFAICS).
There is no functional change, I just had a look at this plug-in while
handling !428 and realized this format was not present in the MimeType
list (which is used to generate the desktop file, in order to have
proper mime types, not detection based on extension only, unlike
Windows in !428).
The saved value for Iptc.Application2.Caption is copied from Xmp.dc.description.
However the last one is multiline but the former should be single line. This
caused only the first line to be saved instead of all lines.
To fix this we set Iptc.Application2.Caption to single and use a different
conversion based on whether the tags from Xmp and Iptc are both
multiline or whether Iptc is single line.
The qbist plug-in shows a grid of 9 patterns, it was not clear at all
which one will be applied. Also it looked like selecting a pattern would
make it disappear and loading a same file twice would not create the
same patterns (as said in commit 7fb696206e).
Actually the plug-in works fine, but it is simply not clear at all what
is happening until we look at the code. The center pattern is the main
one: the one which will be applied and the only one which will be stored
or loaded in/from a file if we decide to save the pattern. Also when
selecting a pattern, it does not disappear, it is moved to the center
(but you don't necessarily realize it, especially as several will look
the same). And the reason why you get different result when loading a
saved pattern is that it only reimports the center pattern; all others
are indeed randomized from this stable source.
So my fix is attempting to have this center pattern standing out a bit
more by framing it with a "Pattern" title. It's not perfect, and I'm not
very happy with this design, but I don't find a nice widget for better
framing the center pattern, nor do I want to spend too much time on
this. It's better than before at least.
While testing the metadata-editor I noticed that certain values for
ModelReleaseStatus and PropertyReleaseStatus were incorrectly
read back.
Apparently while copy pasting some values were forgotten to be
updated which caused incorrect values to be saved.
There probably are not many users of these metadata tags
since in all these years there hasn't been a bug report about this.
The metadata-editor allowed Iptc tags to be set if an empty string was
used contrary to Xmp tags. In the case of DateCreated this cause an
invalid date "0-00-00" to be written.
We added a check to only write text Iptc metadata if the value is
not empty.
Fixes issue #3656 Empty metadata tags are written to XCF at least.
Xmp.plus.ModelReleaseStatus and Xmp.iptcExt.DigitalSourceType are
defined as combo boxes with a fixed number of possible values.
However there was no option to leave it empty so there always was
a value written when saving metadata in the metadata-editor.
We added a "select value" as default option and only write
metadata if a different value was chosen.
As a bonus we replaced the fixed loop numbers with
the current actual number of defined choices.
Contents of these arrays are assumed to be limited to a specific range.
While it did work sometimes (because a further processing would randomly
regenerate some of the indexes and correctly limit the range), it often
crashed.
This commit fixes the crash, but I am not sure this plug-in is working
exactly as expected regarding data load/save. It feels like you would
expect to always get the same patterns with a same source data. Yet
there is further randomization going on.
Oppositely when saving data, and re-loading it later, I would expect
once again to get back the exact same patterns I had when saving the
data. So it would be a way to save the result of randomization (as
chances to get back a pattern one liked are slim by definition when it's
created randomly).
Right now, it doesn't behave at all like this. Files are only used as
some kind of random seed, not as a way to save/load patterns. I feel
this was not the purpose of the file handling here.
We need to work around an issue in Exiv2 that saving to /Iptc4xmpExt bag
records fails which seems related to it registering /iptcExt. Unregistering
that and registering the first one doesn't work.
See: https://gitlab.gnome.org/GNOME/gexiv2/-/issues/24
So let's use /iptcExt for saving even though it's the less common form
for now. When loading metadata we try to load the most common
form first and if that fails we will load /iptcExt.
Set Xmp.iptc.CreatorContactInfo/Iptc4xmpCore:* last since the short
form Xmp.iptc.Ci* could have been used to set this information too.
Becausethe first (longer) form is the most common let that override the
shorter form in the (unlikely) case that both are present and also have
different values.
Due to a bug in the metadata-editor previously only the short form
was saved.
Reordered the strings in artworkorobject to be in the same
order as the columns.
Add write_metadata_tag_multiple function that handles most
of the previously duplicated code and check for special
handling of phonetype when necessary by adding
an enum and a get_phonetype function.
Use a separate function that can be reused and also get rid of
the large output_data array and strcat without length
checking. We will use GString instead.
There was a lot of duplicate code based on
whether or not value was NULL with for
some tags partially unreachable code.
We combine the parts and only test
value for NULL where needed.
Also rename the typos in enum
COL_REGSITRY_* to
COL_REGISTRY_*
There's a lot of duplicate code for adding string values
to the gui (liststore). This is another part of trying
to reduce that.
Besides that the copying code uses fixed size arrays
without length checking as pointed out by flawfinder.
We will instead use a function add_to_store to add a
comma or semicolon separated list of values stored in
value to add rows with values to the gui.
Only count a broken line once and only stop after at
least 10 consecutive broken lines since older fax
images are known for having bad lines.
This enables loading the image in issues #475.
- Miniswhite and Minisblack TIFF images with bps 1, 2 and 4 were loaded as
indexed. We change this to grayscale since that's what they are.
- Fix not handling of miniswhite for 8 bps.
Fixes issue #1838 Tiff image display wrong (tiff tags not interpreted), black
line instead of white line.
Fixes issue #2680 Failure to load 4-bit grayscale TIFF layers.
Testing some fuzzed TIFF images from the imagetestsuite
revealed we should add some more checks for valid input
and function returns.
We now stop whenever the reported bps is above 64.
Even if it is valid we can't handle it anyway and I'm not
aware of any actual valid image like that.
Make sure the image dimensions are valid and in the range
that GIMP can handle.
Return directly when TIFFReadRGBAImage fails instead
of doing further processing and improve the error message.
Check result of TIFFReadTile and return if it fails.
Certain broken tiff fax images can cause a huge amount of warnings
which on Windows eventually causes GIMP to run out of resources
and then crash.
Since the avarage user won't have much use for these warnings
let's just only send them to stderr.
The PNG specs says:
> The cHRM chunk is allowed in all PNG datastreams, although it is of
little value for greyscale images.
Though it doesn't say it's completely useless though. So let's still
extract the info and give it to Little-CMS.
As for the gAMA chunk, it doesn't say it's not usable for grayscale
images.
Note that if one of the 2 values is not set, it will use default values
for sRGB (i.e. default simili-sRGB gamma or d65 whitepoint). Not sure if
this is totally right.
See also issue #6501.
- Store the Layer ID (lyid) block. Use GIMP's layer tattoo as a PSD
layer ID, hence mirroring PSD load processing (we were already reading
this block into our layer tattoos but always exporting with no ID).
- Add support for the Layer Selection ID(s) block (0x042D) both on
import and export in order to store and restore the multi-layer
selection.
We were previously using the Layer state information block (0x0400) to
store the active layer, but it doesn't seem to be usable for multiple
layer selection. Actually it is even doubtful if this was working fine
even for single layer selection but I can't be sure (I could only test
in non-Photoshop software available to me). So the new logics is:
* If more than 1 layer is selected, store only the Layer Selection
ID(s) block.
* If exactly 1 layer is selected, store both the Layer Selection ID(s)
and Layer state information blocks.
* Otherwise (no layers selected) do not store any of these blocks.
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.
Subdivise progression steps inside layer groups.
It is not perfect yet as it would not handle perfectly weird edge cases
where you have for instance a lot of layers in a top layer group with
only many other top layers. But it would still handle it better than
before. Ideally we should simply count all non-group "leaf" layers in
the layer tree and use this as the number of steps. In other words, we
should do a first pass through the whole layer tree.
The Lua API is undergoing changes in darktable master. I've fixed the lua-scripts repository as I've made
changes, but I forgot about the collateral damage.
Attached is a fixed version of file-darktable-export-on-exit.lua which should be good for darktable 3.6 and
beyond.
Fixes#6421
PDF exporting had no progression. In particular when exporting a PDF
with more a hundred pages today, it took maybe 20 secs without any
progression status, which is annoying.
Limitations:
1. When exporting as multi-images, we reset to 0 at each image. I am
actually not sure if this is the expected behavior.
2. I am updating the progression per top layer, not doing finer grained
progression steps. An extreme case would be a single top layer group
with many children layers inside. Yet I wanted to avoid too much
progression updates for the normal cases.
Similarly to the previous commit, it is not only about using the new
API. I also make sure we do not assume that parasite data is
nul-terminated. In many places, we were just assuming so because these
were supposed to be parasite our code set. Yet these are data input
contained in files which can be wrong for any reason (corrupted file,
bugs, other scripts/plug-ins editing these parasites…). So instead of
assuming string parasites are always correctly formatted, I make sure
they are nul-terminated by passing them through g_strndup() when
necessary.
Now when a gAMA chunk is set on an imported PNG (and if there is no
profile attached), an equivalent profile is created out of the gAMA
chunk. So the displayed image looks like it is supposed to.
On export, the generated profile should simply be saved together in the
PNG file. If not, the sRGB chunk is set anyway (and also an equivalent
gAMA/cHRM as recommended in the specs).
So basically we now have a proper support of the gAMA chunk, from import
to export, and display within GIMP takes this metadata into account
appropriately (in the shape of a generated equivalent profile).
Therefore it looks to me keeping the option is redundant. It is even
confusing (because it's actually hard to understand exactly what this
data is about, even our documentation about this metadata seems to be
wrong) hence possibly dangerous if someone thinks it is important,
checks the box, which would override other data or simply create
irrelevant or contradictory metadata.
So let's just remove this option. Development releases are the right
places anyway for people to notice this change and come to us if we
missed some usage regarding this option.
See also #5363.
The openraster plug-in uses the png plug-in in non interactive mode
to load and save layers. With a lot of layers we get a lot of
flashing popups because of the progress dialogs.
When it is called in non interactive mode there should not
be a need for progress dialogs so let's disable them in
that case.
The Openraster plug-in calls file-png-load and
gimp-file-load-layer but didn't check if the result
was SUCCESS. Even though I haven't seen any
failures let's improve error checking by adding
a check for a correct result.
The current openraster specification only has a limited
number of layer modes. Let's improve things a little bit
by mapping both the current and legacy layer modes
to the same openraster composite-op where available.
For the other layer modes we could examine what the
closest composite-op is, or add a GIMP specific op
(which is what Krita seems to do). For now we set
all these to the openraster equivalent of NORMAL.
Let's use the non legacy layer modes when importing/exporting
openraster images. That way we are more likely to be able to
import the correct layer modes since openraster only has a
limited number of layer modes defined.
This feature adds new 'Load in reverse order' toggle box to allow
selection of order of PDF pages before importing them as a layers or
images.
Adds new bool parameter 'reverse-order' to Non-interactive mode.
Order of parameters was also changed from (password, n-pages, pages)
to (password, reverse-order, n-pages, pages)
This commit fixes the issue when requested page width/height was
not honored because DPI resolution value was constrained to whole
integer numbers only.
PNG has support for profiles, which is the state-of-the-art way to
handle color management nowadays. But it also had some very basic color
management based on a gAMA chunk (for the gamma) and a cHRM chunk
(primary chromaticities).
We don't have a core concept with just these and don't want to walk
backward anyway, but we can easily make basic RGB profiles from these.
This is what this commit is about. It generates profiles at import which
were not actually inside the image, but only represented by these 2 PNG
chunks. What gives it away is that GIMP will propose to convert/keep the
profile at start (unless configured otherwise, as any profile), and the
profile name will specify it is a profile generated from PNG chunks'
values. One can even discard the profile later (if you didn't convert),
so it gives a solution if somehow these metadata were not proper.
darktable GUI is "mangled" when opened from GIMP for a reporter. There
was already some debug info outputted (when running with DARKTABLE_DEBUG
environment variable). Adding the environment list (and some separation
titles to make it easier to distinguish what is what). Hopefully it will
help debugging.
See #6195 and https://github.com/darktable-org/darktable/issues/7402
We were not handling all possible record sizes for mask info and
in case it was an unknown size we set everything to 0 instead
of reading the parts we do recognize.
Let's make it more flexible by reading the parts we do know
about and skip the remaining data if any.
Also added a few more g_debug statements to make it
easier to check some values when needed.
Not sure why we didn't see the crash earlier and it suddenly shows up
now. Anyway we must delete the exported image **after** calling
gimp_procedure_config_end_export() on it, obviously.
Commit 01e1587ec1 had directly edited the Makefile.am, which was wrong.
This file is generated by plugin-defs.pl. Add the proper dependencies
for file-heif and regenerate the Makefile.am.
When a profile is set, we should not set the sRGB chunk, because it
would be a contradiction. But when we don't set the profile, then our
code explicitly saves as sRGB. So let's add the sRGB chunk.
Also when we do so, let's also set the relevant gAMA and cHRM chunks
corresponding to sRGB as recommended by the PNG spec:
> An application that writes the sRGB chunk should also write a gAMA
> chunk (and perhaps a cHRM chunk) for compatibility with applications
> that do not use the sRGB chunk. In this situation, only the following
> values may be used: […]
This is why I use png_set_sRGB_gAMA_and_cHRM() and not just
png_set_sRGB().
See also discussions in #5363.
When testing for the presence of extra channels we were not taking into
account all values of TIFFTAG_PHOTOMETRIC causing incorrect messages
that we had a non conformant TIFF without ExtraSamples field.
Since the same test is used twice I separated the code into a new function
and also added another related function to have similar code grouped
together for easier maintenance.
GIMP was exporting certain BMP images with 1, 4, 8 and 24 bpp
with an incorrectly set value of BI_BITFIELDS for compression,
see issue #6144.
According to the specification this is not supported but testing
shows that many image viewers and editors load these
images correctly.
Since allowing these unsupported values for bpp does not
seem to have any negative side effects and is easy to
implement let's add this.
The Microsoft specification for BITMAPV5HEADER says that
BI_BITFIELDS is valid for 16 and 32-bits per pixel, but we
were also using this flag for other values of bpp when
no rle compression was used.
Since this specification doesn't mention 24 bpp or other
values use BI_RGB instead.
Information in issue #6200 revealed that 0 as default value for
the blue channel is a lot more common than 255 so let's use
that instead. The discussion and testing revealed no negative
effects for the other formats that use the same code to
initialize the memory to read blocks of image data.
Since older versions of our GIMP dds file exporter incorrectly
saved BC5 dds images with the red and green channels
swapped we should fix that. Since the exporter already
wrote the plug-ins version number and that it is written by
GIMP we can check for these incorrect images.
To enable that we increase the plug-ins revision in this
commit and swap red and green channels for images
that have an older version number and are of the
correct type.
… org.freedesktop.portal.Desktop (X11 only).
If we don't set this parameter appropriately, the created dialog is not
modal/transient relatively to the main GIMP window. This is especially
annoying because in my tests (GNOME shell), the screenshot dialog would
appear behind GIMP window, which makes it feel like broken behavior.
Note that the supported handles are X11 only so far. We'll need to
generate the appropriate Wayland handles too (created from xdg_foreign
protocol).
Today I have only re-tested the GNOME implementation (in particular not
the KDE one) of the portal API org.freedesktop.portal.Screenshot but I
assume it is similar because the dbus function does not provide any
useful options anyway. This is why the portal comes with its own dialog
providing the various common screenshot features (delay, window picking,
area selection, etc.). Showing our dialog first is therefore redundant.
Also I updated setting the monitor profile only in the case where the
loaded image result from the portal did not have an embedded profile. To
be fair, the GNOME implementation at least does not embed a profile to
this day. So this is only for the future case where they will finally do
the right thing and embed the display profile (or for other desktops'
implementations where they maybe already do the right thing).
This was true in the "old world" where X11 support being built-in meant
that X11 screenshot would be usable. Now we must not only check it is
built, but also that we are actually running under X11. Otherwise with a
Wayland compositor, screenshot would fail. Also it meant that the
Freedesktop portal was never tried if both X11 and Wayland support were
built-in.
If we were loading multi page tif images as layers then only the tif pages
that had a color profile attached would get set as linear (and each tiff page
can have a color profile). Since most of the time only the first page has a
color profile this caused us to load linear tiff pages incorrectly.
Also we were overwriting the color profile if there was more than one since
GIMP can only load one per image. Instead of using the last one we see
use the first one and notify if we encounter more than one.
… GimpSaveProcedureDialog.
See issue #6092 and the discussion with Jacob. Basically we are trying
to improve the metadata situation with more edit abilities and
awareness, while in the same time having the export dialogs less of a
mess (the "Comment" input in particular will most likely move to the
metadata editor itself; I left it for now, until the move is done).
The "(edit)" link will basically just run "plug-in-metadata-editor".
Also as a side note: I realized that gimp_pdb_run_procedure() runs
procedures synchronously and wait for a result, which is fine for quick
non-interactive plug-ins, but freezes the calling process otherwise.
Actually even when we want synchronous result, we should allow for GUI
events to be processed (otherwise the OS just thinks the calling export
plug-in is a zombie and proposes to kill it). This API should probably
be improved (and an alternative async version added as well).
This format name is a public facing name for a file format, such as
"PNG", "JPEG", or "C-source". Since it is public facing, the function
recommends to localize it too.
This is an optional name, yet is made mandatory if you want to use
GimpSaveProcedureDialog because it will be used for the dialog title
(ensuring that all support format have a similar export dialog title).
Following this change, gimp_save_procedure_dialog_new() does not ask for
a title anymore (if anyone absolutely wants to set a custom title,
setting the "title" property on the dialog is always possible anyway,
but a generic and consistent title should be set as a default).
Also updating the 3 plug-ins which were already using the now-changed
API.
Realtime parameter is set for AOM encoder
when Fast encoding speed is selected by user.
Ability to set pixel format (YUV444, RGB)
is enabled for >= libheif 1.10
to avoid issues in older versions.
This fixes various similar warnings:
> file-xmc.c:2276:53: warning: ‘save_rgn.height’ may be used uninitialized in this function
Note that I have not looked in details, but it would seem that these are
actually initialized by get_cropped_region() which uses goto-s quite
extensively, so obviously it's hard to follow (for humans as well as for
the compiler). So let's just assume there is no code error and let's
just add default initialization to silence the compiler.
Per polls on Twitter, Reddit, Patreon and Tipeee, most people were
willing to drop this plug-in as not being relevant anymore (247 for
dropping and 63 for keeping it, so nearly 80% out of 310 participants).
So let's move it to gimp-data-extras allowing it to still have a
possible life, and who knows, maybe to even evolve.
See also corresponding commit beffc6b to gimp-data-extras repository.
This fixes issue #6065 and part of #2680.
Sometime after GIMP 2.8 loading of 2 and 4 bit tif images,
both grayscale and indexed, got broken.
I added routines for handling 2 and 4 bit tifs based on
the handling of 1 bit images.
When not creating an image, we were always returning with status
GIMP_PDB_EXECUTION_ERROR even though it might have been GIMP_PDB_CANCEL
(which is not handled as an error, this is user choice, hence no dialog
pops up).
By using a logging domain we can filter the messages and can choose to
only see them when needed. They seem to be leftovers from coding or
debugging but I don't wanna invest too much time into this.
metadata-editor and metadata-viewer did return GIMP_PDB_SUCCESS when
loading the dialog ui failed, we changed this to GIMP_PDB_EXECUTION_ERROR
and now use a GError to report it back to GIMP.
In metadata-editor also improve the error handling when loading the calendar
dialog fails.
The error also only showed in the console so change that to show a dialog
or the error console.
The string parsing for gps longitude/latitude was not taking all
possible values into account and also the seconds part was
not saved with enough detail.
For altitude lets also increase the detail a bit from .1m to
.10m.
By using g_log with G_LOG_LEVEL_MESSAGE and empty domain
name we make sure the failure to write a tag will be seen by
the user in GIMP's user interface (error console or error
dialog).
Adding metadata-tags to meson.build and Makefile.am for
the metadata-viewer since until now it was only used in
the metadata-editor.
The Exif gps metadata tags for longitude, latitude and
altitude get special handling to show them with more
precision and let the string parts be translated.
The abbreviations M for meter and FT for foot/feet were written in uppercase.
However the offical abbreviations are lowercase so let's just follow that and
make them lowercase here too.
Above and Below Sea Level were using camel case with every word starting
with an uppercase letter and were used on the label of a combobox option
for gps altitude reference. Using standard casing with only the first word
using a capital seems better in this case.
a capital
Saving of longitude and latitude was duplicating the same code,
except that in latitude it forgot to free some strings.
Let's move this code into it's own function to make it easier to
improve it.
The various generic metadata options did not have mnemonic in the base
language (US English).
Also add or fix metadata in file-png|jpeg|tiff so that every option has
a unique mnemonic.
This is nearly 600 lines less for basically the same logics! Removed
code is in particular all the GUI code is favor of the new GUI
generation.
I also cleaned a lot of stuff, removing many global variables or ugly
pieces of code. I also removed a lot of redundant code of things which
are now generic, such as handling of "gimp-comment" parasite (this is
now handled by GimpSaveProcedure and GimpImageMetadata) as well as
saving previous run's values (this is also handled generically).
Note that Advanced Options used to be in an expander. For now I chose to
get them immediately visible (still in their own "Advanced Options"
section, but it's now a normal frame, not an expander hidden by default)
since lately we got some input that many "advanced options" in various
dialogs should not be hidden away. So let's try like this for now (even
though it packs quite a lot of options in the same dialog!).
I thoroughly tested, yet that were so many changes that bugs may have
sneaked in. Please anyone, test JPEG export!
… gimp_label_spin_set_digits() and deleted gimp_prop_opacity_entry_new()
- The "digits" argument for the number of decimal places in
gimp_prop_scale_entry_new() is now mostly useless since GimpLabelSpin
(hence GimpScaleEntry too) got a nice estimation algorithm of sensible
values.
- Add gimp_label_spin_set_digits() function to manually set the digits
property when we don't like the estimated value.
- Also add a new "factor" argument to gimp_prop_scale_entry_new(). Its
role is to allow a GimpScaleEntry showing a factored range, typically
a [0, 100] range for some types of [0, 1] properties.
- Remove gimp_prop_opacity_entry_new() which was basically a
special-case of gimp_prop_scale_entry_new() and which can now be
easily reproduced by simply set factor=100.0.
- Update all usage of gimp_prop_scale_entry_new() in app/ and plug-ins/
with updated arguments. It is interesting to note that all existing
usage were either integers (digits=1) or when double, the estimated
decimal places are the same as the ones which were manually set (hence
showing the generic estimation is not too bad). So the new function
gimp_label_spin_set_digits() was not even needed once in current code.
The values of Description tab - Copyright Status and IPTC tab - Urgency were not saved.
We are fixing that with this commit. Urgency is also saved to Iptc.Application2.Urgency.
Iptc equivalent tags of Xmp tags were only read if the Xmp tag had no value
which doesn't make sense since there is no guarantee that they are the same.
We changed this to always read the Iptc tags too and if the value differs
from the Xmp tag we add both values. That way the user can decide
which values are relevant.
This is one of the problems mentioned in issue #5863. This commit fixes
this problem in the metadata-editor by using gexiv2_metadata_get_tag_multiple.
Empty string values will be skipped.
Basically the same design as before (but a bit prettier, by using a
2 columns flowbox rather than a grid), yet 80 lines smaller according to
git stats. Also still the exact same code logics as before. No feature
loss (and actually a feature gain as there was the "Save comment"
checkbox but no text field to edit the comment as we get in other
plug-ins).
Basically better and prettier dialog in just a few lines. Moreover it's
still work-in-progress. We can even do better, but this is things to
come.
… fails.
My previous commit was just assuming there is one page when the libtiff
function fails. But thinking a bit more on this, there are 2 edge cases
even here:
- If the image is actually really corrupted to the point there are no
TIFF directories to read.
- If there are actually more available directories. It would be much
better if GIMP were able to salvage them from a broken file.
To handle these cases, I actually count manually the number of
directories by looping with TIFFSetDirectory() (which still works fine
on our minimal image). I use 65535 as a max value because in libtiff's
TIFFNumberOfDirectories() implementation, it says this is the max value
(even though I couldn't confirm this in the spec; there is a "Number of
Directory Entries" on 2 bytes, but this is about various fields for each
directories, not a number of directories; still anyway this is already a
huge number and since libtiff won't likely be able to handle directories
over this limit, not need to go over either).
I am also adding a test in the end so that we can output an appropriate
message if we actually failed to read any data from the TIFF (whether or
not TIFFNumberOfDirectories() succeeded, when we actually tried to read
the contents, we couldn't).
On a provided minimal 1×1px TIFF file, TIFFNumberOfDirectories() seems
to choke on some missing field and returns 0. I am not sure if the file
is actually valid or not, but let's make some kind of exception
(outputting a warning of our own when we do) by assuming there is at
least 1 page/image in the file when TIFFNumberOfDirectories() returns 0.
Other than this minimal file (which probably has no other interest than
being a test case), it could actually help salvage corrupted TIFF file
by attempting to read more data.
- New GimpLabelIntWidget which is a label associated to any widget with
an integer "value" property.
- New gimp_procedure_dialog_get_int_combo() which creates a labeled
combo box from an integer property of the GimpProcedureConfig.
- Renamed gimp_procedure_dialog_populate*() with
gimp_procedure_dialog_fill*(). Naming is hard! I hesitated using
_pack() as well (similarly to GtkBox API).
- New gimp_procedure_dialog_fill_flowbox*() functions to create a
GtkFlowBox filled with property widgets (or other container widgets as
we can pack them one in another). This is an alternative way to build
your GUI with sane defaults, with list of property names.
Probably one of the simplest plug-in dialogs, with only few properties
and all in the main box. So it's quite easy to port, even with the still
limited generation API.
file-tiff plugin can read and write GeoTIFF tags
New checkbox 'Save GeoTIFF data' has been added to Export dialog.
Signed-off-by: Ruthra Kumar <ruthrab@gmail.com>
… gimp_scale_entry_set_bounds().
I realized that this function may look like the set() opposite for
gimp_scale_entry_get_range(), which it is not at all. The get_range() is
for getting back the GtkRange widget packed in the GimpScaleEntry,
whereas the set_range() is to change the minimum and maximum allowed
values.
I had recently renamed the former, and could just rename it back into
gimp_scale_entry_get_scale() as it was, but since the class we rely on
is actually called GtkRange (GtkScale is a subclass), I think it could
be misleading. So in the end, let's rather rename the function setting
the widget minimum and maximum as gimp_scale_entry_set_bounds() instead.
Hopefully this is even more understandable. Naming is hard!
Renaming the temporary function gimp_scale_entry_new2() into
gimp_scale_entry_new() now that the original code is entirely gone. This
is now a fully-fledged widget with a nice and proper introspectable API.
Former gimp_color_scale_entry_new() is finally replaced by the
GimpColorScaleEntry implementation.
Both places which used it (GimpColorScales and compose plug-in) now use
the new widget.
Also making "upper" and "lower" properties of GimpScaleEntry into
construction properties, and create the main GtkAjustment at init, so
that we are sure that lower and upper bounds are properly set before we
set the value (when setting all properties at once, we cannot ensure
that these limit properties are set before the "value" one).
Similarly to GimpScaleEntry, this is meant to replace usage of
gimp_color_scale_entry_new() by having a proper widget. This is a child
class of GimpScaleEntry which simply replaces the GtkScale by a
GimpColorScale by adding a GimpScaleEntry class method to create the
scale widget (only restriction: it must be a GtkRange subtype).
This also triggers me to rename gimp_scale_entry_get_scale() into
gimp_scale_entry_get_range() (as well as the 2 plug-ins already using
this function).
This fixes#5838 which was caused by an incomplete port
of this plug-in to the new introspection API.
For now we do keep the n_drawables parameter of save_ora until
issue #5312 is fixed. Not doing this would cause saving to fail.
Jacob Boerema