[PATCH 09/10] d3dx9: Use intermediate surfaces for loading from unmappable source surfaces in D3DXLoadSurfaceFromSurface().
Signed-off-by: Henri Verbeet hverbeet@codeweavers.com --- dlls/d3dx9_36/surface.c | 36 ++++++++++++++++++++++++++---------- dlls/d3dx9_36/tests/surface.c | 4 ---- 2 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c index 7d48cc6b132..cf9a16d9b78 100644 --- a/dlls/d3dx9_36/surface.c +++ b/dlls/d3dx9_36/surface.c @@ -1924,10 +1924,12 @@ HRESULT WINAPI D3DXLoadSurfaceFromSurface(IDirect3DSurface9 *dst_surface, const PALETTEENTRY *dst_palette, const RECT *dst_rect, IDirect3DSurface9 *src_surface, const PALETTEENTRY *src_palette, const RECT *src_rect, DWORD filter, D3DCOLOR color_key) { - RECT rect; + IDirect3DSurface9 *surface = src_surface; + IDirect3DDevice9 *device; + D3DSURFACE_DESC src_desc; D3DLOCKED_RECT lock; - D3DSURFACE_DESC SrcDesc; HRESULT hr; + RECT s;
TRACE("dst_surface %p, dst_palette %p, dst_rect %s, src_surface %p, " "src_palette %p, src_rect %s, filter %#x, color_key 0x%08x.\n", @@ -1937,23 +1939,37 @@ HRESULT WINAPI D3DXLoadSurfaceFromSurface(IDirect3DSurface9 *dst_surface, if (!dst_surface || !src_surface) return D3DERR_INVALIDCALL;
- IDirect3DSurface9_GetDesc(src_surface, &SrcDesc); + IDirect3DSurface9_GetDesc(src_surface, &src_desc); + if (src_desc.Pool == D3DPOOL_DEFAULT && !(src_desc.Usage & D3DUSAGE_DYNAMIC)) + { + IDirect3DSurface9_GetDevice(src_surface, &device); + if (FAILED(IDirect3DDevice9_CreateRenderTarget(device, src_desc.Width, src_desc.Height, + src_desc.Format, D3DMULTISAMPLE_NONE, 0, TRUE, &surface, NULL)) + || FAILED(IDirect3DDevice9_StretchRect(device, src_surface, NULL, surface, NULL, D3DTEXF_NONE))) + surface = src_surface; + IDirect3DDevice9_Release(device); + }
if (!src_rect) - SetRect(&rect, 0, 0, SrcDesc.Width, SrcDesc.Height); - else - rect = *src_rect; + { + SetRect(&s, 0, 0, src_desc.Width, src_desc.Height); + src_rect = &s; + }
- if (FAILED(IDirect3DSurface9_LockRect(src_surface, &lock, NULL, D3DLOCK_READONLY))) + if (FAILED(IDirect3DSurface9_LockRect(surface, &lock, NULL, D3DLOCK_READONLY))) { FIXME("Called for unlockable source surface.\n"); + if (surface != src_surface) + IDirect3DSurface9_Release(surface); return D3DXERR_INVALIDDATA; }
- hr = D3DXLoadSurfaceFromMemory(dst_surface, dst_palette, dst_rect, - lock.pBits, SrcDesc.Format, lock.Pitch, src_palette, &rect, filter, color_key); + hr = D3DXLoadSurfaceFromMemory(dst_surface, dst_palette, dst_rect, lock.pBits, + src_desc.Format, lock.Pitch, src_palette, src_rect, filter, color_key);
- IDirect3DSurface9_UnlockRect(src_surface); + IDirect3DSurface9_UnlockRect(surface); + if (surface != src_surface) + IDirect3DSurface9_Release(surface);
return hr; } diff --git a/dlls/d3dx9_36/tests/surface.c b/dlls/d3dx9_36/tests/surface.c index f153cd92c65..d3450722ef7 100644 --- a/dlls/d3dx9_36/tests/surface.c +++ b/dlls/d3dx9_36/tests/surface.c @@ -869,16 +869,12 @@ static void test_D3DXLoadSurface(IDirect3DDevice9 *device) hr = D3DXLoadSurfaceFromSurface(newsurf, NULL, NULL, surf, NULL, NULL, D3DX_FILTER_NONE, 0); ok(hr == D3D_OK, "D3DXLoadSurfaceFromSurface returned %#x, expected %#x.\n", hr, D3D_OK); hr = D3DXLoadSurfaceFromSurface(surf, NULL, NULL, newsurf, NULL, NULL, D3DX_FILTER_NONE, 0); - todo_wine ok(hr == D3D_OK, "D3DXLoadSurfaceFromSurface returned %#x, expected %#x.\n", hr, D3D_OK); hr = D3DXLoadSurfaceFromSurface(surf, NULL, NULL, newsurf, NULL, NULL, D3DX_FILTER_NONE, 0xff000000); - todo_wine ok(hr == D3D_OK, "D3DXLoadSurfaceFromSurface returned %#x, expected %#x.\n", hr, D3D_OK); hr = D3DXLoadSurfaceFromSurface(surf, NULL, NULL, newsurf, NULL, NULL, D3DX_FILTER_TRIANGLE | D3DX_FILTER_MIRROR, 0); - todo_wine ok(hr == D3D_OK, "D3DXLoadSurfaceFromSurface returned %#x, expected %#x.\n", hr, D3D_OK); hr = D3DXLoadSurfaceFromSurface(surf, NULL, NULL, newsurf, NULL, NULL, D3DX_FILTER_LINEAR, 0); - todo_wine ok(hr == D3D_OK, "D3DXLoadSurfaceFromSurface returned %#x, expected %#x.\n", hr, D3D_OK);
IDirect3DSurface9_Release(newsurf);
Hi,
While running your changed tests on Windows, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check?
Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=44552
Your paranoid android.
=== debian9 (build log) ===
X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig)
=== debian9 (build log) ===
X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig) X Error of failed request: BadValue (integer parameter out of range for operation) Major opcode of failed request: 140 (RANDR) Minor opcode of failed request: 21 (RRSetCrtcConfig)
On Sun, Nov 18, 2018 at 7:40 PM Henri Verbeet hverbeet@codeweavers.com wrote:
@@ -1937,23 +1939,37 @@ HRESULT WINAPI D3DXLoadSurfaceFromSurface(IDirect3DSurface9 *dst_surface, if (!dst_surface || !src_surface) return D3DERR_INVALIDCALL;
- IDirect3DSurface9_GetDesc(src_surface, &SrcDesc);
- IDirect3DSurface9_GetDesc(src_surface, &src_desc);
- if (src_desc.Pool == D3DPOOL_DEFAULT && !(src_desc.Usage & D3DUSAGE_DYNAMIC))
- {
IDirect3DSurface9_GetDevice(src_surface, &device);if (FAILED(IDirect3DDevice9_CreateRenderTarget(device, src_desc.Width, src_desc.Height,src_desc.Format, D3DMULTISAMPLE_NONE, 0, TRUE, &surface, NULL))|| FAILED(IDirect3DDevice9_StretchRect(device, src_surface, NULL, surface, NULL, D3DTEXF_NONE)))surface = src_surface;IDirect3DDevice9_Release(device);- }
Doesn't this leak the staging RT if StretchRect() fails? I'm also unsure whether StretchRect() covers all the cases of our interest. After staring at the StretchRect() tests + docs for a while, my current understanding is that it should always work for us, we're probably doing an extra copy for off-screen plain surfaces but that's no big deal.
I guess GetRenderTargetData() might be a bit better / faster for the cases where it can be used, although it might complicate the function somewhat and it should probably be in a separate patch anyway.
On Mon, 19 Nov 2018 at 21:52, Matteo Bruni matteo.mystral@gmail.com wrote:
On Sun, Nov 18, 2018 at 7:40 PM Henri Verbeet hverbeet@codeweavers.com wrote:
@@ -1937,23 +1939,37 @@ HRESULT WINAPI D3DXLoadSurfaceFromSurface(IDirect3DSurface9 *dst_surface, if (!dst_surface || !src_surface) return D3DERR_INVALIDCALL;
- IDirect3DSurface9_GetDesc(src_surface, &SrcDesc);
- IDirect3DSurface9_GetDesc(src_surface, &src_desc);
- if (src_desc.Pool == D3DPOOL_DEFAULT && !(src_desc.Usage & D3DUSAGE_DYNAMIC))
- {
IDirect3DSurface9_GetDevice(src_surface, &device);if (FAILED(IDirect3DDevice9_CreateRenderTarget(device, src_desc.Width, src_desc.Height,src_desc.Format, D3DMULTISAMPLE_NONE, 0, TRUE, &surface, NULL))|| FAILED(IDirect3DDevice9_StretchRect(device, src_surface, NULL, surface, NULL, D3DTEXF_NONE)))surface = src_surface;IDirect3DDevice9_Release(device);- }
Doesn't this leak the staging RT if StretchRect() fails?
Right, it would. I'll fix that.
I'm also unsure whether StretchRect() covers all the cases of our interest. After staring at the StretchRect() tests + docs for a while, my current understanding is that it should always work for us, we're probably doing an extra copy for off-screen plain surfaces but that's no big deal.
Well, there's one case we really care about, and that's regular non-dynamic default pool textures. StretchRect() is the only way to get the data out of those, GetRenderTargetData() isn't an option. It's true that for e.g. render targets and offscreen plain surfaces there are more efficient options, but I'd consider those optimisations. Note though that after the next patch in this series, this is essentially a fall-back path for when we can't StretchRect() directly between the source and destination.
On Mon, Nov 19, 2018 at 11:04 PM Henri Verbeet hverbeet@gmail.com wrote:
On Mon, 19 Nov 2018 at 21:52, Matteo Bruni matteo.mystral@gmail.com wrote:
I'm also unsure whether StretchRect() covers all the cases of our interest. After staring at the StretchRect() tests + docs for a while, my current understanding is that it should always work for us, we're probably doing an extra copy for off-screen plain surfaces but that's no big deal.
Well, there's one case we really care about, and that's regular non-dynamic default pool textures. StretchRect() is the only way to get the data out of those, GetRenderTargetData() isn't an option. It's true that for e.g. render targets and offscreen plain surfaces there are more efficient options, but I'd consider those optimisations. Note though that after the next patch in this series, this is essentially a fall-back path for when we can't StretchRect() directly between the source and destination.
Right, I guess I'm not as optimistic about applications using suitable filter settings. But sure, certainly stuff for further, possible optimizations.
On Wed, 21 Nov 2018 at 01:38, Matteo Bruni matteo.mystral@gmail.com wrote:
Right, I guess I'm not as optimistic about applications using suitable filter settings. But sure, certainly stuff for further, possible optimizations.
I wasn't trying to convey optimism. :) Mostly just that once you're hitting this path, you've already lost in one way or another.
-
Henri Verbeet -
Henri Verbeet -
Marvin -
Matteo Bruni