This patch adds a parasite on load that retains
the original import settings of a DDS
texture (such as compression, number of mipmaps,
and flags). This parasite is then checked on export,
and if it exists, we default the compression
format to the original to reduce the chance
the user will choose the wrong format for
the game they're creating the texture for.
The other data stored is not currently used,
but can be implemented in future commits.
We use Richard Geldreich's bc7enc_rdo
library to minimize the code changes required
in the existing DDS plug-in, and so it can
more easily be swapped out in the future.
We were already considering the plugins under /common but
not the plug-ins that have their own subdirectories. So,
plugin_executables did not match custom_plugin_targets and
the build started to fail on macOS where install_name_tool
neeeds to process the plugins setting the right LC_RPATH.
Closes#15226
When you previously exported a dds with the pre-existing mipmaps,
that will be the option selected in the export dialog the next time.
If, however, that next time there were an incorrect number of mipmaps
available, the option was grayed out, but selected. Exporting then
caused a crash due to less mipmap layers being available than expected.
When writing the dds settings we need to update that setting in our
config object, so that's what we add here. Also remove updating the
local variable since that will not be used anymore in this function.
On 32-bit systems the computed linear size can overflow, causing a
crash.
Use a function that checks for overflow when multiplying and return
an error if that fails.
As extra security also update the loop to compute the base offset after
each line of data, and convert to gsize first when computing the
size for g_malloc and memset.
In 594afaf9, we changed how texture maps were imported.
The array size is now only loaded if we have a valid DX10 compression
set. However, GIMP allows you to export a texture map without
setting a DDS compression. Thus, any DDS images exported with
no compression would only load the first layer on import.
This patch moves the code that copies over the array items size to
be unconditional once the header is loaded.
In 10b798c198 g_list_next always was used from the beginning of
the layer list for each call for volume map and array export, so
all layers after the first were the same.
Our DDS plug-in checks to see if we have six layers
with certain labels in their name to create a cubemap,
and if we don't, that option is locked.
When porting to GIMP 3 API, we accidentally kept checking
only the first layer's name instead of all six+ layers, thus
making it impossible to verify we had layers with the right labels.
This patch adjusts the iteration code to ensure we check all layers
in the image and not just the name of the first one.
When we removed the drawables parameter for image exports,
we switched to using gimp_image_list_layers () to retrieve the
layers from the image parameter inside the function.
However, for DDS, this provided all layers rather than the selected
ones, so we always exported the top layer. This patch switches
to using gimp_image_get_selected_layers () to only retrieve the
subset of selected layers.
When exporting a grayscale image with alpha channel as DDS while
choosing "default" format, we did not set the DDPF_LUMINANCE flag,
but instead used DDPF_RGB, on loading in the 3.0 branch this caused
a failure to read this format due to unrecognized combination of
settings.
First we make sure that on exporting to also set DDPF_LUMINANCE for
grayscale with alpha.
Second we also make sure to zero the blue and green mask fields,
since that is the expected value when these fields are not used.
To support this type of older exported DDS images, we add an extra
format definition for this unusual combination of DDS settings,
that way we recognize them when opening and are able to load them.
New libgimpbase functions:
- gimp_param_spec_choice_get_choice
- gimp_param_spec_choice_get_default
Now the only GParamSpec in libgimpbase whose struct is visible is
GimpParamSpecObject. This can't change since it is derived into param
specs defined in libgimp and therefore needs to be visible.
that were written by older versions of GIMP.
The improved DDS reader that we got a year ago, caused us to be more
strict in determining what exact DDS format we are loading.
This causes failure in reading certain DDS images exported by older
versions of GIMP.
1. Both the A8 and A8L8 formats as written by GIMP, also wrote 0xff
in the masks of the green and blue channels, which should have been
set to 0, since they are unused. Because of this, they were not
recognized anymore by our import routine.
2. When the source layer didn't have an alpha channel, the BGR8 format
wrote a 24-bit format, which doesn't have any official representation
(only RGB8 exists). This also caused our import routine to fail for
this kind of image.
After updating our export in previous commits, this commit adjusts our
import routines to recognize and correctly load these images.
When the source image/layer didn't have an alpha channel, we were
exporting BGR8 as 24-bit B8G8R8, which is not an official D3D DDS
format.
For compatibility with other programs using DDS, let's instead use
D3DFMT_X8B8G8R8 which is 32-bit with the alpha channel being
ignored/set to 0.
See issue #12660 for more details.
When exporting a dds with types l8 or l8a8 we were also setting the
green and blue masks to 0xff instead of using 0 (since these channels
are not used for these formats).
See issue #12660
Set these channel masks to 0 to be more conforming to dds standards
and update our plug-in revision.
This is a followup of previous commit. We must set the win_subsystem
option on executable() so that the result binary is compiled as a GUI
application (and doesn't output a console every time).
The previous commit is still needed and is what allows us to control
when to actually display a console.
Fix warning:
> plug-ins/file-dds/dxt.c:113:1: warning: unused function 'pack_rgb565'
I don't just remove it as I guess it may be of use at some point for a
not-yet fully implemented feature yet?
… PDB type.
This is a first step for #7369. Clearly our GimpObjectArray was meant to
be used with C arrays, hence the wrapper function
gimp_value_set_object_array() which was taking a C array and actually
creating and setting a GimpObjectArray.
This is why our new type is actually a C array aliased as a boxed type
and containing its own size (thanks to NULL-termination).
Eventually GimpCoreObjectArray is meant to replace GimpObjectArray.
The only issue is that such a type does not allow NULL as a valid
element in such an array, but fact is that I don't think we currently
have any use case where this matters. If ever such a case arise in the
future, we may introduce back GimpObjectArray.
In this first commit, I replaced all itemarray PDB types with a new
drawablearray using this new boxed type when relevant.
When I see that we are just using R'G'B' format with no space
everywhere, I'm pretty sure some of this code must be wrong (even though
for some formats, maybe only sRGB is supported, I am guessing that for
some others, the palette may be in specific color spaces).
This will have to be improved with time.
- Fix annotations for gimp_export_options_get_image() to make it
actually introspectable with the GimpImage being both input and
output. Even though the logic doesn't change much (the input image may
be overriden or not), it doesn't matter for introspection because
images are handled centrally by libgimp and therefore must not be
freed. Actually deleting the image from the central list of images
though remains a manual action depending on code logic, not some
automatic action to be handled by binding engines.
- Add G_GNUC_WARN_UNUSED_RESULT to gimp_export_options_get_image()
because ignoring the returned value is rarely a good idea (as you
usually want to delete the image).
- Remove gimp_export_options_new(): we don't need this constructor
because at this point, the best is to tell plug-in developers to just
pass NULL everywhere. This leaves us free to create a more useful
default constructor if needed, in the future. Main description for
GimpExportOptions has also been updated to say this.
- Add a data_destroy callback for the user data passed in
gimp_export_procedure_set_capabilities().
- Fixing annotations of 'export_options' object from pdb/pdb.pl: input
args would actually be (nullable) and would not transfer ownership
(calling code must still free the object). Return value's ownership on
the other hand is fully transfered.
- Add C and Python unit testing for GimpExportOptions and
gimp_export_options_get_image() in particular.
- Fix or improve various details.
Note that I have also considered for a long time changing the signature
of gimp_export_options_get_image() to return a boolean indicating
whether `image` had been replaced (hence needed deletion) or not. This
also meant getting rid of the GimpExportReturn enum. Right now it would
work because there are no third case, but I was considering the future
possibility that for instance we got some impossible conversion for some
future capability. I'm not sure it would ever happen; and for sure, this
is not desirable because it implies an export failure a bit late in the
workflow. But just in case, let's keep the enum return value. It does
not even make the using code that much more complicated (well just a
value comparison instead of a simple boolean test).
This patch creates a GimpExportOptions class in both
libgimpbase and in libgimp. Currently it is a mostly empty
object, but it will be added to after 3.0 to allow for
additional export options (like resizing on export while
leaving the original image intact)
libgimp/gimpexport.c was removed, and most of its content
was copied into libgimp/gimpexportoptions.c. gimp_export_image ()
was replaced with gimp_export_options_get_image () in all
export plug-ins.
GimpExportProcedure has a new function to set the default
image capabilities for each plug-in on creation. It also sets up
a new callback function, which allows the options to respond to
user setting changes (such as toggling 'Save as Animation' in the
GIF or WEBP Plug-in).
...in non-interactive cases.
gimp_export_image () handles various
tasks like rasterizing NDE filters. It only
runs in interactive cases however, so if the
users calls gimp-file-save the filters are
not exported.
Since Jehan removed the hidden dialogue
in 0dc9ff7c, we can now safely call
gimp_export_image () in all cases to make
image export more consistent. This step is
also preparation for setting up the new
API with GimpExportOptions.
With the new API introduced int d1c4457f,
we next need to port all plug-ins using
the argument macros to functions.
This will allow us to remove the macros
as part of the 3.0 API clean-up.
Port all plug-ins to retrieve the layers
directly from the image rather than
having them passed in. This resolves some
issues with introspection and sets the
foundation for future API work.
Resolves#10932
Since GIMP distinguishes between saving
XCF and exporting image like PNG,
we should change the PDB to show
export rather than save in the function
calls.
We compute the size of the mipmapped layers wrong. We assume that
width, height are always exactly a power of 2, which is the normal case
for dds images. See issue #11256.
Since other sizes are possible, we adjust this by computing the width
times height times bytes per pixel of the current mimmap level.
Comprehensive rewrite of the DDS import routine, in the interest of
easier maintainability and readability. Adds formats.c/h, containing
tables and functions related to reading and parsing uncompressed files.
Importer now supports nearly all non-video uncompressed formats.
Includes a variety of minor-to-moderate fixes made along the way
which could not be pulled out into separate commits due to dependence
on other aspects of the rewrite.
Per the official format specification and hardware implementation,
BC1 cutout pixels are always black with 0 alpha, whereas previous
versions of the plugin interpreted them as white. Commit f97e718e
partially fixed this, but made the behavior an import option that
also ignored the alpha component. This commit reverts the addition
of this option in favor of consistently following the spec.
The export properties "save-type" and "mipmaps" were left as ints
in Commit 427130be due to uncertainty on whether options could be
conditionally disabled on GimpChoice properties. From testing it
this functionality appears to work fine and is used in other plugins.
Now all combo props are GimpChoices and fully configurable from dds.c
Moves most of the code relating to YCoCg and Alpha Exponent into
misc.c/h, in the interest of making the rest of the codebase cleaner.
Removes the decode option from the import menu, as encoded files are
always decoded now (there used to be a menu button for doing this
after import, but with it gone there's no reason ever to not decode).
Finally, the remaining functions in color.c were only ever called once,
so these were extracted and inlined, and the empty file deleted.
Partial refactor of parts of ddsread to better handle DX10+ files.
Though not included here, this is necessary to import formats that
have no DX9 equivalent, namely BC6H and BC7, as the current handling
simply downgrades DX10 files and imports them like DX9 ones.
Extensive formatting and cleanup in the read-write files, and addition
of some comments where they seemed appropriate. Renamed a couple of
single-letter variables to be more descriptive.
Also removed an unnecessary global variable in ddswrite.c, and made
the "config" variable in ddsread a GimpProcedureConfig type, as was
previously done for ddswrite.
Fixes two major issues with mipmap generation, namely sRGB transforms
being applied both backwards and in 8-bit precision, causing severe
color degradation. sRGB transforms are now handled correctly and all
mipmap generation is done at 32-bit floating-point precision.
A new cubic filter has also been added (Catmull-Rom) which rounds-out
the existing lineup of cubic filters.
Also includes extensive code cleanup (sorry I couldn't separate this)
to mipmap.c/h and color.c/h
This adds an import option for DDS DXT1/BC1 images to always use
transparency. This is the default behavior, since this was what always
happened until now and it seems that DDPF_ALPHAPIXELS is very rarely
set for these type of images.
However, as the mentioned issues explains, advanced compression
algorithms can use this transparency data instead to mean a black
pixel. There is however, no certain way to determine this.
For that reason, we add an option here, that, if disabled, will
interpret fully transparent values as a black pixel.
Some games apparently need dds images to be vertically flipped.
We already have an option to flip dds images on export, so it makes
sense to also allow flipping on import.
In addition to the requested DXGI variants we also load the older
D3DF versions, including handling of the signed versions.
We also set signed when the pixelformat flag DDPF_BUMPDUDV (added in
the previous commit) is set.
This adds support for loading RGBA DDS images with 16- and 32-bit per
channel. Loading is supported for 16-bit half float, 32-bit float,
16 and 32-bit unsigned and signed int.
This supports both the DX10 formats and the D3D FOURCC versions.
16- and 32-bit per channel images with less than 4 channels are not
yet supported.
Certain dds images can have non-zero unexpected pitch_or_linsize values.
Until now we were only computing this ourselves in case it was zero.
Let's just always compute it and print an error to the terminal if it
differs from the value in the file.
A sample can be found in Galactic Civilizations 3: Bokeh_Hex.dds.
This change also allows us to safely load the poc in security issue
ZDI-CAN-22093 as that issue was apparently only caused by an invalid
value of pitch_or_linsize.
Optimize dds loading a bit by moving code that doesn't change outside
the loop:
1. The number of bits to be shifted when the source isn't exactly 8 or
16 bits depends on bytes per sample and isn't changing inside the loop.
2. Use rowstride variable to compute width * d->bpp once.
3. The check for rowstride > hdr->pitch_or_linsize doesn't change
inside the loop so move it out.
Inside the loop we only check the DDSD_PITCH flag once and move both
the size check and the fread check inside it.
