[PATCH 0/4] MR6157: d3dx9: More miscellaneous fixes.
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/d3dx9_36/tests/surface.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dlls/d3dx9_36/tests/surface.c b/dlls/d3dx9_36/tests/surface.c index 1d60f9c46aa..5290ab1d38c 100644 --- a/dlls/d3dx9_36/tests/surface.c +++ b/dlls/d3dx9_36/tests/surface.c @@ -936,6 +936,11 @@ static void test_D3DXLoadSurface(IDirect3DDevice9 *device) hr = D3DXLoadSurfaceFromSurface(surf, NULL, NULL, newsurf, NULL, NULL, D3DX_FILTER_NONE, 0); ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr); + /* Can also load from memory into a multisampled render target. */ + SetRect(&rect, 0, 0, 2, 2); + hr = D3DXLoadSurfaceFromMemory(newsurf, NULL, NULL, pixdata, D3DFMT_A8R8G8B8, sizeof(pixdata), NULL, &rect, D3DX_FILTER_NONE, 0); + todo_wine ok(hr == D3D_OK, "D3DXLoadSurfaceFromMemory returned %#lx, expected %#lx\n", hr, D3D_OK); + IDirect3DSurface9_Release(newsurf); } else -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6157
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/d3dx9_36/surface.c | 34 +++++++++++++++++++++++----------- dlls/d3dx9_36/tests/surface.c | 2 +- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c index 344a491765e..8b4f906dcdd 100644 --- a/dlls/d3dx9_36/surface.c +++ b/dlls/d3dx9_36/surface.c @@ -263,8 +263,8 @@ HRESULT lock_surface(IDirect3DSurface9 *surface, const RECT *surface_rect, D3DLO HRESULT unlock_surface(IDirect3DSurface9 *surface, const RECT *surface_rect, IDirect3DSurface9 *temp_surface, BOOL update) { + const POINT surface_point = { (surface_rect ? surface_rect->left : 0), (surface_rect ? surface_rect->top : 0) }; IDirect3DDevice9 *device; - POINT surface_point; HRESULT hr; if (!temp_surface) @@ -274,22 +274,34 @@ HRESULT unlock_surface(IDirect3DSurface9 *surface, const RECT *surface_rect, } hr = IDirect3DSurface9_UnlockRect(temp_surface); - if (update) + if (SUCCEEDED(hr) && update) { - if (surface_rect) + D3DSURFACE_DESC desc; + + IDirect3DSurface9_GetDesc(surface, &desc); + IDirect3DSurface9_GetDevice(surface, &device); + if (desc.MultiSampleType != D3DMULTISAMPLE_NONE) { - surface_point.x = surface_rect->left; - surface_point.y = surface_rect->top; + IDirect3DSurface9 *temp_surface2; + D3DSURFACE_DESC temp_desc; + + IDirect3DSurface9_GetDesc(temp_surface, &temp_desc); + hr = IDirect3DDevice9_CreateRenderTarget(device, temp_desc.Width, temp_desc.Height, temp_desc.Format, + D3DMULTISAMPLE_NONE, 0, FALSE, &temp_surface2, NULL); + if (SUCCEEDED(hr)) + { + hr = IDirect3DDevice9_UpdateSurface(device, temp_surface, NULL, temp_surface2, NULL); + if (SUCCEEDED(hr)) + hr = IDirect3DDevice9_StretchRect(device, temp_surface2, NULL, surface, surface_rect, D3DTEXF_NONE); + IDirect3DSurface9_Release(temp_surface2); + } } else { - surface_point.x = 0; - surface_point.y = 0; + hr = IDirect3DDevice9_UpdateSurface(device, temp_surface, NULL, surface, &surface_point); } - IDirect3DSurface9_GetDevice(surface, &device); - if (FAILED(hr = IDirect3DDevice9_UpdateSurface(device, temp_surface, NULL, surface, &surface_point))) - WARN("Updating surface failed, hr %#lx, surface %p, temp_surface %p.\n", - hr, surface, temp_surface); + if (FAILED(hr)) + WARN("Updating surface failed, hr %#lx, surface %p, temp_surface %p.\n", hr, surface, temp_surface); IDirect3DDevice9_Release(device); } IDirect3DSurface9_Release(temp_surface); diff --git a/dlls/d3dx9_36/tests/surface.c b/dlls/d3dx9_36/tests/surface.c index 5290ab1d38c..772cc6cea10 100644 --- a/dlls/d3dx9_36/tests/surface.c +++ b/dlls/d3dx9_36/tests/surface.c @@ -939,7 +939,7 @@ static void test_D3DXLoadSurface(IDirect3DDevice9 *device) /* Can also load from memory into a multisampled render target. */ SetRect(&rect, 0, 0, 2, 2); hr = D3DXLoadSurfaceFromMemory(newsurf, NULL, NULL, pixdata, D3DFMT_A8R8G8B8, sizeof(pixdata), NULL, &rect, D3DX_FILTER_NONE, 0); - todo_wine ok(hr == D3D_OK, "D3DXLoadSurfaceFromMemory returned %#lx, expected %#lx\n", hr, D3D_OK); + ok(hr == D3D_OK, "D3DXLoadSurfaceFromMemory returned %#lx, expected %#lx\n", hr, D3D_OK); IDirect3DSurface9_Release(newsurf); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6157
From: Connor McAdams <cmcadams(a)codeweavers.com> Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/d3dx9_36/surface.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/dlls/d3dx9_36/surface.c b/dlls/d3dx9_36/surface.c index 8b4f906dcdd..6a78aab8396 100644 --- a/dlls/d3dx9_36/surface.c +++ b/dlls/d3dx9_36/surface.c @@ -2224,13 +2224,16 @@ HRESULT WINAPI D3DXLoadSurfaceFromMemory(IDirect3DSurface9 *dst_surface, if (FAILED(hr = lock_surface(dst_surface, &dst_rect_aligned, &lockrect, &surface, TRUE))) return hr; - set_d3dx_pixels(&dst_pixels, lockrect.pBits, lockrect.Pitch, 0, dst_palette, (dst_rect_aligned.right - dst_rect_aligned.left), (dst_rect_aligned.bottom - dst_rect_aligned.top), 1, dst_rect); OffsetRect(&dst_pixels.unaligned_rect, -dst_rect_aligned.left, -dst_rect_aligned.top); - d3dx_load_pixels_from_pixels(&dst_pixels, destformatdesc, &src_pixels, srcformatdesc, filter, color_key); + if (FAILED(hr = d3dx_load_pixels_from_pixels(&dst_pixels, destformatdesc, &src_pixels, srcformatdesc, filter, color_key))) + { + unlock_surface(dst_surface, &dst_rect_aligned, surface, TRUE); + return hr; + } return unlock_surface(dst_surface, &dst_rect_aligned, surface, TRUE); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6157
From: Connor McAdams <cmcadams(a)codeweavers.com> This used to be handled by the calls to D3DXLoad{Surface,Volume}FromMemory, but now it needs to be handled inside of the texture from file functions themselves. Signed-off-by: Connor McAdams <cmcadams(a)codeweavers.com> --- dlls/d3dx9_36/texture.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dlls/d3dx9_36/texture.c b/dlls/d3dx9_36/texture.c index a65a955cebf..3c0264cc6ce 100644 --- a/dlls/d3dx9_36/texture.c +++ b/dlls/d3dx9_36/texture.c @@ -603,6 +603,8 @@ HRESULT WINAPI D3DXCreateTextureFromFileInMemoryEx(struct IDirect3DDevice9 *devi width = (width == D3DX_DEFAULT) ? make_pow2(imginfo.Width) : imginfo.Width; if (!height || height == D3DX_DEFAULT_NONPOW2 || height == D3DX_FROM_FILE || height == D3DX_DEFAULT) height = (height == D3DX_DEFAULT) ? make_pow2(imginfo.Height) : imginfo.Height; + if (filter == D3DX_DEFAULT) + filter = D3DX_FILTER_TRIANGLE | D3DX_FILTER_DITHER; format_specified = (format != D3DFMT_UNKNOWN && format != D3DX_DEFAULT); if (format == D3DFMT_FROM_FILE || format == D3DFMT_UNKNOWN || format == D3DX_DEFAULT) @@ -1133,6 +1135,8 @@ HRESULT WINAPI D3DXCreateVolumeTextureFromFileInMemoryEx(IDirect3DDevice9 *devic height = (height == D3DX_DEFAULT) ? make_pow2(image_info.Height) : image_info.Height; if (!depth || depth == D3DX_DEFAULT_NONPOW2 || depth == D3DX_FROM_FILE || depth == D3DX_DEFAULT) depth = (depth == D3DX_DEFAULT) ? make_pow2(image_info.Depth) : image_info.Depth; + if (filter == D3DX_DEFAULT) + filter = D3DX_FILTER_TRIANGLE | D3DX_FILTER_DITHER; format_specified = (format != D3DFMT_UNKNOWN && format != D3DX_DEFAULT); if (format == D3DFMT_FROM_FILE || format == D3DFMT_UNKNOWN || format == D3DX_DEFAULT) @@ -1418,6 +1422,8 @@ HRESULT WINAPI D3DXCreateCubeTextureFromFileInMemoryEx(IDirect3DDevice9 *device, size = max(img_info.Width, img_info.Height); else if (size == D3DX_DEFAULT) size = make_pow2(max(img_info.Width, img_info.Height)); + if (filter == D3DX_DEFAULT) + filter = D3DX_FILTER_TRIANGLE | D3DX_FILTER_DITHER; format_specified = (format != D3DFMT_UNKNOWN && format != D3DX_DEFAULT); if (format == D3DFMT_FROM_FILE || format == D3DFMT_UNKNOWN || format == D3DX_DEFAULT) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6157
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/tests/surface.c:
hr = D3DXLoadSurfaceFromSurface(surf, NULL, NULL, newsurf, NULL, NULL, D3DX_FILTER_NONE, 0); ok(hr == D3D_OK, "Unexpected hr %#lx.\n", hr);
+ /* Can also load from memory into a multisampled render target. */ + SetRect(&rect, 0, 0, 2, 2); + hr = D3DXLoadSurfaceFromMemory(newsurf, NULL, NULL, pixdata, D3DFMT_A8R8G8B8, sizeof(pixdata), NULL, &rect, D3DX_FILTER_NONE, 0); + todo_wine ok(hr == D3D_OK, "D3DXLoadSurfaceFromMemory returned %#lx, expected %#lx\n", hr, D3D_OK);
Nitpick, just `..., "Unexpected hr %#lx.\n", hr);` It would be interesting to also check the loaded surface contents. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6157#note_77231
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/surface.c:
{ - if (surface_rect) + D3DSURFACE_DESC desc; + + IDirect3DSurface9_GetDesc(surface, &desc); + IDirect3DSurface9_GetDevice(surface, &device); + if (desc.MultiSampleType != D3DMULTISAMPLE_NONE) { - surface_point.x = surface_rect->left; - surface_point.y = surface_rect->top; + IDirect3DSurface9 *temp_surface2; + D3DSURFACE_DESC temp_desc; + + IDirect3DSurface9_GetDesc(temp_surface, &temp_desc); + hr = IDirect3DDevice9_CreateRenderTarget(device, temp_desc.Width, temp_desc.Height, temp_desc.Format, + D3DMULTISAMPLE_NONE, 0, FALSE, &temp_surface2, NULL);
I think this intermediate rendertarget can be avoided when D3DDEVCAPS2_CAN_STRETCHRECT_FROM_TEXTURES is advertised? I might be overlooking some other reason for it though. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6157#note_77232
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/texture.c:
width = (width == D3DX_DEFAULT) ? make_pow2(imginfo.Width) : imginfo.Width; if (!height || height == D3DX_DEFAULT_NONPOW2 || height == D3DX_FROM_FILE || height == D3DX_DEFAULT) height = (height == D3DX_DEFAULT) ? make_pow2(imginfo.Height) : imginfo.Height; + if (filter == D3DX_DEFAULT) + filter = D3DX_FILTER_TRIANGLE | D3DX_FILTER_DITHER;
So this is effectively a `Fixes: 1215ee024148952525317cc104ce91e650f21ca3`, right? I'd make use of this occasion to introduce a tiny helper like `d3dx9_validate_filter()` for this, even if it's just two lines (but copied around now a handful of times). Also, while thinking of a name for the helper, I started to wonder whether we should actually validate the `filter` value. As it turns out, that's very much the case (e.g. try to use `6`.) Actual validation should be its own patch, probably, but I guess this is one more reason for introducing the helper. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6157#note_77233
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/texture.c:
height = (height == D3DX_DEFAULT) ? make_pow2(image_info.Height) : image_info.Height; if (!depth || depth == D3DX_DEFAULT_NONPOW2 || depth == D3DX_FROM_FILE || depth == D3DX_DEFAULT) depth = (depth == D3DX_DEFAULT) ? make_pow2(image_info.Depth) : image_info.Depth; + if (filter == D3DX_DEFAULT) + filter = D3DX_FILTER_TRIANGLE | D3DX_FILTER_DITHER;
This hunk should be `Fixes: c9e6f6f84f661bdd190f4475296a7ce1d3885516`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6157#note_77234
Matteo Bruni (@Mystral) commented about dlls/d3dx9_36/texture.c:
size = max(img_info.Width, img_info.Height); else if (size == D3DX_DEFAULT) size = make_pow2(max(img_info.Width, img_info.Height)); + if (filter == D3DX_DEFAULT) + filter = D3DX_FILTER_TRIANGLE | D3DX_FILTER_DITHER;
`Fixes: f023fd35cefe6d46849a4a9336905410b4991ac4` I don't think the patch necessarily needs to be split up (not sure on that?) but adding these tags should be nice. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6157#note_77235
While reviewing I found that some d3dx9 tests spuriously fail with traces enabled. I'll send an MR for it, probably tomorrow at this point... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6157#note_77236
participants (3)
-
Connor McAdams -
Connor McAdams (＠cmcadams)
-
Matteo Bruni (＠Mystral)