- This is unneeded in all import procedures. See previous commit. Note though
that this is not because of a change in previous commit. This was already
useless previously. The file set with this PDB function was overridden by the
core anyway (i.e. even before the previous commits).
In app/file/file-import.c:file_import_image(), the imported file is correctly
set (so there is no need to set it from plug-in, which anyway libgimp's
gimp_image_set_file() was not doing) and the XCF file is reset to NULL
(rendering the call to gimp_image_set_file() in a GimpLoadProcedure useless).
- Similarly, this is a useless call in export procedures because
app/file/file-save.c:file_save() overrides such call too. I could only see one
such case for JPEG export, which was quite useless.
- Finally in other types of plug-ins, setting a non-XCF file extension was
interfering with the save feature (similarly to commit e6e73e14c7). I only
fixed the screenshot implementations doing such a thing.
- I left a few usages which will have to be looked at more in details later.
We have some examples of DDS images that have two 16-bit channels that
we could not load.
This commit adds checks to see if we need 16-bit channels, and conversion
of the input to a correct 16-bit red, green, blue or alpha channel.
To be able to use certain variables we use in our DDS loader earlier,
we move the computation of these variables up.
We also add checks to see if the mask needs to be for 8 or 16-bit, since
there are DDS images with 16 bits per sample.
This proved to be both an import and export issue.
Our import set expected format as RGB, causing garbled image output.
Our export for indexed images converted to grayscale first, although the
palette was correctly saved. This caused wrong palette indexes on import.
For indexed images, on import, we request the actual indexed format after
creating the layer with gimp_drawable_get_format, which gives us a correct
indexed Babl format.
Also added logic for indexed with alpha, although I have no sample images
to test this.
For indexed images on export we do the same: use gimp_drawable_get_format
to get an actual indexed Babl format.
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.
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.
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.
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.
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.
Fatal errors should not be done with g_message(), but by setting the
returned GError appropriately. The main GError must therefore be passed
through functions.
Some non-fatal warnings may still be outputted with g_message() if we
want people to be aware of them (for instance if we managed to load some
data yet there might have been data loss or errors).
Finally when we are confident that we recovered from the format error,
it is still nice to output some error on standard error (not be totally
silent about it), but probably not need to bother people with popups.
See #5357.
Also clean up C++-style comments into C-style, to follow our coding
style.
Note: I only cleaned ddsread as a continuation of commit 81a3370e1d. I
expect ddswrite to have similar issues, but I did not look at it.
and in an attack of madness, changes almost all file plug-in
code to use GFile instead of filenames, which means passing
the GFile down to the bottom and get its filename at the very
end where it's actually needed.
None of our load plug-ins have such a checkbox, so this is not
consistent. Moreover one you uncheck it, you just can't get back the
dialog until next GIMP restart. That's very bad usability.
