[PATCH v2 0/3] MR6704: d3dx9: Some changes to pixel format handling on the WIC image decode path.
This adds support for converting to different WIC formats, in one case to more closely match native d3dx9, and in the other to convert to a format that can actually be represented by d3d9. -- v2: d3dx9: Add support for decoding 48bpp RGB PNG files. d3dx9: Report 24bpp RGB as 32bpp XRGB for JPG and PNG files. https://gitlab.winehq.org/wine/wine/-/merge_requests/6704
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/d3dx9_36/tests/d3dx9_test_images.h | 90 +++++++++++++++++++++++++ dlls/d3dx9_36/tests/surface.c | 47 +++++++++++++ 2 files changed, 137 insertions(+) diff --git a/dlls/d3dx9_36/tests/d3dx9_test_images.h b/dlls/d3dx9_36/tests/d3dx9_test_images.h index 0c4a72da57d..0ad9f830a28 100644 --- a/dlls/d3dx9_36/tests/d3dx9_test_images.h +++ b/dlls/d3dx9_36/tests/d3dx9_test_images.h @@ -133,6 +133,96 @@ static const uint8_t png_grayscale[] = 0x42,0x60,0x82 }; +static const uint8_t png_2_2_24bpp_bgr[] = +{ + 0x89,0x50,0x4e,0x47,0x0d,0x0a,0x1a,0x0a,0x00,0x00,0x00,0x0d,0x49,0x48,0x44,0x52, + 0x00,0x00,0x00,0x02,0x00,0x00,0x00,0x02,0x08,0x02,0x00,0x00,0x00,0xfd,0xd4,0x9a, + 0x73,0x00,0x00,0x00,0x16,0x49,0x44,0x41,0x54,0x08,0x99,0x63,0x54,0x10,0x60,0x30, + 0x30,0x30,0x60,0x49,0x48,0x48,0x30,0x30,0x30,0x00,0x00,0x0e,0x68,0x02,0x76,0xd0, + 0x18,0x48,0x79,0x00,0x00,0x00,0x00,0x49,0x45,0x4e,0x44,0xae,0x42,0x60,0x82, +}; + +static const uint8_t png_2_2_48bpp_rgb[] = +{ + 0x89,0x50,0x4e,0x47,0x0d,0x0a,0x1a,0x0a,0x00,0x00,0x00,0x0d,0x49,0x48,0x44,0x52, + 0x00,0x00,0x00,0x02,0x00,0x00,0x00,0x02,0x10,0x02,0x00,0x00,0x00,0xad,0x44,0x46, + 0x30,0x00,0x00,0x00,0x1d,0x49,0x44,0x41,0x54,0x08,0x99,0x63,0x64,0x60,0x10,0x10, + 0x50,0x50,0x30,0x30,0x30,0x30,0x30,0x30,0x60,0x49,0x48,0x48,0x48,0x48,0x48,0x80, + 0x70,0x00,0x34,0x38,0x04,0xe6,0xd6,0xb5,0xab,0x37,0x00,0x00,0x00,0x00,0x49,0x45, + 0x4e,0x44,0xae,0x42,0x60,0x82, +}; + +static const uint8_t jpg_rgb_2_2[] = +{ + 0xff,0xd8,0xff,0xe0,0x00,0x10,0x4a,0x46,0x49,0x46,0x00,0x01,0x01,0x00,0x00,0x01, + 0x00,0x01,0x00,0x00,0xff,0xdb,0x00,0x43,0x00,0x08,0x06,0x06,0x07,0x06,0x05,0x08, + 0x07,0x07,0x07,0x09,0x09,0x08,0x0a,0x0c,0x14,0x0d,0x0c,0x0b,0x0b,0x0c,0x19,0x12, + 0x13,0x0f,0x14,0x1d,0x1a,0x1f,0x1e,0x1d,0x1a,0x1c,0x1c,0x20,0x24,0x2e,0x27,0x20, + 0x22,0x2c,0x23,0x1c,0x1c,0x28,0x37,0x29,0x2c,0x30,0x31,0x34,0x34,0x34,0x1f,0x27, + 0x39,0x3d,0x38,0x32,0x3c,0x2e,0x33,0x34,0x32,0xff,0xdb,0x00,0x43,0x01,0x08,0x09, + 0x09,0x0c,0x0b,0x0c,0x18,0x0d,0x0d,0x18,0x32,0x21,0x1c,0x21,0x32,0x32,0x32,0x32, + 0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32, + 0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32, + 0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0x32,0xff,0xc0, + 0x00,0x11,0x08,0x00,0x02,0x00,0x02,0x03,0x01,0x22,0x00,0x02,0x11,0x01,0x03,0x11, + 0x01,0xff,0xc4,0x00,0x1f,0x00,0x00,0x01,0x05,0x01,0x01,0x01,0x01,0x01,0x01,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x01,0x02,0x03,0x04,0x05,0x06,0x07,0x08,0x09, + 0x0a,0x0b,0xff,0xc4,0x00,0xb5,0x10,0x00,0x02,0x01,0x03,0x03,0x02,0x04,0x03,0x05, + 0x05,0x04,0x04,0x00,0x00,0x01,0x7d,0x01,0x02,0x03,0x00,0x04,0x11,0x05,0x12,0x21, + 0x31,0x41,0x06,0x13,0x51,0x61,0x07,0x22,0x71,0x14,0x32,0x81,0x91,0xa1,0x08,0x23, + 0x42,0xb1,0xc1,0x15,0x52,0xd1,0xf0,0x24,0x33,0x62,0x72,0x82,0x09,0x0a,0x16,0x17, + 0x18,0x19,0x1a,0x25,0x26,0x27,0x28,0x29,0x2a,0x34,0x35,0x36,0x37,0x38,0x39,0x3a, + 0x43,0x44,0x45,0x46,0x47,0x48,0x49,0x4a,0x53,0x54,0x55,0x56,0x57,0x58,0x59,0x5a, + 0x63,0x64,0x65,0x66,0x67,0x68,0x69,0x6a,0x73,0x74,0x75,0x76,0x77,0x78,0x79,0x7a, + 0x83,0x84,0x85,0x86,0x87,0x88,0x89,0x8a,0x92,0x93,0x94,0x95,0x96,0x97,0x98,0x99, + 0x9a,0xa2,0xa3,0xa4,0xa5,0xa6,0xa7,0xa8,0xa9,0xaa,0xb2,0xb3,0xb4,0xb5,0xb6,0xb7, + 0xb8,0xb9,0xba,0xc2,0xc3,0xc4,0xc5,0xc6,0xc7,0xc8,0xc9,0xca,0xd2,0xd3,0xd4,0xd5, + 0xd6,0xd7,0xd8,0xd9,0xda,0xe1,0xe2,0xe3,0xe4,0xe5,0xe6,0xe7,0xe8,0xe9,0xea,0xf1, + 0xf2,0xf3,0xf4,0xf5,0xf6,0xf7,0xf8,0xf9,0xfa,0xff,0xc4,0x00,0x1f,0x01,0x00,0x03, + 0x01,0x01,0x01,0x01,0x01,0x01,0x01,0x01,0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x01, + 0x02,0x03,0x04,0x05,0x06,0x07,0x08,0x09,0x0a,0x0b,0xff,0xc4,0x00,0xb5,0x11,0x00, + 0x02,0x01,0x02,0x04,0x04,0x03,0x04,0x07,0x05,0x04,0x04,0x00,0x01,0x02,0x77,0x00, + 0x01,0x02,0x03,0x11,0x04,0x05,0x21,0x31,0x06,0x12,0x41,0x51,0x07,0x61,0x71,0x13, + 0x22,0x32,0x81,0x08,0x14,0x42,0x91,0xa1,0xb1,0xc1,0x09,0x23,0x33,0x52,0xf0,0x15, + 0x62,0x72,0xd1,0x0a,0x16,0x24,0x34,0xe1,0x25,0xf1,0x17,0x18,0x19,0x1a,0x26,0x27, + 0x28,0x29,0x2a,0x35,0x36,0x37,0x38,0x39,0x3a,0x43,0x44,0x45,0x46,0x47,0x48,0x49, + 0x4a,0x53,0x54,0x55,0x56,0x57,0x58,0x59,0x5a,0x63,0x64,0x65,0x66,0x67,0x68,0x69, + 0x6a,0x73,0x74,0x75,0x76,0x77,0x78,0x79,0x7a,0x82,0x83,0x84,0x85,0x86,0x87,0x88, + 0x89,0x8a,0x92,0x93,0x94,0x95,0x96,0x97,0x98,0x99,0x9a,0xa2,0xa3,0xa4,0xa5,0xa6, + 0xa7,0xa8,0xa9,0xaa,0xb2,0xb3,0xb4,0xb5,0xb6,0xb7,0xb8,0xb9,0xba,0xc2,0xc3,0xc4, + 0xc5,0xc6,0xc7,0xc8,0xc9,0xca,0xd2,0xd3,0xd4,0xd5,0xd6,0xd7,0xd8,0xd9,0xda,0xe2, + 0xe3,0xe4,0xe5,0xe6,0xe7,0xe8,0xe9,0xea,0xf2,0xf3,0xf4,0xf5,0xf6,0xf7,0xf8,0xf9, + 0xfa,0xff,0xda,0x00,0x0c,0x03,0x01,0x00,0x02,0x11,0x03,0x11,0x00,0x3f,0x00,0xb7, + 0x69,0xa4,0xe9,0xbf,0x62,0x83,0xfe,0x25,0xf6,0x9f,0xea,0xd7,0xfe,0x58,0xaf,0xa7, + 0xd2,0x8a,0x28,0xae,0xb3,0x98,0xff,0xd9, +}; + +static const uint8_t jpg_grayscale_2_2[] = +{ + 0xff,0xd8,0xff,0xe0,0x00,0x10,0x4a,0x46,0x49,0x46,0x00,0x01,0x01,0x00,0x00,0x01, + 0x00,0x01,0x00,0x00,0xff,0xdb,0x00,0x43,0x00,0x08,0x06,0x06,0x07,0x06,0x05,0x08, + 0x07,0x07,0x07,0x09,0x09,0x08,0x0a,0x0c,0x14,0x0d,0x0c,0x0b,0x0b,0x0c,0x19,0x12, + 0x13,0x0f,0x14,0x1d,0x1a,0x1f,0x1e,0x1d,0x1a,0x1c,0x1c,0x20,0x24,0x2e,0x27,0x20, + 0x22,0x2c,0x23,0x1c,0x1c,0x28,0x37,0x29,0x2c,0x30,0x31,0x34,0x34,0x34,0x1f,0x27, + 0x39,0x3d,0x38,0x32,0x3c,0x2e,0x33,0x34,0x32,0xff,0xc0,0x00,0x0b,0x08,0x00,0x02, + 0x00,0x02,0x01,0x01,0x11,0x00,0xff,0xc4,0x00,0x1f,0x00,0x00,0x01,0x05,0x01,0x01, + 0x01,0x01,0x01,0x01,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x01,0x02,0x03,0x04, + 0x05,0x06,0x07,0x08,0x09,0x0a,0x0b,0xff,0xc4,0x00,0xb5,0x10,0x00,0x02,0x01,0x03, + 0x03,0x02,0x04,0x03,0x05,0x05,0x04,0x04,0x00,0x00,0x01,0x7d,0x01,0x02,0x03,0x00, + 0x04,0x11,0x05,0x12,0x21,0x31,0x41,0x06,0x13,0x51,0x61,0x07,0x22,0x71,0x14,0x32, + 0x81,0x91,0xa1,0x08,0x23,0x42,0xb1,0xc1,0x15,0x52,0xd1,0xf0,0x24,0x33,0x62,0x72, + 0x82,0x09,0x0a,0x16,0x17,0x18,0x19,0x1a,0x25,0x26,0x27,0x28,0x29,0x2a,0x34,0x35, + 0x36,0x37,0x38,0x39,0x3a,0x43,0x44,0x45,0x46,0x47,0x48,0x49,0x4a,0x53,0x54,0x55, + 0x56,0x57,0x58,0x59,0x5a,0x63,0x64,0x65,0x66,0x67,0x68,0x69,0x6a,0x73,0x74,0x75, + 0x76,0x77,0x78,0x79,0x7a,0x83,0x84,0x85,0x86,0x87,0x88,0x89,0x8a,0x92,0x93,0x94, + 0x95,0x96,0x97,0x98,0x99,0x9a,0xa2,0xa3,0xa4,0xa5,0xa6,0xa7,0xa8,0xa9,0xaa,0xb2, + 0xb3,0xb4,0xb5,0xb6,0xb7,0xb8,0xb9,0xba,0xc2,0xc3,0xc4,0xc5,0xc6,0xc7,0xc8,0xc9, + 0xca,0xd2,0xd3,0xd4,0xd5,0xd6,0xd7,0xd8,0xd9,0xda,0xe1,0xe2,0xe3,0xe4,0xe5,0xe6, + 0xe7,0xe8,0xe9,0xea,0xf1,0xf2,0xf3,0xf4,0xf5,0xf6,0xf7,0xf8,0xf9,0xfa,0xff,0xda, + 0x00,0x08,0x01,0x01,0x00,0x00,0x3f,0x00,0xea,0x2c,0x34,0x1d,0x1f,0xfb,0x3a,0xdb, + 0xfe,0x25,0x36,0x3f,0xea,0x93,0xfe,0x5d,0xd3,0xd0,0x7b,0x57,0xff,0xd9, +}; + /* 2x2 A8R8G8B8 pixel data */ static const uint8_t pixdata[] = { diff --git a/dlls/d3dx9_36/tests/surface.c b/dlls/d3dx9_36/tests/surface.c index c4cd89b2ccc..b57f5e32ad8 100644 --- a/dlls/d3dx9_36/tests/surface.c +++ b/dlls/d3dx9_36/tests/surface.c @@ -927,6 +927,25 @@ static void test_D3DXGetImageInfo(void) ok(hr == D3D_OK, "Got unexpected hr %#lx.\n", hr); ok(info.Format == D3DFMT_A8R8G8B8, "Got unexpected format %u.\n", info.Format); + /* Test JPG support. */ + hr = D3DXGetImageInfoFromFileInMemory(jpg_rgb_2_2, sizeof(jpg_rgb_2_2), &info); + ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr); + check_image_info(&info, 2, 2, 1, 1, D3DFMT_X8R8G8B8, D3DRTYPE_TEXTURE, D3DXIFF_JPG, TRUE); + + hr = D3DXGetImageInfoFromFileInMemory(jpg_grayscale_2_2, sizeof(jpg_grayscale_2_2), &info); + ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr); + check_image_info(&info, 2, 2, 1, 1, D3DFMT_L8, D3DRTYPE_TEXTURE, D3DXIFF_JPG, FALSE); + + /* Test PNG support. */ + hr = D3DXGetImageInfoFromFileInMemory(png_2_2_48bpp_rgb, sizeof(png_2_2_48bpp_rgb), &info); + todo_wine ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr); + if (SUCCEEDED(hr)) + check_image_info(&info, 2, 2, 1, 1, D3DFMT_A16B16G16R16, D3DRTYPE_TEXTURE, D3DXIFF_PNG, FALSE); + + hr = D3DXGetImageInfoFromFileInMemory(png_2_2_24bpp_bgr, sizeof(png_2_2_24bpp_bgr), &info); + ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr); + check_image_info(&info, 2, 2, 1, 1, D3DFMT_X8R8G8B8, D3DRTYPE_TEXTURE, D3DXIFF_PNG, TRUE); + /* Grayscale PNG */ hr = D3DXGetImageInfoFromFileInMemory(png_grayscale, sizeof(png_grayscale), &info); ok(hr == D3D_OK, "D3DXGetImageInfoFromFileInMemory returned %#lx, expected %#lx\n", hr, D3D_OK); @@ -1082,6 +1101,13 @@ static inline void _check_pixel_4bpp(unsigned int line, const D3DLOCKED_RECT *lo ok_(__FILE__, line)(color == expected_color, "Got color 0x%08lx, expected 0x%08lx\n", color, expected_color); } +#define check_pixel_8bpp(lockrect, x, y, color) _check_pixel_8bpp(__LINE__, lockrect, x, y, color) +static inline void _check_pixel_8bpp(unsigned int line, const D3DLOCKED_RECT *lockrect, int x, int y, uint64_t expected_color) +{ + uint64_t color = ((uint64_t *)lockrect->pBits)[x + y * (lockrect->Pitch / 8)]; + ok_(__FILE__, line)(color == expected_color, "Got color %#I64x, expected %#I64x.\n", color, expected_color); +} + #define check_pixel_float4(lockrect, x, y, fx, fy, fz, fw, ulps, todo) \ _check_pixel_float4(__LINE__, lockrect, x, y, fx, fy, fz, fw, ulps, todo) static inline void _check_pixel_float4(uint32_t line, const D3DLOCKED_RECT *lockrect, uint32_t x, uint32_t y, @@ -3372,6 +3398,27 @@ static void test_D3DXLoadSurface(IDirect3DDevice9 *device) check_release((IUnknown*)tex, 0); } + hr = IDirect3DDevice9_CreateOffscreenPlainSurface(device, 2, 2, D3DFMT_A16B16G16R16, D3DPOOL_SCRATCH, &surf, NULL); + ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr); + + hr = D3DXLoadSurfaceFromFileInMemory(surf, NULL, NULL, png_2_2_48bpp_rgb, sizeof(png_2_2_48bpp_rgb), NULL, + D3DX_FILTER_NONE, 0, NULL); + todo_wine ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr); + if (SUCCEEDED(hr)) + { + hr = IDirect3DSurface9_LockRect(surf, &lockrect, NULL, D3DLOCK_READONLY); + ok(hr == D3D_OK, "Failed to lock surface, hr %#lx.\n", hr); + + check_pixel_8bpp(&lockrect, 0, 0, 0xffff202010100000); + check_pixel_8bpp(&lockrect, 1, 0, 0xffff505040403030); + check_pixel_8bpp(&lockrect, 0, 1, 0xffff808070706060); + check_pixel_8bpp(&lockrect, 1, 1, 0xffffb0b0a0a09090); + + hr = IDirect3DSurface9_UnlockRect(surf); + ok(hr == D3D_OK, "Failed to unlock surface, hr %#lx.\n", hr); + } + check_release((IUnknown *)surf, 0); + test_format_conversion(device); test_dxt_premultiplied_alpha(device); test_load_surface_from_tga(device); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6704
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/d3dx9_36/surface.c | 50 ++++++++++++++++++++++++++++++----- dlls/d3dx9_36/tests/surface.c | 4 +-- 2 files changed, 46 insertions(+), 8 deletions(-) diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c index dcffab5e5ce..5010d5990d1 100644 --- a/dlls/d3dx9_36/surface.c +++ b/dlls/d3dx9_36/surface.c @@ -821,7 +821,11 @@ static const char *debug_d3dx_image_file_format(D3DXIMAGE_FILEFORMAT format) static HRESULT d3dx_image_wic_frame_decode(struct d3dx_image *image, IWICImagingFactory *wic_factory, IWICBitmapFrameDecode *bitmap_frame) { + const GUID *dst_pixel_format = d3dformat_to_wic_guid(image->format); + IWICFormatConverter *wic_converter = NULL; const struct pixel_format_desc *fmt_desc; + IWICBitmapSource *wic_bitmap_src = NULL; + WICPixelFormatGUID src_pixel_format; uint32_t row_pitch, slice_pitch; IWICPalette *wic_palette = NULL; PALETTEENTRY *palette = NULL; @@ -838,13 +842,40 @@ static HRESULT d3dx_image_wic_frame_decode(struct d3dx_image *image, if (!(buffer = malloc(slice_pitch))) return E_OUTOFMEMORY; - hr = IWICBitmapFrameDecode_CopyPixels(bitmap_frame, NULL, row_pitch, slice_pitch, buffer); + hr = IWICBitmapFrameDecode_GetPixelFormat(bitmap_frame, &src_pixel_format); if (FAILED(hr)) + goto exit; + + /* + * Needs conversion. Ignore D3DFMT_P8 because multiple different WIC + * formats map to D3DFMT_P8, and they don't require conversion. + */ + if (memcmp(&src_pixel_format, dst_pixel_format, sizeof(*dst_pixel_format)) && image->format != D3DFMT_P8) { - free(buffer); - return hr; + hr = IWICImagingFactory_CreateFormatConverter(wic_factory, &wic_converter); + if (FAILED(hr)) + goto exit; + + hr = IWICFormatConverter_Initialize(wic_converter, (IWICBitmapSource *)bitmap_frame, dst_pixel_format, + WICBitmapDitherTypeErrorDiffusion, 0, 0, WICBitmapPaletteTypeCustom); + if (FAILED(hr)) + goto exit; + + hr = IWICFormatConverter_QueryInterface(wic_converter, &IID_IWICBitmapSource, (void **)&wic_bitmap_src); + if (FAILED(hr)) + goto exit; + } + else + { + hr = IWICBitmapFrameDecode_QueryInterface(bitmap_frame, &IID_IWICBitmapSource, (void **)&wic_bitmap_src); + if (FAILED(hr)) + goto exit; } + hr = IWICBitmapSource_CopyPixels(wic_bitmap_src, NULL, row_pitch, slice_pitch, buffer); + if (FAILED(hr)) + goto exit; + if (is_index_format(fmt_desc)) { uint32_t nb_colors, i; @@ -853,7 +884,7 @@ static HRESULT d3dx_image_wic_frame_decode(struct d3dx_image *image, if (FAILED(hr)) goto exit; - hr = IWICBitmapFrameDecode_CopyPalette(bitmap_frame, wic_palette); + hr = IWICBitmapSource_CopyPalette(wic_bitmap_src, wic_palette); if (FAILED(hr)) goto exit; @@ -890,6 +921,10 @@ exit: free(colors); if (image->image_buf != buffer) free(buffer); + if (wic_converter) + IWICFormatConverter_Release(wic_converter); + if (wic_bitmap_src) + IWICBitmapSource_Release(wic_bitmap_src); if (image->image_palette != palette) free(palette); if (wic_palette) @@ -957,8 +992,8 @@ static HRESULT d3dx_initialize_image_from_wic(const void *src_data, uint32_t src goto exit; } - if (image_is_argb(bitmap_frame, image)) - image->format = D3DFMT_A8R8G8B8; + if (image->format == D3DFMT_R8G8B8 && (d3dx_file_format == D3DXIFF_JPG || d3dx_file_format == D3DXIFF_PNG)) + image->format = D3DFMT_X8R8G8B8; if (!(flags & D3DX_IMAGE_INFO_ONLY)) { @@ -967,6 +1002,9 @@ static HRESULT d3dx_initialize_image_from_wic(const void *src_data, uint32_t src goto exit; } + if (image_is_argb(bitmap_frame, image)) + image->format = D3DFMT_A8R8G8B8; + image->size.depth = 1; image->mip_levels = 1; image->layer_count = 1; diff --git a/dlls/d3dx9_36/tests/surface.c b/dlls/d3dx9_36/tests/surface.c index b57f5e32ad8..3411f03c8e0 100644 --- a/dlls/d3dx9_36/tests/surface.c +++ b/dlls/d3dx9_36/tests/surface.c @@ -930,7 +930,7 @@ static void test_D3DXGetImageInfo(void) /* Test JPG support. */ hr = D3DXGetImageInfoFromFileInMemory(jpg_rgb_2_2, sizeof(jpg_rgb_2_2), &info); ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr); - check_image_info(&info, 2, 2, 1, 1, D3DFMT_X8R8G8B8, D3DRTYPE_TEXTURE, D3DXIFF_JPG, TRUE); + check_image_info(&info, 2, 2, 1, 1, D3DFMT_X8R8G8B8, D3DRTYPE_TEXTURE, D3DXIFF_JPG, FALSE); hr = D3DXGetImageInfoFromFileInMemory(jpg_grayscale_2_2, sizeof(jpg_grayscale_2_2), &info); ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr); @@ -944,7 +944,7 @@ static void test_D3DXGetImageInfo(void) hr = D3DXGetImageInfoFromFileInMemory(png_2_2_24bpp_bgr, sizeof(png_2_2_24bpp_bgr), &info); ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr); - check_image_info(&info, 2, 2, 1, 1, D3DFMT_X8R8G8B8, D3DRTYPE_TEXTURE, D3DXIFF_PNG, TRUE); + check_image_info(&info, 2, 2, 1, 1, D3DFMT_X8R8G8B8, D3DRTYPE_TEXTURE, D3DXIFF_PNG, FALSE); /* Grayscale PNG */ hr = D3DXGetImageInfoFromFileInMemory(png_grayscale, sizeof(png_grayscale), &info); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6704
From: Connor McAdams <cmcadams(a)codeweavers.com> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=48796 Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/d3dx9_36/surface.c | 4 +++- dlls/d3dx9_36/tests/surface.c | 26 +++++++++++--------------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c index 5010d5990d1..844e6fc353f 100644 --- a/dlls/d3dx9_36/surface.c +++ b/dlls/d3dx9_36/surface.c @@ -45,7 +45,9 @@ static const struct { &GUID_WICPixelFormat16bppBGR565, D3DFMT_R5G6B5 }, { &GUID_WICPixelFormat24bppBGR, D3DFMT_R8G8B8 }, { &GUID_WICPixelFormat32bppBGR, D3DFMT_X8R8G8B8 }, - { &GUID_WICPixelFormat32bppBGRA, D3DFMT_A8R8G8B8 } + { &GUID_WICPixelFormat32bppBGRA, D3DFMT_A8R8G8B8 }, + { &GUID_WICPixelFormat64bppRGBA, D3DFMT_A16B16G16R16 }, + { &GUID_WICPixelFormat48bppRGB, D3DFMT_A16B16G16R16 }, }; static D3DFORMAT wic_guid_to_d3dformat(const GUID *guid) diff --git a/dlls/d3dx9_36/tests/surface.c b/dlls/d3dx9_36/tests/surface.c index 3411f03c8e0..a4023cda75c 100644 --- a/dlls/d3dx9_36/tests/surface.c +++ b/dlls/d3dx9_36/tests/surface.c @@ -938,9 +938,8 @@ static void test_D3DXGetImageInfo(void) /* Test PNG support. */ hr = D3DXGetImageInfoFromFileInMemory(png_2_2_48bpp_rgb, sizeof(png_2_2_48bpp_rgb), &info); - todo_wine ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr); - if (SUCCEEDED(hr)) - check_image_info(&info, 2, 2, 1, 1, D3DFMT_A16B16G16R16, D3DRTYPE_TEXTURE, D3DXIFF_PNG, FALSE); + ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr); + check_image_info(&info, 2, 2, 1, 1, D3DFMT_A16B16G16R16, D3DRTYPE_TEXTURE, D3DXIFF_PNG, FALSE); hr = D3DXGetImageInfoFromFileInMemory(png_2_2_24bpp_bgr, sizeof(png_2_2_24bpp_bgr), &info); ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr); @@ -3403,20 +3402,17 @@ static void test_D3DXLoadSurface(IDirect3DDevice9 *device) hr = D3DXLoadSurfaceFromFileInMemory(surf, NULL, NULL, png_2_2_48bpp_rgb, sizeof(png_2_2_48bpp_rgb), NULL, D3DX_FILTER_NONE, 0, NULL); - todo_wine ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr); - if (SUCCEEDED(hr)) - { - hr = IDirect3DSurface9_LockRect(surf, &lockrect, NULL, D3DLOCK_READONLY); - ok(hr == D3D_OK, "Failed to lock surface, hr %#lx.\n", hr); + ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr); - check_pixel_8bpp(&lockrect, 0, 0, 0xffff202010100000); - check_pixel_8bpp(&lockrect, 1, 0, 0xffff505040403030); - check_pixel_8bpp(&lockrect, 0, 1, 0xffff808070706060); - check_pixel_8bpp(&lockrect, 1, 1, 0xffffb0b0a0a09090); + hr = IDirect3DSurface9_LockRect(surf, &lockrect, NULL, D3DLOCK_READONLY); + ok(hr == D3D_OK, "Failed to lock surface, hr %#lx.\n", hr); + check_pixel_8bpp(&lockrect, 0, 0, 0xffff202010100000); + check_pixel_8bpp(&lockrect, 1, 0, 0xffff505040403030); + check_pixel_8bpp(&lockrect, 0, 1, 0xffff808070706060); + check_pixel_8bpp(&lockrect, 1, 1, 0xffffb0b0a0a09090); + hr = IDirect3DSurface9_UnlockRect(surf); + ok(hr == D3D_OK, "Failed to unlock surface, hr %#lx.\n", hr); - hr = IDirect3DSurface9_UnlockRect(surf); - ok(hr == D3D_OK, "Failed to unlock surface, hr %#lx.\n", hr); - } check_release((IUnknown *)surf, 0); test_format_conversion(device); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6704
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=149200 Your paranoid android. === debian11b (64 bit WoW report) === ntoskrnl.exe: ntoskrnl: Timeout
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/surface.c:
static HRESULT d3dx_image_wic_frame_decode(struct d3dx_image *image, IWICImagingFactory *wic_factory, IWICBitmapFrameDecode *bitmap_frame) { + const GUID *dst_pixel_format = d3dformat_to_wic_guid(image->format); + IWICFormatConverter *wic_converter = NULL; const struct pixel_format_desc *fmt_desc; + IWICBitmapSource *wic_bitmap_src = NULL; + WICPixelFormatGUID src_pixel_format; For the usual nitpick: I guess it would be slightly nicer to consistently use `GUID` (or `WICPixelFormatGUID`, which is a `GUID` `typedef` anyway). Mostly mentioning it because of the `memcmp` below.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6704#note_86020
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/surface.c:
static HRESULT d3dx_image_wic_frame_decode(struct d3dx_image *image, IWICImagingFactory *wic_factory, IWICBitmapFrameDecode *bitmap_frame) { + const GUID *dst_pixel_format = d3dformat_to_wic_guid(image->format); Not really relevant to this MR but as a general consideration: I think we usually name those "mapping" functions the other way around, like in this case `wic_guid_from_d3dformat()`. I didn't really notice this earlier but it slightly confused me here, not sure why.
Again, nothing to change here and now, but maybe something to keep in mind when you happen to touch any of those functions... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6704#note_86021
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/surface.c:
return E_OUTOFMEMORY;
- hr = IWICBitmapFrameDecode_CopyPixels(bitmap_frame, NULL, row_pitch, slice_pitch, buffer); + hr = IWICBitmapFrameDecode_GetPixelFormat(bitmap_frame, &src_pixel_format); if (FAILED(hr)) + goto exit; + + /* + * Needs conversion. Ignore D3DFMT_P8 because multiple different WIC + * formats map to D3DFMT_P8, and they don't require conversion. + */ + if (memcmp(&src_pixel_format, dst_pixel_format, sizeof(*dst_pixel_format)) && image->format != D3DFMT_P8) { - free(buffer); - return hr; + hr = IWICImagingFactory_CreateFormatConverter(wic_factory, &wic_converter);
Okay, now for a more architectural point... I'm not particularly keen about making use of WIC conversion stuff. Among other things, d3dx9 has flags to control filtering and such and those don't really map to WIC's. Currently we don't implement most of them, to be fair, but using WIC doesn't seem like a move in the right direction. Could we reasonably use the existing d3dx9 machinery (`d3dx_load_pixels_from_pixels()` and such) to convert the image to the desired format? Are we going to get problematic formats out of WIC, or is there some other issue that makes that complicated? For the two relevant formats in this MR, I expect the 24 -> 32 bpp conversion to already work. The 48 -> 64 bpp one is tricky because we'd need a new `formats[]` entry and there is no suitable `D3DFORMAT` for a 3 channel 16 bpc UNORM format. But that's basically a self-imposed limitation, nothing forces us to use `D3DFORMAT` to identify formats internally. Looking at your branches, I see that you end up with an `enum d3dx_pixel_format_id` to identify formats in d3dx, which should indirectly fix this. I'd suggest to simply defer converting from those "unrepresentable" formats (or maybe this whole conversion business entirely) to some time after `enum d3dx_pixel_format_id` is introduced, unless you really need it sooner. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6704#note_86022
On Thu Oct 24 23:36:21 2024 +0000, Matteo Bruni wrote:
Okay, now for a more architectural point... I'm not particularly keen about making use of WIC conversion stuff. Among other things, d3dx9 has flags to control filtering and such and those don't really map to WIC's. Currently we don't implement most of them, to be fair, but using WIC doesn't seem like a move in the right direction. Could we reasonably use the existing d3dx9 machinery (`d3dx_load_pixels_from_pixels()` and such) to convert the image to the desired format? Are we going to get problematic formats out of WIC, or is there some other issue that makes that complicated? For the two relevant formats in this MR, I expect the 24 -> 32 bpp conversion to already work. The 48 -> 64 bpp one is tricky because we'd need a new `formats[]` entry and there is no suitable `D3DFORMAT` for a 3 channel 16 bpc UNORM format. But that's basically a self-imposed limitation, nothing forces us to use `D3DFORMAT` to identify formats internally. Looking at your branches, I see that you end up with an `enum d3dx_pixel_format_id` to identify formats in d3dx, which should indirectly fix this. I'd suggest to simply defer converting from those "unrepresentable" formats (or maybe this whole conversion business entirely) to some time after `enum d3dx_pixel_format_id` is introduced, unless you really need it sooner. Yeah the 48->64bpp one is the trickier one, but ultimately solvable with `enum d3dx_pixel_format_id` patches. I can defer handling these until that stuff is upstream if that's preferable.
I kind of intended on this to be a stop gap to solve some of the test todos in my surface to file saving patches. Once those were upstream, we could replace this with d3dx conversions instead. Or, I guess I could just do the `enum d3dx_pixel_format_id` patches prior to introducing the new saving code. My main concern with that is introducing an internal format ID seemed like it should be included with code sharing with d3dx10, which is why I initially moved to it. But, I guess having an image with a format that isn't represented by the existing `D3DFORMAT` IDs is also a motivation to introduce `enum d3dx_pixel_format_id` early. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6704#note_86037
On Fri Oct 25 06:45:02 2024 +0000, Connor McAdams wrote:
Yeah the 48->64bpp one is the trickier one, but ultimately solvable with `enum d3dx_pixel_format_id` patches. I can defer handling these until that stuff is upstream if that's preferable. I kind of intended on this to be a stop gap to solve some of the test todos in my surface to file saving patches. Once those were upstream, we could replace this with d3dx conversions instead. Or, I guess I could just do the `enum d3dx_pixel_format_id` patches prior to introducing the new saving code. My main concern with that is introducing an internal format ID seemed like it should be included with code sharing with d3dx10, which is why I initially moved to it. But, I guess having an image with a format that isn't represented by the existing `D3DFORMAT` IDs is also a motivation to introduce `enum d3dx_pixel_format_id` early. I've uploaded a [branch](https://gitlab.winehq.org/cmcadams/wine/-/commits/WIP/d3dx9-new-fmt-id-v1) doing this without WIC conversion, let me know what you think :)
This avoids doing any conversion at all, it just reports a different format in the `D3DXIMAGE_INFO` structure than the format stored in the `d3dx_image` structure. If you think the approach in that branch is better, I can close this MR and open another one with the patches from that branch instead. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6704#note_86056
On Fri Oct 25 11:43:28 2024 +0000, Connor McAdams wrote:
I've uploaded a [branch](https://gitlab.winehq.org/cmcadams/wine/-/commits/WIP/d3dx9-new-fmt-id-v1) doing this without WIC conversion, let me know what you think :) This avoids doing any conversion at all, it just reports a different format in the `D3DXIMAGE_INFO` structure than the format stored in the `d3dx_image` structure. If you think the approach in that branch is better, I can close this MR and open another one with the patches from that branch instead. If I understand the patches in the branch correctly, you're basically deferring conversion to `D3DXLoadSurface*()` time, which all in all seems nicer to me.
I tried to look for anything breaking with the new approach (e.g. I was wondering about `D3DXCreateTextureFromFileInMemoryEx()` - it should be fine since it already takes the format from `d3dximage_info_from_d3dx_image()`) but I couldn't immediately find anything and, in general, tests should help with that concern. I have some specific questions on a couple details, but those are probably better reserved to the actual MR :smile: -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6704#note_86092
On Fri Oct 25 15:54:39 2024 +0000, Matteo Bruni wrote:
If I understand the patches in the branch correctly, you're basically deferring conversion to `D3DXLoadSurface*()` time, which all in all seems nicer to me. I tried to look for anything breaking with the new approach (e.g. I was wondering about `D3DXCreateTextureFromFileInMemoryEx()` - it should be fine since it already takes the format from `d3dximage_info_from_d3dx_image()`) but I couldn't immediately find anything and, in general, tests should help with that concern. I have some specific questions on a couple details, but those are probably better reserved to the actual MR :smile: Yes, the general idea is to do the conversion at the time of loading the volume/texture/surface instead of doing an extra conversion. It's possible that that could cause some confusion due to the `D3DXIMAGE_INFO` structure containing a different format than the `struct d3dx_image`, but I don't think that should be a huge issue.
I've opened a new [MR](https://gitlab.winehq.org/wine/wine/-/merge_requests/6746) with the first part of these patches, going to go ahead and close this one. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6704#note_86319
This merge request was closed by Connor McAdams. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6704
participants (4)
-
Connor McAdams -
Connor McAdams (＠cmcadams)
-
Marvin -
Matteo Bruni (＠Mystral)