[PATCH v2 0/3] MR6673: d3dx9: Move targa decoding into d3dx9, part 1.
This is the first set of patches to move targa decoding out of WIC and into d3dx9. The eventual goal is to add support for saving targa files. I've pushed a branch with the rest of the patches for decoding the image [here](https://gitlab.winehq.org/cmcadams/wine/-/tree/WIP/d3dx9-targa-decode-v2) if extra context would help. I split this into two parts to make review a little less overwhelming. :) -- v2: d3dx9: Use d3dx9 to get image information for targa files. d3dx9/tests: Add TGA header image info tests. d3dx9: Do not use WIC to detect image file format. https://gitlab.winehq.org/wine/wine/-/merge_requests/6673
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/d3dx9_36/surface.c | 182 +++++++++++++++++++++++++++------------- 1 file changed, 122 insertions(+), 60 deletions(-) diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c index 21ff8732550..290f148c4b7 100644 --- a/dlls/d3dx9_36/surface.c +++ b/dlls/d3dx9_36/surface.c @@ -544,6 +544,60 @@ static HRESULT save_dds_surface_to_memory(ID3DXBuffer **dst_buffer, IDirect3DSur return D3D_OK; } +static const uint8_t bmp_file_signature[] = { 'B', 'M' }; +static const uint8_t jpg_file_signature[] = { 0xff, 0xd8 }; +static const uint8_t png_file_signature[] = { 0x89, 'P', 'N', 'G', 0x0d, 0x0a, 0x1a, 0x0a }; +static const uint8_t dds_file_signature[] = { 'D', 'D', 'S', ' ' }; +static const uint8_t ppm_plain_file_signature[] = { 'P', '3' }; +static const uint8_t ppm_raw_file_signature[] = { 'P', '6' }; +static const uint8_t hdr_file_signature[] = { '#', '?', 'R', 'A', 'D', 'I', 'A', 'N', 'C', 'E', '\n' }; +static const uint8_t pfm_color_file_signature[] = { 'P', 'F' }; +static const uint8_t pfm_gray_file_signature[] = { 'P', 'f' }; + +/* + * If none of these match, the file is either DIB, TGA, or something we don't + * support. + */ +struct d3dx_file_format_signature +{ + const uint8_t *file_signature; + uint32_t file_signature_len; + D3DXIMAGE_FILEFORMAT image_file_format; +}; + +static const struct d3dx_file_format_signature file_format_signatures[] = +{ + { bmp_file_signature, sizeof(bmp_file_signature), D3DXIFF_BMP }, + { jpg_file_signature, sizeof(jpg_file_signature), D3DXIFF_JPG }, + { png_file_signature, sizeof(png_file_signature), D3DXIFF_PNG }, + { dds_file_signature, sizeof(dds_file_signature), D3DXIFF_DDS }, + { ppm_plain_file_signature, sizeof(ppm_plain_file_signature), D3DXIFF_PPM }, + { ppm_raw_file_signature, sizeof(ppm_raw_file_signature), D3DXIFF_PPM }, + { hdr_file_signature, sizeof(hdr_file_signature), D3DXIFF_HDR }, + { pfm_color_file_signature, sizeof(pfm_color_file_signature), D3DXIFF_PFM }, + { pfm_gray_file_signature, sizeof(pfm_gray_file_signature), D3DXIFF_PFM }, +}; + +static BOOL d3dx_get_image_file_format_from_file_signature(const void *src_data, uint32_t src_data_size, + D3DXIMAGE_FILEFORMAT *out_iff) +{ + unsigned int i; + + for (i = 0; i < ARRAY_SIZE(file_format_signatures); ++i) + { + const struct d3dx_file_format_signature *signature = &file_format_signatures[i]; + + if ((src_data_size >= signature->file_signature_len) + && !memcmp(src_data, signature->file_signature, signature->file_signature_len)) + { + *out_iff = signature->image_file_format; + return TRUE; + } + } + + return FALSE; +} + #define DDS_PALETTE_SIZE (sizeof(PALETTEENTRY) * 256) static HRESULT d3dx_initialize_image_from_dds(const void *src_data, uint32_t src_data_size, struct d3dx_image *image, uint32_t starting_mip_level) @@ -735,31 +789,18 @@ struct d3dx_wic_file_format D3DXIMAGE_FILEFORMAT d3dx_file_format; }; -/* Sorted by GUID. */ -static const struct d3dx_wic_file_format file_formats[] = -{ - { &GUID_ContainerFormatBmp, D3DXIFF_BMP }, - { &GUID_WineContainerFormatTga, D3DXIFF_TGA }, - { &GUID_ContainerFormatJpeg, D3DXIFF_JPG }, - { &GUID_ContainerFormatPng, D3DXIFF_PNG }, -}; - -static int __cdecl d3dx_wic_file_format_guid_compare(const void *a, const void *b) -{ - const struct d3dx_wic_file_format *format = b; - const GUID *guid = a; - - return memcmp(guid, format->wic_container_guid, sizeof(*guid)); -} - -static D3DXIMAGE_FILEFORMAT wic_container_guid_to_d3dx_file_format(GUID *container_format) +static const GUID *d3dx_file_format_to_wic_container_guid(D3DXIMAGE_FILEFORMAT iff) { - struct d3dx_wic_file_format *format; - - if ((format = bsearch(container_format, file_formats, ARRAY_SIZE(file_formats), sizeof(*format), - d3dx_wic_file_format_guid_compare))) - return format->d3dx_file_format; - return D3DXIFF_FORCE_DWORD; + switch (iff) + { + case D3DXIFF_BMP: return &GUID_ContainerFormatBmp; + case D3DXIFF_TGA: return &GUID_WineContainerFormatTga; + case D3DXIFF_JPG: return &GUID_ContainerFormatJpeg; + case D3DXIFF_PNG: return &GUID_ContainerFormatPng; + default: + assert(0); /* Shouldn't happen. */ + return NULL; + } } static const char *debug_d3dx_image_file_format(D3DXIMAGE_FILEFORMAT format) @@ -863,62 +904,37 @@ exit: } static HRESULT d3dx_initialize_image_from_wic(const void *src_data, uint32_t src_data_size, - struct d3dx_image *image, uint32_t flags) + struct d3dx_image *image, D3DXIMAGE_FILEFORMAT d3dx_file_format, uint32_t flags) { + const GUID *container_format_guid = d3dx_file_format_to_wic_container_guid(d3dx_file_format); IWICBitmapFrameDecode *bitmap_frame = NULL; IWICBitmapDecoder *bitmap_decoder = NULL; - uint32_t src_image_size = src_data_size; IWICImagingFactory *wic_factory; - const void *src_image = src_data; WICPixelFormatGUID pixel_format; IWICStream *wic_stream = NULL; uint32_t frame_count = 0; - GUID container_format; - BOOL is_dib = FALSE; HRESULT hr; hr = WICCreateImagingFactory_Proxy(WINCODEC_SDK_VERSION, &wic_factory); if (FAILED(hr)) return hr; - is_dib = convert_dib_to_bmp(&src_image, &src_image_size); - hr = IWICImagingFactory_CreateStream(wic_factory, &wic_stream); + hr = IWICImagingFactory_CreateDecoder(wic_factory, container_format_guid, NULL, &bitmap_decoder); if (FAILED(hr)) goto exit; - hr = IWICStream_InitializeFromMemory(wic_stream, (BYTE *)src_image, src_image_size); + hr = IWICImagingFactory_CreateStream(wic_factory, &wic_stream); if (FAILED(hr)) goto exit; - hr = IWICImagingFactory_CreateDecoderFromStream(wic_factory, (IStream *)wic_stream, NULL, 0, &bitmap_decoder); + hr = IWICStream_InitializeFromMemory(wic_stream, (BYTE *)src_data, src_data_size); if (FAILED(hr)) - { - if ((src_image_size >= 2) && (!memcmp(src_image, "P3", 2) || !memcmp(src_image, "P6", 2))) - FIXME("File type PPM is not supported yet.\n"); - else if ((src_image_size >= 10) && !memcmp(src_image, "#?RADIANCE", 10)) - FIXME("File type HDR is not supported yet.\n"); - else if ((src_image_size >= 2) && (!memcmp(src_image, "PF", 2) || !memcmp(src_image, "Pf", 2))) - FIXME("File type PFM is not supported yet.\n"); goto exit; - } - hr = IWICBitmapDecoder_GetContainerFormat(bitmap_decoder, &container_format); + hr = IWICBitmapDecoder_Initialize(bitmap_decoder, (IStream *)wic_stream, 0); if (FAILED(hr)) goto exit; - image->image_file_format = wic_container_guid_to_d3dx_file_format(&container_format); - if (is_dib && image->image_file_format == D3DXIFF_BMP) - { - image->image_file_format = D3DXIFF_DIB; - } - else if (image->image_file_format == D3DXIFF_FORCE_DWORD) - { - WARN("Unsupported image file format %s.\n", debugstr_guid(&container_format)); - hr = D3DXERR_INVALIDDATA; - goto exit; - } - - TRACE("File type is %s.\n", debug_d3dx_image_file_format(image->image_file_format)); hr = IWICBitmapDecoder_GetFrameCount(bitmap_decoder, &frame_count); if (FAILED(hr) || (SUCCEEDED(hr) && !frame_count)) { @@ -926,6 +942,7 @@ static HRESULT d3dx_initialize_image_from_wic(const void *src_data, uint32_t src goto exit; } + image->image_file_format = d3dx_file_format; hr = IWICBitmapDecoder_GetFrame(bitmap_decoder, 0, &bitmap_frame); if (FAILED(hr)) goto exit; @@ -961,8 +978,6 @@ static HRESULT d3dx_initialize_image_from_wic(const void *src_data, uint32_t src image->resource_type = D3DRTYPE_TEXTURE; exit: - if (is_dib) - free((void *)src_image); if (bitmap_frame) IWICBitmapFrameDecode_Release(bitmap_frame); if (bitmap_decoder) @@ -977,14 +992,61 @@ exit: HRESULT d3dx_image_init(const void *src_data, uint32_t src_data_size, struct d3dx_image *image, uint32_t starting_mip_level, uint32_t flags) { + D3DXIMAGE_FILEFORMAT iff = D3DXIFF_FORCE_DWORD; + HRESULT hr; + if (!src_data || !src_data_size || !image) return D3DERR_INVALIDCALL; memset(image, 0, sizeof(*image)); - if ((src_data_size >= 4) && !memcmp(src_data, "DDS ", 4)) - return d3dx_initialize_image_from_dds(src_data, src_data_size, image, starting_mip_level); + if (!d3dx_get_image_file_format_from_file_signature(src_data, src_data_size, &iff)) + { + uint32_t src_image_size = src_data_size; + const void *src_image = src_data; - return d3dx_initialize_image_from_wic(src_data, src_data_size, image, flags); + if (convert_dib_to_bmp(&src_image, &src_image_size)) + { + hr = d3dx_image_init(src_image, src_image_size, image, starting_mip_level, flags); + free((void *)src_image); + if (SUCCEEDED(hr)) + image->image_file_format = D3DXIFF_DIB; + return hr; + } + + /* Last resort, try TGA. */ + return d3dx_initialize_image_from_wic(src_data, src_data_size, image, D3DXIFF_TGA, flags); + } + + switch (iff) + { + case D3DXIFF_BMP: + case D3DXIFF_JPG: + case D3DXIFF_PNG: + hr = d3dx_initialize_image_from_wic(src_data, src_data_size, image, iff, flags); + break; + + case D3DXIFF_DDS: + hr = d3dx_initialize_image_from_dds(src_data, src_data_size, image, starting_mip_level); + break; + + case D3DXIFF_PPM: + case D3DXIFF_HDR: + case D3DXIFF_PFM: + WARN("Unsupported file format %s.\n", debug_d3dx_image_file_format(iff)); + hr = E_NOTIMPL; + break; + + case D3DXIFF_FORCE_DWORD: + ERR("Unrecognized file format.\n"); + hr = D3DXERR_INVALIDDATA; + break; + + default: + assert(0); + return E_FAIL; + } + + return hr; } void d3dx_image_cleanup(struct d3dx_image *image) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6673
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/d3dx9_36/tests/d3dx9_test_images.h | 80 +++++++- dlls/d3dx9_36/tests/surface.c | 233 ++++++++++++++++++++++++ 2 files changed, 312 insertions(+), 1 deletion(-) diff --git a/dlls/d3dx9_36/tests/d3dx9_test_images.h b/dlls/d3dx9_36/tests/d3dx9_test_images.h index 266c0c201cf..0c4a72da57d 100644 --- a/dlls/d3dx9_36/tests/d3dx9_test_images.h +++ b/dlls/d3dx9_36/tests/d3dx9_test_images.h @@ -596,6 +596,82 @@ static const uint8_t dds_24bit_8_8[] = 0xff,0x00,0x00,0xff,0x00,0x00,0xff,0x00,0x00,0xff,0x00,0x00,0x00,0x00,0x00 }; +static inline const char *debug_d3dformat(D3DFORMAT format_id) +{ + switch (format_id) + { +#define FMT_TO_STR(format_id) case format_id: return #format_id + FMT_TO_STR(D3DFMT_UNKNOWN); + FMT_TO_STR(D3DFMT_R8G8B8); + FMT_TO_STR(D3DFMT_A8R8G8B8); + FMT_TO_STR(D3DFMT_X8R8G8B8); + FMT_TO_STR(D3DFMT_R5G6B5); + FMT_TO_STR(D3DFMT_X1R5G5B5); + FMT_TO_STR(D3DFMT_A1R5G5B5); + FMT_TO_STR(D3DFMT_A4R4G4B4); + FMT_TO_STR(D3DFMT_R3G3B2); + FMT_TO_STR(D3DFMT_A8); + FMT_TO_STR(D3DFMT_A8R3G3B2); + FMT_TO_STR(D3DFMT_X4R4G4B4); + FMT_TO_STR(D3DFMT_A2B10G10R10); + FMT_TO_STR(D3DFMT_A8B8G8R8); + FMT_TO_STR(D3DFMT_X8B8G8R8); + FMT_TO_STR(D3DFMT_G16R16); + FMT_TO_STR(D3DFMT_A2R10G10B10); + FMT_TO_STR(D3DFMT_A16B16G16R16); + FMT_TO_STR(D3DFMT_A8P8); + FMT_TO_STR(D3DFMT_P8); + FMT_TO_STR(D3DFMT_L8); + FMT_TO_STR(D3DFMT_A8L8); + FMT_TO_STR(D3DFMT_A4L4); + FMT_TO_STR(D3DFMT_V8U8); + FMT_TO_STR(D3DFMT_L6V5U5); + FMT_TO_STR(D3DFMT_X8L8V8U8); + FMT_TO_STR(D3DFMT_Q8W8V8U8); + FMT_TO_STR(D3DFMT_V16U16); + FMT_TO_STR(D3DFMT_A2W10V10U10); + FMT_TO_STR(D3DFMT_UYVY); + FMT_TO_STR(D3DFMT_YUY2); + FMT_TO_STR(D3DFMT_DXT1); + FMT_TO_STR(D3DFMT_DXT2); + FMT_TO_STR(D3DFMT_DXT3); + FMT_TO_STR(D3DFMT_DXT4); + FMT_TO_STR(D3DFMT_DXT5); + FMT_TO_STR(D3DFMT_MULTI2_ARGB8); + FMT_TO_STR(D3DFMT_G8R8_G8B8); + FMT_TO_STR(D3DFMT_R8G8_B8G8); + FMT_TO_STR(D3DFMT_D16_LOCKABLE); + FMT_TO_STR(D3DFMT_D32); + FMT_TO_STR(D3DFMT_D15S1); + FMT_TO_STR(D3DFMT_D24S8); + FMT_TO_STR(D3DFMT_D24X8); + FMT_TO_STR(D3DFMT_D24X4S4); + FMT_TO_STR(D3DFMT_D16); + FMT_TO_STR(D3DFMT_L16); + FMT_TO_STR(D3DFMT_D32F_LOCKABLE); + FMT_TO_STR(D3DFMT_D24FS8); + FMT_TO_STR(D3DFMT_D32_LOCKABLE); + FMT_TO_STR(D3DFMT_S8_LOCKABLE); + FMT_TO_STR(D3DFMT_VERTEXDATA); + FMT_TO_STR(D3DFMT_INDEX16); + FMT_TO_STR(D3DFMT_INDEX32); + FMT_TO_STR(D3DFMT_Q16W16V16U16); + FMT_TO_STR(D3DFMT_R16F); + FMT_TO_STR(D3DFMT_G16R16F); + FMT_TO_STR(D3DFMT_A16B16G16R16F); + FMT_TO_STR(D3DFMT_R32F); + FMT_TO_STR(D3DFMT_G32R32F); + FMT_TO_STR(D3DFMT_A32B32G32R32F); + FMT_TO_STR(D3DFMT_CxV8U8); + FMT_TO_STR(D3DFMT_A1); + FMT_TO_STR(D3DFMT_A2B10G10R10_XR_BIAS); + FMT_TO_STR(D3DFMT_BINARYBUFFER); +#undef FMT_TO_STR + default: + return "unknown"; + } +} + #define check_image_info(info, width, height, depth, mip_levels, format, resource_type, image_file_format, wine_todo) \ check_image_info_(__FILE__, __LINE__, info, width, height, depth, mip_levels, format, resource_type, \ image_file_format, wine_todo) @@ -620,7 +696,9 @@ static inline void check_image_info_(const char *file, uint32_t line, const D3DX todo_wine_if(wine_todo && info->MipLevels != mip_levels) ok_(file, line)(info->MipLevels == mip_levels, "Expected mip_levels %u, got %u.\n", mip_levels, info->MipLevels); - ok_(file, line)(info->Format == format, "Expected texture format %d, got %d.\n", format, info->Format); + todo_wine_if(wine_todo && info->Format != format) + ok_(file, line)(info->Format == format, "Expected texture format %s (%u), got %s (%u).\n", + debug_d3dformat(format), format, debug_d3dformat(info->Format), info->Format); todo_wine_if(wine_todo && info->ResourceType != resource_type) ok_(file, line)(info->ResourceType == resource_type, "Expected resource_type %d, got %d.\n", resource_type, info->ResourceType); diff --git a/dlls/d3dx9_36/tests/surface.c b/dlls/d3dx9_36/tests/surface.c index 747f373ab90..fb2abb27fa2 100644 --- a/dlls/d3dx9_36/tests/surface.c +++ b/dlls/d3dx9_36/tests/surface.c @@ -558,6 +558,238 @@ static void test_dds_header_handling(void) free(dds); } +#define COLORMAP_TYPE_NONE 0 +#define COLORMAP_TYPE_ONE 1 + +#define IMAGETYPE_COLORMAPPED 1 +#define IMAGETYPE_TRUECOLOR 2 +#define IMAGETYPE_GRAYSCALE 3 +#define IMAGETYPE_RLE 8 + +#include "pshpack1.h" +struct tga_header +{ + uint8_t id_length; + uint8_t color_map_type; + uint8_t image_type; + uint16_t color_map_firstentry; + uint16_t color_map_length; + uint8_t color_map_entrysize; + uint16_t xorigin; + uint16_t yorigin; + uint16_t width; + uint16_t height; + uint8_t depth; + uint8_t image_descriptor; +}; + +struct tga_footer +{ + uint32_t extension_area_offset; + uint32_t developer_directory_offset; + uint8_t magic[18]; +}; +#include "poppack.h" + +static const struct tga_footer default_tga_footer = { + 0, 0, + { 'T', 'R', 'U', 'E', 'V', 'I', 'S', 'I', 'O', 'N', '-', 'X', 'F', 'I', 'L', 'E', '.', 0 } +}; + +#define check_tga_image_info(tga, tga_size, expected_width, expected_height, expected_format, expected_hr, todo_hr, todo_info) \ + check_tga_image_info_(__LINE__, tga, tga_size, expected_width, expected_height, expected_format, expected_hr, todo_hr, todo_info) +static void check_tga_image_info_(uint32_t line, const void *tga, uint32_t tga_size, uint32_t expected_width, + uint32_t expected_height, D3DFORMAT expected_format, HRESULT expected_hr, BOOL todo_hr, BOOL todo_info) +{ + D3DXIMAGE_INFO info = { 0 }; + HRESULT hr; + + hr = D3DXGetImageInfoFromFileInMemory(tga, tga_size, &info); + todo_wine_if(todo_hr) ok_(__FILE__, line)(hr == expected_hr, "Unexpected hr %#lx.\n", hr); + if (SUCCEEDED(expected_hr) && SUCCEEDED(hr)) + { + check_image_info_(__FILE__, line, &info, expected_width, expected_height, 1, 1, expected_format, + D3DRTYPE_TEXTURE, D3DXIFF_TGA, todo_info); + } +} + +static void test_tga_header_handling(void) +{ + static const struct + { + struct tga_header header; + struct + { + HRESULT hr; + uint32_t width; + uint32_t height; + D3DFORMAT format; + } expected; + uint32_t extra_header_size; + BOOL todo_hr; + BOOL todo_info; + } info_tests[] = + { + /* 15 bpp true color. */ + { { 0, COLORMAP_TYPE_NONE, IMAGETYPE_TRUECOLOR, 0, 0, 0, 0, 0, 4, 4, 15, 0 }, + { D3D_OK, 4, 4, D3DFMT_X1R5G5B5 }, .todo_hr = TRUE + }, + /* 16 bpp true color. */ + { { 0, COLORMAP_TYPE_NONE, IMAGETYPE_TRUECOLOR, 0, 0, 0, 0, 0, 4, 4, 16, 0 }, + { D3D_OK, 4, 4, D3DFMT_A1R5G5B5 }, .todo_info = TRUE + }, + /* 24 bpp true color. */ + { { 0, COLORMAP_TYPE_NONE, IMAGETYPE_TRUECOLOR, 0, 0, 0, 0, 0, 4, 4, 24, 0 }, + { D3D_OK, 4, 4, D3DFMT_R8G8B8 } + }, + /* 32 bpp true color. */ + { { 0, COLORMAP_TYPE_NONE, IMAGETYPE_TRUECOLOR, 0, 0, 0, 0, 0, 4, 4, 32, 0 }, + { D3D_OK, 4, 4, D3DFMT_A8R8G8B8 }, .todo_info = TRUE + }, + /* 8 bit paletted, 15 bpp palette. */ + { { 0, COLORMAP_TYPE_ONE, IMAGETYPE_COLORMAPPED, 0, 256, 15, 0, 0, 4, 4, 8, 0 }, + { D3D_OK, 4, 4, D3DFMT_P8 }, (256 * 2) + }, + /* 8 bit paletted, 16 bpp palette. */ + { { 0, COLORMAP_TYPE_ONE, IMAGETYPE_COLORMAPPED, 0, 256, 16, 0, 0, 4, 4, 8, 0 }, + { D3D_OK, 4, 4, D3DFMT_P8 }, (256 * 2) + }, + /* 8 bit paletted, 24 bpp palette. */ + { { 0, COLORMAP_TYPE_ONE, IMAGETYPE_COLORMAPPED, 0, 256, 24, 0, 0, 4, 4, 8, 0 }, + { D3D_OK, 4, 4, D3DFMT_P8 }, (256 * 3) + }, + /* 8 bit paletted, 32 bpp palette. */ + { { 0, COLORMAP_TYPE_ONE, IMAGETYPE_COLORMAPPED, 0, 256, 32, 0, 0, 4, 4, 8, 0 }, + { D3D_OK, 4, 4, D3DFMT_P8 }, (256 * 4) + }, + /* Grayscale, 8bpp. */ + { { 0, COLORMAP_TYPE_NONE, IMAGETYPE_GRAYSCALE, 0, 0, 0, 0, 0, 4, 4, 8, 0 }, + { D3D_OK, 4, 4, D3DFMT_L8 } + }, + /* No 16-bit grayscale. */ + { { 0, COLORMAP_TYPE_NONE, IMAGETYPE_GRAYSCALE, 0, 0, 0, 0, 0, 4, 4, 16, 0 }, + { D3DXERR_INVALIDDATA } + }, + }; + struct + { + struct tga_header header; + uint8_t data[4096 * 1024]; + } *tga; + struct tga_footer tmp_footer; + uint32_t i; + + tga = calloc(1, sizeof(*tga)); + if (!tga) + { + skip("Failed to allocate memory.\n"); + return; + } + + for (i = 0; i < ARRAY_SIZE(info_tests); i++) + { + uint32_t file_size = sizeof(tga->header) + info_tests[i].extra_header_size; + assert(file_size <= sizeof(*tga)); + + winetest_push_context("Test %u", i); + + tga->header = info_tests[i].header; + check_tga_image_info(tga, file_size, info_tests[i].expected.width, info_tests[i].expected.height, + info_tests[i].expected.format, info_tests[i].expected.hr, info_tests[i].todo_hr, info_tests[i].todo_info); + + /* X/Y origin fields are ignored. */ + tga->header.xorigin = tga->header.width + 1; + tga->header.yorigin = tga->header.height + 1; + check_tga_image_info(tga, file_size, info_tests[i].expected.width, info_tests[i].expected.height, + info_tests[i].expected.format, info_tests[i].expected.hr, info_tests[i].todo_hr, info_tests[i].todo_info); + + /* Image descriptor field is ignored. */ + tga->header.image_descriptor = 0xcf; + check_tga_image_info(tga, file_size, info_tests[i].expected.width, info_tests[i].expected.height, + info_tests[i].expected.format, info_tests[i].expected.hr, SUCCEEDED(info_tests[i].expected.hr), + info_tests[i].todo_info); + + if (FAILED(info_tests[i].expected.hr)) + goto next; + + /* + * Footer offsets do not seem to be validated. Possible that footer + * isn't even checked for. + */ + tmp_footer = default_tga_footer; + tmp_footer.extension_area_offset = 65536; + memcpy(&tga->data[info_tests[i].extra_header_size], &tmp_footer, sizeof(tmp_footer)); + check_tga_image_info(tga, file_size + sizeof(tmp_footer), info_tests[i].expected.width, info_tests[i].expected.height, + info_tests[i].expected.format, info_tests[i].expected.hr, TRUE, info_tests[i].todo_info); + + /* Check RLE type. */ + tga->header.image_type |= IMAGETYPE_RLE; + check_tga_image_info(tga, file_size, info_tests[i].expected.width, info_tests[i].expected.height, + info_tests[i].expected.format, info_tests[i].expected.hr, TRUE, info_tests[i].todo_info); + tga->header.image_type &= ~IMAGETYPE_RLE; + + if (tga->header.image_type == IMAGETYPE_COLORMAPPED) + goto next; + + /* + * Even if the image isn't color mapped, the color map fields are used + * to validate header size. + */ + tga->header.color_map_length = 1; + check_tga_image_info(tga, file_size, info_tests[i].expected.width, info_tests[i].expected.height, + info_tests[i].expected.format, info_tests[i].expected.hr, SUCCEEDED(info_tests[i].expected.hr), + info_tests[i].todo_info); + + tga->header.color_map_entrysize = 8; + check_tga_image_info(tga, file_size, 0, 0, D3DFMT_UNKNOWN, D3DXERR_INVALIDDATA, FALSE, FALSE); + + /* Add a byte to file size to account for color map. */ + check_tga_image_info(tga, file_size + 1, info_tests[i].expected.width, info_tests[i].expected.height, + info_tests[i].expected.format, info_tests[i].expected.hr, SUCCEEDED(info_tests[i].expected.hr), + info_tests[i].todo_info); + + /* ID length field is also considered. */ + tga->header.id_length = 1; + check_tga_image_info(tga, file_size + 1, 0, 0, D3DFMT_UNKNOWN, D3DXERR_INVALIDDATA, FALSE, FALSE); + + /* Add another byte to file size to account for id length. */ + check_tga_image_info(tga, file_size + 2, info_tests[i].expected.width, info_tests[i].expected.height, + info_tests[i].expected.format, info_tests[i].expected.hr, SUCCEEDED(info_tests[i].expected.hr), + info_tests[i].todo_info); + + /* + * If the color map type field is set but the color map fields + * result in a color map size of 0, header is considered invalid. + * Also, the entrysize value is now validated, e.g it must be 15, + * 16, 24, or 32. + */ + tga->header.id_length = tga->header.color_map_entrysize = tga->header.color_map_length = 0; + tga->header.color_map_type = COLORMAP_TYPE_ONE; + check_tga_image_info(tga, file_size + 2, 0, 0, D3DFMT_UNKNOWN, D3DXERR_INVALIDDATA, FALSE, FALSE); + + /* 8 isn't a valid entry size. */ + tga->header.color_map_entrysize = 8; + tga->header.color_map_length = 1; + check_tga_image_info(tga, file_size + 1, 0, 0, D3DFMT_UNKNOWN, D3DXERR_INVALIDDATA, FALSE, FALSE); + + /* 16 is a valid entry size. */ + tga->header.color_map_entrysize = 16; + check_tga_image_info(tga, file_size + 2, info_tests[i].expected.width, info_tests[i].expected.height, + info_tests[i].expected.format, info_tests[i].expected.hr, SUCCEEDED(info_tests[i].expected.hr), + info_tests[i].todo_info); + + /* First entry doesn't factor into validation. */ + tga->header.color_map_firstentry = 512; + check_tga_image_info(tga, file_size + 2, info_tests[i].expected.width, info_tests[i].expected.height, + info_tests[i].expected.format, info_tests[i].expected.hr, SUCCEEDED(info_tests[i].expected.hr), + info_tests[i].todo_info); +next: + winetest_pop_context(); + } + + free(tga); +} + static void test_D3DXGetImageInfo(void) { HRESULT hr; @@ -818,6 +1050,7 @@ static void test_D3DXGetImageInfo(void) check_dds_pixel_format(DDS_PF_INDEXED | DDS_PF_ALPHA, 0, 16, 0, 0, 0, 0xff00, D3DFMT_A8P8); test_dds_header_handling(); + test_tga_header_handling(); hr = D3DXGetImageInfoFromFileInMemory(dds_16bit, sizeof(dds_16bit) - 1, &info); ok(hr == D3DXERR_INVALIDDATA, "D3DXGetImageInfoFromFileInMemory returned %#lx, expected %#x\n", hr, D3DXERR_INVALIDDATA); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6673
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/d3dx9_36/surface.c | 90 +++++++++++++++++++++++++++++++++++ dlls/d3dx9_36/tests/surface.c | 34 +++++-------- 2 files changed, 103 insertions(+), 21 deletions(-) diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c index 290f148c4b7..c56310982dc 100644 --- a/dlls/d3dx9_36/surface.c +++ b/dlls/d3dx9_36/surface.c @@ -989,6 +989,93 @@ exit: return hr; } +static D3DFORMAT d3dx_get_tga_format_for_bpp(uint8_t bpp) +{ + switch (bpp) + { + case 15: return D3DFMT_X1R5G5B5; + case 16: return D3DFMT_A1R5G5B5; + case 24: return D3DFMT_R8G8B8; + case 32: return D3DFMT_A8R8G8B8; + default: + WARN("Unhandled bpp %u for targa.\n", bpp); + return D3DFMT_UNKNOWN; + } +} + +#define IMAGETYPE_COLORMAPPED 1 +#define IMAGETYPE_TRUECOLOR 2 +#define IMAGETYPE_GRAYSCALE 3 +#define IMAGETYPE_MASK 0x07 +#define IMAGETYPE_RLE 8 + +#include "pshpack1.h" +struct tga_header +{ + uint8_t id_length; + uint8_t color_map_type; + uint8_t image_type; + uint16_t color_map_firstentry; + uint16_t color_map_length; + uint8_t color_map_entrysize; + uint16_t xorigin; + uint16_t yorigin; + uint16_t width; + uint16_t height; + uint8_t depth; + uint8_t image_descriptor; +}; +#include "poppack.h" + +static HRESULT d3dx_initialize_image_from_tga(const void *src_data, uint32_t src_data_size, struct d3dx_image *image) +{ + const struct tga_header *header = (const struct tga_header *)src_data; + uint32_t expected_header_size = sizeof(*header); + + if (src_data_size < sizeof(*header)) + return D3DXERR_INVALIDDATA; + + expected_header_size += header->id_length; + expected_header_size += header->color_map_length * ((header->color_map_entrysize + 7) / CHAR_BIT); + if (src_data_size < expected_header_size) + return D3DXERR_INVALIDDATA; + + if (header->color_map_type && ((header->color_map_type > 1) || (!header->color_map_length) + || (d3dx_get_tga_format_for_bpp(header->color_map_entrysize) == D3DFMT_UNKNOWN))) + return D3DXERR_INVALIDDATA; + + switch (header->image_type & IMAGETYPE_MASK) + { + case IMAGETYPE_COLORMAPPED: + if (header->depth != 8 || !header->color_map_type) + return D3DXERR_INVALIDDATA; + image->format = D3DFMT_P8; + break; + + case IMAGETYPE_TRUECOLOR: + if ((image->format = d3dx_get_tga_format_for_bpp(header->depth)) == D3DFMT_UNKNOWN) + return D3DXERR_INVALIDDATA; + break; + + case IMAGETYPE_GRAYSCALE: + if (header->depth != 8) + return D3DXERR_INVALIDDATA; + image->format = D3DFMT_L8; + break; + + default: + return D3DXERR_INVALIDDATA; + } + + set_volume_struct(&image->size, header->width, header->height, 1); + image->mip_levels = 1; + image->layer_count = 1; + image->resource_type = D3DRTYPE_TEXTURE; + image->image_file_format = D3DXIFF_TGA; + + return D3D_OK; +} + HRESULT d3dx_image_init(const void *src_data, uint32_t src_data_size, struct d3dx_image *image, uint32_t starting_mip_level, uint32_t flags) { @@ -1014,6 +1101,9 @@ HRESULT d3dx_image_init(const void *src_data, uint32_t src_data_size, struct d3d } /* Last resort, try TGA. */ + if (flags & D3DX_IMAGE_INFO_ONLY) + return d3dx_initialize_image_from_tga(src_data, src_data_size, image); + return d3dx_initialize_image_from_wic(src_data, src_data_size, image, D3DXIFF_TGA, flags); } diff --git a/dlls/d3dx9_36/tests/surface.c b/dlls/d3dx9_36/tests/surface.c index fb2abb27fa2..8555464fefb 100644 --- a/dlls/d3dx9_36/tests/surface.c +++ b/dlls/d3dx9_36/tests/surface.c @@ -626,17 +626,15 @@ static void test_tga_header_handling(void) D3DFORMAT format; } expected; uint32_t extra_header_size; - BOOL todo_hr; - BOOL todo_info; } info_tests[] = { /* 15 bpp true color. */ { { 0, COLORMAP_TYPE_NONE, IMAGETYPE_TRUECOLOR, 0, 0, 0, 0, 0, 4, 4, 15, 0 }, - { D3D_OK, 4, 4, D3DFMT_X1R5G5B5 }, .todo_hr = TRUE + { D3D_OK, 4, 4, D3DFMT_X1R5G5B5 } }, /* 16 bpp true color. */ { { 0, COLORMAP_TYPE_NONE, IMAGETYPE_TRUECOLOR, 0, 0, 0, 0, 0, 4, 4, 16, 0 }, - { D3D_OK, 4, 4, D3DFMT_A1R5G5B5 }, .todo_info = TRUE + { D3D_OK, 4, 4, D3DFMT_A1R5G5B5 } }, /* 24 bpp true color. */ { { 0, COLORMAP_TYPE_NONE, IMAGETYPE_TRUECOLOR, 0, 0, 0, 0, 0, 4, 4, 24, 0 }, @@ -644,7 +642,7 @@ static void test_tga_header_handling(void) }, /* 32 bpp true color. */ { { 0, COLORMAP_TYPE_NONE, IMAGETYPE_TRUECOLOR, 0, 0, 0, 0, 0, 4, 4, 32, 0 }, - { D3D_OK, 4, 4, D3DFMT_A8R8G8B8 }, .todo_info = TRUE + { D3D_OK, 4, 4, D3DFMT_A8R8G8B8 } }, /* 8 bit paletted, 15 bpp palette. */ { { 0, COLORMAP_TYPE_ONE, IMAGETYPE_COLORMAPPED, 0, 256, 15, 0, 0, 4, 4, 8, 0 }, @@ -695,19 +693,18 @@ static void test_tga_header_handling(void) tga->header = info_tests[i].header; check_tga_image_info(tga, file_size, info_tests[i].expected.width, info_tests[i].expected.height, - info_tests[i].expected.format, info_tests[i].expected.hr, info_tests[i].todo_hr, info_tests[i].todo_info); + info_tests[i].expected.format, info_tests[i].expected.hr, FALSE, FALSE); /* X/Y origin fields are ignored. */ tga->header.xorigin = tga->header.width + 1; tga->header.yorigin = tga->header.height + 1; check_tga_image_info(tga, file_size, info_tests[i].expected.width, info_tests[i].expected.height, - info_tests[i].expected.format, info_tests[i].expected.hr, info_tests[i].todo_hr, info_tests[i].todo_info); + info_tests[i].expected.format, info_tests[i].expected.hr, FALSE, FALSE); /* Image descriptor field is ignored. */ tga->header.image_descriptor = 0xcf; check_tga_image_info(tga, file_size, info_tests[i].expected.width, info_tests[i].expected.height, - info_tests[i].expected.format, info_tests[i].expected.hr, SUCCEEDED(info_tests[i].expected.hr), - info_tests[i].todo_info); + info_tests[i].expected.format, info_tests[i].expected.hr, FALSE, FALSE); if (FAILED(info_tests[i].expected.hr)) goto next; @@ -720,12 +717,12 @@ static void test_tga_header_handling(void) tmp_footer.extension_area_offset = 65536; memcpy(&tga->data[info_tests[i].extra_header_size], &tmp_footer, sizeof(tmp_footer)); check_tga_image_info(tga, file_size + sizeof(tmp_footer), info_tests[i].expected.width, info_tests[i].expected.height, - info_tests[i].expected.format, info_tests[i].expected.hr, TRUE, info_tests[i].todo_info); + info_tests[i].expected.format, info_tests[i].expected.hr, FALSE, FALSE); /* Check RLE type. */ tga->header.image_type |= IMAGETYPE_RLE; check_tga_image_info(tga, file_size, info_tests[i].expected.width, info_tests[i].expected.height, - info_tests[i].expected.format, info_tests[i].expected.hr, TRUE, info_tests[i].todo_info); + info_tests[i].expected.format, info_tests[i].expected.hr, FALSE, FALSE); tga->header.image_type &= ~IMAGETYPE_RLE; if (tga->header.image_type == IMAGETYPE_COLORMAPPED) @@ -737,16 +734,14 @@ static void test_tga_header_handling(void) */ tga->header.color_map_length = 1; check_tga_image_info(tga, file_size, info_tests[i].expected.width, info_tests[i].expected.height, - info_tests[i].expected.format, info_tests[i].expected.hr, SUCCEEDED(info_tests[i].expected.hr), - info_tests[i].todo_info); + info_tests[i].expected.format, info_tests[i].expected.hr, FALSE, FALSE); tga->header.color_map_entrysize = 8; check_tga_image_info(tga, file_size, 0, 0, D3DFMT_UNKNOWN, D3DXERR_INVALIDDATA, FALSE, FALSE); /* Add a byte to file size to account for color map. */ check_tga_image_info(tga, file_size + 1, info_tests[i].expected.width, info_tests[i].expected.height, - info_tests[i].expected.format, info_tests[i].expected.hr, SUCCEEDED(info_tests[i].expected.hr), - info_tests[i].todo_info); + info_tests[i].expected.format, info_tests[i].expected.hr, FALSE, FALSE); /* ID length field is also considered. */ tga->header.id_length = 1; @@ -754,8 +749,7 @@ static void test_tga_header_handling(void) /* Add another byte to file size to account for id length. */ check_tga_image_info(tga, file_size + 2, info_tests[i].expected.width, info_tests[i].expected.height, - info_tests[i].expected.format, info_tests[i].expected.hr, SUCCEEDED(info_tests[i].expected.hr), - info_tests[i].todo_info); + info_tests[i].expected.format, info_tests[i].expected.hr, FALSE, FALSE); /* * If the color map type field is set but the color map fields @@ -775,14 +769,12 @@ static void test_tga_header_handling(void) /* 16 is a valid entry size. */ tga->header.color_map_entrysize = 16; check_tga_image_info(tga, file_size + 2, info_tests[i].expected.width, info_tests[i].expected.height, - info_tests[i].expected.format, info_tests[i].expected.hr, SUCCEEDED(info_tests[i].expected.hr), - info_tests[i].todo_info); + info_tests[i].expected.format, info_tests[i].expected.hr, FALSE, FALSE); /* First entry doesn't factor into validation. */ tga->header.color_map_firstentry = 512; check_tga_image_info(tga, file_size + 2, info_tests[i].expected.width, info_tests[i].expected.height, - info_tests[i].expected.format, info_tests[i].expected.hr, SUCCEEDED(info_tests[i].expected.hr), - info_tests[i].todo_info); + info_tests[i].expected.format, info_tests[i].expected.hr, FALSE, FALSE); next: winetest_pop_context(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6673
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=149038 Your paranoid android. === debian11b (64 bit WoW report) === Report validation errors: mshtml:script crashed (c0000005)
On Wed Oct 16 14:19:48 2024 +0000, Connor McAdams wrote:
changed this line in [version 2 of the diff](/wine/wine/-/merge_requests/6673/diffs?diff_id=138423&start_sha=5a744831e2abe04fe26136edaee06c866c58b0fa#d49071753fdf2ab21d518dbd8c720b48c7c732fc_666_663) Good catch, not sure how that happened. :)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6673#note_85165
On Wed Oct 16 13:02:23 2024 +0000, Matteo Bruni wrote:
I think it's cleaner to unconditionally assign `src_data` to `header` and move the check directly to the `if`. I _think_ this should be the way you've described now, but I'll let you mark this as resolved because I'm not sure. :)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6673#note_85166
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/surface.c:
tga->header = info_tests[i].header; check_tga_image_info(tga, file_size, info_tests[i].expected.width, info_tests[i].expected.height, - info_tests[i].expected.format, info_tests[i].expected.hr, info_tests[i].todo_hr, info_tests[i].todo_info); + info_tests[i].expected.format, info_tests[i].expected.hr, FALSE, FALSE);
Now both the `todo_hr` and `todo_info` arguments are unused, so they could be dropped as well (unless there is a reason to keep them around). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6673#note_85171
On Wed Oct 16 15:06:50 2024 +0000, Connor McAdams wrote:
I _think_ this should be the way you've described now, but I'll let you mark this as resolved because I'm not sure. :) Yes, that's exactly what I meant, thanks! :slight_smile:
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6673#note_85172
This merge request was approved by Matteo Bruni. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6673
participants (4)
-
Connor McAdams -
Connor McAdams (＠cmcadams)
-
Marvin -
Matteo Bruni (＠Mystral)