[PATCH 0/6] MR7711: d3dx9: Rework color key handling.
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/d3dx9_36/d3dx9_private.h | 12 ------------ dlls/d3dx9_36/surface.c | 16 ++++++++++------ 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/dlls/d3dx9_36/d3dx9_private.h b/dlls/d3dx9_36/d3dx9_private.h index 0e965eb6017..dabe714f611 100644 --- a/dlls/d3dx9_36/d3dx9_private.h +++ b/dlls/d3dx9_36/d3dx9_private.h @@ -374,18 +374,6 @@ void format_to_d3dx_color(const struct pixel_format_desc *format, const BYTE *sr struct d3dx_color *dst); void format_from_d3dx_color(const struct pixel_format_desc *format, const struct d3dx_color *src, BYTE *dst); -void copy_pixels(const BYTE *src, UINT src_row_pitch, UINT src_slice_pitch, - BYTE *dst, UINT dst_row_pitch, UINT dst_slice_pitch, const struct volume *size, - const struct pixel_format_desc *format); -void convert_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT src_slice_pitch, - const struct volume *src_size, const struct pixel_format_desc *src_format, - BYTE *dst, UINT dst_row_pitch, UINT dst_slice_pitch, const struct volume *dst_size, - const struct pixel_format_desc *dst_format, D3DCOLOR color_key, const PALETTEENTRY *palette); -void point_filter_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT src_slice_pitch, - const struct volume *src_size, const struct pixel_format_desc *src_format, - BYTE *dst, UINT dst_row_pitch, UINT dst_slice_pitch, const struct volume *dst_size, - const struct pixel_format_desc *dst_format, D3DCOLOR color_key, const PALETTEENTRY *palette); - HRESULT lock_surface(IDirect3DSurface9 *surface, const RECT *surface_rect, D3DLOCKED_RECT *lock, IDirect3DSurface9 **temp_surface, BOOL write); HRESULT unlock_surface(IDirect3DSurface9 *surface, const RECT *surface_rect, diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c index e3bc77bd52e..decea5677e5 100644 --- a/dlls/d3dx9_36/surface.c +++ b/dlls/d3dx9_36/surface.c @@ -1415,6 +1415,10 @@ static HRESULT d3dx_image_tga_rle_decode_row(const uint8_t **src, uint32_t src_b return D3D_OK; } +static void convert_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT src_slice_pitch, const struct volume *src_size, + const struct pixel_format_desc *src_format, BYTE *dst, UINT dst_row_pitch, UINT dst_slice_pitch, + const struct volume *dst_size, const struct pixel_format_desc *dst_format, D3DCOLOR color_key, + const PALETTEENTRY *palette); static HRESULT d3dx_image_tga_decode(const void *src_data, uint32_t src_data_size, uint32_t src_header_size, struct d3dx_image *image) { @@ -2411,7 +2415,7 @@ void format_from_d3dx_color(const struct pixel_format_desc *format, const struct * Works for any pixel format. * The source and the destination must be block-aligned. */ -void copy_pixels(const BYTE *src, UINT src_row_pitch, UINT src_slice_pitch, +static void copy_pixels(const BYTE *src, UINT src_row_pitch, UINT src_slice_pitch, BYTE *dst, UINT dst_row_pitch, UINT dst_slice_pitch, const struct volume *size, const struct pixel_format_desc *format) { @@ -2442,7 +2446,7 @@ void copy_pixels(const BYTE *src, UINT src_row_pitch, UINT src_slice_pitch, * any necessary format conversion and color keying. * Pixels outsize the source rect are blacked out. */ -void convert_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT src_slice_pitch, const struct volume *src_size, +static void convert_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT src_slice_pitch, const struct volume *src_size, const struct pixel_format_desc *src_format, BYTE *dst, UINT dst_row_pitch, UINT dst_slice_pitch, const struct volume *dst_size, const struct pixel_format_desc *dst_format, D3DCOLOR color_key, const PALETTEENTRY *palette) @@ -2542,10 +2546,10 @@ void convert_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT src_slice_pit * any necessary format conversion, color keying and stretching * using a point filter. */ -void point_filter_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT src_slice_pitch, const struct volume *src_size, - const struct pixel_format_desc *src_format, BYTE *dst, UINT dst_row_pitch, UINT dst_slice_pitch, - const struct volume *dst_size, const struct pixel_format_desc *dst_format, D3DCOLOR color_key, - const PALETTEENTRY *palette) +static void point_filter_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT src_slice_pitch, + const struct volume *src_size, const struct pixel_format_desc *src_format, BYTE *dst, UINT dst_row_pitch, + UINT dst_slice_pitch, const struct volume *dst_size, const struct pixel_format_desc *dst_format, + D3DCOLOR color_key, const PALETTEENTRY *palette) { struct argb_conversion_info conv_info, ck_conv_info; const struct pixel_format_desc *ck_format; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7711
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/d3dx9_36/surface.c | 143 +++++++++++++++------------------------- 1 file changed, 52 insertions(+), 91 deletions(-) diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c index decea5677e5..b9a11f682b4 100644 --- a/dlls/d3dx9_36/surface.c +++ b/dlls/d3dx9_36/surface.c @@ -2439,6 +2439,50 @@ static void copy_pixels(const BYTE *src, UINT src_row_pitch, UINT src_slice_pitc } } +static void convert_argb_pixel(const uint8_t *src_ptr, const struct pixel_format_desc *src_fmt, uint8_t *dst_ptr, + const struct pixel_format_desc *dst_fmt, const PALETTEENTRY *palette, struct argb_conversion_info *conv_info, + D3DCOLOR color_key, const struct pixel_format_desc *ck_format, struct argb_conversion_info *ck_conv_info) +{ + if (format_types_match(src_fmt, dst_fmt) && src_fmt->bytes_per_pixel <= 4 && dst_fmt->bytes_per_pixel <= 4) + { + DWORD channels[4]; + DWORD val; + + get_relevant_argb_components(conv_info, src_ptr, channels); + val = make_argb_color(conv_info, channels); + + if (color_key) + { + DWORD ck_pixel; + + get_relevant_argb_components(ck_conv_info, src_ptr, channels); + ck_pixel = make_argb_color(ck_conv_info, channels); + if (ck_pixel == color_key) + val &= ~conv_info->destmask[0]; + } + memcpy(dst_ptr, &val, dst_fmt->bytes_per_pixel); + } + else + { + struct d3dx_color color, tmp; + + format_to_d3dx_color(src_fmt, src_ptr, palette, &color); + tmp = color; + + if (color_key) + { + DWORD ck_pixel; + + format_from_d3dx_color(ck_format, &tmp, (BYTE *)&ck_pixel); + if (ck_pixel == color_key) + tmp.value.w = 0.0f; + } + + color = tmp; + format_from_d3dx_color(dst_fmt, &color, dst_ptr); + } +} + /************************************************************ * convert_argb_pixels * @@ -2451,9 +2495,9 @@ static void convert_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT src_sl const struct volume *dst_size, const struct pixel_format_desc *dst_format, D3DCOLOR color_key, const PALETTEENTRY *palette) { + /* Color keys are always represented in D3DFMT_A8R8G8B8 format. */ + const struct pixel_format_desc *ck_format = color_key ? get_format_info(D3DFMT_A8R8G8B8) : NULL; struct argb_conversion_info conv_info, ck_conv_info; - const struct pixel_format_desc *ck_format; - DWORD channels[4]; UINT min_width, min_height, min_depth; UINT x, y, z; @@ -2462,7 +2506,6 @@ static void convert_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT src_sl src, src_row_pitch, src_slice_pitch, src_size, src_format, dst, dst_row_pitch, dst_slice_pitch, dst_size, dst_format, color_key, palette); - ZeroMemory(channels, sizeof(channels)); init_argb_conversion_info(src_format, dst_format, &conv_info); min_width = min(src_size->width, dst_size->width); @@ -2470,11 +2513,7 @@ static void convert_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT src_sl min_depth = min(src_size->depth, dst_size->depth); if (color_key) - { - /* Color keys are always represented in D3DFMT_A8R8G8B8 format. */ - ck_format = get_format_info(D3DFMT_A8R8G8B8); init_argb_conversion_info(src_format, ck_format, &ck_conv_info); - } for (z = 0; z < min_depth; z++) { const BYTE *src_slice_ptr = src + z * src_slice_pitch; @@ -2485,44 +2524,8 @@ static void convert_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT src_sl BYTE *dst_ptr = dst_slice_ptr + y * dst_row_pitch; for (x = 0; x < min_width; x++) { - if (format_types_match(src_format, dst_format) - && src_format->bytes_per_pixel <= 4 && dst_format->bytes_per_pixel <= 4) - { - DWORD val; - - get_relevant_argb_components(&conv_info, src_ptr, channels); - val = make_argb_color(&conv_info, channels); - - if (color_key) - { - DWORD ck_pixel; - - get_relevant_argb_components(&ck_conv_info, src_ptr, channels); - ck_pixel = make_argb_color(&ck_conv_info, channels); - if (ck_pixel == color_key) - val &= ~conv_info.destmask[0]; - } - memcpy(dst_ptr, &val, dst_format->bytes_per_pixel); - } - else - { - struct d3dx_color color, tmp; - - format_to_d3dx_color(src_format, src_ptr, palette, &color); - tmp = color; - - if (color_key) - { - DWORD ck_pixel; - - format_from_d3dx_color(ck_format, &tmp, (BYTE *)&ck_pixel); - if (ck_pixel == color_key) - tmp.value.w = 0.0f; - } - - color = tmp; - format_from_d3dx_color(dst_format, &color, dst_ptr); - } + convert_argb_pixel(src_ptr, src_format, dst_ptr, dst_format, palette, + &conv_info, color_key, ck_format, &ck_conv_info); src_ptr += src_format->bytes_per_pixel; dst_ptr += dst_format->bytes_per_pixel; @@ -2551,9 +2554,9 @@ static void point_filter_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT s UINT dst_slice_pitch, const struct volume *dst_size, const struct pixel_format_desc *dst_format, D3DCOLOR color_key, const PALETTEENTRY *palette) { + /* Color keys are always represented in D3DFMT_A8R8G8B8 format. */ + const struct pixel_format_desc *ck_format = color_key ? get_format_info(D3DFMT_A8R8G8B8) : NULL; struct argb_conversion_info conv_info, ck_conv_info; - const struct pixel_format_desc *ck_format; - DWORD channels[4]; UINT x, y, z; TRACE("src %p, src_row_pitch %u, src_slice_pitch %u, src_size %p, src_format %p, dst %p, " @@ -2561,15 +2564,10 @@ static void point_filter_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT s src, src_row_pitch, src_slice_pitch, src_size, src_format, dst, dst_row_pitch, dst_slice_pitch, dst_size, dst_format, color_key, palette); - ZeroMemory(channels, sizeof(channels)); init_argb_conversion_info(src_format, dst_format, &conv_info); if (color_key) - { - /* Color keys are always represented in D3DFMT_A8R8G8B8 format. */ - ck_format = get_format_info(D3DFMT_A8R8G8B8); init_argb_conversion_info(src_format, ck_format, &ck_conv_info); - } for (z = 0; z < dst_size->depth; z++) { @@ -2585,45 +2583,8 @@ static void point_filter_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT s { const BYTE *src_ptr = src_row_ptr + (x * src_size->width / dst_size->width) * src_format->bytes_per_pixel; - if (format_types_match(src_format, dst_format) - && src_format->bytes_per_pixel <= 4 && dst_format->bytes_per_pixel <= 4) - { - DWORD val; - - get_relevant_argb_components(&conv_info, src_ptr, channels); - val = make_argb_color(&conv_info, channels); - - if (color_key) - { - DWORD ck_pixel; - - get_relevant_argb_components(&ck_conv_info, src_ptr, channels); - ck_pixel = make_argb_color(&ck_conv_info, channels); - if (ck_pixel == color_key) - val &= ~conv_info.destmask[0]; - } - memcpy(dst_ptr, &val, dst_format->bytes_per_pixel); - } - else - { - struct d3dx_color color, tmp; - - format_to_d3dx_color(src_format, src_ptr, palette, &color); - tmp = color; - - if (color_key) - { - DWORD ck_pixel; - - format_from_d3dx_color(ck_format, &tmp, (BYTE *)&ck_pixel); - if (ck_pixel == color_key) - tmp.value.w = 0.0f; - } - - color = tmp; - format_from_d3dx_color(dst_format, &color, dst_ptr); - } - + convert_argb_pixel(src_ptr, src_format, dst_ptr, dst_format, palette, + &conv_info, color_key, ck_format, &ck_conv_info); dst_ptr += dst_format->bytes_per_pixel; } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7711
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/d3dx9_36/tests/surface.c | 227 ++++++++++++++++++++++++++++++++++ 1 file changed, 227 insertions(+) diff --git a/dlls/d3dx9_36/tests/surface.c b/dlls/d3dx9_36/tests/surface.c index c618ff6c48a..7a150724cd6 100644 --- a/dlls/d3dx9_36/tests/surface.c +++ b/dlls/d3dx9_36/tests/surface.c @@ -1150,10 +1150,12 @@ static uint32_t get_bpp_for_d3dformat(D3DFORMAT format) case D3DFMT_Q16W16V16U16: return 8; + case D3DFMT_Q8W8V8U8: case D3DFMT_A8B8G8R8: case D3DFMT_A8R8G8B8: case D3DFMT_V16U16: case D3DFMT_G16R16: + case D3DFMT_R32F: return 4; case D3DFMT_R8G8B8: @@ -1165,6 +1167,7 @@ static uint32_t get_bpp_for_d3dformat(D3DFORMAT format) case D3DFMT_A8P8: return 2; + case D3DFMT_DXT5: case D3DFMT_L8: case D3DFMT_P8: return 1; @@ -2015,6 +2018,229 @@ static void test_load_surface_from_tga(IDirect3DDevice9 *device) free(tga); } +static const uint8_t r8g8b8_4_4[] = +{ + 0xff,0x00,0x00,0xff,0x00,0x00,0x00,0xff,0x00,0x00,0xff,0x00,0xff,0x00,0x00,0xff, + 0x00,0x00,0x00,0xff,0x00,0x00,0xff,0x00,0x00,0x00,0xff,0x00,0x00,0xff,0xff,0xff, + 0x00,0xff,0xff,0x00,0x00,0x00,0xff,0x00,0x00,0xff,0xff,0xff,0x00,0x80,0x80,0x00, +}; + +static const uint8_t r8g8b8_4_4_expected[] = +{ + 0xff,0x00,0x00,0xff,0xff,0x00,0x00,0xff,0x00,0xff,0x00,0xff,0x00,0xff,0x00,0xff, + 0xff,0x00,0x00,0xff,0xff,0x00,0x00,0xff,0x00,0xff,0x00,0xff,0x00,0xff,0x00,0xff, + 0x00,0x00,0xff,0xff,0x00,0x00,0xff,0xff,0xff,0xff,0x00,0xff,0xff,0xff,0x00,0xff, + 0x00,0x00,0xff,0xff,0x00,0x00,0xff,0xff,0xff,0xff,0x00,0xff,0x00,0x00,0x00,0x00, +}; + +static const uint8_t dxt5_4_4[] = +{ + 0xff,0x00,0x00,0x00,0x00,0x49,0x92,0x24,0x00,0xf8,0x00,0xf8,0x00,0x00,0x00,0x00, +}; + +static const uint8_t dxt5_4_4_expected[] = +{ + 0x00,0x00,0xff,0xff,0x00,0x00,0xff,0xff,0x00,0x00,0xff,0xff,0x00,0x00,0xff,0xff, + 0x00,0x00,0xff,0xff,0x00,0x00,0xff,0xff,0x00,0x00,0xff,0xff,0x00,0x00,0xff,0xff, + 0x00,0x00,0xff,0x00,0x00,0x00,0xff,0x00,0x00,0x00,0xff,0x00,0x00,0x00,0xff,0x00, + 0x00,0x00,0xff,0x00,0x00,0x00,0xff,0x00,0x00,0x00,0xff,0x00,0x00,0x00,0xff,0x00, +}; + +static const uint8_t p8_4_4[] = +{ + 0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xf0,0xf0,0xf0,0xf0,0xf0,0xf0,0xf0,0xf0, +}; + +static const uint8_t p8_4_4_expected[] = +{ + 0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff, + 0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, +}; + +static const uint8_t a8p8_4_4[] = +{ + 0xff,0x00,0xff,0x00,0xff,0x00,0xff,0x00,0xff,0x00,0xff,0x00,0xff,0x00,0xff,0x00, + 0xf0,0x10,0xf0,0x10,0xf0,0x10,0xf0,0x10,0xf0,0x10,0xf0,0x10,0xf0,0x10,0xf0,0x10, +}; + +static const uint8_t a8p8_4_4_expected[] = +{ + 0xff,0xff,0xff,0x00,0xff,0xff,0xff,0x00,0xff,0xff,0xff,0x00,0xff,0xff,0xff,0x00, + 0xff,0xff,0xff,0x00,0xff,0xff,0xff,0x00,0xff,0xff,0xff,0x00,0xff,0xff,0xff,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, +}; + +static const uint8_t a32r32g32b32_4_4[] = +{ + 0x00,0x00,0x80,0x3f,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x80,0x3f, + 0x00,0x00,0x80,0x3f,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x80,0x3f, + 0x00,0x00,0x80,0x3f,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x80,0x3f, + 0x00,0x00,0x80,0x3f,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x80,0x3f, + 0x00,0x00,0x80,0x3f,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x80,0x3f, + 0x00,0x00,0x80,0x3f,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x80,0x3f, + 0x00,0x00,0x80,0x3f,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x80,0x3f, + 0x00,0x00,0x80,0x3f,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x80,0x3f, + 0x00,0x00,0x80,0x3f,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x80,0x3f,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x80,0x3f,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x80,0x3f,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x80,0x3f,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x80,0x3f,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x80,0x3f,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x80,0x3f,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, +}; + +static const uint8_t a32r32g32b32_4_4_expected[] = +{ + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0xff,0x00,0x00,0x00,0xff,0x00,0x00,0x00,0xff,0x00,0x00,0x00,0xff,0x00, + 0x00,0x00,0xff,0x00,0x00,0x00,0xff,0x00,0x00,0x00,0xff,0x00,0x00,0x00,0xff,0x00, +}; + +static const uint8_t r32_4_4[] = +{ + 0x00,0x00,0x80,0x3f,0x00,0x00,0x80,0x3f,0x00,0x00,0x80,0x3f,0x00,0x00,0x80,0x3f, + 0x00,0x00,0x80,0x3f,0x00,0x00,0x80,0x3f,0x00,0x00,0x80,0x3f,0x00,0x00,0x80,0x3f, + 0x00,0x00,0x00,0x3f,0x00,0x00,0x00,0x3f,0x00,0x00,0x00,0x3f,0x00,0x00,0x00,0x3f, + 0x00,0x00,0x00,0x3f,0x00,0x00,0x00,0x3f,0x00,0x00,0x00,0x3f,0x00,0x00,0x00,0x3f, +}; + +static const uint8_t r32_4_4_expected[] = +{ + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, + 0xff,0xff,0x80,0xff,0xff,0xff,0x80,0xff,0xff,0xff,0x80,0xff,0xff,0xff,0x80,0xff, + 0xff,0xff,0x80,0xff,0xff,0xff,0x80,0xff,0xff,0xff,0x80,0xff,0xff,0xff,0x80,0xff, +}; + +static const uint8_t q8w8v8u8_4_4[] = +{ + 0x7f,0x82,0x82,0x7f,0x7f,0x82,0x82,0x7f,0x7f,0x82,0x82,0x7f,0x7f,0x82,0x82,0x7f, + 0x7f,0x82,0x82,0x7f,0x7f,0x82,0x82,0x7f,0x7f,0x82,0x82,0x7f,0x7f,0x82,0x82,0x7f, + 0x7f,0x82,0x82,0x82,0x7f,0x82,0x82,0x82,0x7f,0x82,0x82,0x82,0x7f,0x82,0x82,0x82, + 0x7f,0x82,0x82,0x82,0x7f,0x82,0x82,0x82,0x7f,0x82,0x82,0x82,0x7f,0x82,0x82,0x82, +}; + +static const uint8_t q8w8v8u8_4_4_expected[] = +{ + 0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80, + 0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80, + 0x01,0x01,0xff,0x01,0x01,0x01,0xff,0x01,0x01,0x01,0xff,0x01,0x01,0x01,0xff,0x01, + 0x01,0x01,0xff,0x01,0x01,0x01,0xff,0x01,0x01,0x01,0xff,0x01,0x01,0x01,0xff,0x01, +}; + +static void test_color_key(IDirect3DDevice9 *device) +{ + struct + { + D3DFORMAT src_format; + const PALETTEENTRY *src_palette; + const RECT src_rect; + const void *src_data; + + const void *expected_dst_data; + D3DCOLOR color_key; + BOOL todo; + } tests[] = + { + /* Color key with alpha channel unset. */ + { + D3DFMT_R8G8B8, NULL, { 0, 0, 4, 4 }, r8g8b8_4_4, r8g8b8_4_4_expected, 0x00008080, + .todo = TRUE + }, + /* Same color key as before except the alpha channel is set. */ + { + D3DFMT_R8G8B8, NULL, { 0, 0, 4, 4 }, r8g8b8_4_4, r8g8b8_4_4_expected, 0xff008080, + .todo = TRUE + }, + /* Color key on a palette. */ + { + D3DFMT_P8, test_palette, { 0, 0, 4, 4 }, p8_4_4, p8_4_4_expected, 0xf0c0c000, + .todo = TRUE + }, + { + D3DFMT_A8P8, test_palette, { 0, 0, 4, 4 }, a8p8_4_4, a8p8_4_4_expected, 0x10c0c000, + .todo = TRUE + }, + { + D3DFMT_A32B32G32R32F, NULL, { 0, 0, 4, 4 }, a32r32g32b32_4_4, a32r32g32b32_4_4_expected, 0xffff0000, + .todo = TRUE + }, + /* 5. */ + { + D3DFMT_R32F, NULL, { 0, 0, 4, 4 }, r32_4_4, r32_4_4_expected, 0xffff0000, + .todo = TRUE + }, + { + D3DFMT_Q8W8V8U8, NULL, { 0, 0, 4, 4 }, q8w8v8u8_4_4, q8w8v8u8_4_4_expected, 0xffff0000, + .todo = TRUE + }, + /* Color key is ignored for compressed formats. */ + { + D3DFMT_DXT5, NULL, { 0, 0, 4, 4 }, dxt5_4_4, dxt5_4_4_expected, 0xffff0000, + .todo = TRUE + }, + }; + D3DLOCKED_RECT lock_rect; + IDirect3DSurface9 *surf; + unsigned int i, x, y; + HRESULT hr; + + hr = IDirect3DDevice9_CreateOffscreenPlainSurface(device, 4, 4, D3DFMT_A8R8G8B8, D3DPOOL_SCRATCH, &surf, NULL); + ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr); + for (i = 0; i < ARRAY_SIZE(tests); ++i) + { + const uint32_t src_pitch = get_bpp_for_d3dformat(tests[i].src_format) * tests[i].src_rect.right; + unsigned int mismatch_count = 0; + + winetest_push_context("Test %u", i); + + hr = D3DXLoadSurfaceFromMemory(surf, NULL, NULL, tests[i].src_data, tests[i].src_format, + src_pitch, tests[i].src_palette, &tests[i].src_rect, D3DX_FILTER_NONE, tests[i].color_key); + ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr); + + IDirect3DSurface9_LockRect(surf, &lock_rect, NULL, D3DLOCK_READONLY); + for (y = 0; y < 4; ++y) + { + const uint8_t *dst_expected_row = ((uint8_t *)tests[i].expected_dst_data) + (16 * y); + const uint8_t *dst_row = ((uint8_t *)lock_rect.pBits) + (lock_rect.Pitch * y); + + for (x = 0; x < 4; ++x) + { + const uint8_t *dst_expected_pixel = dst_expected_row + (4 * x); + const uint8_t *dst_pixel = dst_row + (4 * x); + + if (memcmp(dst_pixel, dst_expected_pixel, 4)) + mismatch_count++; + } + } + IDirect3DSurface9_UnlockRect(surf); + + todo_wine_if(tests[i].todo) ok(!mismatch_count, "Unexpected number of mismatched pixels %u.\n", mismatch_count); + winetest_pop_context(); + } + + /* + * Trying to use D3DX_FILTER_SRGB_IN with a color key results in + * STATUS_ACCESS_VIOLATION on native. + */ + if (0) + { + const uint32_t src_pitch = get_bpp_for_d3dformat(tests[0].src_format) * tests[0].src_rect.right; + + hr = D3DXLoadSurfaceFromMemory(surf, NULL, NULL, tests[0].src_data, tests[0].src_format, + src_pitch, tests[0].src_palette, &tests[0].src_rect, D3DX_FILTER_NONE | D3DX_FILTER_SRGB_IN, + tests[0].color_key); + ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr); + } + + check_release((IUnknown *)surf, 0); +} + static void test_D3DXLoadSurface(IDirect3DDevice9 *device) { HRESULT hr; @@ -3280,6 +3506,7 @@ static void test_D3DXLoadSurface(IDirect3DDevice9 *device) test_format_conversion(device); test_dxt_premultiplied_alpha(device); test_load_surface_from_tga(device); + test_color_key(device); /* cleanup */ if(testdummy_ok) DeleteFileA("testdummy.bmp"); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7711
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/d3dx9_36/surface.c | 104 ++++++++++++++++++---------------- dlls/d3dx9_36/tests/surface.c | 5 -- 2 files changed, 56 insertions(+), 53 deletions(-) diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c index b9a11f682b4..1b6a4c0ad19 100644 --- a/dlls/d3dx9_36/surface.c +++ b/dlls/d3dx9_36/surface.c @@ -1417,7 +1417,7 @@ static HRESULT d3dx_image_tga_rle_decode_row(const uint8_t **src, uint32_t src_b static void convert_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT src_slice_pitch, const struct volume *src_size, const struct pixel_format_desc *src_format, BYTE *dst, UINT dst_row_pitch, UINT dst_slice_pitch, - const struct volume *dst_size, const struct pixel_format_desc *dst_format, D3DCOLOR color_key, + const struct volume *dst_size, const struct pixel_format_desc *dst_format, uint8_t *color_key, const PALETTEENTRY *palette); static HRESULT d3dx_image_tga_decode(const void *src_data, uint32_t src_data_size, uint32_t src_header_size, struct d3dx_image *image) @@ -1463,7 +1463,7 @@ static HRESULT d3dx_image_tga_decode(const void *src_data, uint32_t src_data_siz dst_desc = get_format_info(D3DFMT_A8B8G8R8); d3dx_calculate_pixels_size(dst_desc->format, 256, 1, &dst_row_pitch, &dst_slice_pitch); convert_argb_pixels(src_palette, src_row_pitch, src_slice_pitch, &image_map_size, src_desc, (BYTE *)palette, - dst_row_pitch, dst_slice_pitch, &image_map_size, dst_desc, 0, NULL); + dst_row_pitch, dst_slice_pitch, &image_map_size, dst_desc, NULL, NULL); /* Initialize unused palette entries to 0xff. */ if (header->color_map_length < 256) @@ -2441,44 +2441,33 @@ static void copy_pixels(const BYTE *src, UINT src_row_pitch, UINT src_slice_pitc static void convert_argb_pixel(const uint8_t *src_ptr, const struct pixel_format_desc *src_fmt, uint8_t *dst_ptr, const struct pixel_format_desc *dst_fmt, const PALETTEENTRY *palette, struct argb_conversion_info *conv_info, - D3DCOLOR color_key, const struct pixel_format_desc *ck_format, struct argb_conversion_info *ck_conv_info) + uint8_t *color_key) { + const BOOL is_color_key = (color_key && !memcmp(color_key, src_ptr, src_fmt->bytes_per_pixel)); + if (format_types_match(src_fmt, dst_fmt) && src_fmt->bytes_per_pixel <= 4 && dst_fmt->bytes_per_pixel <= 4) { DWORD channels[4]; DWORD val; - get_relevant_argb_components(conv_info, src_ptr, channels); - val = make_argb_color(conv_info, channels); - - if (color_key) + if (!is_color_key) { - DWORD ck_pixel; - - get_relevant_argb_components(ck_conv_info, src_ptr, channels); - ck_pixel = make_argb_color(ck_conv_info, channels); - if (ck_pixel == color_key) - val &= ~conv_info->destmask[0]; + get_relevant_argb_components(conv_info, src_ptr, channels); + val = make_argb_color(conv_info, channels); + } + else + { + val = 0; } memcpy(dst_ptr, &val, dst_fmt->bytes_per_pixel); } else { - struct d3dx_color color, tmp; + struct d3dx_color color; format_to_d3dx_color(src_fmt, src_ptr, palette, &color); - tmp = color; - - if (color_key) - { - DWORD ck_pixel; - - format_from_d3dx_color(ck_format, &tmp, (BYTE *)&ck_pixel); - if (ck_pixel == color_key) - tmp.value.w = 0.0f; - } - - color = tmp; + if (is_color_key) + memset(&color.value, 0, sizeof(color.value)); format_from_d3dx_color(dst_fmt, &color, dst_ptr); } } @@ -2492,17 +2481,15 @@ static void convert_argb_pixel(const uint8_t *src_ptr, const struct pixel_format */ static void convert_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT src_slice_pitch, const struct volume *src_size, const struct pixel_format_desc *src_format, BYTE *dst, UINT dst_row_pitch, UINT dst_slice_pitch, - const struct volume *dst_size, const struct pixel_format_desc *dst_format, D3DCOLOR color_key, + const struct volume *dst_size, const struct pixel_format_desc *dst_format, uint8_t *color_key, const PALETTEENTRY *palette) { - /* Color keys are always represented in D3DFMT_A8R8G8B8 format. */ - const struct pixel_format_desc *ck_format = color_key ? get_format_info(D3DFMT_A8R8G8B8) : NULL; - struct argb_conversion_info conv_info, ck_conv_info; + struct argb_conversion_info conv_info; UINT min_width, min_height, min_depth; UINT x, y, z; TRACE("src %p, src_row_pitch %u, src_slice_pitch %u, src_size %p, src_format %p, dst %p, " - "dst_row_pitch %u, dst_slice_pitch %u, dst_size %p, dst_format %p, color_key 0x%08lx, palette %p.\n", + "dst_row_pitch %u, dst_slice_pitch %u, dst_size %p, dst_format %p, color_key %p, palette %p.\n", src, src_row_pitch, src_slice_pitch, src_size, src_format, dst, dst_row_pitch, dst_slice_pitch, dst_size, dst_format, color_key, palette); @@ -2512,9 +2499,6 @@ static void convert_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT src_sl min_height = min(src_size->height, dst_size->height); min_depth = min(src_size->depth, dst_size->depth); - if (color_key) - init_argb_conversion_info(src_format, ck_format, &ck_conv_info); - for (z = 0; z < min_depth; z++) { const BYTE *src_slice_ptr = src + z * src_slice_pitch; BYTE *dst_slice_ptr = dst + z * dst_slice_pitch; @@ -2524,8 +2508,7 @@ static void convert_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT src_sl BYTE *dst_ptr = dst_slice_ptr + y * dst_row_pitch; for (x = 0; x < min_width; x++) { - convert_argb_pixel(src_ptr, src_format, dst_ptr, dst_format, palette, - &conv_info, color_key, ck_format, &ck_conv_info); + convert_argb_pixel(src_ptr, src_format, dst_ptr, dst_format, palette, &conv_info, color_key); src_ptr += src_format->bytes_per_pixel; dst_ptr += dst_format->bytes_per_pixel; @@ -2552,23 +2535,18 @@ static void convert_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT src_sl static void point_filter_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT src_slice_pitch, const struct volume *src_size, const struct pixel_format_desc *src_format, BYTE *dst, UINT dst_row_pitch, UINT dst_slice_pitch, const struct volume *dst_size, const struct pixel_format_desc *dst_format, - D3DCOLOR color_key, const PALETTEENTRY *palette) + uint8_t *color_key, const PALETTEENTRY *palette) { - /* Color keys are always represented in D3DFMT_A8R8G8B8 format. */ - const struct pixel_format_desc *ck_format = color_key ? get_format_info(D3DFMT_A8R8G8B8) : NULL; - struct argb_conversion_info conv_info, ck_conv_info; + struct argb_conversion_info conv_info; UINT x, y, z; TRACE("src %p, src_row_pitch %u, src_slice_pitch %u, src_size %p, src_format %p, dst %p, " - "dst_row_pitch %u, dst_slice_pitch %u, dst_size %p, dst_format %p, color_key 0x%08lx, palette %p.\n", + "dst_row_pitch %u, dst_slice_pitch %u, dst_size %p, dst_format %p, color_key %p, palette %p.\n", src, src_row_pitch, src_slice_pitch, src_size, src_format, dst, dst_row_pitch, dst_slice_pitch, dst_size, dst_format, color_key, palette); init_argb_conversion_info(src_format, dst_format, &conv_info); - if (color_key) - init_argb_conversion_info(src_format, ck_format, &ck_conv_info); - for (z = 0; z < dst_size->depth; z++) { BYTE *dst_slice_ptr = dst + z * dst_slice_pitch; @@ -2583,8 +2561,7 @@ static void point_filter_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT s { const BYTE *src_ptr = src_row_ptr + (x * src_size->width / dst_size->width) * src_format->bytes_per_pixel; - convert_argb_pixel(src_ptr, src_format, dst_ptr, dst_format, palette, - &conv_info, color_key, ck_format, &ck_conv_info); + convert_argb_pixel(src_ptr, src_format, dst_ptr, dst_format, palette, &conv_info, color_key); dst_ptr += dst_format->bytes_per_pixel; } } @@ -2727,11 +2704,38 @@ static const char *debug_d3dx_pixels(struct d3dx_pixels *pixels) pixels->size.width, pixels->size.height, pixels->size.depth, wine_dbgstr_rect(&pixels->unaligned_rect)); } +static BOOL get_format_color_key(const struct pixel_format_desc *fmt, const PALETTEENTRY *palette, + uint32_t in_color_key, uint8_t *out_color_key) +{ + /* Color keys are always represented in D3DFMT_A8R8G8B8 format. */ + const struct pixel_format_desc *ck_format = get_format_info(D3DFMT_A8R8G8B8); + struct d3dx_color tmp_color; + + if (fmt == ck_format) + { + memcpy(out_color_key, &in_color_key, sizeof(in_color_key)); + return TRUE; + } + + if (is_index_format(fmt)) + { + FIXME("Color keying for indexed formats is currently unimplemented.\n"); + return FALSE; + } + + format_to_d3dx_color(ck_format, (const uint8_t *)&in_color_key, palette, &tmp_color); + format_from_d3dx_color(fmt, &tmp_color, out_color_key); + + return TRUE; +} + HRESULT d3dx_load_pixels_from_pixels(struct d3dx_pixels *dst_pixels, const struct pixel_format_desc *dst_desc, struct d3dx_pixels *src_pixels, const struct pixel_format_desc *src_desc, uint32_t filter_flags, uint32_t color_key) { struct volume src_size, dst_size, dst_size_aligned; + uint8_t *src_color_key = NULL; + uint8_t color_key_buf[16]; HRESULT hr = S_OK; TRACE("dst_pixels %s, dst_desc %p, src_pixels %s, src_desc %p, filter_flags %#x, color_key %#x.\n", @@ -2859,11 +2863,15 @@ HRESULT d3dx_load_pixels_from_pixels(struct d3dx_pixels *dst_pixels, goto exit; } + assert(src_desc->bytes_per_pixel <= sizeof(color_key_buf)); + if (color_key && get_format_color_key(src_desc, src_pixels->palette, color_key, color_key_buf)) + src_color_key = color_key_buf; + if ((filter_flags & 0xf) == D3DX_FILTER_NONE) { convert_argb_pixels(src_pixels->data, src_pixels->row_pitch, src_pixels->slice_pitch, &src_size, src_desc, (BYTE *)dst_pixels->data, dst_pixels->row_pitch, dst_pixels->slice_pitch, &dst_size, dst_desc, - color_key, src_pixels->palette); + src_color_key, src_pixels->palette); } else /* if ((filter & 0xf) == D3DX_FILTER_POINT) */ { @@ -2874,7 +2882,7 @@ HRESULT d3dx_load_pixels_from_pixels(struct d3dx_pixels *dst_pixels, * D3DX_FILTER_TRIANGLE and D3DX_FILTER_BOX are implemented. */ point_filter_argb_pixels(src_pixels->data, src_pixels->row_pitch, src_pixels->slice_pitch, &src_size, src_desc, (BYTE *)dst_pixels->data, dst_pixels->row_pitch, dst_pixels->slice_pitch, &dst_size, - dst_desc, color_key, src_pixels->palette); + dst_desc, src_color_key, src_pixels->palette); } exit: diff --git a/dlls/d3dx9_36/tests/surface.c b/dlls/d3dx9_36/tests/surface.c index 7a150724cd6..082abca5dc5 100644 --- a/dlls/d3dx9_36/tests/surface.c +++ b/dlls/d3dx9_36/tests/surface.c @@ -2150,12 +2150,10 @@ static void test_color_key(IDirect3DDevice9 *device) /* Color key with alpha channel unset. */ { D3DFMT_R8G8B8, NULL, { 0, 0, 4, 4 }, r8g8b8_4_4, r8g8b8_4_4_expected, 0x00008080, - .todo = TRUE }, /* Same color key as before except the alpha channel is set. */ { D3DFMT_R8G8B8, NULL, { 0, 0, 4, 4 }, r8g8b8_4_4, r8g8b8_4_4_expected, 0xff008080, - .todo = TRUE }, /* Color key on a palette. */ { @@ -2168,16 +2166,13 @@ static void test_color_key(IDirect3DDevice9 *device) }, { D3DFMT_A32B32G32R32F, NULL, { 0, 0, 4, 4 }, a32r32g32b32_4_4, a32r32g32b32_4_4_expected, 0xffff0000, - .todo = TRUE }, /* 5. */ { D3DFMT_R32F, NULL, { 0, 0, 4, 4 }, r32_4_4, r32_4_4_expected, 0xffff0000, - .todo = TRUE }, { D3DFMT_Q8W8V8U8, NULL, { 0, 0, 4, 4 }, q8w8v8u8_4_4, q8w8v8u8_4_4_expected, 0xffff0000, - .todo = TRUE }, /* Color key is ignored for compressed formats. */ { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7711
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/d3dx9_36/surface.c | 2 +- dlls/d3dx9_36/tests/surface.c | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c index 1b6a4c0ad19..1d571d89f65 100644 --- a/dlls/d3dx9_36/surface.c +++ b/dlls/d3dx9_36/surface.c @@ -2801,7 +2801,7 @@ HRESULT d3dx_load_pixels_from_pixels(struct d3dx_pixels *dst_pixels, 0, src_pixels->size.depth, &uncompressed_pixels); hr = d3dx_load_pixels_from_pixels(dst_pixels, dst_desc, &uncompressed_pixels, uncompressed_desc, - filter_flags, color_key); + filter_flags, 0); } free(uncompressed_mem); goto exit; diff --git a/dlls/d3dx9_36/tests/surface.c b/dlls/d3dx9_36/tests/surface.c index 082abca5dc5..9cd989680fa 100644 --- a/dlls/d3dx9_36/tests/surface.c +++ b/dlls/d3dx9_36/tests/surface.c @@ -2177,7 +2177,6 @@ static void test_color_key(IDirect3DDevice9 *device) /* Color key is ignored for compressed formats. */ { D3DFMT_DXT5, NULL, { 0, 0, 4, 4 }, dxt5_4_4, dxt5_4_4_expected, 0xffff0000, - .todo = TRUE }, }; D3DLOCKED_RECT lock_rect; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7711
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/d3dx9_36/surface.c | 36 +++++++++++++++++++++++++++++++++-- dlls/d3dx9_36/tests/surface.c | 9 +-------- 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c index 1d571d89f65..c2b67bba17e 100644 --- a/dlls/d3dx9_36/surface.c +++ b/dlls/d3dx9_36/surface.c @@ -2719,8 +2719,40 @@ static BOOL get_format_color_key(const struct pixel_format_desc *fmt, const PALE if (is_index_format(fmt)) { - FIXME("Color keying for indexed formats is currently unimplemented.\n"); - return FALSE; + const uint8_t cmp_bytes = (fmt->a_type == CTYPE_INDEX) ? 4 : 3; + unsigned int color_key_idx = 256; + PALETTEENTRY tmp_palette; + unsigned int i; + + tmp_palette.peRed = (in_color_key >> 16) & 0xff; + tmp_palette.peGreen = (in_color_key >> 8) & 0xff; + tmp_palette.peBlue = in_color_key & 0xff; + tmp_palette.peFlags = (in_color_key >> 24) & 0xff; + for (i = 0; i < 256; ++i) + { + if (!memcmp(&tmp_palette, &palette[i], cmp_bytes)) + { + color_key_idx = i; + break; + } + } + + /* No match in the palette for this color key. */ + if (color_key_idx == 256) + return FALSE; + + /* A8P8. */ + if (fmt->a_type == CTYPE_UNORM) + { + out_color_key[0] = color_key_idx; + out_color_key[1] = (in_color_key >> 24) & 0xff; + } + else + { + out_color_key[0] = color_key_idx; + } + + return TRUE; } format_to_d3dx_color(ck_format, (const uint8_t *)&in_color_key, palette, &tmp_color); diff --git a/dlls/d3dx9_36/tests/surface.c b/dlls/d3dx9_36/tests/surface.c index 9cd989680fa..cfd7e54c4fe 100644 --- a/dlls/d3dx9_36/tests/surface.c +++ b/dlls/d3dx9_36/tests/surface.c @@ -2144,7 +2144,6 @@ static void test_color_key(IDirect3DDevice9 *device) const void *expected_dst_data; D3DCOLOR color_key; - BOOL todo; } tests[] = { /* Color key with alpha channel unset. */ @@ -2158,11 +2157,9 @@ static void test_color_key(IDirect3DDevice9 *device) /* Color key on a palette. */ { D3DFMT_P8, test_palette, { 0, 0, 4, 4 }, p8_4_4, p8_4_4_expected, 0xf0c0c000, - .todo = TRUE }, { D3DFMT_A8P8, test_palette, { 0, 0, 4, 4 }, a8p8_4_4, a8p8_4_4_expected, 0x10c0c000, - .todo = TRUE }, { D3DFMT_A32B32G32R32F, NULL, { 0, 0, 4, 4 }, a32r32g32b32_4_4, a32r32g32b32_4_4_expected, 0xffff0000, @@ -2189,7 +2186,6 @@ static void test_color_key(IDirect3DDevice9 *device) for (i = 0; i < ARRAY_SIZE(tests); ++i) { const uint32_t src_pitch = get_bpp_for_d3dformat(tests[i].src_format) * tests[i].src_rect.right; - unsigned int mismatch_count = 0; winetest_push_context("Test %u", i); @@ -2208,13 +2204,10 @@ static void test_color_key(IDirect3DDevice9 *device) const uint8_t *dst_expected_pixel = dst_expected_row + (4 * x); const uint8_t *dst_pixel = dst_row + (4 * x); - if (memcmp(dst_pixel, dst_expected_pixel, 4)) - mismatch_count++; + ok(!memcmp(dst_pixel, dst_expected_pixel, 4), "Pixel mismatch at (%u,%u).\n", x, y); } } IDirect3DSurface9_UnlockRect(surf); - - todo_wine_if(tests[i].todo) ok(!mismatch_count, "Unexpected number of mismatched pixels %u.\n", mismatch_count); winetest_pop_context(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7711
Apparently 64-bit supports color keying on compressed formats but 32-bit does not, very odd :) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_99509
On Tue Apr 1 12:40:15 2025 +0000, Connor McAdams wrote:
Apparently 64-bit supports color keying on compressed formats but 32-bit does not, very odd :) Not sure if this is a behavior we should match, or if we should support it on both 32-bit/64-bit and mark the 32-bit test on Windows as broken?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_99510
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/surface.c:
+ 0x7f,0x82,0x82,0x7f,0x7f,0x82,0x82,0x7f,0x7f,0x82,0x82,0x7f,0x7f,0x82,0x82,0x7f, + 0x7f,0x82,0x82,0x82,0x7f,0x82,0x82,0x82,0x7f,0x82,0x82,0x82,0x7f,0x82,0x82,0x82, + 0x7f,0x82,0x82,0x82,0x7f,0x82,0x82,0x82,0x7f,0x82,0x82,0x82,0x7f,0x82,0x82,0x82, +}; + +static const uint8_t q8w8v8u8_4_4_expected[] = +{ + 0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80, + 0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80, + 0x01,0x01,0xff,0x01,0x01,0x01,0xff,0x01,0x01,0x01,0xff,0x01,0x01,0x01,0xff,0x01, + 0x01,0x01,0xff,0x01,0x01,0x01,0xff,0x01,0x01,0x01,0xff,0x01,0x01,0x01,0xff,0x01, +}; + +static void test_color_key(IDirect3DDevice9 *device) +{ + struct The array should probably be `static const`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_99956
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/surface.c:
+ { + const uint8_t *dst_expected_row = ((uint8_t *)tests[i].expected_dst_data) + (16 * y); + const uint8_t *dst_row = ((uint8_t *)lock_rect.pBits) + (lock_rect.Pitch * y); + + for (x = 0; x < 4; ++x) + { + const uint8_t *dst_expected_pixel = dst_expected_row + (4 * x); + const uint8_t *dst_pixel = dst_row + (4 * x); + + if (memcmp(dst_pixel, dst_expected_pixel, 4)) + mismatch_count++; + } + } + IDirect3DSurface9_UnlockRect(surf); + + todo_wine_if(tests[i].todo) ok(!mismatch_count, "Unexpected number of mismatched pixels %u.\n", mismatch_count); Technically you're counting bytes on a 4 byte-per-pixel format, so the message isn't entirely correct :nerd:
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_99958
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/surface.c:
+ for (i = 0; i < ARRAY_SIZE(tests); ++i) + { + const uint32_t src_pitch = get_bpp_for_d3dformat(tests[i].src_format) * tests[i].src_rect.right; + unsigned int mismatch_count = 0; + + winetest_push_context("Test %u", i); + + hr = D3DXLoadSurfaceFromMemory(surf, NULL, NULL, tests[i].src_data, tests[i].src_format, + src_pitch, tests[i].src_palette, &tests[i].src_rect, D3DX_FILTER_NONE, tests[i].color_key); + ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr); + + IDirect3DSurface9_LockRect(surf, &lock_rect, NULL, D3DLOCK_READONLY); + for (y = 0; y < 4; ++y) + { + const uint8_t *dst_expected_row = ((uint8_t *)tests[i].expected_dst_data) + (16 * y); + const uint8_t *dst_row = ((uint8_t *)lock_rect.pBits) + (lock_rect.Pitch * y); Not really an issue but ideally the cast should be to `(const uint8_t *)`.
Also bikeshedding but, that 16 could be "spelled out" as `4 * 4`, or `sizeof(uint32_t) * 4` or such. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_99957
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/surface.c:
test_format_conversion(device); test_dxt_premultiplied_alpha(device); test_load_surface_from_tga(device); + test_color_key(device);
I just now realized the new test is being called from `test_D3DXLoadSurface()`. I think it would be better to get rid of one level of nesting i.e. call the new test directly from the main function, creating a separate `device` in the new test. It might make sense to add helpers like `create_device()` while at it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_99961
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/surface.c:
+ + const void *expected_dst_data; + D3DCOLOR color_key; + BOOL todo; + } tests[] = + { + /* Color key with alpha channel unset. */ + { + D3DFMT_R8G8B8, NULL, { 0, 0, 4, 4 }, r8g8b8_4_4, r8g8b8_4_4_expected, 0x00008080, + .todo = TRUE + }, + /* Same color key as before except the alpha channel is set. */ + { + D3DFMT_R8G8B8, NULL, { 0, 0, 4, 4 }, r8g8b8_4_4, r8g8b8_4_4_expected, 0xff008080, + .todo = TRUE + }, I'd be curious to see a test with some pixels matching the color key in the RGB part but not alpha, to make sure that alpha isn't just ignored.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_99960
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/surface.c:
} }
+static void convert_argb_pixel(const uint8_t *src_ptr, const struct pixel_format_desc *src_fmt, uint8_t *dst_ptr, + const struct pixel_format_desc *dst_fmt, const PALETTEENTRY *palette, struct argb_conversion_info *conv_info, + D3DCOLOR color_key, const struct pixel_format_desc *ck_format, struct argb_conversion_info *ck_conv_info)
I intentionally didn't factor out this chunk at the time, because calling a function once per pixel is potentially a lot of overhead and such a large function might end up not being inlined (regardless of the presence of any `inline` qualifier). It looks like, indeed, it's not inlined by the compiler for me. I guess the question is how much this actually affects performance. Can you run some quick benchmarks on a variety of images to have some idea of the effect of this change? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_99959
On Tue Apr 1 12:40:15 2025 +0000, Connor McAdams wrote:
Not sure if this is a behavior we should match, or if we should support it on both 32-bit/64-bit and mark the 32-bit test on Windows as broken? It's tricky, since it's conceivable that some 32-bit game depends on the color key being ignored for compressed formats...
I guess ignoring for now is okay but we probably want a `FIXME()` in the implementation or a `todo_wine` in the tests. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_99963
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/surface.c:
+ .todo = TRUE + }, + /* Same color key as before except the alpha channel is set. */ + { + D3DFMT_R8G8B8, NULL, { 0, 0, 4, 4 }, r8g8b8_4_4, r8g8b8_4_4_expected, 0xff008080, + .todo = TRUE + }, + /* Color key on a palette. */ + { + D3DFMT_P8, test_palette, { 0, 0, 4, 4 }, p8_4_4, p8_4_4_expected, 0xf0c0c000, + .todo = TRUE + }, + { + D3DFMT_A8P8, test_palette, { 0, 0, 4, 4 }, a8p8_4_4, a8p8_4_4_expected, 0x10c0c000, + .todo = TRUE + }, What happens if the color key matches two colors in the palette? Or for the A8P8 to RGB case, what happens if the alpha part doesn't match?
Fundamentally though, I'm not convinced (yet) that the matching happens in the source color space. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_99962
While reviewing this I did explore the idea of fixing the color key check without moving it to the source format. I'm attaching the result of that here, for reference. It's a bunch of debug stuff, test changes, fixes to the color key checks which kinda required splitting up the rgb_range into individual per-component ranges. It's not exactly beautiful or thought out (e.g. I more or less blindly replaced rgb_range with r_range all over the place, which isn't necessarily correct), but I guess it might be somewhat useful. [d3dx-per-channel-range.txt](/uploads/7398e770a0f207ef0799b586ec79334d/d3dx-per-channel-range.txt) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_99964
might end up not being inlined
FWIW `static FORCEINLINE void ...` should result in it always being inlined, at least with GCC and Clang. `FORCEINLINE` being the Wine macro for `__attribute__((always_inline)) inline`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_99966
On Fri Apr 4 00:12:14 2025 +0000, William Horvath wrote:
might end up not being inlined FWIW `static FORCEINLINE void ...` should result in it always being inlined, at least with GCC and Clang. `FORCEINLINE` being the Wine macro for `__attribute__((always_inline)) inline`. I wrote a test that converts a source of 4096x4096 pixels to a destination surface of 4096x4096 pixels. I ran the test 40 times for each format then took the average, these are the results between the function without `FORCEINLINE` and with `FORCEINLINE`:
| Source | Dest | non-inline | inline | diff | | ------ | ------ | ------ | ------ | ------ | | D3DFMT_P8 | D3DFMT_A8R8G8B8 | 1.1341057 | 1.1172716 | 0.0168341 | | D3DFMT_A8P8 | D3DFMT_A8R8G8B8 | 1.2521212 | 1.2365495 | 0.0155717 | | D3DFMT_Q8W8V8U8 | D3DFMT_A8R8G8B8 | 1.1889339 | 1.1724054 | 0.0165285 | | D3DFMT_A32B32G32R32F | D3DFMT_A8R8G8B8 | 0.7444296 | 0.7330986 | 0.011331 | | D3DFMT_P8 | D3DFMT_A32B32G32R32F | 1.0119892 | 0.9935202 | 0.018469 | | D3DFMT_A8P8 | D3DFMT_A32B32G32R32F | 1.1156599 | 1.0982643 | 0.0173956 | | D3DFMT_Q8W8V8U8 | D3DFMT_A32B32G32R32F | 1.0075875 | 0.9763689 | 0.0312186 | | D3DFMT_A8R8G8B8 | D3DFMT_A32B32G32R32F | 1.4340131 | 1.4098957 | 0.0241174 | | D3DFMT_P8 | D3DFMT_Q8W8V8U8 | 1.2236793 | 1.1992818 | 0.0243975 | | D3DFMT_A8P8 | D3DFMT_Q8W8V8U8 | 1.3356345 | 1.3049285 | 0.030706 | | D3DFMT_A32B32G32R32F | D3DFMT_Q8W8V8U8 | 0.7572374 | 0.7376824 | 0.019555 | | D3DFMT_A8R8G8B8 | D3DFMT_Q8W8V8U8 | 1.6424342 | 1.6185154 | 0.0239188 | So, there is a consistent performance difference between the two, with `FORCEINLINE` being faster. Compared to native we're pretty slow either way, here are the results from native: | Source | Dest | Time | | ------ | ------ | ------ | | D3DFMT_P8 | D3DFMT_A8R8G8B8 | 0.2615006 | | D3DFMT_A8P8 | D3DFMT_A8R8G8B8 | 0.2651899 | | D3DFMT_Q8W8V8U8 | D3DFMT_A8R8G8B8 | 0.2894723 | | D3DFMT_A32B32G32R32F | D3DFMT_A8R8G8B8 | 0.2726750 | | D3DFMT_P8 | D3DFMT_A32B32G32R32F | 0.1033063 | | D3DFMT_A8P8 | D3DFMT_A32B32G32R32F | 0.1009229 | | D3DFMT_Q8W8V8U8 | D3DFMT_A32B32G32R32F | 0.0864582 | | D3DFMT_A8R8G8B8 | D3DFMT_A32B32G32R32F | 0.0719979 | | D3DFMT_P8 | D3DFMT_Q8W8V8U8 | 0.3161181 | | D3DFMT_A8P8 | D3DFMT_Q8W8V8U8 | 0.3253104 | | D3DFMT_A32B32G32R32F | D3DFMT_Q8W8V8U8 | 0.2874802 | | D3DFMT_A8R8G8B8 | D3DFMT_Q8W8V8U8 | 0.2879247 | I think it makes sense to use `FORCEINLINE` here. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_99998
On Fri Apr 4 15:26:30 2025 +0000, Connor McAdams wrote:
I wrote a test that converts a source of 4096x4096 pixels to a destination surface of 4096x4096 pixels. I ran the test 40 times for each format then took the average, these are the results between the function without `FORCEINLINE` and with `FORCEINLINE`:
| Source | Dest | non-inline | inline | diff | | ------ | ------ | ------ | ------ | ------ | | D3DFMT_P8 | D3DFMT_A8R8G8B8 | 1.1341057 | 1.1172716 | 0.0168341 | | D3DFMT_A8P8 | D3DFMT_A8R8G8B8 | 1.2521212 | 1.2365495 | 0.0155717 | | D3DFMT_Q8W8V8U8 | D3DFMT_A8R8G8B8 | 1.1889339 | 1.1724054 | 0.0165285 | | D3DFMT_A32B32G32R32F | D3DFMT_A8R8G8B8 | 0.7444296 | 0.7330986 | 0.011331 | | D3DFMT_P8 | D3DFMT_A32B32G32R32F | 1.0119892 | 0.9935202 | 0.018469 | | D3DFMT_A8P8 | D3DFMT_A32B32G32R32F | 1.1156599 | 1.0982643 | 0.0173956 | | D3DFMT_Q8W8V8U8 | D3DFMT_A32B32G32R32F | 1.0075875 | 0.9763689 | 0.0312186 | | D3DFMT_A8R8G8B8 | D3DFMT_A32B32G32R32F | 1.4340131 | 1.4098957 | 0.0241174 | | D3DFMT_P8 | D3DFMT_Q8W8V8U8 | 1.2236793 | 1.1992818 | 0.0243975 | | D3DFMT_A8P8 | D3DFMT_Q8W8V8U8 | 1.3356345 | 1.3049285 | 0.030706 | | D3DFMT_A32B32G32R32F | D3DFMT_Q8W8V8U8 | 0.7572374 | 0.7376824 | 0.019555 | | D3DFMT_A8R8G8B8 | D3DFMT_Q8W8V8U8 | 1.6424342 | 1.6185154 | 0.0239188 | So, there is a consistent performance difference between the two, with `FORCEINLINE` being faster. Compared to native we're pretty slow either way, here are the results from native: | Source | Dest | Time | | ------ | ------ | ------ | | D3DFMT_P8 | D3DFMT_A8R8G8B8 | 0.2615006 | | D3DFMT_A8P8 | D3DFMT_A8R8G8B8 | 0.2651899 | | D3DFMT_Q8W8V8U8 | D3DFMT_A8R8G8B8 | 0.2894723 | | D3DFMT_A32B32G32R32F | D3DFMT_A8R8G8B8 | 0.2726750 | | D3DFMT_P8 | D3DFMT_A32B32G32R32F | 0.1033063 | | D3DFMT_A8P8 | D3DFMT_A32B32G32R32F | 0.1009229 | | D3DFMT_Q8W8V8U8 | D3DFMT_A32B32G32R32F | 0.0864582 | | D3DFMT_A8R8G8B8 | D3DFMT_A32B32G32R32F | 0.0719979 | | D3DFMT_P8 | D3DFMT_Q8W8V8U8 | 0.3161181 | | D3DFMT_A8P8 | D3DFMT_Q8W8V8U8 | 0.3253104 | | D3DFMT_A32B32G32R32F | D3DFMT_Q8W8V8U8 | 0.2874802 | | D3DFMT_A8R8G8B8 | D3DFMT_Q8W8V8U8 | 0.2879247 | I think it makes sense to use `FORCEINLINE` here. I assume there is no difference between `FORCEINLINE convert_argb_pixel()` and the patch before splitting that function out?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_99999
On Fri Apr 4 15:55:02 2025 +0000, Matteo Bruni wrote:
I assume there is no difference between `FORCEINLINE convert_argb_pixel()` and the patch before splitting that function out? Correct. No two runs are identical in either case, but it seems to be within the same variance as beefore the function was split off.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_100000
On Fri Apr 4 15:55:02 2025 +0000, Connor McAdams wrote:
Correct. No two runs are identical in either case, but it seems to be within the same variance as beefore the function was split off. For what it's worth, the difference doesn't seem *that* large that I would consider `FORCEINLINE` necessary here[*]. What I feared was that we'd be falling off a performance cliff with the additional function call overhead, but that definitely doesn't seem to be the case from your tests.
The gap with native is significant. No need to tackle that until necessary, but it's something to keep in mind. BTW, what are the time units in the tables above? Milliseconds? [*]: There are other possible concerns with inlining (e.g. additional cache pressure) that are hard to evaluate in a standalone test and might discourage it, all things being (roughly) equal otherwise. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_100009
On Fri Apr 4 17:56:14 2025 +0000, Matteo Bruni wrote:
For what it's worth, the difference doesn't seem *that* large that I would consider `FORCEINLINE` necessary here[*]. What I feared was that we'd be falling off a performance cliff with the additional function call overhead, but that definitely doesn't seem to be the case from your tests. The gap with native is significant. No need to tackle that until necessary, but it's something to keep in mind. BTW, what are the time units in the tables above? Milliseconds? [*]: There are other possible concerns with inlining (e.g. additional cache pressure) that are hard to evaluate in a standalone test and might discourage it, all things being (roughly) equal otherwise. The time unit is in seconds. Initially I used `GetTickCount()`, the difference was still noticeable but I wasn't sure if that was just due to the resolution of the timer there. I switched to `QueryPerformanceCounter()` instead to make sure it wasn't just a timer resolution issue.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_100010
On Fri Apr 4 19:44:30 2025 +0000, Connor McAdams wrote:
The time unit is in seconds. Initially I used `GetTickCount()`, the difference was still noticeable but I wasn't sure if that was just due to the resolution of the timer there. I switched to `QueryPerformanceCounter()` instead to make sure it wasn't just a timer resolution issue. Here's the test I was using if you want to take a look and make sure my measurements make sense :) [test-conversion-speed.diff](/uploads/5a785bc62b5b67b831b26dad91379ba4/test-conversion-speed.diff)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_100011
participants (4)
-
Connor McAdams -
Connor McAdams (＠cmcadams)
-
Matteo Bruni (＠Mystral)
-
William Horvath (＠whrvt)