[PATCH 0/1] MR7091: windowscodecs: Make values returned from CanConvert consistent
Any time CanConvert returns FALSE for the output parameter, also return an appropriate failing result code. This also adds tests for CanConvert. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7091
From: Jeff Smith <whydoubt(a)gmail.com> Any time CanConvert returns FALSE for the output parameter, also return an appropriate failing result code. This also adds tests for CanConvert. --- dlls/windowscodecs/converter.c | 42 ++++++- dlls/windowscodecs/tests/converter.c | 165 ++++++++++++++++++++++++++- 2 files changed, 200 insertions(+), 7 deletions(-) diff --git a/dlls/windowscodecs/converter.c b/dlls/windowscodecs/converter.c index 39fcce61dd8..e86a513e6a4 100644 --- a/dlls/windowscodecs/converter.c +++ b/dlls/windowscodecs/converter.c @@ -1643,6 +1643,22 @@ static const struct pixelformatinfo supported_formats[] = { {0} }; +/* Formats supported in Windows as both source and destination, but not supported in Wine */ +static const WICPixelFormatGUID *missing_native_formats[] = { + &GUID_WICPixelFormat8bppAlpha, + &GUID_WICPixelFormat48bppBGR, + &GUID_WICPixelFormat64bppRGB, + &GUID_WICPixelFormat64bppBGRA, + &GUID_WICPixelFormat64bppPRGBA, + &GUID_WICPixelFormat64bppPBGRA, + &GUID_WICPixelFormat32bppBGR101010, + &GUID_WICPixelFormat96bppRGBFloat, + &GUID_WICPixelFormat128bppRGBAFloat, + &GUID_WICPixelFormat128bppPRGBAFloat, + &GUID_WICPixelFormat128bppRGBFloat, + NULL +}; + static const struct pixelformatinfo *get_formatinfo(const WICPixelFormatGUID *format) { UINT i; @@ -1653,6 +1669,16 @@ static const struct pixelformatinfo *get_formatinfo(const WICPixelFormatGUID *fo return NULL; } +static BOOL is_missing_native_format(const WICPixelFormatGUID *format) +{ + UINT i; + + for (i=0; missing_native_formats[i]; i++) + if (IsEqualGUID(missing_native_formats[i], format)) return TRUE; + + return FALSE; +} + static HRESULT WINAPI FormatConverter_QueryInterface(IWICFormatConverter *iface, REFIID iid, void **ppv) { @@ -1914,26 +1940,30 @@ static HRESULT WINAPI FormatConverter_CanConvert(IWICFormatConverter *iface, srcinfo = get_formatinfo(srcPixelFormat); if (!srcinfo) { - FIXME("Unsupported source format %s\n", debugstr_guid(srcPixelFormat)); + if (is_missing_native_format(srcPixelFormat) || IsEqualGUID(&GUID_WICPixelFormat32bppR10G10B10A2, srcPixelFormat)) + FIXME("Unsupported source format %s\n", debugstr_guid(srcPixelFormat)); + *pfCanConvert = FALSE; return WINCODEC_ERR_UNSUPPORTEDPIXELFORMAT; } dstinfo = get_formatinfo(dstPixelFormat); if (!dstinfo) { - FIXME("Unsupported destination format %s\n", debugstr_guid(dstPixelFormat)); + if (is_missing_native_format(dstPixelFormat)) + FIXME("Unsupported destination format %s\n", debugstr_guid(dstPixelFormat)); + *pfCanConvert = FALSE; return WINCODEC_ERR_UNSUPPORTEDPIXELFORMAT; } - if (dstinfo->copy_function && - SUCCEEDED(dstinfo->copy_function(This, NULL, 0, 0, NULL, dstinfo->format))) - *pfCanConvert = TRUE; - else + if (!dstinfo->copy_function || + FAILED(dstinfo->copy_function(This, NULL, 0, 0, NULL, dstinfo->format))) { FIXME("Unsupported conversion %s -> %s\n", debugstr_guid(srcPixelFormat), debugstr_guid(dstPixelFormat)); *pfCanConvert = FALSE; + return WINCODEC_ERR_UNSUPPORTEDPIXELFORMAT; } + *pfCanConvert = TRUE; return S_OK; } diff --git a/dlls/windowscodecs/tests/converter.c b/dlls/windowscodecs/tests/converter.c index f6cef841d56..6fdb54783f8 100644 --- a/dlls/windowscodecs/tests/converter.c +++ b/dlls/windowscodecs/tests/converter.c @@ -707,6 +707,168 @@ static void test_default_converter(void) DeleteTestBitmap(src_obj); } +static void test_can_convert(void) +{ +#define WIC_PIXEL_FORMAT(fmt) #fmt, &GUID_WICPixelFormat ## fmt + static const struct test_data + { + const char *name; + const WICPixelFormatGUID *format; + BOOL src_valid; + BOOL dst_valid; + BOOL src_todo; + BOOL dst_todo; + BOOL src_broken; + } + td[] = + { + {WIC_PIXEL_FORMAT(Undefined)}, + {WIC_PIXEL_FORMAT(1bppIndexed), TRUE, TRUE, FALSE, TRUE}, + {WIC_PIXEL_FORMAT(2bppIndexed), TRUE, TRUE, FALSE, TRUE}, + {WIC_PIXEL_FORMAT(4bppIndexed), TRUE, TRUE, FALSE, TRUE}, + {WIC_PIXEL_FORMAT(8bppIndexed), TRUE, TRUE}, + {WIC_PIXEL_FORMAT(BlackWhite), TRUE, TRUE, FALSE, TRUE}, + {WIC_PIXEL_FORMAT(2bppGray), TRUE, TRUE, FALSE, TRUE}, + {WIC_PIXEL_FORMAT(4bppGray), TRUE, TRUE, FALSE, TRUE}, + {WIC_PIXEL_FORMAT(8bppGray), TRUE, TRUE}, + {WIC_PIXEL_FORMAT(16bppGray), TRUE, TRUE, FALSE, TRUE}, + + {WIC_PIXEL_FORMAT(8bppAlpha), TRUE, TRUE, TRUE, TRUE}, + + {WIC_PIXEL_FORMAT(16bppBGR555), TRUE, TRUE, FALSE, TRUE}, + {WIC_PIXEL_FORMAT(16bppBGR565), TRUE, TRUE, FALSE, TRUE}, + {WIC_PIXEL_FORMAT(16bppBGRA5551), TRUE, TRUE}, + {WIC_PIXEL_FORMAT(24bppBGR), TRUE, TRUE}, + {WIC_PIXEL_FORMAT(24bppRGB), TRUE, TRUE}, + {WIC_PIXEL_FORMAT(32bppBGR), TRUE, TRUE}, + {WIC_PIXEL_FORMAT(32bppBGRA), TRUE, TRUE}, + {WIC_PIXEL_FORMAT(32bppPBGRA), TRUE, TRUE}, + {WIC_PIXEL_FORMAT(32bppRGB), TRUE, TRUE}, + {WIC_PIXEL_FORMAT(32bppRGBA), TRUE, TRUE}, + {WIC_PIXEL_FORMAT(32bppPRGBA), TRUE, TRUE}, + {WIC_PIXEL_FORMAT(32bppGrayFloat), TRUE, TRUE}, + + {WIC_PIXEL_FORMAT(48bppRGB), TRUE, TRUE, FALSE, TRUE}, + {WIC_PIXEL_FORMAT(48bppBGR), TRUE, TRUE, TRUE, TRUE}, + {WIC_PIXEL_FORMAT(64bppRGB), TRUE, TRUE, TRUE, TRUE}, + {WIC_PIXEL_FORMAT(64bppRGBA), TRUE, TRUE}, + {WIC_PIXEL_FORMAT(64bppBGRA), TRUE, TRUE, TRUE, TRUE}, + {WIC_PIXEL_FORMAT(64bppPRGBA), TRUE, TRUE, TRUE, TRUE}, + {WIC_PIXEL_FORMAT(64bppPBGRA), TRUE, TRUE, TRUE, TRUE}, + + {WIC_PIXEL_FORMAT(16bppGrayFixedPoint)}, + {WIC_PIXEL_FORMAT(32bppBGR101010), TRUE, TRUE, TRUE, TRUE}, + {WIC_PIXEL_FORMAT(48bppRGBFixedPoint)}, + {WIC_PIXEL_FORMAT(48bppBGRFixedPoint)}, + {WIC_PIXEL_FORMAT(96bppRGBFixedPoint)}, + {WIC_PIXEL_FORMAT(96bppRGBFloat), TRUE, TRUE, TRUE, TRUE}, + {WIC_PIXEL_FORMAT(128bppRGBAFloat), TRUE, TRUE, TRUE, TRUE}, + {WIC_PIXEL_FORMAT(128bppPRGBAFloat), TRUE, TRUE, TRUE, TRUE}, + {WIC_PIXEL_FORMAT(128bppRGBFloat), TRUE, TRUE, TRUE, TRUE}, + + /* Not actually TODO, but it succeeds where Windows fails */ + {WIC_PIXEL_FORMAT(32bppCMYK), FALSE, FALSE, TRUE}, + + {WIC_PIXEL_FORMAT(64bppRGBAFixedPoint)}, + {WIC_PIXEL_FORMAT(64bppBGRAFixedPoint)}, + {WIC_PIXEL_FORMAT(64bppRGBFixedPoint)}, + {WIC_PIXEL_FORMAT(128bppRGBAFixedPoint)}, + {WIC_PIXEL_FORMAT(128bppRGBFixedPoint)}, + + {WIC_PIXEL_FORMAT(64bppRGBAHalf)}, + {WIC_PIXEL_FORMAT(64bppPRGBAHalf)}, + {WIC_PIXEL_FORMAT(64bppRGBHalf)}, + {WIC_PIXEL_FORMAT(48bppRGBHalf)}, + + {WIC_PIXEL_FORMAT(32bppRGBE)}, + + {WIC_PIXEL_FORMAT(16bppGrayHalf)}, + {WIC_PIXEL_FORMAT(32bppGrayFixedPoint)}, + + {WIC_PIXEL_FORMAT(32bppRGBA1010102)}, + {WIC_PIXEL_FORMAT(32bppRGBA1010102XR)}, + + /* Starting with Windows 10 v1809, this works as a source format */ + {WIC_PIXEL_FORMAT(32bppR10G10B10A2), TRUE, FALSE, TRUE, FALSE, TRUE}, + + {WIC_PIXEL_FORMAT(32bppR10G10B10A2HDR10)}, + + {WIC_PIXEL_FORMAT(64bppCMYK)}, + + {WIC_PIXEL_FORMAT(24bpp3Channels)}, + {WIC_PIXEL_FORMAT(32bpp4Channels)}, + {WIC_PIXEL_FORMAT(40bpp5Channels)}, + {WIC_PIXEL_FORMAT(48bpp6Channels)}, + {WIC_PIXEL_FORMAT(56bpp7Channels)}, + {WIC_PIXEL_FORMAT(64bpp8Channels)}, + + {WIC_PIXEL_FORMAT(48bpp3Channels)}, + {WIC_PIXEL_FORMAT(64bpp4Channels)}, + {WIC_PIXEL_FORMAT(80bpp5Channels)}, + {WIC_PIXEL_FORMAT(96bpp6Channels)}, + {WIC_PIXEL_FORMAT(112bpp7Channels)}, + {WIC_PIXEL_FORMAT(128bpp8Channels)}, + + {WIC_PIXEL_FORMAT(40bppCMYKAlpha)}, + {WIC_PIXEL_FORMAT(80bppCMYKAlpha)}, + + {WIC_PIXEL_FORMAT(32bpp3ChannelsAlpha)}, + {WIC_PIXEL_FORMAT(40bpp4ChannelsAlpha)}, + {WIC_PIXEL_FORMAT(48bpp5ChannelsAlpha)}, + {WIC_PIXEL_FORMAT(56bpp6ChannelsAlpha)}, + {WIC_PIXEL_FORMAT(64bpp7ChannelsAlpha)}, + {WIC_PIXEL_FORMAT(72bpp8ChannelsAlpha)}, + + {WIC_PIXEL_FORMAT(64bpp3ChannelsAlpha)}, + {WIC_PIXEL_FORMAT(80bpp4ChannelsAlpha)}, + {WIC_PIXEL_FORMAT(96bpp5ChannelsAlpha)}, + {WIC_PIXEL_FORMAT(112bpp6ChannelsAlpha)}, + {WIC_PIXEL_FORMAT(128bpp7ChannelsAlpha)}, + {WIC_PIXEL_FORMAT(144bpp8ChannelsAlpha)}, + + {WIC_PIXEL_FORMAT(8bppY)}, + {WIC_PIXEL_FORMAT(8bppCb)}, + {WIC_PIXEL_FORMAT(8bppCr)}, + {WIC_PIXEL_FORMAT(16bppCbCr)}, + + {WIC_PIXEL_FORMAT(16bppYQuantizedDctCoefficients)}, + {WIC_PIXEL_FORMAT(16bppCbQuantizedDctCoefficients)}, + {WIC_PIXEL_FORMAT(16bppCrQuantizedDctCoefficients)}, + }; +#undef WIC_PIXEL_FORMAT + BOOL can_convert, can_native, can_broken, can_wine; + HRESULT hr, expect_hr, broken_hr; + IWICFormatConverter *converter; + UINT i, j; + + hr = CoCreateInstance(&CLSID_WICDefaultFormatConverter, NULL, CLSCTX_INPROC_SERVER, + &IID_IWICFormatConverter, (void**)&converter); + ok(SUCCEEDED(hr), "CoCreateInstance failed, hr=%lx\n", hr); + if (FAILED(hr)) + return; + + for (i = 0; i < ARRAY_SIZE(td); i++) + { + for (j = 0; j < ARRAY_SIZE(td); j++) + { + can_native = td[i].src_valid && td[j].dst_valid; + can_broken = (td[i].src_valid ^ td[i].src_broken) && td[j].dst_valid; + can_wine = (td[i].src_valid ^ td[i].src_todo) && (td[j].dst_valid ^ td[j].dst_todo); + expect_hr = can_native ? S_OK : WINCODEC_ERR_UNSUPPORTEDPIXELFORMAT; + broken_hr = can_broken ? S_OK : WINCODEC_ERR_UNSUPPORTEDPIXELFORMAT; + + can_convert = -1; + hr = IWICFormatConverter_CanConvert(converter, td[i].format, td[j].format, &can_convert); + todo_wine_if (can_native != can_wine) { + ok(hr == expect_hr || broken(hr == broken_hr), + "CanConvert (%s -> %s) returned %lx\n", td[i].name, td[j].name, hr); + ok(can_convert == can_native || broken(can_convert == can_broken), + "expected %i, got %i \n", can_native, can_convert); + } + } + } +} + static void test_converter_4bppGray(void) { BitmapTestSrc *src_obj; @@ -723,7 +885,7 @@ static void test_converter_4bppGray(void) { hr = IWICFormatConverter_CanConvert(converter, &GUID_WICPixelFormat32bppBGRA, &GUID_WICPixelFormat4bppGray, &can_convert); - ok(SUCCEEDED(hr), "CanConvert returned %lx\n", hr); + todo_wine ok(SUCCEEDED(hr), "CanConvert returned %lx\n", hr); todo_wine ok(can_convert, "expected TRUE, got %i\n", can_convert); hr = IWICFormatConverter_Initialize(converter, &src_obj->IWICBitmapSource_iface, @@ -2088,6 +2250,7 @@ START_TEST(converter) test_invalid_conversion(); test_default_converter(); + test_can_convert(); test_converter_4bppGray(); test_converter_8bppGray(); test_converter_8bppIndexed(); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7091
Esme Povirk (@madewokherd) commented about dlls/windowscodecs/tests/converter.c:
+ {WIC_PIXEL_FORMAT(8bppCb)}, + {WIC_PIXEL_FORMAT(8bppCr)}, + {WIC_PIXEL_FORMAT(16bppCbCr)}, + + {WIC_PIXEL_FORMAT(16bppYQuantizedDctCoefficients)}, + {WIC_PIXEL_FORMAT(16bppCbQuantizedDctCoefficients)}, + {WIC_PIXEL_FORMAT(16bppCrQuantizedDctCoefficients)}, + }; +#undef WIC_PIXEL_FORMAT + BOOL can_convert, can_native, can_broken, can_wine; + HRESULT hr, expect_hr, broken_hr; + IWICFormatConverter *converter; + UINT i, j; + + hr = CoCreateInstance(&CLSID_WICDefaultFormatConverter, NULL, CLSCTX_INPROC_SERVER, + &IID_IWICFormatConverter, (void**)&converter); converter is leaked.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7091#note_91302
Esme Povirk (@madewokherd) commented about dlls/windowscodecs/converter.c:
+/* Formats supported in Windows as both source and destination, but not supported in Wine */ +static const WICPixelFormatGUID *missing_native_formats[] = { + &GUID_WICPixelFormat8bppAlpha, + &GUID_WICPixelFormat48bppBGR, + &GUID_WICPixelFormat64bppRGB, + &GUID_WICPixelFormat64bppBGRA, + &GUID_WICPixelFormat64bppPRGBA, + &GUID_WICPixelFormat64bppPBGRA, + &GUID_WICPixelFormat32bppBGR101010, + &GUID_WICPixelFormat96bppRGBFloat, + &GUID_WICPixelFormat128bppRGBAFloat, + &GUID_WICPixelFormat128bppPRGBAFloat, + &GUID_WICPixelFormat128bppRGBFloat, + NULL +};
Wouldn't it be simpler to add these to the existing table without any conversion path? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7091#note_91303
participants (3)
-
Esme Povirk (＠madewokherd) -
Jeff Smith
-
Jeffrey Smith (＠whydoubt)