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

[Ziqing Hui]

Hi Bruni,

Thanks for reviewing. I'll sent a v3 version later.

According to my tests, at least GetImageInfoFromMemory() supports uncompressed DDS. Native WIC doesn't support uncompressed DDS, but I would like to support it in wine's implementation for WIC.

On 7/8/20 5:15 AM, Matteo Bruni wrote:
> 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...