Re: [PATCH] d3dx9: Signature isn't part of the DDS header

[Matteo Bruni]

2016-01-27 10:29 GMT+01:00 Alistair Leslie-Hughes :
> Signed-off-by: Alistair Leslie-Hughes
>
> dlls/d3dx9_36/surface.c | 78 ++++++++++++++++++++++++++-----------------------
>  1 file changed, 41 insertions(+), 37 deletions(-)
>
> diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c
> index 4fa2a76..a7bf9b7 100644
> --- a/dlls/d3dx9_36/surface.c
> +++ b/dlls/d3dx9_36/surface.c
> @@ -125,7 +125,6 @@ struct dds_pixel_format
>
> struct dds_header
>  {
> - DWORD signature;
> DWORD size;
> DWORD flags;
> DWORD height;
>
> @@ -142,6 +141,12 @@ struct dds_header
>      DWORD reserved2;
>  };
>
> +struct DDS

Probably better to pick a lowercase struct name.
> +{
> - DWORD signature;
> - struct dds_header header;
>
> +};
> -
>
> static D3DFORMAT dds_fourcc_to_d3dformat(DWORD fourcc)
>  {
>      unsigned int i;
> @@ -355,7 +360,7 @@ static UINT calculate_dds_file_size(D3DFORMAT format, UINT width, UINT height, U
>      }
>  file_size *= faces;
> - file_size += sizeof(struct dds_header);
> - file_size += sizeof(struct DDS);
> return file_size;
>
> }
>
> @@ -379,37 +384,37 @@ static HRESULT get_image_info_from_dds(const void *buffer, UINT length, D3DXIMAG
>  {
>      UINT faces = 1;
>      UINT expected_length;
> - const struct dds_header *header = buffer;
> - const struct DDS *dds = buffer;

I'd keep the *header variable, initializing it to &dds->header...
> - if (length < sizeof(*header) || !info)
> - if (length < sizeof(*dds) || !info)
>     return D3DXERR_INVALIDDATA;
> - if (header->pixel_format.size != sizeof(header->pixel_format))
> - if (dds->header.pixel_format.size != sizeof(dds->header.pixel_format))
>     return D3DXERR_INVALIDDATA;

... so that you can leave this and the other similar lines below unchanged.
> - info->Width = header->width;
> - info->Height = header->height;
> - info->Width = dds->header.width;
> - info->Height = dds->header.height;
> info->Depth = 1;
> - info->MipLevels = header->miplevels ? header->miplevels : 1;
> - info->MipLevels = dds->header.miplevels ? dds->header.miplevels : 1;
> - info->Format = dds_pixel_format_to_d3dformat(&header->pixel_format);
> -
>
> info->Format = dds_pixel_format_to_d3dformat(&dds->header.pixel_format);
> if (info->Format == D3DFMT_UNKNOWN)
>     return D3DXERR_INVALIDDATA;
>
> TRACE("Pixel format is %#x\n", info->Format);
> - if (header->caps2 & DDS_CAPS2_VOLUME)
> - if (dds->header.caps2 & DDS_CAPS2_VOLUME)
> {
> -
>    info->Depth = header->depth;
> -
>    info->Depth = dds->header.depth;
>    info->ResourceType = D3DRTYPE_VOLUMETEXTURE;
> }
> - else if (header->caps2 & DDS_CAPS2_CUBEMAP)
> - else if (dds->header.caps2 & DDS_CAPS2_CUBEMAP)
> {
>     DWORD face;
>     faces = 0;
>     for (face = DDS_CAPS2_CUBEMAP_POSITIVEX; face <= DDS_CAPS2_CUBEMAP_NEGATIVEZ; face <<= 1)
>     {
> -
>        if (header->caps2 & face)
> -
>        if (dds->header.caps2 & face)
>            faces++;
>    }
>    info->ResourceType = D3DRTYPE_CUBETEXTURE;
>
> @@ -437,8 +442,8 @@ static HRESULT load_surface_from_dds(IDirect3DSurface9 *dst_surface, const PALET
>  {
>      UINT size;
>      UINT src_pitch;
> - const struct dds_header *header = src_data;
> - const BYTE *pixels = (BYTE *)(header + 1);
> - const struct DDS *dds = src_data;
> - const BYTE *pixels = (BYTE *)(dds + 1);

I guess you could instead make the src_data parameter BYTE * and
compute "pixels" as src_data + sizeof(struct dds) (or however you want
to call the new struct). It's mostly a matter of taste though so it's
fine to me either way.

[PATCH] d3dx9: Signature isn't part of the DDS header

[Henri Verbeet]

On 28 January 2016 at 00:20, Matteo Bruni  wrote:
> > @@ -437,8 +442,8 @@ static HRESULT load_surface_from_dds(IDirect3DSurface9 *dst_surface, const PALET
> >  {
> >      UINT size;
> >      UINT src_pitch;
> > - const struct dds_header *header = src_data;
> > - const BYTE *pixels = (BYTE *)(header + 1);
> > - const struct DDS *dds = src_data;
> > - const BYTE *pixels = (BYTE *)(dds + 1);
>
> I guess you could instead make the src_data parameter BYTE * and
> compute "pixels" as src_data + sizeof(struct dds) (or however you want
> to call the new struct). It's mostly a matter of taste though so it's
> fine to me either way.

If you define struct dds as
    struct dds
    {
        DWORD signature;
       struct dds_header header;
       BYTE data[1];
    };
you can just change load_surface_from_dds() to take "const struct dds
*dds" as parameter instead of src_data, and pass dds->data to
D3DXLoadSurfaceFromMemory(). Or just pass dds->data to
load_surface_from_dds() and rename that function, since it doesn't do
all that much with the information in the header.

It also means you can do things like "return FIELD_OFFSET(struct dds,
data[data_size]);" in calculate_dds_file_size().

You may also want to consider just merging surface.c and volume.c into
texture.c.

On 27 January 2016 at 10:29, Alistair Leslie-Hughes
 wrote:
> - memset(dds, 0, sizeof(*dds));
> - dds->signature = MAKEFOURCC('D','D','S',' ');
> - dds->header.size = sizeof(struct dds_header);

+    dds->header.size = sizeof(dds->header);