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

[Matteo Bruni]

2015-12-03 7:55 GMT+01:00 Alistair Leslie-Hughes :
> From: Christian Costa
>
> The different fixes are:
> - Fix header size of the DDS file
> - Remove DDS_MIPMAPCOUNT as mipmap levels are not supported yet

That's okay but not for the the right reason (a good reason is
matching native, as the test in the previous patch shows).
> - Do not set depth and miplevels fields as their flags are not set (to match native)

That's not really significant, the DDS_MIPMAPCOUNT flag is effectively
ignored when loading DDS files (see test_dds_header_handling()).
Again, setting depth and miplevels to 0 matches native so that's fine
for now.
> Signed-off-by: Alistair Leslie-Hughes
>
> dlls/d3dx9_36/surface.c       | 7 +++----
>  dlls/d3dx9_36/tests/surface.c | 8 ++++----
>  2 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c
> index b14f5e7..6857c56 100644
> --- a/dlls/d3dx9_36/surface.c
> +++ b/dlls/d3dx9_36/surface.c
> @@ -487,13 +487,12 @@ static HRESULT save_dds_surface_to_memory(ID3DXBuffer **dst_buffer, IDirect3DSur
>  memset(header, 0, sizeof(*header));

Idea for a possible followup, we allocate the ID3DXBuffer objects data
with HEAP_ZERO_MEMORY so that memset is unnecessary.
There might be more of that kind of unnecessary explicit zeroing in d3dx9.
This requires a test for D3DXCreateBuffer() (probably in
test_ID3DXBuffer()) to check that we're actually supposed to zero the
memory in the first place.
>  header->signature = MAKEFOURCC('D','D','S',' ');
> - 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);
> - 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;
> - header->depth = 1;
> - header->miplevels = 1;
> header->caps = DDS_CAPS_TEXTURE;
> hr = d3dformat_to_dds_pixel_format(&header->pixel_format, src_desc.Format);
> if (FAILED(hr))
>
> diff --git a/dlls/d3dx9_36/tests/surface.c b/dlls/d3dx9_36/tests/surface.c
> index 1c85ca3..b735af0 100644
> --- a/dlls/d3dx9_36/tests/surface.c
> +++ b/dlls/d3dx9_36/tests/surface.c
> @@ -1274,12 +1274,12 @@ static void test_D3DXSaveSurfaceToFileInMemory(IDirect3DDevice9 *device)
>          header = ID3DXBuffer_GetBufferPointer(buffer);
>      ok(header->magic == MAKEFOURCC('D','D','S',' '), "Invalid DDS signature\n");
> -
>    todo_wine ok(header->size == 124, "Invalid DDS size %d\n", header->size);
> -
>    ok(header->size == 124, "Invalid DDS size %d\n", header->size);
>    ok(header->height == 4, "Wrong height %d\n", header->height);
>    ok(header->width == 4, "Wrong width %d\n", header->width);
> -
>    todo_wine ok(header->depth == 0, "Wrong depth %d\n", header->depth);
> -
>    todo_wine ok(header->miplevels == 0, "Wrong miplevels %d\n", header->miplevels);
> -
>    todo_wine ok(header->flags == (DDS_CAPS | DDS_HEIGHT | DDS_WIDTH | DDS_PIXELFORMAT),
> -
>    ok(header->depth == 0, "Wrong depth %d\n", header->depth);
> -
>    ok(header->miplevels == 0, "Wrong miplevels %d\n", header->miplevels);
> -
>    ok(header->flags == (DDS_CAPS | DDS_HEIGHT | DDS_WIDTH | DDS_PIXELFORMAT),
>                 "Wrong flags %x\n", header->flags);
>    ID3DXBuffer_Release(buffer);
> }
>
> --
> 1.9.1