[PATCH v2 2/2] d3dx10: Implement D3DX10GetImageInfoFromMemory().

[Matteo Bruni]

On Fri, Jul 3, 2020 at 1:06 PM Ziqing Hui <zhui at codeweavers.com> wrote:
>
>
> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=48856
> Signed-off-by: Ziqing Hui <zhui at codeweavers.com>
> ---
> v2:
> * Add Wine-Bug link in commit message.
> * Add parameter name for WICCreateImagingFactory_Proxy() declaration.
> * Change variable declarations order to a reverse Christmas tree.
> * Move FIXME() for WMP format to the beginning of the function.
>
>  dlls/d3dx10_43/Makefile.in      |   1 +
>  dlls/d3dx10_43/d3dx10_43_main.c | 143 +++++++++++++++++++++++++++++++-
>  dlls/d3dx10_43/tests/d3dx10.c   |   7 +-
>  3 files changed, 144 insertions(+), 7 deletions(-)

The patch is generally okay. I do have a number of small nits though.

+static D3DX10_IMAGE_FILE_FORMAT
wic_container_guid_to_file_format(GUID *container_format)
+{
+    if (IsEqualGUID(container_format, &GUID_ContainerFormatBmp))
+        return D3DX10_IFF_BMP;
+    else if(IsEqualGUID(container_format, &GUID_ContainerFormatJpeg))
+        return D3DX10_IFF_JPG;
+    else if (IsEqualGUID(container_format, &GUID_ContainerFormatPng))
+        return D3DX10_IFF_PNG;
+    else if(IsEqualGUID(container_format, &GUID_ContainerFormatDds))
+        return D3DX10_IFF_DDS;
+    else if(IsEqualGUID(container_format, &GUID_ContainerFormatTiff))
+        return D3DX10_IFF_TIFF;
+    else if(IsEqualGUID(container_format, &GUID_ContainerFormatGif))
+        return D3DX10_IFF_GIF;
+    else if(IsEqualGUID(container_format, &GUID_ContainerFormatWmp))
+        return D3DX10_IFF_WMP;
+    else
+        return D3DX10_IFF_FORCE_DWORD;
+}

All the elses are unnecessary and should be dropped. In fact, it's
probably better to replicate d3dx9 here and implement this with a
table instead.

+    if (pump)
+        FIXME("Thread pump is not supported yet.");

Missing '\n' at the end of the message. More of the same below.

+    if ((src_data_size >= 4) && (!strncmp(src_data, "II\xbc\x00", 4)
|| !strncmp(src_data, "II\xbc\x01", 4)))
+    {
+        FIXME("File type WMP is not supported yet.\n");
+        return E_FAIL;
+    }

This is not ideal. In particular, doing this with string comparisons
looks awkward.
I'd get rid of the check entirely and turn the "Invalid or unsupported
image file" TRACE at the end of the file into a WARN instead.

+    hr = IWICStream_InitializeFromMemory(stream, (BYTE*)src_data,
(DWORD)src_data_size);

The src_data_size cast shouldn't be needed.

+end:
+    if (factory)
+        IWICImagingFactory_Release(factory);
+    if (stream)
+        IWICStream_Release(stream);
+    if (decoder)
+        IWICBitmapDecoder_Release(decoder);
+    if (frame)
+        IWICBitmapFrameDecode_Release(frame);
+    if (dds_decoder)
+        IWICDdsDecoder_Release(dds_decoder);

Nitpick: I'd prefer to have the release happen in reverse order of
initialization.

I have a somewhat related question. Does d3dx10 support loading
uncompressed DDS files? I know that WIC doesn't, but d3dx10 might have
internal support for that...