[PATCH v3 0/5] MR5202: d3dx9: Refactor D3DXLoadSurfaceFromMemory and fix a few compressed surface bugs.
This is the first set of patches in an effort to create more generic image loading/conversion code that can eventually be shared. -- v3: d3dx9: Preserve the contents of unaligned compressed destination surfaces. d3dx9: Split off image decompression into a helper function. d3dx9: Split D3DXLoadSurfaceFromMemory functionality into a separate function. d3dx9: Use base image pointer when decompressing source image. d3dx9/tests: Add more tests for misaligned compressed surface loading. https://gitlab.winehq.org/wine/wine/-/merge_requests/5202
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/d3dx9_36/tests/surface.c | 143 ++++++++++++++++++++++++++++++++++ 1 file changed, 143 insertions(+) diff --git a/dlls/d3dx9_36/tests/surface.c b/dlls/d3dx9_36/tests/surface.c index 8b48d13d45b..ccbf25925d3 100644 --- a/dlls/d3dx9_36/tests/surface.c +++ b/dlls/d3dx9_36/tests/surface.c @@ -279,6 +279,87 @@ static const BYTE dds_dxt5_8_8[] = 0xff,0xff,0x00,0x00,0x00,0x00,0x00,0x00,0xff,0xff,0x00,0x00,0x05,0x05,0x50,0x50, }; +/* + * 8x8 dxt5 image data, four 4x4 blocks: + * +-----+-----+ + * |Blue |Green| + * | | | + * +-----+-----+ + * |Red |Black| + * | | | + * +-----+-----+ + */ +static const BYTE dxt5_8_8[] = +{ + 0xff,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x1f,0x00,0x1f,0x00,0x00,0x00,0x00,0x00, + 0xff,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0xe0,0x07,0xe0,0x07,0x00,0x00,0x00,0x00, + 0xff,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0xf8,0x00,0xf8,0x00,0x00,0x00,0x00, + 0xff,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, +}; + +struct surface_readback +{ + IDirect3DSurface9 *surface; + D3DLOCKED_RECT locked_rect; +}; + +static DWORD get_readback_color(struct surface_readback *rb, unsigned int x, unsigned int y) +{ + return rb->locked_rect.pBits + ? ((DWORD *)rb->locked_rect.pBits)[y * rb->locked_rect.Pitch / sizeof(DWORD) + x] : 0xdeadbeef; +} + +static void release_surface_readback(struct surface_readback *rb) +{ + HRESULT hr; + + if (!rb->surface) + return; + if (rb->locked_rect.pBits && FAILED(hr = IDirect3DSurface9_UnlockRect(rb->surface))) + trace("Can't unlock the offscreen surface, hr %#lx.\n", hr); + IDirect3DSurface9_Release(rb->surface); +} + +static void get_surface_decompressed_readback(IDirect3DDevice9 *device, IDirect3DSurface9 *compressed_surface, + struct surface_readback *rb) +{ + D3DSURFACE_DESC desc; + HRESULT hr; + + memset(rb, 0, sizeof(*rb)); + hr = IDirect3DSurface9_GetDesc(compressed_surface, &desc); + if (FAILED(hr)) + { + trace("Failed to get compressed surface description, hr %#lx.\n", hr); + return; + } + + hr = IDirect3DDevice9_CreateOffscreenPlainSurface(device, desc.Width, desc.Height, D3DFMT_A8R8G8B8, D3DPOOL_SYSTEMMEM, + &rb->surface, NULL); + if (FAILED(hr)) + { + trace("Can't create the decompressed surface, hr %#lx.\n", hr); + return; + } + + hr = D3DXLoadSurfaceFromSurface(rb->surface, NULL, NULL, compressed_surface, NULL, NULL, D3DX_FILTER_NONE, 0); + if (FAILED(hr)) + { + trace("Can't load the decompressed surface surface, hr %#lx.\n", hr); + IDirect3DSurface9_Release(rb->surface); + rb->surface = NULL; + return; + } + + hr = IDirect3DSurface9_LockRect(rb->surface, &rb->locked_rect, NULL, D3DLOCK_READONLY); + if (FAILED(hr)) + { + trace("Can't lock the offscreen surface, hr %#lx.\n", hr); + IDirect3DSurface9_Release(rb->surface); + rb->surface = NULL; + } +} + static HRESULT create_file(const char *filename, const unsigned char *data, const unsigned int size) { DWORD received; @@ -832,6 +913,14 @@ static inline void _check_pixel_4bpp(unsigned int line, const D3DLOCKED_RECT *lo ok_(__FILE__, line)(color == expected_color, "Got color 0x%08lx, expected 0x%08lx\n", color, expected_color); } +#define check_readback_pixel_4bpp(rb, x, y, color, todo) _check_readback_pixel_4bpp(__LINE__, rb, x, y, color, todo) +static inline void _check_readback_pixel_4bpp(unsigned int line, struct surface_readback *rb, int x, int y, DWORD expected_color, + BOOL todo) +{ + DWORD color = get_readback_color(rb, x, y); + todo_wine_if(todo) ok_(__FILE__, line)(color == expected_color, "Got color 0x%08lx, expected 0x%08lx\n", color, expected_color); +} + static void test_D3DXLoadSurface(IDirect3DDevice9 *device) { HRESULT hr; @@ -1450,6 +1539,8 @@ static void test_D3DXLoadSurface(IDirect3DDevice9 *device) skip("Failed to create DXT5 texture, hr %#lx.\n", hr); else { + struct surface_readback surface_rb; + hr = IDirect3DTexture9_GetSurfaceLevel(tex, 0, &newsurf); ok(SUCCEEDED(hr), "Failed to get the surface, hr %#lx.\n", hr); hr = D3DXLoadSurfaceFromSurface(newsurf, NULL, NULL, surf, NULL, NULL, D3DX_FILTER_NONE, 0); @@ -1521,6 +1612,58 @@ static void test_D3DXLoadSurface(IDirect3DDevice9 *device) check_release((IUnknown *)newsurf, 1); check_release((IUnknown *)tex, 0); + + + /* Misalignment tests but check the resulting image. */ + hr = IDirect3DDevice9_CreateTexture(device, 8, 8, 1, 0, D3DFMT_DXT5, D3DPOOL_SYSTEMMEM, &tex, NULL); + ok(hr == D3D_OK, "Got unexpected hr %#lx.\n", hr); + hr = IDirect3DTexture9_GetSurfaceLevel(tex, 0, &newsurf); + ok(hr == D3D_OK, "Got unexpected hr %#lx.\n", hr); + + SetRect(&rect, 0, 0, 8, 8); + hr = D3DXLoadSurfaceFromMemory(newsurf, NULL, NULL, dxt5_8_8, + D3DFMT_DXT5, 16 * 2, NULL, &rect, D3DX_FILTER_NONE, 0); + ok(hr == D3D_OK, "Got unexpected hr %#lx.\n", hr); + + get_surface_decompressed_readback(device, newsurf, &surface_rb); + + check_readback_pixel_4bpp(&surface_rb, 0, 0, 0xff0000ff, FALSE); /* Blue block, top left. */ + check_readback_pixel_4bpp(&surface_rb, 3, 3, 0xff0000ff, FALSE); /* Blue block, bottom right. */ + check_readback_pixel_4bpp(&surface_rb, 7, 0, 0xff00ff00, FALSE); /* Green block, top right. */ + check_readback_pixel_4bpp(&surface_rb, 4, 3, 0xff00ff00, FALSE); /* Green block, bottom left. */ + check_readback_pixel_4bpp(&surface_rb, 3, 4, 0xffff0000, FALSE); /* Red block, top right. */ + check_readback_pixel_4bpp(&surface_rb, 0, 7, 0xffff0000, FALSE); /* Red block, bottom left. */ + check_readback_pixel_4bpp(&surface_rb, 4, 4, 0xff000000, FALSE); /* Black block, top left. */ + check_readback_pixel_4bpp(&surface_rb, 7, 7, 0xff000000, FALSE); /* Black block, bottom right. */ + + release_surface_readback(&surface_rb); + + /* + * Load our surface into a destination rectangle that overlaps + * multiple blocks. Original data in the blocks should be + * preserved. + */ + SetRect(&rect, 4, 4, 8, 8); + SetRect(&destrect, 2, 2, 6, 6); + hr = D3DXLoadSurfaceFromMemory(newsurf, NULL, &destrect, dxt5_8_8, + D3DFMT_DXT5, 16 * 2, NULL, &rect, D3DX_FILTER_NONE, 0); + ok(hr == D3D_OK, "Got unexpected hr %#lx.\n", hr); + + get_surface_decompressed_readback(device, newsurf, &surface_rb); + + check_readback_pixel_4bpp(&surface_rb, 0, 0, 0xff0000ff, TRUE); /* Blue block, top left. */ + check_readback_pixel_4bpp(&surface_rb, 3, 3, 0xff000000, TRUE); /* Blue block, bottom right. */ + check_readback_pixel_4bpp(&surface_rb, 7, 0, 0xff00ff00, TRUE); /* Green block, top right. */ + check_readback_pixel_4bpp(&surface_rb, 4, 3, 0xff000000, TRUE); /* Green block, bottom left. */ + check_readback_pixel_4bpp(&surface_rb, 3, 4, 0xff000000, TRUE); /* Red block, top right. */ + check_readback_pixel_4bpp(&surface_rb, 0, 7, 0xffff0000, TRUE); /* Red block, bottom left. */ + check_readback_pixel_4bpp(&surface_rb, 4, 4, 0xff000000, TRUE); /* Black block, top left. */ + check_readback_pixel_4bpp(&surface_rb, 7, 7, 0xff000000, TRUE); /* Black block, bottom right. */ + + release_surface_readback(&surface_rb); + + check_release((IUnknown *)newsurf, 1); + check_release((IUnknown *)tex, 0); } hr = IDirect3DDevice9_CreateTexture(device, 4, 4, 1, 0, D3DFMT_DXT1, D3DPOOL_SYSTEMMEM, &tex, NULL); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5202
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/d3dx9_36/surface.c | 11 ++++++----- dlls/d3dx9_36/tests/surface.c | 8 ++++---- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c index 25a762e7e8a..b8dd67d106f 100644 --- a/dlls/d3dx9_36/surface.c +++ b/dlls/d3dx9_36/surface.c @@ -1914,6 +1914,7 @@ HRESULT WINAPI D3DXLoadSurfaceFromMemory(IDirect3DSurface9 *dst_surface, { const struct pixel_format_desc *srcformatdesc, *destformatdesc; struct volume src_size, dst_size, dst_size_aligned; + const BYTE *src_memory_offset = src_memory; RECT dst_rect_temp, dst_rect_aligned; IDirect3DSurface9 *surface; D3DSURFACE_DESC surfdesc; @@ -2000,7 +2001,7 @@ HRESULT WINAPI D3DXLoadSurfaceFromMemory(IDirect3DSurface9 *dst_surface, if (FAILED(hr = lock_surface(dst_surface, &dst_rect_aligned, &lockrect, &surface, TRUE))) return hr; - src_memory = (BYTE *)src_memory + src_rect->top / srcformatdesc->block_height * src_pitch + src_memory_offset += src_rect->top / srcformatdesc->block_height * src_pitch + src_rect->left / srcformatdesc->block_width * srcformatdesc->block_byte_count; if (src_format == surfdesc.Format @@ -2013,7 +2014,7 @@ HRESULT WINAPI D3DXLoadSurfaceFromMemory(IDirect3DSurface9 *dst_surface, && !(dst_rect->top & (destformatdesc->block_height - 1))) { TRACE("Simple copy.\n"); - copy_pixels(src_memory, src_pitch, 0, lockrect.pBits, lockrect.Pitch, 0, + copy_pixels(src_memory_offset, src_pitch, 0, lockrect.pBits, lockrect.Pitch, 0, &src_size, srcformatdesc); } else /* Stretching or format conversion. */ @@ -2075,7 +2076,7 @@ HRESULT WINAPI D3DXLoadSurfaceFromMemory(IDirect3DSurface9 *dst_surface, ++ptr; } } - src_memory = src_uncompressed; + src_memory_offset = (BYTE *)src_uncompressed; src_pitch = src_size.width * sizeof(DWORD); srcformatdesc = get_format_info(D3DFMT_A8B8G8R8); } @@ -2110,7 +2111,7 @@ HRESULT WINAPI D3DXLoadSurfaceFromMemory(IDirect3DSurface9 *dst_surface, if ((filter & 0xf) == D3DX_FILTER_NONE) { - convert_argb_pixels(src_memory, src_pitch, 0, &src_size, srcformatdesc, + convert_argb_pixels(src_memory_offset, src_pitch, 0, &src_size, srcformatdesc, dst_mem, dst_pitch, 0, &dst_size, dst_format, color_key, src_palette); } else /* if ((filter & 0xf) == D3DX_FILTER_POINT) */ @@ -2120,7 +2121,7 @@ HRESULT WINAPI D3DXLoadSurfaceFromMemory(IDirect3DSurface9 *dst_surface, /* Always apply a point filter until D3DX_FILTER_LINEAR, * D3DX_FILTER_TRIANGLE and D3DX_FILTER_BOX are implemented. */ - point_filter_argb_pixels(src_memory, src_pitch, 0, &src_size, srcformatdesc, + point_filter_argb_pixels(src_memory_offset, src_pitch, 0, &src_size, srcformatdesc, dst_mem, dst_pitch, 0, &dst_size, dst_format, color_key, src_palette); } diff --git a/dlls/d3dx9_36/tests/surface.c b/dlls/d3dx9_36/tests/surface.c index ccbf25925d3..f5139c4690b 100644 --- a/dlls/d3dx9_36/tests/surface.c +++ b/dlls/d3dx9_36/tests/surface.c @@ -1652,12 +1652,12 @@ static void test_D3DXLoadSurface(IDirect3DDevice9 *device) get_surface_decompressed_readback(device, newsurf, &surface_rb); check_readback_pixel_4bpp(&surface_rb, 0, 0, 0xff0000ff, TRUE); /* Blue block, top left. */ - check_readback_pixel_4bpp(&surface_rb, 3, 3, 0xff000000, TRUE); /* Blue block, bottom right. */ + check_readback_pixel_4bpp(&surface_rb, 3, 3, 0xff000000, FALSE); /* Blue block, bottom right. */ check_readback_pixel_4bpp(&surface_rb, 7, 0, 0xff00ff00, TRUE); /* Green block, top right. */ - check_readback_pixel_4bpp(&surface_rb, 4, 3, 0xff000000, TRUE); /* Green block, bottom left. */ - check_readback_pixel_4bpp(&surface_rb, 3, 4, 0xff000000, TRUE); /* Red block, top right. */ + check_readback_pixel_4bpp(&surface_rb, 4, 3, 0xff000000, FALSE); /* Green block, bottom left. */ + check_readback_pixel_4bpp(&surface_rb, 3, 4, 0xff000000, FALSE); /* Red block, top right. */ check_readback_pixel_4bpp(&surface_rb, 0, 7, 0xffff0000, TRUE); /* Red block, bottom left. */ - check_readback_pixel_4bpp(&surface_rb, 4, 4, 0xff000000, TRUE); /* Black block, top left. */ + check_readback_pixel_4bpp(&surface_rb, 4, 4, 0xff000000, FALSE); /* Black block, top left. */ check_readback_pixel_4bpp(&surface_rb, 7, 7, 0xff000000, TRUE); /* Black block, bottom right. */ release_surface_readback(&surface_rb); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5202
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/d3dx9_36/surface.c | 353 +++++++++++++++++++++------------------- 1 file changed, 187 insertions(+), 166 deletions(-) diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c index b8dd67d106f..38fa5705da2 100644 --- a/dlls/d3dx9_36/surface.c +++ b/dlls/d3dx9_36/surface.c @@ -1875,6 +1875,189 @@ void point_filter_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT src_slic } } +static void set_volume_struct(struct volume *volume, uint32_t width, uint32_t height, uint32_t depth) +{ + volume->width = width; + volume->height = height; + volume->depth = depth; +} + +static HRESULT d3dx_load_image_from_memory(void *dst_memory, uint32_t dst_row_pitch, const struct pixel_format_desc *dst_desc, + const PALETTEENTRY *dst_palette, const RECT *dst_rect, const RECT *dst_rect_aligned, const void *src_memory, + uint32_t src_row_pitch, const struct pixel_format_desc *src_desc, const PALETTEENTRY *src_palette, const RECT *src_rect, + uint32_t filter_flags, uint32_t color_key) +{ + struct volume src_size, dst_size, dst_size_aligned; + const BYTE *src_memory_offset = src_memory; + HRESULT hr = S_OK; + + TRACE("dst_memory %p, dst_row_pitch %d, dst_desc %p, dst_palette %p, dst_rect %s, dst_rect_aligned %s, src_memory %p, " + "src_row_pitch %d, src_desc %p, src_palette %p, src_rect %s, filter %#x, color_key 0x%08x.\n", + dst_memory, dst_row_pitch, dst_desc, dst_palette, wine_dbgstr_rect(dst_rect), wine_dbgstr_rect(dst_rect_aligned), + src_memory, src_row_pitch, src_desc, src_palette, wine_dbgstr_rect(src_rect), filter_flags, color_key); + + set_volume_struct(&src_size, (src_rect->right - src_rect->left), (src_rect->bottom - src_rect->top), 1); + set_volume_struct(&dst_size, (dst_rect->right - dst_rect->left), (dst_rect->bottom - dst_rect->top), 1); + set_volume_struct(&dst_size_aligned, (dst_rect_aligned->right - dst_rect_aligned->left), + (dst_rect_aligned->bottom - dst_rect_aligned->top), 1); + + src_memory_offset += (src_rect->top / src_desc->block_height) * src_row_pitch; + src_memory_offset += (src_rect->left / src_desc->block_width) * src_desc->block_byte_count; + + /* Everything matches, simply copy the pixels. */ + if (src_desc->format == dst_desc->format + && dst_size.width == src_size.width + && dst_size.height == src_size.height + && color_key == 0 + && !(src_rect->left & (src_desc->block_width - 1)) + && !(src_rect->top & (src_desc->block_height - 1)) + && !(dst_rect->left & (dst_desc->block_width - 1)) + && !(dst_rect->top & (dst_desc->block_height - 1))) + { + TRACE("Simple copy.\n"); + copy_pixels(src_memory_offset, src_row_pitch, 0, dst_memory, dst_row_pitch, 0, &src_size, src_desc); + return S_OK; + } + + /* Stretching or format conversion. */ + if (!is_conversion_from_supported(src_desc) + || !is_conversion_to_supported(dst_desc)) + { + FIXME("Unsupported format conversion %#x -> %#x.\n", src_desc->format, dst_desc->format); + return E_NOTIMPL; + } + + /* + * If the source is a compressed image, we need to decompress it first + * before doing any modifications. + */ + if (src_desc->type == FORMAT_DXT) + { + void (*fetch_dxt_texel)(int srcRowStride, const BYTE *pixdata, int i, int j, void *texel); + const struct pixel_format_desc *src_uncompressed_desc; + uint32_t x, y, src_uncompressed_row_pitch, tmp_pitch; + DWORD *src_uncompressed = NULL; + RECT src_uncompressed_rect; + + tmp_pitch = src_row_pitch * src_desc->block_width / src_desc->block_byte_count; + + src_uncompressed = malloc(src_size.width * src_size.height * sizeof(DWORD)); + if (!src_uncompressed) + return E_OUTOFMEMORY; + + switch (src_desc->format) + { + case D3DFMT_DXT1: + fetch_dxt_texel = fetch_2d_texel_rgba_dxt1; + break; + case D3DFMT_DXT2: + case D3DFMT_DXT3: + fetch_dxt_texel = fetch_2d_texel_rgba_dxt3; + break; + case D3DFMT_DXT4: + case D3DFMT_DXT5: + fetch_dxt_texel = fetch_2d_texel_rgba_dxt5; + break; + default: + FIXME("Unexpected compressed texture format %u.\n", src_desc->format); + fetch_dxt_texel = NULL; + } + + TRACE("Uncompressing DXTn surface.\n"); + for (y = 0; y < src_size.height; ++y) + { + DWORD *ptr = &src_uncompressed[y * src_size.width]; + for (x = 0; x < src_size.width; ++x) + { + fetch_dxt_texel(tmp_pitch, src_memory, x + src_rect->left, y + src_rect->top, ptr); + ++ptr; + } + } + + src_uncompressed_row_pitch = src_size.width * sizeof(DWORD); + src_uncompressed_desc = get_format_info(D3DFMT_A8B8G8R8); + SetRect(&src_uncompressed_rect, 0, 0, src_size.width, src_size.height); + hr = d3dx_load_image_from_memory(dst_memory, dst_row_pitch, dst_desc, dst_palette, dst_rect, dst_rect_aligned, + src_uncompressed, src_uncompressed_row_pitch, src_uncompressed_desc, src_palette, &src_uncompressed_rect, + filter_flags, color_key); + free(src_uncompressed); + return hr; + } + + /* Same as the above, need to decompress the destination prior to modifying. */ + if (dst_desc->type == FORMAT_DXT) + { + size_t dst_uncompressed_size = dst_size_aligned.width * dst_size_aligned.height * sizeof(DWORD); + BOOL dst_misaligned = dst_rect->left != dst_rect_aligned->left + || dst_rect->top != dst_rect_aligned->top + || dst_rect->right != dst_rect_aligned->right + || dst_rect->bottom != dst_rect_aligned->bottom; + const struct pixel_format_desc *dst_uncompressed_desc; + BYTE *dst_uncompressed, *dst_uncompressed_aligned; + uint32_t dst_uncompressed_row_pitch; + RECT dst_uncompressed_rect; + + dst_uncompressed_aligned = malloc(dst_uncompressed_size); + if (!dst_uncompressed_aligned) + return E_OUTOFMEMORY; + + if (dst_misaligned) memset(dst_uncompressed_aligned, 0, dst_uncompressed_size); + dst_uncompressed_row_pitch = dst_size_aligned.width * sizeof(DWORD); + dst_uncompressed_desc = get_format_info(D3DFMT_A8B8G8R8); + dst_uncompressed = dst_uncompressed_aligned + (dst_rect->top - dst_rect_aligned->top) * dst_uncompressed_row_pitch + + (dst_rect->left - dst_rect_aligned->left) * sizeof(DWORD); + + SetRect(&dst_uncompressed_rect, 0, 0, dst_size.width, dst_size.height); + hr = d3dx_load_image_from_memory(dst_uncompressed, dst_uncompressed_row_pitch, dst_uncompressed_desc, dst_palette, + &dst_uncompressed_rect, &dst_uncompressed_rect, src_memory_offset, src_row_pitch, src_desc, src_palette, + src_rect, filter_flags, color_key); + if (SUCCEEDED(hr)) + { + GLenum gl_format = 0; + + TRACE("Compressing DXTn surface.\n"); + switch (dst_desc->format) + { + case D3DFMT_DXT1: + gl_format = GL_COMPRESSED_RGBA_S3TC_DXT1_EXT; + break; + case D3DFMT_DXT2: + case D3DFMT_DXT3: + gl_format = GL_COMPRESSED_RGBA_S3TC_DXT3_EXT; + break; + case D3DFMT_DXT4: + case D3DFMT_DXT5: + gl_format = GL_COMPRESSED_RGBA_S3TC_DXT5_EXT; + break; + default: + ERR("Unexpected destination compressed format %u.\n", dst_desc->format); + } + tx_compress_dxtn(4, dst_size_aligned.width, dst_size_aligned.height, dst_uncompressed_aligned, gl_format, + dst_memory, dst_row_pitch); + } + free(dst_uncompressed_aligned); + return hr; + } + + if ((filter_flags & 0xf) == D3DX_FILTER_NONE) + { + convert_argb_pixels(src_memory_offset, src_row_pitch, 0, &src_size, src_desc, + dst_memory, dst_row_pitch, 0, &dst_size, dst_desc, color_key, src_palette); + } + else /* if ((filter & 0xf) == D3DX_FILTER_POINT) */ + { + if ((filter_flags & 0xf) != D3DX_FILTER_POINT) + FIXME("Unhandled filter %#x.\n", filter_flags); + + /* Always apply a point filter until D3DX_FILTER_LINEAR, + * D3DX_FILTER_TRIANGLE and D3DX_FILTER_BOX are implemented. */ + point_filter_argb_pixels(src_memory_offset, src_row_pitch, 0, &src_size, src_desc, dst_memory, dst_row_pitch, 0, + &dst_size, dst_desc, color_key, src_palette); + } + + return hr; +} + /************************************************************ * D3DXLoadSurfaceFromMemory * @@ -1913,8 +2096,6 @@ HRESULT WINAPI D3DXLoadSurfaceFromMemory(IDirect3DSurface9 *dst_surface, DWORD filter, D3DCOLOR color_key) { const struct pixel_format_desc *srcformatdesc, *destformatdesc; - struct volume src_size, dst_size, dst_size_aligned; - const BYTE *src_memory_offset = src_memory; RECT dst_rect_temp, dst_rect_aligned; IDirect3DSurface9 *surface; D3DSURFACE_DESC surfdesc; @@ -1946,10 +2127,6 @@ HRESULT WINAPI D3DXLoadSurfaceFromMemory(IDirect3DSurface9 *dst_surface, return E_NOTIMPL; } - src_size.width = src_rect->right - src_rect->left; - src_size.height = src_rect->bottom - src_rect->top; - src_size.depth = 1; - IDirect3DSurface9_GetDesc(dst_surface, &surfdesc); destformatdesc = get_format_info(surfdesc.Format); if (!dst_rect) @@ -1988,172 +2165,16 @@ HRESULT WINAPI D3DXLoadSurfaceFromMemory(IDirect3DSurface9 *dst_surface, dst_rect_aligned.bottom = min((dst_rect_aligned.bottom + destformatdesc->block_height - 1) & ~(destformatdesc->block_height - 1), surfdesc.Height); - dst_size.width = dst_rect->right - dst_rect->left; - dst_size.height = dst_rect->bottom - dst_rect->top; - dst_size.depth = 1; - dst_size_aligned.width = dst_rect_aligned.right - dst_rect_aligned.left; - dst_size_aligned.height = dst_rect_aligned.bottom - dst_rect_aligned.top; - dst_size_aligned.depth = 1; - if (filter == D3DX_DEFAULT) filter = D3DX_FILTER_TRIANGLE | D3DX_FILTER_DITHER; if (FAILED(hr = lock_surface(dst_surface, &dst_rect_aligned, &lockrect, &surface, TRUE))) return hr; - src_memory_offset += src_rect->top / srcformatdesc->block_height * src_pitch - + src_rect->left / srcformatdesc->block_width * srcformatdesc->block_byte_count; - - if (src_format == surfdesc.Format - && dst_size.width == src_size.width - && dst_size.height == src_size.height - && color_key == 0 - && !(src_rect->left & (srcformatdesc->block_width - 1)) - && !(src_rect->top & (srcformatdesc->block_height - 1)) - && !(dst_rect->left & (destformatdesc->block_width - 1)) - && !(dst_rect->top & (destformatdesc->block_height - 1))) - { - TRACE("Simple copy.\n"); - copy_pixels(src_memory_offset, src_pitch, 0, lockrect.pBits, lockrect.Pitch, 0, - &src_size, srcformatdesc); - } - else /* Stretching or format conversion. */ - { - const struct pixel_format_desc *dst_format; - DWORD *src_uncompressed = NULL; - BYTE *dst_uncompressed = NULL; - unsigned int dst_pitch; - BYTE *dst_mem; - - if (!is_conversion_from_supported(srcformatdesc) - || !is_conversion_to_supported(destformatdesc)) - { - FIXME("Unsupported format conversion %#x -> %#x.\n", src_format, surfdesc.Format); - unlock_surface(dst_surface, &dst_rect_aligned, surface, FALSE); - return E_NOTIMPL; - } - - if (srcformatdesc->type == FORMAT_DXT) - { - void (*fetch_dxt_texel)(int srcRowStride, const BYTE *pixdata, - int i, int j, void *texel); - unsigned int x, y; - - src_pitch = src_pitch * srcformatdesc->block_width / srcformatdesc->block_byte_count; - - src_uncompressed = malloc(src_size.width * src_size.height * sizeof(DWORD)); - if (!src_uncompressed) - { - unlock_surface(dst_surface, &dst_rect_aligned, surface, FALSE); - return E_OUTOFMEMORY; - } - - switch(src_format) - { - case D3DFMT_DXT1: - fetch_dxt_texel = fetch_2d_texel_rgba_dxt1; - break; - case D3DFMT_DXT2: - case D3DFMT_DXT3: - fetch_dxt_texel = fetch_2d_texel_rgba_dxt3; - break; - case D3DFMT_DXT4: - case D3DFMT_DXT5: - fetch_dxt_texel = fetch_2d_texel_rgba_dxt5; - break; - default: - FIXME("Unexpected compressed texture format %u.\n", src_format); - fetch_dxt_texel = NULL; - } - - TRACE("Uncompressing DXTn surface.\n"); - for (y = 0; y < src_size.height; ++y) - { - DWORD *ptr = &src_uncompressed[y * src_size.width]; - for (x = 0; x < src_size.width; ++x) - { - fetch_dxt_texel(src_pitch, src_memory, x + src_rect->left, y + src_rect->top, ptr); - ++ptr; - } - } - src_memory_offset = (BYTE *)src_uncompressed; - src_pitch = src_size.width * sizeof(DWORD); - srcformatdesc = get_format_info(D3DFMT_A8B8G8R8); - } - - if (destformatdesc->type == FORMAT_DXT) - { - BOOL dst_misaligned = dst_rect->left != dst_rect_aligned.left - || dst_rect->top != dst_rect_aligned.top - || dst_rect->right != dst_rect_aligned.right - || dst_rect->bottom != dst_rect_aligned.bottom; - size_t dst_uncompressed_size = dst_size_aligned.width * dst_size_aligned.height * sizeof(DWORD); - - dst_uncompressed = malloc(dst_uncompressed_size); - if (!dst_uncompressed) - { - free(src_uncompressed); - unlock_surface(dst_surface, &dst_rect_aligned, surface, FALSE); - return E_OUTOFMEMORY; - } - if (dst_misaligned) memset(dst_uncompressed, 0, dst_uncompressed_size); - dst_pitch = dst_size_aligned.width * sizeof(DWORD); - dst_format = get_format_info(D3DFMT_A8B8G8R8); - dst_mem = dst_uncompressed + (dst_rect->top - dst_rect_aligned.top) * dst_pitch - + (dst_rect->left - dst_rect_aligned.left) * sizeof(DWORD); - } - else - { - dst_mem = lockrect.pBits; - dst_pitch = lockrect.Pitch; - dst_format = destformatdesc; - } - - if ((filter & 0xf) == D3DX_FILTER_NONE) - { - convert_argb_pixels(src_memory_offset, src_pitch, 0, &src_size, srcformatdesc, - dst_mem, dst_pitch, 0, &dst_size, dst_format, color_key, src_palette); - } - else /* if ((filter & 0xf) == D3DX_FILTER_POINT) */ - { - if ((filter & 0xf) != D3DX_FILTER_POINT) - FIXME("Unhandled filter %#lx.\n", filter); - - /* Always apply a point filter until D3DX_FILTER_LINEAR, - * D3DX_FILTER_TRIANGLE and D3DX_FILTER_BOX are implemented. */ - point_filter_argb_pixels(src_memory_offset, src_pitch, 0, &src_size, srcformatdesc, - dst_mem, dst_pitch, 0, &dst_size, dst_format, color_key, src_palette); - } - - free(src_uncompressed); - - if (dst_uncompressed) - { - GLenum gl_format = 0; - - TRACE("Compressing DXTn surface.\n"); - switch(surfdesc.Format) - { - case D3DFMT_DXT1: - gl_format = GL_COMPRESSED_RGBA_S3TC_DXT1_EXT; - break; - case D3DFMT_DXT2: - case D3DFMT_DXT3: - gl_format = GL_COMPRESSED_RGBA_S3TC_DXT3_EXT; - break; - case D3DFMT_DXT4: - case D3DFMT_DXT5: - gl_format = GL_COMPRESSED_RGBA_S3TC_DXT5_EXT; - break; - default: - ERR("Unexpected destination compressed format %u.\n", surfdesc.Format); - } - tx_compress_dxtn(4, dst_size_aligned.width, dst_size_aligned.height, - dst_uncompressed, gl_format, lockrect.pBits, - lockrect.Pitch); - free(dst_uncompressed); - } - } + hr = d3dx_load_image_from_memory(lockrect.pBits, lockrect.Pitch, destformatdesc, dst_palette, dst_rect, + &dst_rect_aligned, src_memory, src_pitch, srcformatdesc, src_palette, src_rect, filter, color_key); + if (FAILED(hr)) + WARN("d3dx_load_image_from_memory failed with hr %#lx\n", hr); return unlock_surface(dst_surface, &dst_rect_aligned, surface, TRUE); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5202
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/d3dx9_36/surface.c | 109 +++++++++++++++++++++++----------------- 1 file changed, 64 insertions(+), 45 deletions(-) diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c index 38fa5705da2..59791a76f05 100644 --- a/dlls/d3dx9_36/surface.c +++ b/dlls/d3dx9_36/surface.c @@ -1875,6 +1875,59 @@ void point_filter_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT src_slic } } +static HRESULT d3dx_image_decompress(const void *memory, uint32_t row_pitch, const RECT *rect, + const struct volume *size, const struct pixel_format_desc *desc, void **out_memory, + uint32_t *out_row_pitch, RECT *out_rect, const struct pixel_format_desc **out_desc) +{ + void (*fetch_dxt_texel)(int srcRowStride, const BYTE *pixdata, int i, int j, void *texel); + const struct pixel_format_desc *uncompressed_desc = NULL; + uint32_t x, y, tmp_pitch; + BYTE *uncompressed_mem; + + switch (desc->format) + { + case D3DFMT_DXT1: + uncompressed_desc = get_format_info(D3DFMT_A8B8G8R8); + fetch_dxt_texel = fetch_2d_texel_rgba_dxt1; + break; + case D3DFMT_DXT2: + case D3DFMT_DXT3: + uncompressed_desc = get_format_info(D3DFMT_A8B8G8R8); + fetch_dxt_texel = fetch_2d_texel_rgba_dxt3; + break; + case D3DFMT_DXT4: + case D3DFMT_DXT5: + uncompressed_desc = get_format_info(D3DFMT_A8B8G8R8); + fetch_dxt_texel = fetch_2d_texel_rgba_dxt5; + break; + default: + FIXME("Unexpected compressed texture format %u.\n", desc->format); + return E_NOTIMPL; + } + + if (!(uncompressed_mem = malloc(size->width * size->height * size->depth * uncompressed_desc->bytes_per_pixel))) + return E_OUTOFMEMORY; + + TRACE("Decompressing image.\n"); + tmp_pitch = row_pitch * desc->block_width / desc->block_byte_count; + for (y = 0; y < size->height; ++y) + { + BYTE *ptr = &uncompressed_mem[y * size->width * uncompressed_desc->bytes_per_pixel]; + for (x = 0; x < size->width; ++x) + { + fetch_dxt_texel(tmp_pitch, (BYTE *)memory, x + rect->left, y + rect->top, ptr); + ptr += uncompressed_desc->bytes_per_pixel; + } + } + + *out_memory = uncompressed_mem; + *out_row_pitch = size->width * uncompressed_desc->bytes_per_pixel; + SetRect(out_rect, 0, 0, size->width, size->height); + *out_desc = uncompressed_desc; + + return S_OK; +} + static void set_volume_struct(struct volume *volume, uint32_t width, uint32_t height, uint32_t depth) { volume->width = width; @@ -1933,54 +1986,20 @@ static HRESULT d3dx_load_image_from_memory(void *dst_memory, uint32_t dst_row_pi */ if (src_desc->type == FORMAT_DXT) { - void (*fetch_dxt_texel)(int srcRowStride, const BYTE *pixdata, int i, int j, void *texel); - const struct pixel_format_desc *src_uncompressed_desc; - uint32_t x, y, src_uncompressed_row_pitch, tmp_pitch; - DWORD *src_uncompressed = NULL; - RECT src_uncompressed_rect; - - tmp_pitch = src_row_pitch * src_desc->block_width / src_desc->block_byte_count; - - src_uncompressed = malloc(src_size.width * src_size.height * sizeof(DWORD)); - if (!src_uncompressed) - return E_OUTOFMEMORY; + const struct pixel_format_desc *uncompressed_desc; + uint32_t uncompressed_row_pitch; + void *uncompressed_mem = NULL; + RECT uncompressed_rect; - switch (src_desc->format) + hr = d3dx_image_decompress(src_memory, src_row_pitch, src_rect, &src_size, src_desc, + &uncompressed_mem, &uncompressed_row_pitch, &uncompressed_rect, &uncompressed_desc); + if (SUCCEEDED(hr)) { - case D3DFMT_DXT1: - fetch_dxt_texel = fetch_2d_texel_rgba_dxt1; - break; - case D3DFMT_DXT2: - case D3DFMT_DXT3: - fetch_dxt_texel = fetch_2d_texel_rgba_dxt3; - break; - case D3DFMT_DXT4: - case D3DFMT_DXT5: - fetch_dxt_texel = fetch_2d_texel_rgba_dxt5; - break; - default: - FIXME("Unexpected compressed texture format %u.\n", src_desc->format); - fetch_dxt_texel = NULL; + hr = d3dx_load_image_from_memory(dst_memory, dst_row_pitch, dst_desc, dst_palette, dst_rect, dst_rect_aligned, + uncompressed_mem, uncompressed_row_pitch, uncompressed_desc, src_palette, &uncompressed_rect, + filter_flags, color_key); } - - TRACE("Uncompressing DXTn surface.\n"); - for (y = 0; y < src_size.height; ++y) - { - DWORD *ptr = &src_uncompressed[y * src_size.width]; - for (x = 0; x < src_size.width; ++x) - { - fetch_dxt_texel(tmp_pitch, src_memory, x + src_rect->left, y + src_rect->top, ptr); - ++ptr; - } - } - - src_uncompressed_row_pitch = src_size.width * sizeof(DWORD); - src_uncompressed_desc = get_format_info(D3DFMT_A8B8G8R8); - SetRect(&src_uncompressed_rect, 0, 0, src_size.width, src_size.height); - hr = d3dx_load_image_from_memory(dst_memory, dst_row_pitch, dst_desc, dst_palette, dst_rect, dst_rect_aligned, - src_uncompressed, src_uncompressed_row_pitch, src_uncompressed_desc, src_palette, &src_uncompressed_rect, - filter_flags, color_key); - free(src_uncompressed); + free(uncompressed_mem); return hr; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5202
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/d3dx9_36/surface.c | 62 ++++++++++++++++++----------------- dlls/d3dx9_36/tests/surface.c | 8 ++--- 2 files changed, 36 insertions(+), 34 deletions(-) diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c index 59791a76f05..bbeba9c8b23 100644 --- a/dlls/d3dx9_36/surface.c +++ b/dlls/d3dx9_36/surface.c @@ -1876,8 +1876,8 @@ void point_filter_argb_pixels(const BYTE *src, UINT src_row_pitch, UINT src_slic } static HRESULT d3dx_image_decompress(const void *memory, uint32_t row_pitch, const RECT *rect, - const struct volume *size, const struct pixel_format_desc *desc, void **out_memory, - uint32_t *out_row_pitch, RECT *out_rect, const struct pixel_format_desc **out_desc) + const RECT *unaligned_rect, const struct volume *size, const struct pixel_format_desc *desc, + void **out_memory, uint32_t *out_row_pitch, RECT *out_rect, const struct pixel_format_desc **out_desc) { void (*fetch_dxt_texel)(int srcRowStride, const BYTE *pixdata, int i, int j, void *texel); const struct pixel_format_desc *uncompressed_desc = NULL; @@ -1908,6 +1908,9 @@ static HRESULT d3dx_image_decompress(const void *memory, uint32_t row_pitch, con if (!(uncompressed_mem = malloc(size->width * size->height * size->depth * uncompressed_desc->bytes_per_pixel))) return E_OUTOFMEMORY; + if (unaligned_rect && EqualRect(rect, unaligned_rect)) + goto exit; + TRACE("Decompressing image.\n"); tmp_pitch = row_pitch * desc->block_width / desc->block_byte_count; for (y = 0; y < size->height; ++y) @@ -1915,14 +1918,21 @@ static HRESULT d3dx_image_decompress(const void *memory, uint32_t row_pitch, con BYTE *ptr = &uncompressed_mem[y * size->width * uncompressed_desc->bytes_per_pixel]; for (x = 0; x < size->width; ++x) { - fetch_dxt_texel(tmp_pitch, (BYTE *)memory, x + rect->left, y + rect->top, ptr); + const POINT pt = { x, y }; + + if (!PtInRect(unaligned_rect, pt)) + fetch_dxt_texel(tmp_pitch, (BYTE *)memory, x + rect->left, y + rect->top, ptr); ptr += uncompressed_desc->bytes_per_pixel; } } +exit: *out_memory = uncompressed_mem; *out_row_pitch = size->width * uncompressed_desc->bytes_per_pixel; - SetRect(out_rect, 0, 0, size->width, size->height); + if (unaligned_rect) + *out_rect = *unaligned_rect; + else + SetRect(out_rect, 0, 0, size->width, size->height); *out_desc = uncompressed_desc; return S_OK; @@ -1991,7 +2001,7 @@ static HRESULT d3dx_load_image_from_memory(void *dst_memory, uint32_t dst_row_pi void *uncompressed_mem = NULL; RECT uncompressed_rect; - hr = d3dx_image_decompress(src_memory, src_row_pitch, src_rect, &src_size, src_desc, + hr = d3dx_image_decompress(src_memory, src_row_pitch, src_rect, NULL, &src_size, src_desc, &uncompressed_mem, &uncompressed_row_pitch, &uncompressed_rect, &uncompressed_desc); if (SUCCEEDED(hr)) { @@ -2006,29 +2016,21 @@ static HRESULT d3dx_load_image_from_memory(void *dst_memory, uint32_t dst_row_pi /* Same as the above, need to decompress the destination prior to modifying. */ if (dst_desc->type == FORMAT_DXT) { - size_t dst_uncompressed_size = dst_size_aligned.width * dst_size_aligned.height * sizeof(DWORD); - BOOL dst_misaligned = dst_rect->left != dst_rect_aligned->left - || dst_rect->top != dst_rect_aligned->top - || dst_rect->right != dst_rect_aligned->right - || dst_rect->bottom != dst_rect_aligned->bottom; - const struct pixel_format_desc *dst_uncompressed_desc; - BYTE *dst_uncompressed, *dst_uncompressed_aligned; - uint32_t dst_uncompressed_row_pitch; - RECT dst_uncompressed_rect; - - dst_uncompressed_aligned = malloc(dst_uncompressed_size); - if (!dst_uncompressed_aligned) - return E_OUTOFMEMORY; - - if (dst_misaligned) memset(dst_uncompressed_aligned, 0, dst_uncompressed_size); - dst_uncompressed_row_pitch = dst_size_aligned.width * sizeof(DWORD); - dst_uncompressed_desc = get_format_info(D3DFMT_A8B8G8R8); - dst_uncompressed = dst_uncompressed_aligned + (dst_rect->top - dst_rect_aligned->top) * dst_uncompressed_row_pitch - + (dst_rect->left - dst_rect_aligned->left) * sizeof(DWORD); - - SetRect(&dst_uncompressed_rect, 0, 0, dst_size.width, dst_size.height); - hr = d3dx_load_image_from_memory(dst_uncompressed, dst_uncompressed_row_pitch, dst_uncompressed_desc, dst_palette, - &dst_uncompressed_rect, &dst_uncompressed_rect, src_memory_offset, src_row_pitch, src_desc, src_palette, + const struct pixel_format_desc *uncompressed_desc; + uint32_t uncompressed_row_pitch; + void *uncompressed_mem = NULL; + BYTE *uncompressed_mem_offset; + RECT uncompressed_rect; + + hr = d3dx_image_decompress(dst_memory, dst_row_pitch, dst_rect_aligned, dst_rect, &dst_size_aligned, dst_desc, + &uncompressed_mem, &uncompressed_row_pitch, &uncompressed_rect, &uncompressed_desc); + if (FAILED(hr)) + return hr; + + uncompressed_mem_offset = (BYTE *)uncompressed_mem + (dst_rect->top - dst_rect_aligned->top) * uncompressed_row_pitch + + (dst_rect->left - dst_rect_aligned->left) * uncompressed_desc->bytes_per_pixel; + hr = d3dx_load_image_from_memory(uncompressed_mem_offset, uncompressed_row_pitch, uncompressed_desc, dst_palette, + &uncompressed_rect, &uncompressed_rect, src_memory, src_row_pitch, src_desc, src_palette, src_rect, filter_flags, color_key); if (SUCCEEDED(hr)) { @@ -2051,10 +2053,10 @@ static HRESULT d3dx_load_image_from_memory(void *dst_memory, uint32_t dst_row_pi default: ERR("Unexpected destination compressed format %u.\n", dst_desc->format); } - tx_compress_dxtn(4, dst_size_aligned.width, dst_size_aligned.height, dst_uncompressed_aligned, gl_format, + tx_compress_dxtn(4, dst_size_aligned.width, dst_size_aligned.height, uncompressed_mem, gl_format, dst_memory, dst_row_pitch); } - free(dst_uncompressed_aligned); + free(uncompressed_mem); return hr; } diff --git a/dlls/d3dx9_36/tests/surface.c b/dlls/d3dx9_36/tests/surface.c index f5139c4690b..3bac0b93601 100644 --- a/dlls/d3dx9_36/tests/surface.c +++ b/dlls/d3dx9_36/tests/surface.c @@ -1651,14 +1651,14 @@ static void test_D3DXLoadSurface(IDirect3DDevice9 *device) get_surface_decompressed_readback(device, newsurf, &surface_rb); - check_readback_pixel_4bpp(&surface_rb, 0, 0, 0xff0000ff, TRUE); /* Blue block, top left. */ + check_readback_pixel_4bpp(&surface_rb, 0, 0, 0xff0000ff, FALSE); /* Blue block, top left. */ check_readback_pixel_4bpp(&surface_rb, 3, 3, 0xff000000, FALSE); /* Blue block, bottom right. */ - check_readback_pixel_4bpp(&surface_rb, 7, 0, 0xff00ff00, TRUE); /* Green block, top right. */ + check_readback_pixel_4bpp(&surface_rb, 7, 0, 0xff00ff00, FALSE); /* Green block, top right. */ check_readback_pixel_4bpp(&surface_rb, 4, 3, 0xff000000, FALSE); /* Green block, bottom left. */ check_readback_pixel_4bpp(&surface_rb, 3, 4, 0xff000000, FALSE); /* Red block, top right. */ - check_readback_pixel_4bpp(&surface_rb, 0, 7, 0xffff0000, TRUE); /* Red block, bottom left. */ + check_readback_pixel_4bpp(&surface_rb, 0, 7, 0xffff0000, FALSE); /* Red block, bottom left. */ check_readback_pixel_4bpp(&surface_rb, 4, 4, 0xff000000, FALSE); /* Black block, top left. */ - check_readback_pixel_4bpp(&surface_rb, 7, 7, 0xff000000, TRUE); /* Black block, bottom right. */ + check_readback_pixel_4bpp(&surface_rb, 7, 7, 0xff000000, FALSE); /* Black block, bottom right. */ release_surface_readback(&surface_rb); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5202
On Tue Mar 5 14:07:21 2024 +0000, Matteo Bruni wrote:
I have been confused about this image data for a bit. The first row is alright, but there is no way that the others are what they claim to be: ``` [0x1f, 0xf8] -> this sets the first and last color components to "1.0", the middle one to 0. [0xff, 0x07] -> this sets the first two (LSB to MSB) components to "1.0", last to 0. Notice also how the last row has the same color data as the first. ``` Problem is, the tests pass and trying to "fix" the colors obviously broke the test. Turns out, this is a combination of "comment misdirection" and slight test confusion. The uncompressed texture uses D3DFMT_A8R8G8B8, which is a BGRA format - channels are sort of "reversed". So the last (MSB -> LSB) component in the expected value isn't alpha but blue. Which means the second block is purple and the third one is cyan. The last one is transparent blue. So, I'd either fix the comments to match the actual colors or (probably slightly preferable) fix the image and expected data to match the comment. Thanks for taking the time to investigate this, hopefully I didn't waste too much of your time. The code comments should now match the test data. :) Switching between DXGI and D3DFORMATs had me confused WRT endianness.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5202#note_63717
On Wed Mar 6 16:55:18 2024 +0000, Connor McAdams wrote:
changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/5202/diffs?diff_id=103436&start_sha=5ff64e106b0c4a03fc846c418a2df1ff850efddd#d49071753fdf2ab21d518dbd8c720b48c7c732fc_868_920) The inefficiency of this bothered me as well, but I figured since it was a test it didn't matter so much. I've changed it in the current revision to hopefully be closer to what you were imagining.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5202#note_63718
On Wed Mar 6 16:55:20 2024 +0000, Connor McAdams wrote:
changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/5202/diffs?diff_id=103436&start_sha=5ff64e106b0c4a03fc846c418a2df1ff850efddd#8fffeae09861a99974e55842964630933974cdc8_1886_1886) I've gone through and changed all internal `UINT` usages into `uint32_t`, hopefully that works.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5202#note_63719
On Wed Mar 6 16:55:21 2024 +0000, Connor McAdams wrote:
changed this line in [version 3 of the diff](/wine/wine/-/merge_requests/5202/diffs?diff_id=103436&start_sha=5ff64e106b0c4a03fc846c418a2df1ff850efddd#8fffeae09861a99974e55842964630933974cdc8_1880_1878) This is the only comment I'm iffy about, technically this is the `unaligned` rectangle, which is what I had named the argument initially. I felt like `exclude` made more sense WRT functionality, but I've switched back to `unaligned_rect` in the current revision.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5202#note_63720
On Wed Mar 6 20:30:02 2024 +0000, Connor McAdams wrote:
This is the only comment I'm iffy about, technically this is the `unaligned` rectangle, which is what I had named the argument initially. I felt like `exclude` made more sense WRT functionality, but I've switched back to `unaligned_rect` in the current revision. Ah yes, I had it backwards. I think in this case I prefer this anyways; `exclude` looked a bit confusing to me, e.g. in
if (exclude_rect)
*out_rect = *exclude_rect;
I agree that, in general, naming arguments by what they do rather than what they are is nicer; this one is kind of the exception. In any case, not a big deal. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5202#note_63746
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/surface.c:
+ 0xff,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x1f,0x00,0x1f,0x00,0x00,0x00,0x00,0x00, + 0xff,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0xe0,0x07,0xe0,0x07,0x00,0x00,0x00,0x00, + 0xff,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0xf8,0x00,0xf8,0x00,0x00,0x00,0x00, + 0xff,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00, +}; + +struct surface_readback +{ + IDirect3DSurface9 *surface; + D3DLOCKED_RECT locked_rect; +}; + +static DWORD get_readback_color(struct surface_readback *rb, unsigned int x, unsigned int y) +{ + return rb->locked_rect.pBits + ? ((DWORD *)rb->locked_rect.pBits)[y * rb->locked_rect.Pitch / sizeof(DWORD) + x] : 0xdeadbeef; Nitpicking, these could be `uint32_t` as well.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5202#note_63747
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/surface.c:
+ trace("Failed to get compressed surface description, hr %#lx.\n", hr); + return; + } + + hr = IDirect3DDevice9_CreateOffscreenPlainSurface(device, desc.Width, desc.Height, D3DFMT_A8R8G8B8, D3DPOOL_SYSTEMMEM, + &rb->surface, NULL); + if (FAILED(hr)) + { + trace("Can't create the decompressed surface, hr %#lx.\n", hr); + return; + } + + hr = D3DXLoadSurfaceFromSurface(rb->surface, NULL, NULL, compressed_surface, NULL, NULL, D3DX_FILTER_NONE, 0); + if (FAILED(hr)) + { + trace("Can't load the decompressed surface surface, hr %#lx.\n", hr); Doubled "surface".
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5202#note_63748
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/surface.c:
ok_(__FILE__, line)(color == expected_color, "Got color 0x%08lx, expected 0x%08lx\n", color, expected_color); }
+#define check_readback_pixel_4bpp(rb, x, y, color, todo) _check_readback_pixel_4bpp(__LINE__, rb, x, y, color, todo) +static inline void _check_readback_pixel_4bpp(unsigned int line, struct surface_readback *rb, int x, int y, DWORD expected_color,
`unsigned int` for `x` and `y`, also to match with `get_readback_color()`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5202#note_63749
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/surface.c:
ok_(__FILE__, line)(color == expected_color, "Got color 0x%08lx, expected 0x%08lx\n", color, expected_color); }
+#define check_readback_pixel_4bpp(rb, x, y, color, todo) _check_readback_pixel_4bpp(__LINE__, rb, x, y, color, todo) +static inline void _check_readback_pixel_4bpp(unsigned int line, struct surface_readback *rb, int x, int y, DWORD expected_color, + BOOL todo) +{ + DWORD color = get_readback_color(rb, x, y); + todo_wine_if(todo) ok_(__FILE__, line)(color == expected_color, "Got color 0x%08lx, expected 0x%08lx\n", color, expected_color);
Period at the end of the `ok()` message. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5202#note_63750
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/surface.c:
check_release((IUnknown *)newsurf, 1); check_release((IUnknown *)tex, 0); + +
One blank line is probably enough. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5202#note_63751
I only have a few nitpicky comments. I'm not approving right away to leave the chance of fixing those, but I can be convinced otherwise :smile: -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5202#note_63752
participants (3)
-
Connor McAdams -
Connor McAdams (＠cmcadams)
-
Matteo Bruni (＠Mystral)