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

[Matteo Bruni]

2016-01-27 10:29 GMT+01:00 Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com>:
> Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com>
> ---
>  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 <matteo.mystral(a)gmail.com> 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
<leslie_alistair(a)hotmail.com> 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);