[PATCH 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. :) -- 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..eca87f75322 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) +{ + uint32_t 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 | 235 ++++++++++++++++++++++++ 2 files changed, 314 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..17ac86cb38e 100644 --- a/dlls/d3dx9_36/tests/surface.c +++ b/dlls/d3dx9_36/tests/surface.c @@ -558,6 +558,240 @@ 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) + }, + /* 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 +1052,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 | 28 +++++------ 2 files changed, 101 insertions(+), 17 deletions(-) diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c index eca87f75322..0fa918bbf2e 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 = (src_data_size >= sizeof(*header)) ? (const struct tga_header *)src_data : NULL; + uint32_t expected_header_size = sizeof(*header); + + if (!header) + return D3DXERR_INVALIDDATA; + + expected_header_size += header->id_length; + expected_header_size += header->color_map_length * ((header->color_map_entrysize + 7) >> 3); + 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 (d3dx_get_tga_format_for_bpp(header->depth) == D3DFMT_UNKNOWN) + return D3DXERR_INVALIDDATA; + image->format = d3dx_get_tga_format_for_bpp(header->depth); + 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 17ac86cb38e..7092c8972b8 100644 --- a/dlls/d3dx9_36/tests/surface.c +++ b/dlls/d3dx9_36/tests/surface.c @@ -630,11 +630,11 @@ static void test_tga_header_handling(void) } 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 }, @@ -642,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 }, @@ -708,8 +708,7 @@ static void test_tga_header_handling(void) /* 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, info_tests[i].todo_hr, info_tests[i].todo_info); if (FAILED(info_tests[i].expected.hr)) goto next; @@ -722,12 +721,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, info_tests[i].todo_hr, 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); + info_tests[i].expected.format, info_tests[i].expected.hr, info_tests[i].todo_hr, info_tests[i].todo_info); tga->header.image_type &= ~IMAGETYPE_RLE; if (tga->header.image_type == IMAGETYPE_COLORMAPPED) @@ -739,16 +738,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, info_tests[i].todo_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); + info_tests[i].expected.format, info_tests[i].expected.hr, info_tests[i].todo_hr, info_tests[i].todo_info); /* ID length field is also considered. */ tga->header.id_length = 1; @@ -756,8 +753,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, info_tests[i].todo_hr, info_tests[i].todo_info); /* * If the color map type field is set but the color map fields @@ -777,14 +773,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, info_tests[i].todo_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); + info_tests[i].expected.format, info_tests[i].expected.hr, info_tests[i].todo_hr, info_tests[i].todo_info); next: winetest_pop_context(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6673
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/surface.c:
+{ + { 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) +{ + uint32_t i; This one can just be `unsigned int`, we don't particularly care for the specific integer type.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6673#note_85136
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/surface.c:
+ 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 { `{` on its own line.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6673#note_85143
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/surface.c:
+ 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 = (src_data_size >= sizeof(*header)) ? (const struct tga_header *)src_data : NULL; + uint32_t expected_header_size = sizeof(*header); + + if (!header) + return D3DXERR_INVALIDDATA; I think it's cleaner to unconditionally assign `src_data` to `header` and move the check directly to the `if`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6673#note_85144
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/surface.c:
+ } +} + +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; The `expected` can actually go on the same line as the `}`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6673#note_85138
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/surface.c:
+{ + 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[] = { ... while this opening bracket, confusingly perhaps, can also be on its own line.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6673#note_85139
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/surface.c:
+ /* 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) + }, + /* 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) + }, These two tests look identical to me.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6673#note_85140
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/surface.c:
+ + /* + * 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; Oh wow, I didn't expect that.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6673#note_85141
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/surface.c:
} 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
I haven't checked but, if there are no more of these tests coming, we could get rid of the `todo_` fields entirely. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6673#note_85142
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/surface.c:
+ { 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) +{ + uint32_t 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)) `&&` should be at the start of the next line.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6673#note_85137
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/surface.c:
+ 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 (d3dx_get_tga_format_for_bpp(header->depth) == D3DFMT_UNKNOWN) + return D3DXERR_INVALIDDATA; + image->format = d3dx_get_tga_format_for_bpp(header->depth); Another nitpick but:
if ((image->format = d3dx_get_tga_format_for_bpp(header->depth)) == D3DFMT_UNKNOWN)
return D3DXERR_INVALIDDATA;
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6673#note_85146
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/surface.c:
+ 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 = (src_data_size >= sizeof(*header)) ? (const struct tga_header *)src_data : NULL; + uint32_t expected_header_size = sizeof(*header); + + if (!header) + return D3DXERR_INVALIDDATA; + + expected_header_size += header->id_length; + expected_header_size += header->color_map_length * ((header->color_map_entrysize + 7) >> 3); Very nitpicky but it seems clearer to do `/ 8` (or even better `/ CHAR_BIT`) instead of `>> 3`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/6673#note_85145
I've got a bunch of comments, definitely nothing substantial. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6673#note_85147
participants (3)
-
Connor McAdams -
Connor McAdams (＠cmcadams)
-
Matteo Bruni (＠Mystral)