[PATCH 0/1] MR2725: d3dx9: Fix dst pitch for compressed format in D3DXLoadSurfaceFromMemory().

[Paul Gofman (＠gofman)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54901

The pitch is treated as a byte offset between the rows of blocks in tx_compress_dxtn(), so is the Pitch from _LockRect() so it doesn't need a fixup.

That pitch has any effect only when dst texture stride is larger than width being compressed, that's why it was mostly not causing problems before.

[PATCH 1/1] d3dx9: Fix dst pitch for compressed format in D3DXLoadSurfaceFromMemory().

[Paul Gofman]

From: Paul Gofman

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54901
---
 dlls/d3dx9_36/surface.c       |  2 +-
 dlls/d3dx9_36/tests/surface.c | 24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c
index 26809a47e18..ccb5c2ffadb 100644
--- a/dlls/d3dx9_36/surface.c
+++ b/dlls/d3dx9_36/surface.c
@@ -2148,7 +2148,7 @@ HRESULT WINAPI D3DXLoadSurfaceFromMemory(IDirect3DSurface9 *dst_surface,
             }
             tx_compress_dxtn(4, dst_size_aligned.width, dst_size_aligned.height,
                     dst_uncompressed, gl_format, lockrect.pBits,
-                    lockrect.Pitch * destformatdesc->block_width / destformatdesc->block_byte_count);
+                    lockrect.Pitch);
             heap_free(dst_uncompressed);
         }
     }
diff --git a/dlls/d3dx9_36/tests/surface.c b/dlls/d3dx9_36/tests/surface.c
index c3ac4ba74b3..726fb111a66 100644
--- a/dlls/d3dx9_36/tests/surface.c
+++ b/dlls/d3dx9_36/tests/surface.c
@@ -847,6 +847,7 @@ static void test_D3DXLoadSurface(IDirect3DDevice9 *device)
     static const DWORD pixdata_g16r16[] = { 0x07d23fbe, 0xdc7f44a4, 0xe4d8976b, 0x9a84fe89 };
     static const DWORD pixdata_a8b8g8r8[] = { 0xc3394cf0, 0x235ae892, 0x09b197fd, 0x8dc32bf6 };
     static const DWORD pixdata_a2r10g10b10[] = { 0x57395aff, 0x5b7668fd, 0xb0d856b5, 0xff2c61d6 };
+    BYTE buffer[4 * 8 * 4];

hr = create_file("testdummy.bmp", noimage, sizeof(noimage));  /* invalid image */
     testdummy_ok = SUCCEEDED(hr);
@@ -1463,6 +1464,29 @@ static void test_D3DXLoadSurface(IDirect3DDevice9 *device)
             check_release((IUnknown *)newsurf, 1);
             check_release((IUnknown *)tex, 0);

+            /* Test updating subarea of compressed texture. */
+            hr = IDirect3DDevice9_CreateTexture(device, 32, 16, 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(&destrect, 0, 0, 4, 8);
+            SetRect(&rect, 0, 0, 4, 8);
+            memset(buffer, 0x40, 4 * 8 * 4);
+            hr = D3DXLoadSurfaceFromMemory(newsurf, NULL, &destrect, buffer,
+                    D3DFMT_A8B8G8R8, 4 * 4, NULL, &rect, D3DX_FILTER_NONE, 0);
+            ok(hr == D3D_OK, "Got unexpected hr %#lx.\n", hr);
+
+            hr = IDirect3DSurface9_LockRect(newsurf, &lockrect, &destrect, D3DLOCK_READONLY);
+            ok(hr == D3D_OK, "Got unexpected hr %#lx.\n", hr);
+            /* 2 identical 16 bytes DXT5 blocks. The exact values in blocks may differ from Windows due to
+             * different compression algorithms. */
+            ok(!memcmp(lockrect.pBits, (char *)lockrect.pBits + lockrect.Pitch, 16), "data mismatch.\n");
+            hr = IDirect3DSurface9_UnlockRect(newsurf);
+            ok(hr == D3D_OK, "Got unexpected hr %#lx.\n", hr);
+
+            check_release((IUnknown *)newsurf, 1);
+            check_release((IUnknown *)tex, 0);
+
             /* Test a rect larger than but not an integer multiple of the block size. */
             hr = IDirect3DDevice9_CreateTexture(device, 4, 8, 1, 0, D3DFMT_DXT5, D3DPOOL_SYSTEMMEM, &tex, NULL);
             ok(hr == D3D_OK, "Got unexpected hr %#lx.\n", hr);

[Matteo Bruni (＠Mystral)]

Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/surface.c:
>          check_release((IUnknown *)newsurf, 1);
>          check_release((IUnknown *)tex, 0);
>
> -
>        /* Test updating subarea of compressed texture. */
> -
>        hr = IDirect3DDevice9_CreateTexture(device, 32, 16, 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(&destrect, 0, 0, 4, 8);
> -
>        SetRect(&rect, 0, 0, 4, 8);
> -
>        memset(buffer, 0x40, 4 * 8 * 4);

I know this is just a test but it seems nicer to put a sizeof(buffer) as the third argument here.

[PATCH 0/1] MR2725: d3dx9: Fix dst pitch for compressed format in D3DXLoadSurfaceFromMemory(). - approved

[Matteo Bruni (＠Mystral)]

This merge request was approved by Matteo Bruni.

[Matteo Bruni (＠Mystral)]

This happens to be a partial revert of 9069913dbba0914b80544ce4afb9e7285efad2a5. It looks correct, my guess is that I was confused by the fetch functions requiring the (source) pitch in pixels while the compression function wants the (destination) pitch in bytes. It might be a good idea to check that this MR doesn't regress https://bugs.winehq.org/show_bug.cgi?id=47862 - good chance that the game from that bug doesn't use compression at all.

[Paul Gofman (＠gofman)]

I tried to look at Art of Murded: Cards of Destiny (Steam version) and could not reproduce the main character's corruption even with 9069913d completely reverted. So it is not clear what exactly is going on with this game, but since the change here looks correct I hope it should be fine. FWIW Medieval II Total War loads DXT1 texture into a big DXT5 with offsets (offsets and sizes are aligned), decompression worked fine while compression had this issue.