Re: [PATCH 2/2] wined3d: Fix calculation of pitch for ATI2N format

[Stefan Dösinger]

On the big picture I am not sure if the test conclusively shows what is going on. If your test's offset of 2160 bytes is correct something odd is going on.  Please add a test explicitly testing the pitch returned in D3DLOCKED_RECT.Pitch.
> +    hr = IDirect3DDevice9_CreateOffscreenPlainSurface(device, 128, 128, MAKEFOURCC('A','T','I','2'), D3DPOOL_SCRATCH, &surface, 0);
> +    ok(SUCCEEDED(hr), "CreateOffscreenPlainSurface failed (%08x)\n", hr);
> I am not sure if ATI2N surfaces are available everywhere. Please call CheckDeviceFormat to see if it is supported before trying to create a surface, and if it is not available call skip() and skip the test.

+    todo_wine ok(offset == 2160, "Got offset %u, expected offset 2160 for format ATI2N\n", offset);
I may be a bit dizzy from playing games for 2 days, but I don't understand how you get to an offset of 2160 with the normal surface calculation. According to our table in utils.c ATI2N has 1 byte per pixel, so the offset would be * 1(bpp) * (128(width) * 60(yoffset) + 60(xoffset)) = 7740 bytes.

Following the formula given in the GL_EXT_texture_compression_latc spec I'd get
16[blocksize in bytes] * (128[width]/4 * 60[y] / 4 + 60[x] / 4) = 7920
> From patch 2:
> @@ -330,7 +330,7 @@ DWORD WINAPI IWineD3DBaseSurfaceImpl_GetPitch(IWineD3DSurface *iface) {
>     DWORD ret;
>     TRACE("(%p)\n", This);
> -    if (format_desc->Flags & WINED3DFMT_FLAG_COMPRESSED)
> +    if ((format_desc->Flags & WINED3DFMT_FLAG_COMPRESSED) && (format_desc->format != WINED3DFMT_ATI2N))
> IwineD3DBaseSurfaceImpl_LockRect assumes that the pitch gives the number of bytes to the next line of blocks rather than the next line in the surface, so LockRect will need some changes as well.

Hardcoding ATI2N is ugly, a new flag for this behavior would be better(or something entirely different depending on what more detailed tests on windows reveal)

[PATCH 2/2] wined3d: Fix calculation of pitch for ATI2N format

[Henri Verbeet]

2010/2/21 Stefan Dösinger <stefandoesinger(a)gmx.at>:
> I may be a bit dizzy from playing games for 2 days, but I don't understand how you get to an offset of 2160 with the normal surface calculation. According to our table in utils.c ATI2N has 1 byte per pixel, so the offset would be * 1(bpp) * (128(width) * 60(yoffset) + 60(xoffset)) = 7740 bytes.
>
> Following the formula given in the GL_EXT_texture_compression_latc spec I'd get
> 16[blocksize in bytes] * (128[width]/4 * 60[y] / 4 + 60[x] / 4) = 7920

You get 2160 when you use correct block sizes, but a broken pitch. I.e.
60[yoffest]/4[bh] * 128[broken_pitch] + 60[xoffset]/4[bw] * 16[block_size]

I don't see how that will result in working locks though, so I'm
curious if this test actually passes on native Windows.

[PATCH 2/2] wined3d: Fix calculation of pitch for ATI2N format

[Stefan Dösinger]

Am 21.02.2010 um 22:53 schrieb Henri Verbeet:
> You get 2160 when you use correct block sizes, but a broken pitch. I.e.
> 60[yoffest]/4[bh] * 128[broken_pitch] + 60[xoffset]/4[bw] * 16[block_size]
>
> I don't see how that will result in working locks though, so I'm
> curious if this test actually passes on native Windows.
> I ran a quick test, and Windows gives offset=7920, following the normal DXTN formula. That was Windows 7, Nvidia Geforce 9600, driver from somewhen in late January 2010.

I'll send the attached patch to wine-devel after making sure the CheckDeviceFormat part works on Windows. I coded it on OSX and think that there was some magic about D3DRTYPE_SURFACE.