[PATCH v2 0/6] MR7711: d3dx9: Rework color key handling.
-- v2: d3dx9: Don't color key compressed pixel formats on 32-bit d3dx9. d3dx9: Fixup color key value in d3dx_load_pixels_from_pixels() if necessary. d3dx9: Set all color channels to zero when color keying. d3dx9/tests: Add some color key tests. d3dx9: Move code for format conversion of a single pixel into a common helper function. d3dx9: Make functions for pixel copying/conversion/filtering static. 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/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 | 144 +++++++++++++++------------------------- 1 file changed, 53 insertions(+), 91 deletions(-) diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c index decea5677e5..3a6b8b78ebf 100644 --- a/dlls/d3dx9_36/surface.c +++ b/dlls/d3dx9_36/surface.c @@ -2439,6 +2439,51 @@ 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 +2496,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 +2507,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 +2514,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 +2525,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 +2555,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 +2565,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 +2584,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 | 332 ++++++++++++++++++++++++++++++++++ 1 file changed, 332 insertions(+) diff --git a/dlls/d3dx9_36/tests/surface.c b/dlls/d3dx9_36/tests/surface.c index c618ff6c48a..8c657b702a1 100644 --- a/dlls/d3dx9_36/tests/surface.c +++ b/dlls/d3dx9_36/tests/surface.c @@ -133,6 +133,46 @@ static HRESULT create_file(const char *filename, const unsigned char *data, cons return D3DERR_INVALIDCALL; } +static IDirect3DDevice9 *create_device(HWND *window) +{ + D3DPRESENT_PARAMETERS present_parameters = { 0 }; + IDirect3DDevice9 *device; + IDirect3D9 *d3d; + HRESULT hr; + HWND wnd; + + *window = NULL; + + if (!(wnd = CreateWindowA("static", "d3dx9_test", WS_OVERLAPPEDWINDOW, 0, 0, + 640, 480, NULL, NULL, NULL, NULL))) + { + skip("Couldn't create application window.\n"); + return NULL; + } + + if (!(d3d = Direct3DCreate9(D3D_SDK_VERSION))) + { + skip("Couldn't create IDirect3D9 object.\n"); + DestroyWindow(wnd); + return NULL; + } + + present_parameters.Windowed = TRUE; + present_parameters.SwapEffect = D3DSWAPEFFECT_DISCARD; + hr = IDirect3D9_CreateDevice(d3d, D3DADAPTER_DEFAULT, D3DDEVTYPE_HAL, wnd, D3DCREATE_HARDWARE_VERTEXPROCESSING, + &present_parameters, &device); + IDirect3D9_Release(d3d); + if (FAILED(hr)) + { + skip("Failed to create IDirect3DDevice9 object %#lx.\n", hr); + DestroyWindow(wnd); + return NULL; + } + + *window = wnd; + return device; +} + /* fills dds_header with reasonable default values */ static void fill_dds_header(struct dds_header *header) { @@ -1150,10 +1190,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 +1207,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 +2058,293 @@ 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_32_bit[] = +{ + 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 dxt5_4_4_expected_64_bit[] = +{ + 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 p8_4_4[] = +{ + 0x10,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xf0,0xf0,0xf0,0xf0,0xf0,0xf0,0xf0,0xf0, +}; + +static const uint8_t p8_4_4_expected[] = +{ + 0x00,0x40,0x00,0x10,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,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,0x10,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[] = +{ + 0x00,0x00,0x00,0x00,0x00,0xc0,0xc0,0x00,0x00,0xc0,0xc0,0x00,0x00,0xc0,0xc0,0x00, + 0x00,0xc0,0xc0,0x00,0x00,0xc0,0xc0,0x00,0x00,0xc0,0xc0,0x00,0x00,0xc0,0xc0,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 const uint8_t a8r8g8b8_4_4[] = +{ + 0x00,0xff,0x00,0x80,0x00,0xff,0x00,0x80,0x00,0xff,0x00,0xff,0x00,0xff,0x00,0xff, + 0x80,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,0xff,0xff,0x00,0xff, +}; + +static const uint8_t a8r8g8b8_4_4_expected[] = +{ + 0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0xff,0x00,0xff,0x00,0xff,0x00,0xff, + 0x80,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,0xff,0xff,0x00,0xff, +}; + +static BOOL is_dxt_d3dformat(D3DFORMAT fmt); +static void test_color_key(void) +{ + static const struct + { + D3DFORMAT src_format; + 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, { 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, { 0, 0, 4, 4 }, r8g8b8_4_4, r8g8b8_4_4_expected, 0xff008080, + .todo = TRUE + }, + /* Color key on a palette. */ + { + D3DFMT_P8, { 0, 0, 4, 4 }, p8_4_4, p8_4_4_expected, 0xf0c0c000, + .todo = TRUE + }, + { + D3DFMT_A8P8, { 0, 0, 4, 4 }, a8p8_4_4, a8p8_4_4_expected, 0x10c0c000, + .todo = TRUE + }, + { + D3DFMT_A32B32G32R32F, { 0, 0, 4, 4 }, a32r32g32b32_4_4, a32r32g32b32_4_4_expected, 0xffff0000, + .todo = TRUE + }, + /* 5. */ + /* + * Only channels that exist in the source format matter for the color + * key, other channels can be set to any value and are ignored. + */ + { + D3DFMT_R32F, { 0, 0, 4, 4 }, r32_4_4, r32_4_4_expected, 0xffff3080, + .todo = TRUE + }, + { + D3DFMT_Q8W8V8U8, { 0, 0, 4, 4 }, q8w8v8u8_4_4, q8w8v8u8_4_4_expected, 0xffff0000, + .todo = TRUE + }, + /* Alpha channel factors into the color key check. */ + { + D3DFMT_A8R8G8B8, { 0, 0, 4, 4 }, a8r8g8b8_4_4, a8r8g8b8_4_4_expected, 0x8000ff00, + .todo = TRUE + }, + { + D3DFMT_A8R8G8B8, { 0, 0, 4, 4 }, a8r8g8b8_4_4, a8r8g8b8_4_4, 0x0000ff00, + }, + }; + unsigned int i, y, mismatch_count; + PALETTEENTRY tmp_palette[256]; + const uint8_t *expected_dst; + IDirect3DDevice9 *device; + D3DLOCKED_RECT lock_rect; + IDirect3DSurface9 *surf; + uint32_t src_pitch; + RECT rect; + HRESULT hr; + HWND hwnd; + + if (!(device = create_device(&hwnd))) + return; + + memcpy(tmp_palette, test_palette, sizeof(test_palette)); + /* Set palette entries 0xf1-0xff to the same value. */ + for (i = 0; i < 0xf; ++i) + tmp_palette[0xf1 + i] = tmp_palette[0xf0]; + + 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) + { + winetest_push_context("Test %u", i); + + mismatch_count = 0; + src_pitch = get_bpp_for_d3dformat(tests[i].src_format) * tests[i].src_rect.right; + hr = D3DXLoadSurfaceFromMemory(surf, NULL, NULL, tests[i].src_data, tests[i].src_format, + src_pitch, tmp_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 = ((const uint8_t *)tests[i].expected_dst_data) + (sizeof(uint32_t) * 4 * y); + const uint8_t *dst_row = ((const uint8_t *)lock_rect.pBits) + (lock_rect.Pitch * y); + + if (memcmp(dst_row, dst_expected_row, sizeof(uint32_t) * 4)) + mismatch_count++; + } + IDirect3DSurface9_UnlockRect(surf); + + todo_wine_if(tests[i].todo) ok(!mismatch_count, "Unexpected number of mismatched lines %u.\n", mismatch_count); + winetest_pop_context(); + } + + /* + * Test color key handling for compressed formats. On 64-bit the color key + * is used, but on 32-bit it is ignored. + */ + SetRect(&rect, 0, 0, 4, 4); + hr = D3DXLoadSurfaceFromMemory(surf, NULL, NULL, dxt5_4_4, D3DFMT_DXT5, 16, NULL, &rect, D3DX_FILTER_NONE, + 0xffff0000); + ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr); + + mismatch_count = 0; + expected_dst = (sizeof(void *) == 8) ? dxt5_4_4_expected_64_bit : dxt5_4_4_expected_32_bit; + IDirect3DSurface9_LockRect(surf, &lock_rect, NULL, D3DLOCK_READONLY); + for (y = 0; y < 4; ++y) + { + const uint8_t *dst_expected_row = ((const uint8_t *)expected_dst) + (sizeof(uint32_t) * 4 * y); + const uint8_t *dst_row = ((const uint8_t *)lock_rect.pBits) + (lock_rect.Pitch * y); + + if (memcmp(dst_row, dst_expected_row, sizeof(uint32_t) * 4)) + mismatch_count++; + } + IDirect3DSurface9_UnlockRect(surf); + + todo_wine ok(!mismatch_count, "Unexpected number of mismatched lines %u.\n", mismatch_count); + + /* + * 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, NULL, &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); + check_release((IUnknown *)device, 0); + DestroyWindow(hwnd); +} + static void test_D3DXLoadSurface(IDirect3DDevice9 *device) { HRESULT hr; @@ -4651,4 +4981,6 @@ START_TEST(surface) check_release((IUnknown*)device, 0); check_release((IUnknown*)d3d, 0); DestroyWindow(wnd); + + test_color_key(); } -- 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 | 4 ++-- dlls/d3dx9_36/tests/surface.c | 7 +------ 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c index 3a6b8b78ebf..4786b29bfe3 100644 --- a/dlls/d3dx9_36/surface.c +++ b/dlls/d3dx9_36/surface.c @@ -2459,7 +2459,7 @@ static void convert_argb_pixel(const uint8_t *src_ptr, const struct pixel_format 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]; + val = 0; } memcpy(dst_ptr, &val, dst_fmt->bytes_per_pixel); } @@ -2476,7 +2476,7 @@ static void convert_argb_pixel(const uint8_t *src_ptr, const struct pixel_format format_from_d3dx_color(ck_format, &tmp, (BYTE *)&ck_pixel); if (ck_pixel == color_key) - tmp.value.w = 0.0f; + tmp.value.x = tmp.value.y = tmp.value.z = tmp.value.w = 0.0f; } color = tmp; diff --git a/dlls/d3dx9_36/tests/surface.c b/dlls/d3dx9_36/tests/surface.c index 8c657b702a1..7bc52914184 100644 --- a/dlls/d3dx9_36/tests/surface.c +++ b/dlls/d3dx9_36/tests/surface.c @@ -2219,20 +2219,16 @@ static void test_color_key(void) /* Same color key as before except the alpha channel is set. */ { D3DFMT_R8G8B8, { 0, 0, 4, 4 }, r8g8b8_4_4, r8g8b8_4_4_expected, 0xff008080, - .todo = TRUE }, /* Color key on a palette. */ { D3DFMT_P8, { 0, 0, 4, 4 }, p8_4_4, p8_4_4_expected, 0xf0c0c000, - .todo = TRUE }, { D3DFMT_A8P8, { 0, 0, 4, 4 }, a8p8_4_4, a8p8_4_4_expected, 0x10c0c000, - .todo = TRUE }, { D3DFMT_A32B32G32R32F, { 0, 0, 4, 4 }, a32r32g32b32_4_4, a32r32g32b32_4_4_expected, 0xffff0000, - .todo = TRUE }, /* 5. */ /* @@ -2250,7 +2246,6 @@ static void test_color_key(void) /* Alpha channel factors into the color key check. */ { D3DFMT_A8R8G8B8, { 0, 0, 4, 4 }, a8r8g8b8_4_4, a8r8g8b8_4_4_expected, 0x8000ff00, - .todo = TRUE }, { D3DFMT_A8R8G8B8, { 0, 0, 4, 4 }, a8r8g8b8_4_4, a8r8g8b8_4_4, 0x0000ff00, @@ -2324,7 +2319,7 @@ static void test_color_key(void) } IDirect3DSurface9_UnlockRect(surf); - todo_wine ok(!mismatch_count, "Unexpected number of mismatched lines %u.\n", mismatch_count); + todo_wine_if(sizeof(void *) == 4) ok(!mismatch_count, "Unexpected number of mismatched lines %u.\n", mismatch_count); /* * Trying to use D3DX_FILTER_SRGB_IN with a color key results in -- 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/d3dx9_private.h | 5 +++++ dlls/d3dx9_36/surface.c | 19 +++++++++++++++++++ dlls/d3dx9_36/tests/surface.c | 10 +--------- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/dlls/d3dx9_36/d3dx9_private.h b/dlls/d3dx9_36/d3dx9_private.h index dabe714f611..194d9879f7d 100644 --- a/dlls/d3dx9_36/d3dx9_private.h +++ b/dlls/d3dx9_36/d3dx9_private.h @@ -359,6 +359,11 @@ static inline BOOL is_conversion_to_supported(const struct pixel_format_desc *fo return !is_index_format(format) && !is_packed_format(format) && !is_unknown_format(format); } +static inline uint8_t format_channel_count(const struct pixel_format_desc *fmt) +{ + return !!fmt->bits[0] + !!fmt->bits[1] + !!fmt->bits[2] + !!fmt->bits[3]; +} + HRESULT map_view_of_file(const WCHAR *filename, void **buffer, DWORD *length); HRESULT load_resource_into_memory(HMODULE module, HRSRC resinfo, void **buffer, DWORD *length); diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c index 4786b29bfe3..f90a4d30456 100644 --- a/dlls/d3dx9_36/surface.c +++ b/dlls/d3dx9_36/surface.c @@ -2732,6 +2732,7 @@ 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) { + const struct pixel_format_desc *ck_format = get_format_info(D3DFMT_A8R8G8B8); struct volume src_size, dst_size, dst_size_aligned; HRESULT hr = S_OK; @@ -2860,6 +2861,24 @@ HRESULT d3dx_load_pixels_from_pixels(struct d3dx_pixels *dst_pixels, goto exit; } + /* + * Convert color key to the source format and then back again if + * necessary. This ensures the color key represents a value that + * is obtainable when converting from the source format to the color + * key format. + */ + if (color_key && !is_index_format(src_desc) && (!format_types_match(src_desc, ck_format) + || format_channel_count(src_desc) < 4)) + { + struct argb_conversion_info ck_conv_info, conv_info; + uint8_t tmp_buf[16]; + + init_argb_conversion_info(ck_format, src_desc, &ck_conv_info); + init_argb_conversion_info(src_desc, ck_format, &conv_info); + convert_argb_pixel((uint8_t *)&color_key, ck_format, tmp_buf, src_desc, NULL, &ck_conv_info, 0, NULL, NULL); + convert_argb_pixel(tmp_buf, src_desc, (uint8_t *)&color_key, ck_format, NULL, &conv_info, 0, NULL, NULL); + } + 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, diff --git a/dlls/d3dx9_36/tests/surface.c b/dlls/d3dx9_36/tests/surface.c index 7bc52914184..2a5569f7836 100644 --- a/dlls/d3dx9_36/tests/surface.c +++ b/dlls/d3dx9_36/tests/surface.c @@ -2208,13 +2208,11 @@ static void test_color_key(void) const void *expected_dst_data; D3DCOLOR color_key; - BOOL todo; } tests[] = { /* Color key with alpha channel unset. */ { D3DFMT_R8G8B8, { 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. */ { @@ -2237,11 +2235,9 @@ static void test_color_key(void) */ { D3DFMT_R32F, { 0, 0, 4, 4 }, r32_4_4, r32_4_4_expected, 0xffff3080, - .todo = TRUE }, { D3DFMT_Q8W8V8U8, { 0, 0, 4, 4 }, q8w8v8u8_4_4, q8w8v8u8_4_4_expected, 0xffff0000, - .todo = TRUE }, /* Alpha channel factors into the color key check. */ { @@ -2276,7 +2272,6 @@ static void test_color_key(void) { winetest_push_context("Test %u", i); - mismatch_count = 0; src_pitch = get_bpp_for_d3dformat(tests[i].src_format) * tests[i].src_rect.right; hr = D3DXLoadSurfaceFromMemory(surf, NULL, NULL, tests[i].src_data, tests[i].src_format, src_pitch, tmp_palette, &tests[i].src_rect, D3DX_FILTER_NONE, tests[i].color_key); @@ -2288,12 +2283,9 @@ static void test_color_key(void) const uint8_t *dst_expected_row = ((const uint8_t *)tests[i].expected_dst_data) + (sizeof(uint32_t) * 4 * y); const uint8_t *dst_row = ((const uint8_t *)lock_rect.pBits) + (lock_rect.Pitch * y); - if (memcmp(dst_row, dst_expected_row, sizeof(uint32_t) * 4)) - mismatch_count++; + ok(!memcmp(dst_row, dst_expected_row, sizeof(uint32_t) * 4), "Unexpected line %u.\n", y); } IDirect3DSurface9_UnlockRect(surf); - - todo_wine_if(tests[i].todo) ok(!mismatch_count, "Unexpected number of mismatched lines %u.\n", mismatch_count); winetest_pop_context(); } -- 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 | 5 +++++ dlls/d3dx9_36/tests/surface.c | 8 ++------ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c index f90a4d30456..2dc3e9b5c54 100644 --- a/dlls/d3dx9_36/surface.c +++ b/dlls/d3dx9_36/surface.c @@ -2798,6 +2798,11 @@ HRESULT d3dx_load_pixels_from_pixels(struct d3dx_pixels *dst_pixels, uncompressed_desc->format, 0, 0, src_pixels->size.width, src_pixels->size.height, 0, src_pixels->size.depth, &uncompressed_pixels); + if (sizeof(void *) == 4 && color_key) + { + TRACE("Clearing color key value on compressed source pixels.\n"); + color_key = 0; + } hr = d3dx_load_pixels_from_pixels(dst_pixels, dst_desc, &uncompressed_pixels, uncompressed_desc, filter_flags, color_key); } diff --git a/dlls/d3dx9_36/tests/surface.c b/dlls/d3dx9_36/tests/surface.c index 2a5569f7836..1efea54462c 100644 --- a/dlls/d3dx9_36/tests/surface.c +++ b/dlls/d3dx9_36/tests/surface.c @@ -2247,13 +2247,13 @@ static void test_color_key(void) D3DFMT_A8R8G8B8, { 0, 0, 4, 4 }, a8r8g8b8_4_4, a8r8g8b8_4_4, 0x0000ff00, }, }; - unsigned int i, y, mismatch_count; PALETTEENTRY tmp_palette[256]; const uint8_t *expected_dst; IDirect3DDevice9 *device; D3DLOCKED_RECT lock_rect; IDirect3DSurface9 *surf; uint32_t src_pitch; + unsigned int i, y; RECT rect; HRESULT hr; HWND hwnd; @@ -2298,7 +2298,6 @@ static void test_color_key(void) 0xffff0000); ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr); - mismatch_count = 0; expected_dst = (sizeof(void *) == 8) ? dxt5_4_4_expected_64_bit : dxt5_4_4_expected_32_bit; IDirect3DSurface9_LockRect(surf, &lock_rect, NULL, D3DLOCK_READONLY); for (y = 0; y < 4; ++y) @@ -2306,13 +2305,10 @@ static void test_color_key(void) const uint8_t *dst_expected_row = ((const uint8_t *)expected_dst) + (sizeof(uint32_t) * 4 * y); const uint8_t *dst_row = ((const uint8_t *)lock_rect.pBits) + (lock_rect.Pitch * y); - if (memcmp(dst_row, dst_expected_row, sizeof(uint32_t) * 4)) - mismatch_count++; + ok(!memcmp(dst_row, dst_expected_row, sizeof(uint32_t) * 4), "Unexpected line %u.\n", y); } IDirect3DSurface9_UnlockRect(surf); - todo_wine_if(sizeof(void *) == 4) ok(!mismatch_count, "Unexpected number of mismatched lines %u.\n", mismatch_count); - /* * Trying to use D3DX_FILTER_SRGB_IN with a color key results in * STATUS_ACCESS_VIOLATION on native. -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/7711
On Mon Apr 7 14:40:37 2025 +0000, Matteo Bruni wrote:
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. It wasn't hard to add support for this so I just went ahead and matched native here in the current revision, alongside adding tests to show the difference in behavior.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_100155
On Thu Apr 3 22:40:04 2025 +0000, Matteo Bruni wrote:
Technically you're counting bytes on a 4 byte-per-pixel format, so the message isn't entirely correct :nerd: Erm ackshully I'm checking lines now in the current revision, I'm not sure if that is better or not though so I'll let you mark this as resolved just in case :)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_100156
On Thu Apr 3 22:40:04 2025 +0000, Matteo Bruni wrote:
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. I _think_ I've added tests that do what you're describing here now in tests 7 and 8, but I'll let you mark as resolved just to be sure.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_100161
On Mon Apr 7 14:45:00 2025 +0000, Matteo Bruni wrote:
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. I hadn't considered that initially, and you were correct that if there were multiple palette entries that had the same color converting to the source color space wouldn't work. I've added tests for this now, and changed the way things are done so that this works.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_100163
On Mon Apr 7 14:46:33 2025 +0000, Matteo Bruni wrote:
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) This was on top of commit 3/6 This felt a little bit overkill, I've tried a different method in patch 5 that solves the same problem. Let me know what you think :)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_100164
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/surface.c:
goto exit; }
+ /* + * Convert color key to the source format and then back again if + * necessary. This ensures the color key represents a value that + * is obtainable when converting from the source format to the color + * key format. + */ + if (color_key && !is_index_format(src_desc) && (!format_types_match(src_desc, ck_format) + || format_channel_count(src_desc) < 4))
I'm not sure this is safe in general, even leaving the palette case out[*]. Are all the color space conversions bijective? I seem to remember that signed <-> unsigned conversion isn't, to name one. At the moment I'd be more convinced if we did a per-component comparison in `D3DX_PIXEL_FORMAT_B8G8R8A8_UNORM` space, ignoring channels not in the source format. No need to go all the way to splitting `rgb_range` like in my diff (I did that mostly just to see how it would look :grinning:). It should be possible to test this further, actually. What happens if we're loading a `D3DX_PIXEL_FORMAT_B4G4R4A4_UNORM` image and we give a (`D3DX_PIXEL_FORMAT_B8G8R8A8_UNORM`) color key value not exactly representable in the source format? Does the nearest value get color-keyed? Do multiple closest? None of them? [*]: In fact, having to special case palette is probably another hint against this kind of solution. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_100805
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/surface.c:
const uint8_t *dst_expected_row = ((const uint8_t *)tests[i].expected_dst_data) + (sizeof(uint32_t) * 4 * y); const uint8_t *dst_row = ((const uint8_t *)lock_rect.pBits) + (lock_rect.Pitch * y);
- if (memcmp(dst_row, dst_expected_row, sizeof(uint32_t) * 4)) - mismatch_count++; + ok(!memcmp(dst_row, dst_expected_row, sizeof(uint32_t) * 4), "Unexpected line %u.\n", y);
Either way it's fine but I don't see a reason for introducing one way two patches earlier and changing it here. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_100804
I'm sorry this color-key sidetrack is becoming larger and larger... I can probably pick it up if you rather want to get back to your series right away. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_100807
On Tue Apr 15 16:30:17 2025 +0000, Matteo Bruni wrote:
I'm sorry this color-key sidetrack is becoming larger and larger... I can probably pick it up if you rather want to get back to your series right away. It's no big deal, I'd rather fix it properly now than try to work around rebasing on top of any changes we make here :) I'll see what I can do. I wasn't opposed to doing it per-component, I just thought I had found a viable middle-ground that was less work and hadn't considered the case of something like `D3DX_PIXEL_FORMAT_B4G4R4A4_UNORM` that you described above.
Thank you for taking the time to review this thoroughly! -- https://gitlab.winehq.org/wine/wine/-/merge_requests/7711#note_100941
participants (3)
-
Connor McAdams -
Connor McAdams (＠cmcadams)
-
Matteo Bruni (＠Mystral)