[PATCH v4 0/3] MR4452: d2d1: Acquire lock before attempt to draw to device context
Fixes periodic crashes in PowerPoint 365 -- v4: d2d1: Acquire lock before attempt to draw to device context https://gitlab.winehq.org/wine/wine/-/merge_requests/4452
From: Brendan McGrath <bmcgrath(a)codeweavers.com> Tests that Direct2D can not access D2D exclusive resources whilst the Direct 2D lock is held --- dlls/d2d1/tests/d2d1.c | 51 +++++++++++++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 8 deletions(-) diff --git a/dlls/d2d1/tests/d2d1.c b/dlls/d2d1/tests/d2d1.c index 58f905ca1dc..e49deea3993 100644 --- a/dlls/d2d1/tests/d2d1.c +++ b/dlls/d2d1/tests/d2d1.c @@ -1163,13 +1163,14 @@ static IDXGISwapChain *create_d3d10_swapchain(ID3D10Device1 *device, HWND window } static ID2D1RenderTarget *create_render_target_desc(IDXGISurface *surface, - const D2D1_RENDER_TARGET_PROPERTIES *desc, BOOL d3d11) + const D2D1_RENDER_TARGET_PROPERTIES *desc, BOOL d3d11, BOOL multithreaded) { + D2D1_FACTORY_TYPE factory_type = multithreaded ? D2D1_FACTORY_TYPE_MULTI_THREADED : D2D1_FACTORY_TYPE_SINGLE_THREADED; ID2D1RenderTarget *render_target; ID2D1Factory *factory; HRESULT hr; - hr = D2D1CreateFactory(D2D1_FACTORY_TYPE_SINGLE_THREADED, &IID_ID2D1Factory, NULL, (void **)&factory); + hr = D2D1CreateFactory(factory_type, &IID_ID2D1Factory, NULL, (void **)&factory); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); hr = ID2D1Factory_CreateDxgiSurfaceRenderTarget(factory, surface, desc, &render_target); ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); @@ -1178,7 +1179,7 @@ static ID2D1RenderTarget *create_render_target_desc(IDXGISurface *surface, return render_target; } -static ID2D1RenderTarget *create_render_target(IDXGISurface *surface, BOOL d3d11) +static ID2D1RenderTarget *create_render_target(IDXGISurface *surface, BOOL d3d11, BOOL multithreaded) { D2D1_RENDER_TARGET_PROPERTIES desc; @@ -1190,7 +1191,7 @@ static ID2D1RenderTarget *create_render_target(IDXGISurface *surface, BOOL d3d11 desc.usage = D2D1_RENDER_TARGET_USAGE_NONE; desc.minLevel = D2D1_FEATURE_LEVEL_DEFAULT; - return create_render_target_desc(surface, &desc, d3d11); + return create_render_target_desc(surface, &desc, d3d11, multithreaded); } #define release_test_context(ctx) release_test_context_(__LINE__, ctx) @@ -1216,8 +1217,9 @@ static void release_test_context_(unsigned int line, struct d2d1_test_context *c ok_(__FILE__, line)(!ref, "Device has %lu references left.\n", ref); } -#define init_test_context(ctx, d3d11) init_test_context_(__LINE__, ctx, d3d11) -static BOOL init_test_context_(unsigned int line, struct d2d1_test_context *ctx, BOOL d3d11) +#define init_test_context(ctx, d3d11) init_test_context_(__LINE__, ctx, d3d11, FALSE) +#define init_test_multithreaded_context(ctx, d3d11) init_test_context_(__LINE__, ctx, d3d11, TRUE) +static BOOL init_test_context_(unsigned int line, struct d2d1_test_context *ctx, BOOL d3d11, BOOL multithreaded) { HRESULT hr; @@ -1237,7 +1239,7 @@ static BOOL init_test_context_(unsigned int line, struct d2d1_test_context *ctx, hr = IDXGISwapChain_GetBuffer(ctx->swapchain, 0, &IID_IDXGISurface, (void **)&ctx->surface); ok_(__FILE__, line)(hr == S_OK, "Failed to get buffer, hr %#lx.\n", hr); - ctx->rt = create_render_target(ctx->surface, d3d11); + ctx->rt = create_render_target(ctx->surface, d3d11, multithreaded); if (!ctx->rt && d3d11) { todo_wine win_skip_(__FILE__, line)("Skipping d3d11 tests.\n"); @@ -4887,7 +4889,7 @@ static void test_alpha_mode(BOOL d3d11) rt_desc.dpiY = 0.0f; rt_desc.usage = D2D1_RENDER_TARGET_USAGE_NONE; rt_desc.minLevel = D2D1_FEATURE_LEVEL_DEFAULT; - rt = create_render_target_desc(ctx.surface, &rt_desc, d3d11); + rt = create_render_target_desc(ctx.surface, &rt_desc, d3d11, FALSE); ok(!!rt, "Failed to create render target.\n"); ID2D1RenderTarget_SetAntialiasMode(rt, D2D1_ANTIALIAS_MODE_ALIASED); @@ -10694,8 +10696,23 @@ static DWORD WINAPI mt_factory_test_thread_func(void *param) return 0; } +static DWORD WINAPI mt_factory_test_thread_draw_func(void *param) { + ID2D1RenderTarget *rt = param; + D2D1_COLOR_F color; + HRESULT hr; + + ID2D1RenderTarget_BeginDraw(rt); + set_color(&color, 1.0f, 1.0f, 0.0f, 1.0f); + ID2D1RenderTarget_Clear(rt, &color); + hr = ID2D1RenderTarget_EndDraw(rt, NULL, NULL); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + + return 0; +} + static void test_mt_factory(BOOL d3d11) { + struct d2d1_test_context ctx; ID2D1Multithread *multithread; ID2D1Factory *factory; HANDLE thread; @@ -10750,6 +10767,24 @@ static void test_mt_factory(BOOL d3d11) ID2D1Multithread_Release(multithread); ID2D1Factory_Release(factory); + + if (!init_test_multithreaded_context(&ctx, d3d11)) + return; + + hr = ID2D1Factory_QueryInterface(ctx.factory, &IID_ID2D1Multithread, (void **)&multithread); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + + ID2D1Multithread_Enter(multithread); + thread = CreateThread(NULL, 0, mt_factory_test_thread_draw_func, ctx.rt, 0, NULL); + ok(!!thread, "Failed to create a thread.\n"); + ret = WaitForSingleObject(thread, 1000); + todo_wine ok(ret == WAIT_TIMEOUT, "Expected timeout.\n"); + ID2D1Multithread_Leave(multithread); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); + + ID2D1Multithread_Release(multithread); + release_test_context(&ctx); } #define check_system_properties(effect, is_builtin) check_system_properties_(__LINE__, effect, is_builtin) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4452
From: Brendan McGrath <bmcgrath(a)codeweavers.com> Tests that Direct2D can access D2D exclusive resources whilst the Direct 2D lock is held if the factory type is single threaded --- dlls/d2d1/tests/d2d1.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/dlls/d2d1/tests/d2d1.c b/dlls/d2d1/tests/d2d1.c index e49deea3993..f0e6f1ffb38 100644 --- a/dlls/d2d1/tests/d2d1.c +++ b/dlls/d2d1/tests/d2d1.c @@ -10785,6 +10785,24 @@ static void test_mt_factory(BOOL d3d11) ID2D1Multithread_Release(multithread); release_test_context(&ctx); + + if (!init_test_context(&ctx, d3d11)) + return; + + hr = ID2D1Factory_QueryInterface(ctx.factory, &IID_ID2D1Multithread, (void **)&multithread); + ok(hr == S_OK, "Got unexpected hr %#lx.\n", hr); + + ID2D1Multithread_Enter(multithread); + thread = CreateThread(NULL, 0, mt_factory_test_thread_draw_func, ctx.rt, 0, NULL); + ok(!!thread, "Failed to create a thread.\n"); + ret = WaitForSingleObject(thread, 1000); + ok(ret == WAIT_OBJECT_0, "Didn't expect timeout.\n"); + ID2D1Multithread_Leave(multithread); + WaitForSingleObject(thread, INFINITE); + CloseHandle(thread); + + ID2D1Multithread_Release(multithread); + release_test_context(&ctx); } #define check_system_properties(effect, is_builtin) check_system_properties_(__LINE__, effect, is_builtin) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4452
From: Brendan McGrath <bmcgrath(a)codeweavers.com> Ensures the Direct 2D lock is held before attempting to access Direct 2D exclusive resources. This fixes periodic crashes in PowerPoint 365 --- dlls/d2d1/d2d1_private.h | 7 ++++++- dlls/d2d1/device.c | 21 +++++++++++++++++++++ dlls/d2d1/factory.c | 12 ++++++++++++ dlls/d2d1/tests/d2d1.c | 2 +- 4 files changed, 40 insertions(+), 2 deletions(-) diff --git a/dlls/d2d1/d2d1_private.h b/dlls/d2d1/d2d1_private.h index 1dd5cdb0a39..6a780c6334f 100644 --- a/dlls/d2d1/d2d1_private.h +++ b/dlls/d2d1/d2d1_private.h @@ -159,6 +159,8 @@ enum d2d_device_context_target_type D2D_TARGET_COMMAND_LIST, }; +struct d2d_factory; + struct d2d_device_context { ID2D1DeviceContext1 ID2D1DeviceContext1_iface; @@ -171,6 +173,7 @@ struct d2d_device_context const struct d2d_device_context_ops *ops; ID2D1Factory *factory; + struct d2d_factory *factory_impl; struct d2d_device *device; ID3D11Device1 *d3d_device; ID3DDeviceContextState *d3d_state; @@ -678,12 +681,14 @@ struct d2d_effect_registration struct d2d_effect_properties properties; }; -struct d2d_factory; void d2d_effects_init_builtins(struct d2d_factory *factory); struct d2d_effect_registration * d2d_factory_get_registered_effect(ID2D1Factory *factory, const GUID *effect_id); void d2d_factory_register_effect(struct d2d_factory *factory, struct d2d_effect_registration *effect); +struct d2d_factory *d2d_factory_from_ID2D1Multithread(ID2D1Multithread *iface); +void d2d_factory_enter_cs(struct d2d_factory *factory); +void d2d_factory_leave_cs(struct d2d_factory *factory); struct d2d_transform_graph { diff --git a/dlls/d2d1/device.c b/dlls/d2d1/device.c index e5548878ee9..75ac3a5b7c1 100644 --- a/dlls/d2d1/device.c +++ b/dlls/d2d1/device.c @@ -115,6 +115,14 @@ static void d2d_clip_stack_pop(struct d2d_clip_stack *stack) --stack->count; } +static inline void d2d_device_context_enter_cs(struct d2d_device_context *context) { + if (context->factory_impl) d2d_factory_enter_cs(context->factory_impl); +} + +static inline void d2d_device_context_leave_cs(struct d2d_device_context *context) { + if (context->factory_impl) d2d_factory_leave_cs(context->factory_impl); +} + static void d2d_device_context_draw(struct d2d_device_context *render_target, enum d2d_shape_type shape_type, ID3D11Buffer *ib, unsigned int index_count, ID3D11Buffer *vb, unsigned int vb_stride, struct d2d_brush *brush, struct d2d_brush *opacity_brush) @@ -135,6 +143,8 @@ static void d2d_device_context_draw(struct d2d_device_context *render_target, en vp.MinDepth = 0.0f; vp.MaxDepth = 1.0f; + d2d_device_context_enter_cs(render_target); + ID3D11Device1_GetImmediateContext1(device, &context); ID3D11DeviceContext1_SwapDeviceContextState(context, render_target->d3d_state, &prev_state); @@ -188,6 +198,8 @@ static void d2d_device_context_draw(struct d2d_device_context *render_target, en ID3D11DeviceContext1_SwapDeviceContextState(context, prev_state, NULL); ID3D11DeviceContext1_Release(context); ID3DDeviceContextState_Release(prev_state); + + d2d_device_context_leave_cs(render_target); } static void d2d_device_context_set_error(struct d2d_device_context *context, HRESULT code) @@ -3196,6 +3208,7 @@ static HRESULT d2d_device_context_init(struct d2d_device_context *render_target, IDWriteFactory *dwrite_factory; D3D11_RASTERIZER_DESC rs_desc; D3D11_BUFFER_DESC buffer_desc; + ID2D1Multithread *multithread; unsigned int i; HRESULT hr; @@ -4256,6 +4269,14 @@ static HRESULT d2d_device_context_init(struct d2d_device_context *render_target, render_target->device = device; ID2D1Device1_AddRef(&render_target->device->ID2D1Device1_iface); + ID2D1Factory_QueryInterface(render_target->factory, &IID_ID2D1Multithread, (void **)&multithread); + if (ID2D1Multithread_GetMultithreadProtected(multithread)) { + render_target->factory_impl = d2d_factory_from_ID2D1Multithread(multithread); + } else { + render_target->factory_impl = NULL; + } + ID2D1Multithread_Release(multithread); + render_target->outer_unknown = outer_unknown ? outer_unknown : &render_target->IUnknown_iface; render_target->ops = ops; diff --git a/dlls/d2d1/factory.c b/dlls/d2d1/factory.c index 3b2b1f5e4ec..1c49383d445 100644 --- a/dlls/d2d1/factory.c +++ b/dlls/d2d1/factory.c @@ -68,6 +68,10 @@ static inline struct d2d_factory *impl_from_ID2D1Multithread(ID2D1Multithread *i return CONTAINING_RECORD(iface, struct d2d_factory, ID2D1Multithread_iface); } +struct d2d_factory *d2d_factory_from_ID2D1Multithread(ID2D1Multithread *iface) { + return impl_from_ID2D1Multithread(iface); +} + static BOOL WINAPI d2d_factory_builtins_initonce(INIT_ONCE *once, void *param, void **context) { d2d_effects_init_builtins(param); @@ -1173,6 +1177,14 @@ static void STDMETHODCALLTYPE d2d_factory_mt_Leave(ID2D1Multithread *iface) LeaveCriticalSection(&factory->cs); } +void d2d_factory_enter_cs(struct d2d_factory *factory) { + EnterCriticalSection(&factory->cs); +} + +void d2d_factory_leave_cs(struct d2d_factory *factory) { + LeaveCriticalSection(&factory->cs); +} + static BOOL STDMETHODCALLTYPE d2d_factory_st_GetMultithreadProtected(ID2D1Multithread *iface) { return FALSE; diff --git a/dlls/d2d1/tests/d2d1.c b/dlls/d2d1/tests/d2d1.c index f0e6f1ffb38..3b7b59ef49f 100644 --- a/dlls/d2d1/tests/d2d1.c +++ b/dlls/d2d1/tests/d2d1.c @@ -10778,7 +10778,7 @@ static void test_mt_factory(BOOL d3d11) thread = CreateThread(NULL, 0, mt_factory_test_thread_draw_func, ctx.rt, 0, NULL); ok(!!thread, "Failed to create a thread.\n"); ret = WaitForSingleObject(thread, 1000); - todo_wine ok(ret == WAIT_TIMEOUT, "Expected timeout.\n"); + ok(ret == WAIT_TIMEOUT, "Expected timeout.\n"); ID2D1Multithread_Leave(multithread); WaitForSingleObject(thread, INFINITE); CloseHandle(thread); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4452
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=140204 Your paranoid android. === w10pro64_ar (64 bit report) === d2d1: d2d1.c:586: Test failed: Got unexpected hr 0x80004002. 239c:d2d1: unhandled exception c0000005 at 0000000000E336B7
participants (3)
-
Brendan McGrath -
Brendan McGrath (＠redmcg)
-
Marvin