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. :)
From: Connor McAdams cmcadams@codeweavers.com
Signed-off-by: Connor McAdams cmcadams@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)
From: Connor McAdams cmcadams@codeweavers.com
Signed-off-by: Connor McAdams cmcadams@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);
From: Connor McAdams cmcadams@codeweavers.com
Signed-off-by: Connor McAdams cmcadams@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(); }
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.
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.
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 `}`.
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.
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.
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.
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.
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.
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`.
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`.
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: ```suggestion:-2+0 if ((image->format = d3dx_get_tga_format_for_bpp(header->depth)) == D3DFMT_UNKNOWN) return D3DXERR_INVALIDDATA; ```
I've got a bunch of comments, definitely nothing substantial.