[PATCH 0/2] MR2821: wined3d: Improve d3d8 compatibility of minification and magnification filters.
This fixes the visual glitches and crashes of Worms Blast that were reported here: https://bugs.winehq.org/show_bug.cgi?id=54898 (and here: https://github.com/ValveSoftware/Proton/issues/706). There is, however, still an issue with the background music, which is not playing. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2821
From: Sebastian Mayr <me(a)sam.st> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54898 --- dlls/d3d8/tests/device.c | 60 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/dlls/d3d8/tests/device.c b/dlls/d3d8/tests/device.c index 6a585458bae..a2d40b4b154 100644 --- a/dlls/d3d8/tests/device.c +++ b/dlls/d3d8/tests/device.c @@ -10910,6 +10910,65 @@ static void test_window_position(void) IDirect3D8_Release(d3d); } +static void test_min_mag_mip_filter_validate(void) +{ + static const D3DTEXTUREFILTERTYPE filters[] = {D3DTEXF_NONE, D3DTEXF_POINT, D3DTEXF_LINEAR, D3DTEXF_ANISOTROPIC}; + IDirect3DTexture8 *texture; + IDirect3DDevice8 *device; + unsigned int i, j, k, l; + IDirect3D8 *d3d; + ULONG refcount; + HWND window; + HRESULT hr; + + window = create_window(); + ok(!!window, "Failed to create a window.\n"); + d3d = Direct3DCreate8(D3D_SDK_VERSION); + ok(!!d3d, "Failed to create a D3D object.\n"); + if (!(device = create_device(d3d, window, NULL))) + { + skip("Failed to create a 3D device, skipping test.\n"); + goto cleanup; + } + + hr = IDirect3DDevice8_CreateTexture(device, 16, 16, 1, 0, D3DFMT_A8R8G8B8, D3DPOOL_MANAGED, &texture); + ok(hr == D3D_OK, "CreateTexture failed, error %#lx.\n", hr); + + for (i = 0; i < 8; ++i) + { + hr = IDirect3DDevice8_SetTexture(device, i, (IDirect3DBaseTexture8 *)texture); + ok(hr == D3D_OK, "SetTexture failed, error %#lx.\n", hr); + + for (j = 0; j < ARRAY_SIZE(filters); ++j) + for (k = 0; k < ARRAY_SIZE(filters); ++k) + for (l = 0; l < ARRAY_SIZE(filters); ++l) + { + DWORD passes; + + hr = IDirect3DDevice8_SetTextureStageState(device, i, D3DTSS_MINFILTER, filters[j]); + ok(hr == D3D_OK, "SetTextureStageState MINFILTER failed, error %#lx.\n", hr); + hr = IDirect3DDevice8_SetTextureStageState(device, i, D3DTSS_MAGFILTER, filters[k]); + ok(hr == D3D_OK, "SetTextureStageState MAGFILTER failed, error %#lx.\n", hr); + hr = IDirect3DDevice8_SetTextureStageState(device, i, D3DTSS_MIPFILTER, filters[l]); + ok(hr == D3D_OK, "SetTextureStageState MIPFILTER failed, error %#lx.\n", hr); + + hr = IDirect3DDevice8_ValidateDevice(device, &passes); + ok(hr == D3D_OK, "ValidateDevice failed, error %#lx.\n", hr); + } + + hr = IDirect3DDevice8_SetTexture(device, i, NULL); + ok(hr == D3D_OK, "SetTexture NULL failed, error %#lx.\n", hr); + } + + refcount = IDirect3DTexture8_Release(texture); + ok(!refcount, "Texture has %lu references left.\n", refcount); + refcount = IDirect3DDevice8_Release(device); + ok(!refcount, "Device has %lu references left.\n", refcount); +cleanup: + IDirect3D8_Release(d3d); + DestroyWindow(window); +} + START_TEST(device) { HMODULE d3d8_handle = GetModuleHandleA("d3d8.dll"); @@ -11031,6 +11090,7 @@ START_TEST(device) test_creation_parameters(); test_cursor_clipping(); test_window_position(); + test_min_mag_mip_filter_validate(); UnregisterClassA("d3d8_test_wc", GetModuleHandleA(NULL)); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2821
From: Sebastian Mayr <me(a)sam.st> In d3d8, setting the minification and magnifications filters to D3DTEXF_NONE does not yield an error when calling IDirect3DDevice8::ValidateDevice. Instead, it behaves identical to D3DTEXF_POINT. This resolves crashes and various visual glitches in Worms Blast. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54898 --- dlls/d3d8/directx.c | 2 +- dlls/wined3d/device.c | 19 +++++++++++-------- dlls/wined3d/sampler.c | 1 + include/wine/wined3d.h | 1 + 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/dlls/d3d8/directx.c b/dlls/d3d8/directx.c index b35abc6b699..fed0e86b22e 100644 --- a/dlls/d3d8/directx.c +++ b/dlls/d3d8/directx.c @@ -476,7 +476,7 @@ BOOL d3d8_init(struct d3d8 *d3d8) DWORD flags = WINED3D_LEGACY_DEPTH_BIAS | WINED3D_HANDLE_RESTORE | WINED3D_PIXEL_CENTER_INTEGER | WINED3D_LEGACY_UNBOUND_RESOURCE_COLOR | WINED3D_NO_PRIMITIVE_RESTART | WINED3D_LEGACY_CUBEMAP_FILTERING - | WINED3D_NO_DRAW_INDIRECT; + | WINED3D_NO_DRAW_INDIRECT | WINED3D_LEGACY_MINMAG_FILTERING; unsigned int adapter_idx, output_idx, adapter_count, output_count = 0; struct wined3d_adapter *wined3d_adapter; diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c index df186d9ffa2..5b54f4fc856 100644 --- a/dlls/wined3d/device.c +++ b/dlls/wined3d/device.c @@ -4798,15 +4798,18 @@ HRESULT CDECL wined3d_device_validate_device(const struct wined3d_device *device for (i = 0; i < WINED3D_MAX_COMBINED_SAMPLERS; ++i) { - if (state->sampler_states[i][WINED3D_SAMP_MIN_FILTER] == WINED3D_TEXF_NONE) + if (!(device->wined3d->flags & WINED3D_LEGACY_MINMAG_FILTERING)) { - WARN("Sampler state %u has minfilter D3DTEXF_NONE, returning D3DERR_UNSUPPORTEDTEXTUREFILTER\n", i); - return WINED3DERR_UNSUPPORTEDTEXTUREFILTER; - } - if (state->sampler_states[i][WINED3D_SAMP_MAG_FILTER] == WINED3D_TEXF_NONE) - { - WARN("Sampler state %u has magfilter D3DTEXF_NONE, returning D3DERR_UNSUPPORTEDTEXTUREFILTER\n", i); - return WINED3DERR_UNSUPPORTEDTEXTUREFILTER; + if (state->sampler_states[i][WINED3D_SAMP_MIN_FILTER] == WINED3D_TEXF_NONE) + { + WARN("Sampler state %u has minfilter D3DTEXF_NONE, returning D3DERR_UNSUPPORTEDTEXTUREFILTER\n", i); + return WINED3DERR_UNSUPPORTEDTEXTUREFILTER; + } + if (state->sampler_states[i][WINED3D_SAMP_MAG_FILTER] == WINED3D_TEXF_NONE) + { + WARN("Sampler state %u has magfilter D3DTEXF_NONE, returning D3DERR_UNSUPPORTEDTEXTUREFILTER\n", i); + return WINED3DERR_UNSUPPORTEDTEXTUREFILTER; + } } texture = state->textures[i]; diff --git a/dlls/wined3d/sampler.c b/dlls/wined3d/sampler.c index d6e19fc521c..4ebc8c7f844 100644 --- a/dlls/wined3d/sampler.c +++ b/dlls/wined3d/sampler.c @@ -143,6 +143,7 @@ static VkFilter vk_filter_from_wined3d(enum wined3d_texture_filter_type f) { default: ERR("Invalid filter type %#x.\n", f); + case WINED3D_TEXF_NONE: case WINED3D_TEXF_POINT: return VK_FILTER_NEAREST; case WINED3D_TEXF_LINEAR: diff --git a/include/wine/wined3d.h b/include/wine/wined3d.h index 71a5d92c301..e04aba2233f 100644 --- a/include/wine/wined3d.h +++ b/include/wine/wined3d.h @@ -1324,6 +1324,7 @@ enum wined3d_memory_segment_group #define WINED3D_LEGACY_CUBEMAP_FILTERING 0x00001000 #define WINED3D_NORMALIZED_DEPTH_BIAS 0x00002000 #define WINED3D_NO_DRAW_INDIRECT 0x00004000 +#define WINED3D_LEGACY_MINMAG_FILTERING 0x00008000 #define WINED3D_RESZ_CODE 0x7fa05000 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2821
Stefan Dösinger (@stefan) commented about dlls/d3d8/tests/device.c:
IDirect3D8_Release(d3d); }
+static void test_min_mag_mip_filter_validate(void) +{ + static const D3DTEXTUREFILTERTYPE filters[] = {D3DTEXF_NONE, D3DTEXF_POINT, D3DTEXF_LINEAR, D3DTEXF_ANISOTROPIC};
I am curious: Does native d3d8 maybe accept every possible filter value? What if you set it to e.g. 0xdeadbeef? For consistency I'd prefer a port of d3d9's test_filter(), obviously with different expected return values. The loop iterating over all possible combinations makes a point though. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2821#note_32812
Stefan Dösinger (@stefan) commented about dlls/d3d8/tests/device.c:
+ ok(!!window, "Failed to create a window.\n"); + d3d = Direct3DCreate8(D3D_SDK_VERSION); + ok(!!d3d, "Failed to create a D3D object.\n"); + if (!(device = create_device(d3d, window, NULL))) + { + skip("Failed to create a 3D device, skipping test.\n"); + goto cleanup; + } + + hr = IDirect3DDevice8_CreateTexture(device, 16, 16, 1, 0, D3DFMT_A8R8G8B8, D3DPOOL_MANAGED, &texture); + ok(hr == D3D_OK, "CreateTexture failed, error %#lx.\n", hr); + + for (i = 0; i < 8; ++i) + { + hr = IDirect3DDevice8_SetTexture(device, i, (IDirect3DBaseTexture8 *)texture); + ok(hr == D3D_OK, "SetTexture failed, error %#lx.\n", hr); What about GPUs that support fewer than 8 textures and/or simultaneous active stages? I have an old r200 GPU around, I'll give that test a try there.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2821#note_32813
On Mon May 15 20:35:52 2023 +0000, Stefan Dösinger wrote:
I am curious: Does native d3d8 maybe accept every possible filter value? What if you set it to e.g. 0xdeadbeef? For consistency I'd prefer a port of d3d9's test_filter(), obviously with different expected return values. The loop iterating over all possible combinations makes a point though. As far as I can tell, d3d8 does indeed accept every filter value, it's probably not validated at all. I'll look into porting `test_filter()` and also test some invalid values.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2821#note_32847
On Mon May 15 20:35:52 2023 +0000, Stefan Dösinger wrote:
What about GPUs that support fewer than 8 textures and/or simultaneous active stages? I have an old r200 GPU around, I'll give that test a try there. Hmm, good point... For the most part I just copied this from `test_limits()`. Do you know if this limit also affects the usable indices, or does it just limit the number of textures that are set simultaneously? If it's the latter, the test should be fine, because the texture is unset at the end of the loop.
But I guess testing all possible texture stages is not very relevant here, it might be better to just test stage 0? (like `test_filter()` is doing). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2821#note_32848
On Tue May 16 07:24:05 2023 +0000, Sebastian Mayr wrote:
As far as I can tell, d3d8 does indeed accept every filter value, it's probably not validated at all. I'll look into porting `test_filter()` and also test some invalid values. In that case the fix for the bug might be a lot easier - just do something like if (hr == WINED3DERR_UNSUPPORTEDTEXTUREFILTER) return D3D_OK; in IDirect3DDevice8::ValidateDevice.
(Unless we validate the device state internally somehow, but I don't think we do) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2821#note_32852
On Tue May 16 07:24:05 2023 +0000, Sebastian Mayr wrote:
Hmm, good point... For the most part I just copied this from `test_limits()`. Do you know if this limit also affects the usable indices, or does it just limit the number of textures that are set simultaneously? If it's the latter, the test should be fine, because the texture is unset at the end of the loop. But I guess testing all possible texture stages is not very relevant here, it might be better to just test stage 0? (like `test_filter()` is doing). --- random musings, ignore anything that doesn't make sense ---
I ran the test on my r200 GPU (from circa 2002 - a directx 8 native chip, shader model 1 support, but not shader model 2). It has MaxSimultaneousTextures=6 and MaxTextureBlendStages=8. The test passes as is. I think though that binding something to e.g. texture unit 7 is fine as long as no more than 6 textures are read in total by the fixed function pipeline setup. I think version 1.x shaders can only access textures 0-5. There are only 6 "texture registers". A shader trying to use t6 or t7 should fail static validation. I don't think we have tests for this or enforce this restriction though. Fixed function fragment processing can access textures 6 and 7 by using D3DTA_TEXTURE in texture stage 6 or 7. A sufficient number of lower texture stages will have to not read a texture for this to work though. We implement support for this, see wined3d_context_gl_map_fixed_function_samplers. A similar setup is used to make vertex and fragment samplers coexist peacefully. In this case the application will have to use D3DTSS_TEXCOORDINDEX to source an appropriate set of texture coordinates. --- end random musings --- So my 2c: 1) Try to change the implementation to simply ignore D3DERR_UNSUPPORTEDTEXTUREFILTER in d3d8's ValidateDevice and return D3D_OK. There is the question of priority of errors. E.g. assume a d3d9 application fails two of the checks in wined3d_device_validate_device - the filter type and using a depth stencil that is smaller than the render target (D3DERR_CONFLICTINGRENDERSTATE). If d3d9 gets D3DERR_UNSUPPORTEDTEXTUREFILTER, but d3d8 expects D3DERR_CONFLICTINGRENDERSTATE, there's an issue. But I am happy to ignore this situation until we find an application that cares about this. 2) Add more invalid values to d3d9's test_filter(), port the test to d3d8 and adjust the expected return values accordingly. 3) If you are motivated, write tests testing the rendering result with 'invalid' filters. wined3d_sampler_desc_from_sampler_states() already has handling for them, but I don't think we check if the result is correct. E.g. we'd treat a mag filter of D3DTEXF_ANISOTROPIC as D3DTEXF_LINEAR. It's possible the correct behavior is to treat it as D3DTEXF_POINT. This is a question that applies to d3d8 and d3d9. D3d9's ValidateDevice() rejects the bad mag filter, but there's no guarantee applications call ValidateDevice or do something sensible if it returns failure. Personally I don't care about rendering tests - what we seem to need for bug 54898 is for ValidateDevice to succeed. The existing handling of invalid filter values in rendering seems to be ok. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2821#note_32920
On Tue May 16 20:41:25 2023 +0000, Stefan Dösinger wrote:
--- random musings, ignore anything that doesn't make sense --- I ran the test on my r200 GPU (from circa 2002 - a directx 8 native chip, shader model 1 support, but not shader model 2). It has MaxSimultaneousTextures=6 and MaxTextureBlendStages=8. The test passes as is. I think though that binding something to e.g. texture unit 7 is fine as long as no more than 6 textures are read in total by the fixed function pipeline setup. I think version 1.x shaders can only access textures 0-5. There are only 6 "texture registers". A shader trying to use t6 or t7 should fail static validation. I don't think we have tests for this or enforce this restriction though. Fixed function fragment processing can access textures 6 and 7 by using D3DTA_TEXTURE in texture stage 6 or 7. A sufficient number of lower texture stages will have to not read a texture for this to work though. We implement support for this, see wined3d_context_gl_map_fixed_function_samplers. A similar setup is used to make vertex and fragment samplers coexist peacefully. In this case the application will have to use D3DTSS_TEXCOORDINDEX to source an appropriate set of texture coordinates. --- end random musings --- So my 2c: 1) Try to change the implementation to simply ignore D3DERR_UNSUPPORTEDTEXTUREFILTER in d3d8's ValidateDevice and return D3D_OK. There is the question of priority of errors. E.g. assume a d3d9 application fails two of the checks in wined3d_device_validate_device - the filter type and using a depth stencil that is smaller than the render target (D3DERR_CONFLICTINGRENDERSTATE). If d3d9 gets D3DERR_UNSUPPORTEDTEXTUREFILTER, but d3d8 expects D3DERR_CONFLICTINGRENDERSTATE, there's an issue. But I am happy to ignore this situation until we find an application that cares about this. 2) Add more invalid values to d3d9's test_filter(), port the test to d3d8 and adjust the expected return values accordingly. 3) If you are motivated, write tests testing the rendering result with 'invalid' filters. wined3d_sampler_desc_from_sampler_states() already has handling for them, but I don't think we check if the result is correct. E.g. we'd treat a mag filter of D3DTEXF_ANISOTROPIC as D3DTEXF_LINEAR. It's possible the correct behavior is to treat it as D3DTEXF_POINT. This is a question that applies to d3d8 and d3d9. D3d9's ValidateDevice() rejects the bad mag filter, but there's no guarantee applications call ValidateDevice or do something sensible if it returns failure. Personally I don't care about rendering tests - what we seem to need for bug 54898 is for ValidateDevice to succeed. The existing handling of invalid filter values in rendering seems to be ok. Thanks for all the (arcane) details, much appreciated!
Ignoring the error in d3d8's ValidateDevice is definitely much simpler, I'll go with that. Concerning test_filter(), I'm not yet sure if d3d8 supports a similar "unfilterable" texture format, but I'll try to figure something out... or simply remove this particular case. I did some _very_ quick rendering tests with invalid min/mag filters and apart from WINED3D_TEXF_NONE they seem to get treated as D3DTEXF_LINEAR. So it looks to me as if wine already does the right thing in wined3d_sampler_desc_from_sampler_states(). (Thanks for pointing me to this function, btw.) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2821#note_32948
participants (3)
-
Sebastian Mayr -
Sebastian Mayr (＠smayr)
-
Stefan Dösinger