Re: [PATCH 2/2] d3dx9_36: Fix several issues in save_dds_surface_to_memory. (try 2)

[Henri Verbeet]

Any time you find yourself writing something along the lines of "Fix
several issues ..." in a patch description, that's an indication you
should probably be writing several patches instead.

On 7 December 2015 at 07:53, Alistair Leslie-Hughes
<leslie_alistair(a)hotmail.com> wrote:
> From: Christian Costa <titan.costa(a)gmail.com>
>
> This makes the default values line up with native.
>
> Signed-off-by: Alistair Leslie-Hughes <leslie_alistair(a)hotmail.com>
> I suppose a Signed-off-by from the original author as well would too
> much to ask. /shrug.
> -    header->size = sizeof(*header);
> -    header->flags = DDS_CAPS | DDS_HEIGHT | DDS_WIDTH | DDS_PITCH | DDS_PIXELFORMAT | DDS_MIPMAPCOUNT;
> +    /* The signature is not really part of the DDS header */
> +    header->size = sizeof(*header) - sizeof(header->signature);
> This isn't necessarily wrong, although it would be more correct to
> write this kind of thing as "sizeof(*header) - FIELD_OFFSET(struct
> dds_header, size)". As the comment already indicates though, the real
> issue is that the signature field shouldn't be part of the same
> structure.
> +    header->flags = DDS_CAPS | DDS_HEIGHT | DDS_WIDTH | DDS_PIXELFORMAT;
>      header->height = src_desc.Height;
>      header->width = src_desc.Width;
>      header->pitch_or_linear_size = dst_pitch;
> You remove DDS_PITCH from the flags, but set the field here anyway.
> The test doesn't test it. Somewhat similarly, the test doesn't verify
> the value of the "caps" field.