The GetWorkItemCount implementation may look strange. It's done this way because of how WaitForAllItems is implemented. Alternative solution is to introduce separate counter for GetWorkItemCount.
-- v3: d3dx10/tests: Add D3DX10CreateThreadPump tests. d3dx10: Add ID3DX10ThreadPump:PurgeAllItems implementation. d3dx10: Add ID3DX10ThreadPump:GetQueueStatus implementation. d3dx10: Add ID3DX10ThreadPump:WaitForAllItems implementation. d3dx10: Add ID3DX10ThreadPump:ProcessDeviceWorkItems implementation. d3dx10: Add ID3DX10ThreadPump:GetWorkItemCount implementation. d3dx10: Add ID3DX10ThreadPump:AddWorkItem implementation. d3dx10: Add D3DX10CreateThreadPump stub. d3dx10/tests: Fix texture leak in check_resource_data.
From: Piotr Caban piotr@codeweavers.com
Signed-off-by: Piotr Caban piotr@codeweavers.com --- dlls/d3dx10_43/tests/d3dx10.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/dlls/d3dx10_43/tests/d3dx10.c b/dlls/d3dx10_43/tests/d3dx10.c index 1c28a62b700..d1c772197b9 100644 --- a/dlls/d3dx10_43/tests/d3dx10.c +++ b/dlls/d3dx10_43/tests/d3dx10.c @@ -1237,7 +1237,7 @@ static void check_resource_data(ID3D10Resource *resource, const struct test_imag ok_(__FILE__, line)(hr == S_OK, "Map failed, hr %#x.\n", hr); if (hr != S_OK) { - ID3D10Texture2D_Unmap(readback, 0); + ID3D10Texture2D_Release(readback); return; }
@@ -1253,6 +1253,7 @@ static void check_resource_data(ID3D10Resource *resource, const struct test_imag }
ID3D10Texture2D_Unmap(readback, 0); + ID3D10Texture2D_Release(readback); }
static void test_D3DX10UnsetAllDeviceObjects(void) @@ -2062,7 +2063,7 @@ static void test_D3DX10CreateAsyncTextureProcessor(void)
CoUninitialize();
- ID3D10Device_Release(device); + ok(!ID3D10Device_Release(device), "Unexpected refcount.\n"); }
static void test_get_image_info(void) @@ -2417,7 +2418,7 @@ static void test_create_texture(void)
CoUninitialize();
- ID3D10Device_Release(device); + ok(!ID3D10Device_Release(device), "Unexpected refcount.\n"); }
#define check_rect(rect, left, top, right, bottom) _check_rect(__LINE__, rect, left, top, right, bottom)
From: Piotr Caban piotr@codeweavers.com
Signed-off-by: Piotr Caban piotr@codeweavers.com --- dlls/d3dx10_43/d3dx10_43.spec | 2 +- dlls/d3dx10_43/d3dx10_43_main.c | 123 ++++++++++++++++++++++++++++++++ include/d3dx10core.h | 1 + 3 files changed, 125 insertions(+), 1 deletion(-)
diff --git a/dlls/d3dx10_43/d3dx10_43.spec b/dlls/d3dx10_43/d3dx10_43.spec index 95160a067c5..2359c7c6f02 100644 --- a/dlls/d3dx10_43/d3dx10_43.spec +++ b/dlls/d3dx10_43/d3dx10_43.spec @@ -1,4 +1,4 @@ -@ stub D3DX10CreateThreadPump(long long ptr) +@ stdcall D3DX10CreateThreadPump(long long ptr) @ stdcall D3DX10CheckVersion(long long) @ stub D3DX10CompileFromFileA(str ptr ptr str str long long ptr ptr ptr ptr) @ stub D3DX10CompileFromFileW(wstr ptr ptr str str long long ptr ptr ptr ptr) diff --git a/dlls/d3dx10_43/d3dx10_43_main.c b/dlls/d3dx10_43/d3dx10_43_main.c index ec7407508d3..357b0257496 100644 --- a/dlls/d3dx10_43/d3dx10_43_main.c +++ b/dlls/d3dx10_43/d3dx10_43_main.c @@ -176,3 +176,126 @@ HRESULT WINAPI D3DX10LoadTextureFromTexture(ID3D10Resource *src_texture, D3DX10_
return E_NOTIMPL; } + +struct thread_pump +{ + ID3DX10ThreadPump ID3DX10ThreadPump_iface; + LONG refcount; +}; + +static inline struct thread_pump *impl_from_ID3DX10ThreadPump(ID3DX10ThreadPump *iface) +{ + return CONTAINING_RECORD(iface, struct thread_pump, ID3DX10ThreadPump_iface); +} + +static HRESULT WINAPI thread_pump_QueryInterface(ID3DX10ThreadPump *iface, REFIID riid, void **out) +{ + TRACE("iface %p, riid %s, out %p.\n", iface, debugstr_guid(riid), out); + + if (IsEqualGUID(riid, &IID_ID3DX10ThreadPump) + || IsEqualGUID(riid, &IID_IUnknown)) + { + ID3DX10ThreadPump_AddRef(iface); + *out = iface; + return S_OK; + } + + WARN("%s not implemented, returning E_NOINTERFACE.\n", debugstr_guid(riid)); + *out = NULL; + return E_NOINTERFACE; +} + +static ULONG WINAPI thread_pump_AddRef(ID3DX10ThreadPump *iface) +{ + struct thread_pump *thread_pump = impl_from_ID3DX10ThreadPump(iface); + ULONG refcount = InterlockedIncrement(&thread_pump->refcount); + + TRACE("%p increasing refcount to %lu.\n", iface, refcount); + + return refcount; +} + +static ULONG WINAPI thread_pump_Release(ID3DX10ThreadPump *iface) +{ + struct thread_pump *thread_pump = impl_from_ID3DX10ThreadPump(iface); + ULONG refcount = InterlockedDecrement(&thread_pump->refcount); + + TRACE("%p decreasing refcount to %lu.\n", iface, refcount); + + if (!refcount) + free(thread_pump); + + return refcount; +} + +static HRESULT WINAPI thread_pump_AddWorkItem(ID3DX10ThreadPump *iface, ID3DX10DataLoader *loader, + ID3DX10DataProcessor *processor, HRESULT *result, void **object) +{ + FIXME("iface %p, loader %p, processor %p, result %p, object %p stub!\n", + iface, loader, processor, result, object); + return E_NOTIMPL; +} + +static UINT WINAPI thread_pump_GetWorkItemCount(ID3DX10ThreadPump *iface) +{ + FIXME("iface %p stub!\n", iface); + return 0; +} + +static HRESULT WINAPI thread_pump_WaitForAllItems(ID3DX10ThreadPump *iface) +{ + FIXME("iface %p stub!\n", iface); + return E_NOTIMPL; +} + +static HRESULT WINAPI thread_pump_ProcessDeviceWorkItems(ID3DX10ThreadPump *iface, UINT count) +{ + FIXME("iface %p, count %u stub!\n", iface, count); + return E_NOTIMPL; +} + +static HRESULT WINAPI thread_pump_PurgeAllItems(ID3DX10ThreadPump *iface) +{ + FIXME("iface %p stub!\n", iface); + return E_NOTIMPL; +} + +static HRESULT WINAPI thread_pump_GetQueueStatus(ID3DX10ThreadPump *iface, + UINT *io_queue, UINT *process_queue, UINT *device_queue) +{ + FIXME("iface %p, io_queue %p, process_queue %p, device_queue %p stub!\n", + iface, io_queue, process_queue, device_queue); + return E_NOTIMPL; +} + +static const ID3DX10ThreadPumpVtbl thread_pump_vtbl = +{ + thread_pump_QueryInterface, + thread_pump_AddRef, + thread_pump_Release, + thread_pump_AddWorkItem, + thread_pump_GetWorkItemCount, + thread_pump_WaitForAllItems, + thread_pump_ProcessDeviceWorkItems, + thread_pump_PurgeAllItems, + thread_pump_GetQueueStatus +}; + +HRESULT WINAPI D3DX10CreateThreadPump(UINT io_threads, UINT proc_threads, ID3DX10ThreadPump **pump) +{ + struct thread_pump *object; + + TRACE("io_threads %u, proc_threads %u, pump %p.\n", io_threads, proc_threads, pump); + + if (io_threads >= 1024 || proc_threads >= 1024) + return E_FAIL; + + if (!(object = calloc(1, sizeof(*object)))) + return E_OUTOFMEMORY; + + object->ID3DX10ThreadPump_iface.lpVtbl = &thread_pump_vtbl; + object->refcount = 1; + + *pump = &object->ID3DX10ThreadPump_iface; + return S_OK; +} diff --git a/include/d3dx10core.h b/include/d3dx10core.h index a9ba7854e90..cca9052cc13 100644 --- a/include/d3dx10core.h +++ b/include/d3dx10core.h @@ -298,3 +298,4 @@ HRESULT WINAPI D3DX10CreateFontW(ID3D10Device *device, INT height, UINT width, U UINT miplevels, BOOL italic, UINT charset, UINT precision, UINT quality, UINT pitchandfamily, const WCHAR *facename, ID3DX10Font **font); HRESULT WINAPI D3DX10CreateSprite(ID3D10Device *device, UINT size, ID3DX10Sprite **sprite); +HRESULT WINAPI D3DX10CreateThreadPump(UINT io_threads, UINT proc_threads, ID3DX10ThreadPump **pump);
From: Piotr Caban piotr@codeweavers.com
Signed-off-by: Piotr Caban piotr@codeweavers.com --- dlls/d3dx10_43/d3dx10_43_main.c | 251 +++++++++++++++++++++++++++++++- 1 file changed, 248 insertions(+), 3 deletions(-)
diff --git a/dlls/d3dx10_43/d3dx10_43_main.c b/dlls/d3dx10_43/d3dx10_43_main.c index 357b0257496..d66d83d3e7f 100644 --- a/dlls/d3dx10_43/d3dx10_43_main.c +++ b/dlls/d3dx10_43/d3dx10_43_main.c @@ -29,10 +29,13 @@ #include "winbase.h" #include "winuser.h" #include "objbase.h" +#include "winternl.h"
#include "d3d10_1.h" #include "d3dx10.h"
+#include "wine/list.h" + WINE_DEFAULT_DEBUG_CHANNEL(d3dx);
/*********************************************************************** @@ -177,10 +180,45 @@ HRESULT WINAPI D3DX10LoadTextureFromTexture(ID3D10Resource *src_texture, D3DX10_ return E_NOTIMPL; }
+struct work_item +{ + struct list entry; + + ID3DX10DataLoader *loader; + ID3DX10DataProcessor *processor; + HRESULT *result; + void **object; +}; + +static inline void work_item_free(struct work_item *work_item, BOOL cancel) +{ + ID3DX10DataLoader_Destroy(work_item->loader); + ID3DX10DataProcessor_Destroy(work_item->processor); + if (cancel && work_item->result) + *work_item->result = S_FALSE; + free(work_item); +} + +#define THREAD_PUMP_EXITING UINT_MAX struct thread_pump { ID3DX10ThreadPump ID3DX10ThreadPump_iface; LONG refcount; + + SRWLOCK io_lock; + UINT io_count; + struct list io_queue; + + SRWLOCK proc_lock; + UINT proc_count; + struct list proc_queue; + + SRWLOCK device_lock; + UINT device_count; + struct list device_queue; + + int threads_no; + HANDLE threads[1]; };
static inline struct thread_pump *impl_from_ID3DX10ThreadPump(ID3DX10ThreadPump *iface) @@ -219,11 +257,48 @@ static ULONG WINAPI thread_pump_Release(ID3DX10ThreadPump *iface) { struct thread_pump *thread_pump = impl_from_ID3DX10ThreadPump(iface); ULONG refcount = InterlockedDecrement(&thread_pump->refcount); + struct work_item *iter, *next; + struct list list; + int i;
TRACE("%p decreasing refcount to %lu.\n", iface, refcount);
if (!refcount) + { + AcquireSRWLockExclusive(&thread_pump->io_lock); + thread_pump->io_count = THREAD_PUMP_EXITING; + ReleaseSRWLockExclusive(&thread_pump->io_lock); + RtlWakeAddressAll((void *)&thread_pump->io_count); + + AcquireSRWLockExclusive(&thread_pump->proc_lock); + thread_pump->proc_count = THREAD_PUMP_EXITING; + ReleaseSRWLockExclusive(&thread_pump->proc_lock); + RtlWakeAddressAll((void *)&thread_pump->proc_count); + + AcquireSRWLockExclusive(&thread_pump->device_lock); + thread_pump->device_count = THREAD_PUMP_EXITING; + ReleaseSRWLockExclusive(&thread_pump->device_lock); + + for (i = 0; i < thread_pump->threads_no; i++) + { + if (!thread_pump->threads[i]) continue; + + WaitForSingleObject(thread_pump->threads[i], INFINITE); + CloseHandle(thread_pump->threads[i]); + } + + list_init(&list); + list_move_tail(&list, &thread_pump->io_queue); + list_move_tail(&list, &thread_pump->proc_queue); + list_move_tail(&list, &thread_pump->device_queue); + LIST_FOR_EACH_ENTRY_SAFE(iter, next, &list, struct work_item, entry) + { + list_remove(&iter->entry); + work_item_free(iter, TRUE); + } + free(thread_pump); + }
return refcount; } @@ -231,9 +306,30 @@ static ULONG WINAPI thread_pump_Release(ID3DX10ThreadPump *iface) static HRESULT WINAPI thread_pump_AddWorkItem(ID3DX10ThreadPump *iface, ID3DX10DataLoader *loader, ID3DX10DataProcessor *processor, HRESULT *result, void **object) { - FIXME("iface %p, loader %p, processor %p, result %p, object %p stub!\n", + struct thread_pump *thread_pump = impl_from_ID3DX10ThreadPump(iface); + struct work_item *work_item; + + TRACE("iface %p, loader %p, processor %p, result %p, object %p.\n", iface, loader, processor, result, object); - return E_NOTIMPL; + + work_item = malloc(sizeof(*work_item)); + if (!work_item) + return E_OUTOFMEMORY; + + work_item->loader = loader; + work_item->processor = processor; + work_item->result = result; + work_item->object = object; + + if (object) + *object = NULL; + + AcquireSRWLockExclusive(&thread_pump->io_lock); + thread_pump->io_count++; + list_add_tail(&thread_pump->io_queue, &work_item->entry); + ReleaseSRWLockExclusive(&thread_pump->io_lock); + RtlWakeAddressAll((void *)&thread_pump->io_count); + return S_OK; }
static UINT WINAPI thread_pump_GetWorkItemCount(ID3DX10ThreadPump *iface) @@ -281,20 +377,169 @@ static const ID3DX10ThreadPumpVtbl thread_pump_vtbl = thread_pump_GetQueueStatus };
+static DWORD WINAPI io_thread(void *arg) +{ + struct thread_pump *thread_pump = arg; + struct work_item *work_item; + UINT zero = 0; + HRESULT hr; + + TRACE("%p thread started.\n", thread_pump); + + while (1) + { + RtlWaitOnAddress((void *)&thread_pump->io_count, &zero, sizeof(zero), NULL); + AcquireSRWLockExclusive(&thread_pump->io_lock); + if (!thread_pump->io_count) + { + ReleaseSRWLockExclusive(&thread_pump->io_lock); + continue; + } + if (thread_pump->io_count == THREAD_PUMP_EXITING) + { + ReleaseSRWLockExclusive(&thread_pump->io_lock); + return 0; + } + + thread_pump->io_count--; + work_item = LIST_ENTRY(list_head(&thread_pump->io_queue), struct work_item, entry); + list_remove(&work_item->entry); + ReleaseSRWLockExclusive(&thread_pump->io_lock); + + if (FAILED(hr = ID3DX10DataLoader_Load(work_item->loader))) + { + if (work_item->result) + *work_item->result = hr; + work_item_free(work_item, FALSE); + continue; + } + + AcquireSRWLockExclusive(&thread_pump->proc_lock); + if (thread_pump->proc_count == THREAD_PUMP_EXITING) + { + ReleaseSRWLockExclusive(&thread_pump->proc_lock); + work_item_free(work_item, TRUE); + return 0; + } + + list_add_tail(&thread_pump->proc_queue, &work_item->entry); + thread_pump->proc_count++; + ReleaseSRWLockExclusive(&thread_pump->proc_lock); + RtlWakeAddressAll((void *)&thread_pump->proc_count); + } + return 0; +} + +static DWORD WINAPI proc_thread(void *arg) +{ + struct thread_pump *thread_pump = arg; + struct work_item *work_item; + UINT zero = 0; + SIZE_T size; + void *data; + HRESULT hr; + + TRACE("%p thread started.\n", thread_pump); + + while (1) + { + RtlWaitOnAddress((void *)&thread_pump->proc_count, &zero, sizeof(zero), NULL); + AcquireSRWLockExclusive(&thread_pump->proc_lock); + if (!thread_pump->proc_count) + { + ReleaseSRWLockExclusive(&thread_pump->proc_lock); + continue; + } + if (thread_pump->proc_count == THREAD_PUMP_EXITING) + { + ReleaseSRWLockExclusive(&thread_pump->proc_lock); + return 0; + } + + thread_pump->proc_count--; + work_item = LIST_ENTRY(list_head(&thread_pump->proc_queue), struct work_item, entry); + list_remove(&work_item->entry); + ReleaseSRWLockExclusive(&thread_pump->proc_lock); + + if (FAILED(hr = ID3DX10DataLoader_Decompress(work_item->loader, &data, &size))) + { + if (work_item->result) + *work_item->result = hr; + work_item_free(work_item, FALSE); + continue; + } + + if (thread_pump->device_count == THREAD_PUMP_EXITING) + { + work_item_free(work_item, TRUE); + return 0; + } + + if (FAILED(hr = ID3DX10DataProcessor_Process(work_item->processor, data, size))) + { + if (work_item->result) + *work_item->result = hr; + work_item_free(work_item, FALSE); + continue; + } + + AcquireSRWLockExclusive(&thread_pump->device_lock); + if (thread_pump->device_count == THREAD_PUMP_EXITING) + { + ReleaseSRWLockExclusive(&thread_pump->device_lock); + work_item_free(work_item, TRUE); + return 0; + } + + list_add_tail(&thread_pump->device_queue, &work_item->entry); + thread_pump->device_count++; + ReleaseSRWLockExclusive(&thread_pump->device_lock); + } + return 0; +} + HRESULT WINAPI D3DX10CreateThreadPump(UINT io_threads, UINT proc_threads, ID3DX10ThreadPump **pump) { struct thread_pump *object; + int i;
TRACE("io_threads %u, proc_threads %u, pump %p.\n", io_threads, proc_threads, pump);
if (io_threads >= 1024 || proc_threads >= 1024) return E_FAIL;
- if (!(object = calloc(1, sizeof(*object)))) + if (!io_threads) + io_threads = 1; + if (!proc_threads) + { + SYSTEM_INFO info; + + GetSystemInfo(&info); + proc_threads = info.dwNumberOfProcessors; + } + + if (!(object = calloc(1, FIELD_OFFSET(struct thread_pump, threads[io_threads + proc_threads])))) return E_OUTOFMEMORY;
object->ID3DX10ThreadPump_iface.lpVtbl = &thread_pump_vtbl; object->refcount = 1; + InitializeSRWLock(&object->io_lock); + list_init(&object->io_queue); + InitializeSRWLock(&object->proc_lock); + list_init(&object->proc_queue); + InitializeSRWLock(&object->device_lock); + list_init(&object->device_queue); + object->threads_no = io_threads + proc_threads; + + for (i = 0; i < object->threads_no; i++) + { + object->threads[i] = CreateThread(NULL, 0, i < io_threads ? io_thread : proc_thread, object, 0, NULL); + if (!object->threads[i]) + { + ID3DX10ThreadPump_Release(&object->ID3DX10ThreadPump_iface); + return E_FAIL; + } + }
*pump = &object->ID3DX10ThreadPump_iface; return S_OK;
On 6/18/22 14:21, Piotr Caban wrote:
+static DWORD WINAPI io_thread(void *arg) +{
- struct thread_pump *thread_pump = arg;
- struct work_item *work_item;
- UINT zero = 0;
- HRESULT hr;
- TRACE("%p thread started.\n", thread_pump);
- while (1)
- {
RtlWaitOnAddress((void *)&thread_pump->io_count, &zero, sizeof(zero), NULL);AcquireSRWLockExclusive(&thread_pump->io_lock);if (!thread_pump->io_count){ReleaseSRWLockExclusive(&thread_pump->io_lock);continue;}
This works, but it strikes me as simpler (and more idiomatic) just to use a condition variable. Any reason not to do that?
Note also that "zero" can be static const.
From: Piotr Caban piotr@codeweavers.com
Signed-off-by: Piotr Caban piotr@codeweavers.com --- dlls/d3dx10_43/d3dx10_43_main.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/dlls/d3dx10_43/d3dx10_43_main.c b/dlls/d3dx10_43/d3dx10_43_main.c index d66d83d3e7f..5d718f6de2b 100644 --- a/dlls/d3dx10_43/d3dx10_43_main.c +++ b/dlls/d3dx10_43/d3dx10_43_main.c @@ -205,6 +205,8 @@ struct thread_pump ID3DX10ThreadPump ID3DX10ThreadPump_iface; LONG refcount;
+ LONG processing_count; + SRWLOCK io_lock; UINT io_count; struct list io_queue; @@ -324,6 +326,7 @@ static HRESULT WINAPI thread_pump_AddWorkItem(ID3DX10ThreadPump *iface, ID3DX10D if (object) *object = NULL;
+ InterlockedIncrement(&thread_pump->processing_count); AcquireSRWLockExclusive(&thread_pump->io_lock); thread_pump->io_count++; list_add_tail(&thread_pump->io_queue, &work_item->entry); @@ -334,8 +337,15 @@ static HRESULT WINAPI thread_pump_AddWorkItem(ID3DX10ThreadPump *iface, ID3DX10D
static UINT WINAPI thread_pump_GetWorkItemCount(ID3DX10ThreadPump *iface) { - FIXME("iface %p stub!\n", iface); - return 0; + struct thread_pump *thread_pump = impl_from_ID3DX10ThreadPump(iface); + UINT ret; + + TRACE("iface %p.\n", iface); + + AcquireSRWLockExclusive(&thread_pump->device_lock); + ret = thread_pump->processing_count + thread_pump->device_count; + ReleaseSRWLockExclusive(&thread_pump->device_lock); + return ret; }
static HRESULT WINAPI thread_pump_WaitForAllItems(ID3DX10ThreadPump *iface) @@ -411,6 +421,7 @@ static DWORD WINAPI io_thread(void *arg) if (work_item->result) *work_item->result = hr; work_item_free(work_item, FALSE); + InterlockedDecrement(&thread_pump->processing_count); continue; }
@@ -466,6 +477,7 @@ static DWORD WINAPI proc_thread(void *arg) if (work_item->result) *work_item->result = hr; work_item_free(work_item, FALSE); + InterlockedDecrement(&thread_pump->processing_count); continue; }
@@ -480,6 +492,7 @@ static DWORD WINAPI proc_thread(void *arg) if (work_item->result) *work_item->result = hr; work_item_free(work_item, FALSE); + InterlockedDecrement(&thread_pump->processing_count); continue; }
@@ -493,6 +506,7 @@ static DWORD WINAPI proc_thread(void *arg)
list_add_tail(&thread_pump->device_queue, &work_item->entry); thread_pump->device_count++; + InterlockedDecrement(&thread_pump->processing_count); ReleaseSRWLockExclusive(&thread_pump->device_lock); } return 0;
From: Piotr Caban piotr@codeweavers.com
Signed-off-by: Piotr Caban piotr@codeweavers.com --- dlls/d3dx10_43/d3dx10_43_main.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/dlls/d3dx10_43/d3dx10_43_main.c b/dlls/d3dx10_43/d3dx10_43_main.c index 5d718f6de2b..795d052af83 100644 --- a/dlls/d3dx10_43/d3dx10_43_main.c +++ b/dlls/d3dx10_43/d3dx10_43_main.c @@ -356,8 +356,34 @@ static HRESULT WINAPI thread_pump_WaitForAllItems(ID3DX10ThreadPump *iface)
static HRESULT WINAPI thread_pump_ProcessDeviceWorkItems(ID3DX10ThreadPump *iface, UINT count) { - FIXME("iface %p, count %u stub!\n", iface, count); - return E_NOTIMPL; + struct thread_pump *thread_pump = impl_from_ID3DX10ThreadPump(iface); + struct work_item *work_item; + HRESULT hr; + UINT i; + + TRACE("iface %p, count %u.\n", iface, count); + + for (i = 0; i < count; i++) + { + AcquireSRWLockExclusive(&thread_pump->device_lock); + if (!thread_pump->device_count) + { + ReleaseSRWLockExclusive(&thread_pump->device_lock); + break; + } + + thread_pump->device_count--; + work_item = LIST_ENTRY(list_head(&thread_pump->device_queue), struct work_item, entry); + list_remove(&work_item->entry); + ReleaseSRWLockExclusive(&thread_pump->device_lock); + + hr = ID3DX10DataProcessor_CreateDeviceObject(work_item->processor, work_item->object); + if (work_item->result) + *work_item->result = hr; + work_item_free(work_item, FALSE); + } + + return S_OK; }
static HRESULT WINAPI thread_pump_PurgeAllItems(ID3DX10ThreadPump *iface)
From: Piotr Caban piotr@codeweavers.com
Signed-off-by: Piotr Caban piotr@codeweavers.com --- dlls/d3dx10_43/d3dx10_43_main.c | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/dlls/d3dx10_43/d3dx10_43_main.c b/dlls/d3dx10_43/d3dx10_43_main.c index 795d052af83..198cf14b7d0 100644 --- a/dlls/d3dx10_43/d3dx10_43_main.c +++ b/dlls/d3dx10_43/d3dx10_43_main.c @@ -350,8 +350,24 @@ static UINT WINAPI thread_pump_GetWorkItemCount(ID3DX10ThreadPump *iface)
static HRESULT WINAPI thread_pump_WaitForAllItems(ID3DX10ThreadPump *iface) { - FIXME("iface %p stub!\n", iface); - return E_NOTIMPL; + struct thread_pump *thread_pump = impl_from_ID3DX10ThreadPump(iface); + HRESULT hr; + LONG v; + + TRACE("iface %p.\n", iface); + + while(1) + { + if (FAILED((hr = ID3DX10ThreadPump_ProcessDeviceWorkItems(iface, UINT_MAX)))) + return hr; + + if (!(v = thread_pump->processing_count)) + break; + + RtlWaitOnAddress((void *)&thread_pump->processing_count, &v, sizeof(v), NULL); + } + + return ID3DX10ThreadPump_ProcessDeviceWorkItems(iface, UINT_MAX); }
static HRESULT WINAPI thread_pump_ProcessDeviceWorkItems(ID3DX10ThreadPump *iface, UINT count) @@ -447,7 +463,8 @@ static DWORD WINAPI io_thread(void *arg) if (work_item->result) *work_item->result = hr; work_item_free(work_item, FALSE); - InterlockedDecrement(&thread_pump->processing_count); + if (!InterlockedDecrement(&thread_pump->processing_count)) + RtlWakeAddressAll((void *)&thread_pump->processing_count); continue; }
@@ -503,7 +520,8 @@ static DWORD WINAPI proc_thread(void *arg) if (work_item->result) *work_item->result = hr; work_item_free(work_item, FALSE); - InterlockedDecrement(&thread_pump->processing_count); + if (!InterlockedDecrement(&thread_pump->processing_count)) + RtlWakeAddressAll((void *)&thread_pump->processing_count); continue; }
@@ -518,7 +536,8 @@ static DWORD WINAPI proc_thread(void *arg) if (work_item->result) *work_item->result = hr; work_item_free(work_item, FALSE); - InterlockedDecrement(&thread_pump->processing_count); + if (!InterlockedDecrement(&thread_pump->processing_count)) + RtlWakeAddressAll((void *)&thread_pump->processing_count); continue; }
@@ -533,6 +552,7 @@ static DWORD WINAPI proc_thread(void *arg) list_add_tail(&thread_pump->device_queue, &work_item->entry); thread_pump->device_count++; InterlockedDecrement(&thread_pump->processing_count); + RtlWakeAddressAll((void *)&thread_pump->processing_count); ReleaseSRWLockExclusive(&thread_pump->device_lock); } return 0;
From: Piotr Caban piotr@codeweavers.com
Signed-off-by: Piotr Caban piotr@codeweavers.com --- dlls/d3dx10_43/d3dx10_43_main.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/dlls/d3dx10_43/d3dx10_43_main.c b/dlls/d3dx10_43/d3dx10_43_main.c index 198cf14b7d0..5804a1058b0 100644 --- a/dlls/d3dx10_43/d3dx10_43_main.c +++ b/dlls/d3dx10_43/d3dx10_43_main.c @@ -411,9 +411,15 @@ static HRESULT WINAPI thread_pump_PurgeAllItems(ID3DX10ThreadPump *iface) static HRESULT WINAPI thread_pump_GetQueueStatus(ID3DX10ThreadPump *iface, UINT *io_queue, UINT *process_queue, UINT *device_queue) { - FIXME("iface %p, io_queue %p, process_queue %p, device_queue %p stub!\n", + struct thread_pump *thread_pump = impl_from_ID3DX10ThreadPump(iface); + + TRACE("iface %p, io_queue %p, process_queue %p, device_queue %p.\n", iface, io_queue, process_queue, device_queue); - return E_NOTIMPL; + + *io_queue = thread_pump->io_count; + *process_queue = thread_pump->proc_count; + *device_queue = thread_pump->device_count; + return S_OK; }
static const ID3DX10ThreadPumpVtbl thread_pump_vtbl =
From: Piotr Caban piotr@codeweavers.com
Signed-off-by: Piotr Caban piotr@codeweavers.com --- dlls/d3dx10_43/d3dx10_43_main.c | 48 +++++++++++++++++++++++++++++++-- 1 file changed, 46 insertions(+), 2 deletions(-)
diff --git a/dlls/d3dx10_43/d3dx10_43_main.c b/dlls/d3dx10_43/d3dx10_43_main.c index 5804a1058b0..df1e3f413f5 100644 --- a/dlls/d3dx10_43/d3dx10_43_main.c +++ b/dlls/d3dx10_43/d3dx10_43_main.c @@ -404,8 +404,52 @@ static HRESULT WINAPI thread_pump_ProcessDeviceWorkItems(ID3DX10ThreadPump *ifac
static HRESULT WINAPI thread_pump_PurgeAllItems(ID3DX10ThreadPump *iface) { - FIXME("iface %p stub!\n", iface); - return E_NOTIMPL; + struct thread_pump *thread_pump = impl_from_ID3DX10ThreadPump(iface); + struct list list = LIST_INIT(list); + struct work_item *work_item; + LONG v; + + TRACE("iface %p.\n", iface); + + while (1) + { + AcquireSRWLockExclusive(&thread_pump->io_lock); + list_move_tail(&list, &thread_pump->io_queue); + thread_pump->io_count = 0; + ReleaseSRWLockExclusive(&thread_pump->io_lock); + + AcquireSRWLockExclusive(&thread_pump->proc_lock); + list_move_tail(&list, &thread_pump->proc_queue); + thread_pump->proc_count = 0; + ReleaseSRWLockExclusive(&thread_pump->proc_lock); + + while (!list_empty(&list)) + { + work_item = LIST_ENTRY(list_head(&list), struct work_item, entry); + list_remove(&work_item->entry); + work_item_free(work_item, TRUE); + if (!InterlockedDecrement(&thread_pump->processing_count)) + RtlWakeAddressAll((void *)&thread_pump->processing_count); + } + + if (!(v = thread_pump->processing_count)) + break; + + RtlWaitOnAddress((void *)&thread_pump->processing_count, &v, sizeof(v), NULL); + } + + AcquireSRWLockExclusive(&thread_pump->device_lock); + list_move_tail(&list, &thread_pump->device_queue); + thread_pump->device_count = 0; + ReleaseSRWLockExclusive(&thread_pump->device_lock); + + while(!list_empty(&list)) + { + work_item = LIST_ENTRY(list_head(&list), struct work_item, entry); + list_remove(&work_item->entry); + work_item_free(work_item, TRUE); + } + return S_OK; }
static HRESULT WINAPI thread_pump_GetQueueStatus(ID3DX10ThreadPump *iface,
From: Piotr Caban piotr@codeweavers.com
Signed-off-by: Piotr Caban piotr@codeweavers.com --- dlls/d3dx10_43/tests/d3dx10.c | 356 ++++++++++++++++++++++++++++++++++ 1 file changed, 356 insertions(+)
diff --git a/dlls/d3dx10_43/tests/d3dx10.c b/dlls/d3dx10_43/tests/d3dx10.c index d1c772197b9..5300a568999 100644 --- a/dlls/d3dx10_43/tests/d3dx10.c +++ b/dlls/d3dx10_43/tests/d3dx10.c @@ -2066,6 +2066,361 @@ static void test_D3DX10CreateAsyncTextureProcessor(void) ok(!ID3D10Device_Release(device), "Unexpected refcount.\n"); }
+static DWORD main_tid; +static DWORD io_tid; + +struct data_object +{ + ID3DX10DataLoader ID3DX10DataLoader_iface; + ID3DX10DataProcessor ID3DX10DataProcessor_iface; + + HANDLE load_started; + HANDLE load_done; + HANDLE decompress_done; + HRESULT load_ret; + + DWORD process_tid; +}; + +static struct data_object* data_object_from_ID3DX10DataLoader(ID3DX10DataLoader *iface) +{ + return CONTAINING_RECORD(iface, struct data_object, ID3DX10DataLoader_iface); +} + +static LONG data_loader_load_count; +static WINAPI HRESULT data_loader_Load(ID3DX10DataLoader *iface) +{ + struct data_object *data_object = data_object_from_ID3DX10DataLoader(iface); + DWORD ret; + + ok(InterlockedDecrement(&data_loader_load_count) >= 0, "unexpected call\n"); + + if (io_tid) + io_tid = GetCurrentThreadId(); + ok(io_tid != main_tid, "Load called in main thread.\n"); + + SetEvent(data_object->load_started); + ret = WaitForSingleObject(data_object->load_done, INFINITE); + ok(ret == WAIT_OBJECT_0, "WaitForSingleObject returned %x\n", ret); + return data_object->load_ret; +} + +static LONG data_loader_decompress_count; +static WINAPI HRESULT data_loader_Decompress(ID3DX10DataLoader *iface, void **data, SIZE_T *bytes) +{ + struct data_object *data_object = data_object_from_ID3DX10DataLoader(iface); + DWORD ret; + + ok(InterlockedDecrement(&data_loader_decompress_count) >= 0, "unexpected call\n"); + ok(data != NULL, "data == NULL\n"); + ok(bytes != NULL, "bytes == NULL\n"); + + data_object->process_tid = GetCurrentThreadId(); + ok(data_object->process_tid != main_tid, "Decompress called in main thread.\n"); + ok(data_object->process_tid != io_tid, "Decompress called in IO thread.\n"); + + *data = (void*)0xdeadbeef; + *bytes = 0xdead; + ret = WaitForSingleObject(data_object->decompress_done, INFINITE); + ok(ret == WAIT_OBJECT_0, "WaitForSingleObject returned %x\n", ret); + return S_OK; +} + +static LONG data_loader_destroy_count; +static WINAPI HRESULT data_loader_Destroy(ID3DX10DataLoader *iface) +{ + ok(InterlockedDecrement(&data_loader_destroy_count) >= 0, "unexpected call\n"); + return S_OK; +} + +static ID3DX10DataLoaderVtbl D3DX10DataLoaderVtbl = +{ + data_loader_Load, + data_loader_Decompress, + data_loader_Destroy +}; + +static struct data_object* data_object_from_ID3DX10DataProcessor(ID3DX10DataProcessor *iface) +{ + return CONTAINING_RECORD(iface, struct data_object, ID3DX10DataProcessor_iface); +} + +static LONG data_processor_process_count; +static HRESULT WINAPI data_processor_Process(ID3DX10DataProcessor *iface, void *data, SIZE_T bytes) +{ + struct data_object *data_object = data_object_from_ID3DX10DataProcessor(iface); + + ok(InterlockedDecrement(&data_processor_process_count) >= 0, "unexpected call\n"); + ok(GetCurrentThreadId() == data_object->process_tid, "Process called in different thread.\n"); + + ok(data == (void*)0xdeadbeef, "data = %p\n", data); + ok(bytes == 0xdead, "bytes = %lu\n", bytes); + return S_OK; +} + +static LONG data_processor_create_count; +static HRESULT WINAPI data_processor_CreateDeviceObject(ID3DX10DataProcessor *iface, void **object) +{ + ok(InterlockedDecrement(&data_processor_create_count) >= 0, "unexpected call\n"); + ok(GetCurrentThreadId() == main_tid, "CreateDeviceObject not called in main thread.\n"); + + *object = (void*)0xdeadf00d; + return S_OK; +} + +static LONG data_processor_destroy_count; +static HRESULT WINAPI data_processor_Destroy(ID3DX10DataProcessor *iface) +{ + struct data_object *data_object = data_object_from_ID3DX10DataProcessor(iface); + + ok(InterlockedDecrement(&data_processor_destroy_count) >= 0, "unexpected call\n"); + + CloseHandle(data_object->load_started); + CloseHandle(data_object->load_done); + CloseHandle(data_object->decompress_done); + free(data_object); + return S_OK; +} + +static ID3DX10DataProcessorVtbl D3DX10DataProcessorVtbl = +{ + data_processor_Process, + data_processor_CreateDeviceObject, + data_processor_Destroy +}; + +static struct data_object* create_data_object(HRESULT load_ret) +{ + struct data_object *data_object = malloc(sizeof(*data_object)); + + data_object->ID3DX10DataLoader_iface.lpVtbl = &D3DX10DataLoaderVtbl; + data_object->ID3DX10DataProcessor_iface.lpVtbl = &D3DX10DataProcessorVtbl; + + data_object->load_started = CreateEventW(NULL, FALSE, FALSE, NULL); + ok(data_object->load_started != NULL, "CreateEvent failed (%u)\n", GetLastError()); + data_object->load_done = CreateEventW(NULL, FALSE, FALSE, NULL); + ok(data_object->load_done != NULL, "CreateEvent failed (%u)\n", GetLastError()); + data_object->decompress_done = CreateEventW(NULL, FALSE, FALSE, NULL); + ok(data_object->decompress_done != NULL, "CreateEvent failed (%u)\n", GetLastError()); + data_object->load_ret = load_ret; + + return data_object; +} + +static void test_D3DX10CreateThreadPump(void) +{ + UINT io_count, process_count, device_count, count; + struct data_object *data_object[2]; + ID3DX10DataProcessor *processor; + D3DX10_IMAGE_INFO image_info; + ID3DX10DataLoader *loader; + HRESULT hr, work_item_hr; + ID3D10Resource *resource; + ID3DX10ThreadPump *pump; + ID3D10Device *device; + SYSTEM_INFO info; + void *object; + DWORD ret; + int i; + + main_tid = GetCurrentThreadId(); + + hr = D3DX10CreateThreadPump(1024, 0, &pump); + ok(hr == E_FAIL, "Got unexpected hr %#x.\n", hr); + hr = D3DX10CreateThreadPump(0, 1024, &pump); + ok(hr == E_FAIL, "Got unexpected hr %#x.\n", hr); + + GetSystemInfo(&info); + if (info.dwNumberOfProcessors > 1) + hr = D3DX10CreateThreadPump(0, 0, &pump); + else + hr = D3DX10CreateThreadPump(0, 2, &pump); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + + count = ID3DX10ThreadPump_GetWorkItemCount(pump); + ok(!count, "GetWorkItemCount returned %u.\n", count); + hr = ID3DX10ThreadPump_GetQueueStatus(pump, &io_count, &process_count, &device_count); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + ok(!io_count, "Got unexpected io_count = %u.\n", io_count); + ok(!process_count, "Got unexpected process_count = %u.\n", process_count); + ok(!device_count, "Got unexpected device_count = %u.\n", device_count); + + data_object[0] = create_data_object(E_NOTIMPL); + data_object[1] = create_data_object(S_OK); + + data_loader_load_count = 1; + hr = ID3DX10ThreadPump_AddWorkItem(pump, &data_object[0]->ID3DX10DataLoader_iface, + &data_object[0]->ID3DX10DataProcessor_iface, &work_item_hr, NULL); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + ret = WaitForSingleObject(data_object[0]->load_started, INFINITE); + ok(ret == WAIT_OBJECT_0, "WaitForSingleObject returned %x.\n", ret); + ok(!data_loader_load_count, "Got unexpected data_loader_load_count %d.\n", data_loader_load_count); + count = ID3DX10ThreadPump_GetWorkItemCount(pump); + ok(count == 1, "GetWorkItemCount returned %u.\n", count); + hr = ID3DX10ThreadPump_GetQueueStatus(pump, &io_count, &process_count, &device_count); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + ok(!io_count, "Got unexpected io_count = %u.\n", io_count); + ok(!process_count, "Got unexpected process_count = %u.\n", process_count); + ok(!device_count, "Got unexpected device_count = %u.\n", device_count); + + hr = ID3DX10ThreadPump_AddWorkItem(pump, &data_object[1]->ID3DX10DataLoader_iface, + &data_object[1]->ID3DX10DataProcessor_iface, NULL, &object); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + ret = WaitForSingleObject(data_object[0]->load_started, 50); + ok(ret == WAIT_TIMEOUT, "WaitForSingleObject returned %x.\n", ret); + count = ID3DX10ThreadPump_GetWorkItemCount(pump); + ok(count == 2, "GetWorkItemCount returned %u.\n", count); + hr = ID3DX10ThreadPump_GetQueueStatus(pump, &io_count, &process_count, &device_count); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + ok(io_count == 1, "Got unexpected io_count = %u.\n", io_count); + ok(!process_count, "Got unexpected process_count = %u.\n", process_count); + ok(!device_count, "Got unexpected device_count = %u.\n", device_count); + + data_loader_load_count = 1; + data_loader_destroy_count = 1; + data_processor_destroy_count = 1; + SetEvent(data_object[0]->load_done); + ret = WaitForSingleObject(data_object[1]->load_started, INFINITE); + ok(ret == WAIT_OBJECT_0, "WaitForSingleObject returned %x.\n", ret); + ok(work_item_hr == E_NOTIMPL, "Got unexpected work_item_hr = %#x.\n", work_item_hr); + ok(!data_loader_destroy_count, "Got unexpected data_loader_destroy_count %d.\n", data_loader_destroy_count); + ok(!data_processor_destroy_count, "Got unexpected data_processor_destroy_count %d.\n", + data_processor_destroy_count); + ok(!data_loader_load_count, "Got unexpected data_loader_load_count %d.\n", data_loader_load_count); + + data_loader_decompress_count = 1; + data_processor_process_count = 1; + SetEvent(data_object[1]->load_done); + SetEvent(data_object[1]->decompress_done); + + data_processor_create_count = 1; + data_loader_destroy_count = 1; + data_processor_destroy_count = 1; + hr = ID3DX10ThreadPump_WaitForAllItems(pump); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + ok(object == (void*)0xdeadf00d, "Got unexpected object = %p.\n", object); + ok(!data_loader_decompress_count, "Got unexpected data_loader_decompress_count %d.\n", data_loader_decompress_count); + ok(!data_processor_process_count, "Got unexpected data_processor_process_count %d.\n", data_processor_process_count); + ok(!data_processor_create_count, "Got unexpected data_processor_create_count %d.\n", data_processor_create_count); + ok(!data_loader_destroy_count, "Got unexpected data_loader_destroy_count %d.\n", data_loader_destroy_count); + ok(!data_processor_destroy_count, "Got unexpected data_processor_destroy_count %d.\n", data_processor_destroy_count); + + data_object[0] = create_data_object(S_OK); + data_object[1] = create_data_object(S_OK); + SetEvent(data_object[0]->load_done); + SetEvent(data_object[1]->load_done); + SetEvent(data_object[1]->decompress_done); + + data_loader_load_count = 2; + data_loader_decompress_count = 2; + data_processor_process_count = 1; + hr = ID3DX10ThreadPump_AddWorkItem(pump, &data_object[0]->ID3DX10DataLoader_iface, + &data_object[0]->ID3DX10DataProcessor_iface, NULL, &object); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + hr = ID3DX10ThreadPump_AddWorkItem(pump, &data_object[1]->ID3DX10DataLoader_iface, + &data_object[1]->ID3DX10DataProcessor_iface, NULL, &object); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + while(1) + { + hr = ID3DX10ThreadPump_GetQueueStatus(pump, &io_count, &process_count, &device_count); + if (hr != S_OK || device_count) + break; + Sleep(1); + } + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + ok(!io_count, "Got unexpected io_count = %u.\n", io_count); + ok(!process_count, "Got unexpected process_count = %u.\n", process_count); + ok(device_count == 1, "Got unexpected device_count = %u.\n", device_count); + ok(!data_loader_load_count, "Got unexpected data_loader_load_count %d.\n", data_loader_load_count); + ok(!data_loader_decompress_count, "Got unexpected data_loader_decompress_count %d.\n", data_loader_decompress_count); + ok(!data_processor_process_count, "Got unexpected data_processor_process_count %d.\n", data_processor_process_count); + + hr = ID3DX10ThreadPump_ProcessDeviceWorkItems(pump, 0); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + hr = ID3DX10ThreadPump_GetQueueStatus(pump, &io_count, &process_count, &device_count); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + ok(!io_count, "Got unexpected io_count = %u.\n", io_count); + ok(!process_count, "Got unexpected process_count = %u.\n", process_count); + ok(device_count == 1, "Got unexpected device_count = %u.\n", device_count); + + data_processor_create_count = 1; + data_loader_destroy_count = 1; + data_processor_destroy_count = 1; + hr = ID3DX10ThreadPump_ProcessDeviceWorkItems(pump, 1); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + hr = ID3DX10ThreadPump_GetQueueStatus(pump, &io_count, &process_count, &device_count); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + ok(!io_count, "Got unexpected io_count = %u.\n", io_count); + ok(!process_count, "Got unexpected process_count = %u.\n", process_count); + ok(!device_count, "Got unexpected device_count = %u.\n", device_count); + ok(!data_processor_create_count, "Got unexpected data_processor_create_count %d.\n", data_processor_create_count); + ok(!data_loader_destroy_count, "Got unexpected data_loader_destroy_count %d.\n", data_loader_destroy_count); + ok(!data_processor_destroy_count, "Got unexpected data_processor_destroy_count %d.\n", data_processor_destroy_count); + + data_processor_process_count = 1; + data_loader_destroy_count = 1; + data_processor_destroy_count = 1; + SetEvent(data_object[0]->decompress_done); + hr = ID3DX10ThreadPump_PurgeAllItems(pump); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + data_processor_process_count = 0; + ok(!data_loader_destroy_count, "Got unexpected data_loader_destroy_count %d.\n", data_loader_destroy_count); + ok(!data_processor_destroy_count, "Got unexpected data_processor_destroy_count %d.\n", data_processor_destroy_count); + + device = create_device(); + if (!device) + { + skip("Failed to create device, skipping tests.\n"); + ID3DX10ThreadPump_Release(pump); + return; + } + + for (i = 0; i < ARRAY_SIZE(test_image); ++i) + { + winetest_push_context("Test %u", i); + + hr = D3DX10CreateAsyncMemoryLoader(test_image[i].data, test_image[i].size, &loader); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + hr = D3DX10CreateAsyncTextureInfoProcessor(&image_info, &processor); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + hr = ID3DX10ThreadPump_AddWorkItem(pump, loader, processor, &work_item_hr, NULL); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + hr = ID3DX10ThreadPump_WaitForAllItems(pump); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + ok(work_item_hr == S_OK || (work_item_hr == E_FAIL + && test_image[i].expected_info.ImageFileFormat == D3DX10_IFF_WMP), + "Got unexpected hr %#x.\n", work_item_hr); + if (work_item_hr == S_OK) + check_image_info(&image_info, test_image + i, __LINE__); + + hr = D3DX10CreateAsyncMemoryLoader(test_image[i].data, test_image[i].size, &loader); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + hr = D3DX10CreateAsyncTextureProcessor(device, NULL, &processor); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + hr = ID3DX10ThreadPump_AddWorkItem(pump, loader, processor, &work_item_hr, (void **)&resource); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + hr = ID3DX10ThreadPump_WaitForAllItems(pump); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + todo_wine_if(test_image[i].expected_info.MiscFlags & D3D10_RESOURCE_MISC_TEXTURECUBE) + ok(work_item_hr == S_OK || (work_item_hr == E_FAIL + && test_image[i].expected_info.ImageFileFormat == D3DX10_IFF_WMP), + "Got unexpected hr %#x.\n", work_item_hr); + if (work_item_hr == S_OK) + { + check_resource_info(resource, test_image + i, __LINE__); + check_resource_data(resource, test_image + i, __LINE__); + ID3D10Resource_Release(resource); + } + + winetest_pop_context(); + } + + ok(!ID3D10Device_Release(device), "Unexpected refcount.\n"); + + ret = ID3DX10ThreadPump_Release(pump); + ok(!ret, "Got unexpected refcount %u.\n", ret); +} + static void test_get_image_info(void) { static const WCHAR test_resource_name[] = L"resource.data"; @@ -3573,6 +3928,7 @@ START_TEST(d3dx10) test_D3DX10CreateAsyncResourceLoader(); test_D3DX10CreateAsyncTextureInfoProcessor(); test_D3DX10CreateAsyncTextureProcessor(); + test_D3DX10CreateThreadPump(); test_get_image_info(); test_create_texture(); test_font();
I'm still looking through the patches but I might as well mention right away that we don't want to add any new code to d3dx10_43_main.c. In fact we moved a few functions out of that file when implementing them (but, as far as I'm concerned, pretty much any excuse is good to move stuff out).
async.c seems like a good place for the thread pump implementation but I'd consider other options too.
On Sat Jun 18 12:13:35 2022 +0000, **** wrote:
Marvin replied on the mailing list:
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 full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=117237 Your paranoid android. === debian11 (32 bit report) === d3dx10_43: Unhandled exception: page fault on write access to 0x00000000 in 32-bit code (0x70bb0040). === debian11 (32 bit Arabic:Morocco report) === d3dx10_43: Unhandled exception: page fault on write access to 0x00000000 in 32-bit code (0x70bb0040). === debian11 (32 bit German report) === d3dx10_43: Unhandled exception: page fault on write access to 0x00000000 in 32-bit code (0x70bb0040). === debian11 (32 bit French report) === d3dx10_43: Unhandled exception: page fault on write access to 0x00000000 in 32-bit code (0x70bb0040). === debian11 (32 bit Hebrew:Israel report) === d3dx10_43: Unhandled exception: page fault on write access to 0x00000000 in 32-bit code (0x70bb0040). === debian11 (32 bit Hindi:India report) === d3dx10_43: Unhandled exception: page fault on write access to 0x00000000 in 32-bit code (0x70bb0040). === debian11 (32 bit Japanese:Japan report) === d3dx10_43: Unhandled exception: page fault on write access to 0x00000000 in 32-bit code (0x70bb0040). === debian11 (32 bit Chinese:China report) === d3dx10_43: Unhandled exception: page fault on write access to 0x00000000 in 32-bit code (0x70bb0040). === debian11 (32 bit WoW report) === d3dx10_43: Unhandled exception: page fault on write access to 0x00000000 in 32-bit code (0x70bb0040).
I don't see any case where d3dx10_43:d3dx10 crashed in WineTest: https://test.winehq.org/data/patterns.html#d3dx10_43:d3dx10
So it does look like this patchset introduces a new failure.
On Mon Jun 20 15:42:37 2022 +0000, Francois Gouget wrote:
I don't see any case where d3dx10_43:d3dx10 crashed in WineTest: https://test.winehq.org/data/patterns.html#d3dx10_43:d3dx10 So it does look like this patchset introduces a new failure.
It crashed in v1 (due to leak in check_resource_data). It's already fixed.
Matteo Bruni (@Mystral) commented about dlls/d3dx10_43/d3dx10_43_main.c:
LONG refcount;
- SRWLOCK io_lock;
- UINT io_count;
- struct list io_queue;
- SRWLOCK proc_lock;
- UINT proc_count;
- struct list proc_queue;
- SRWLOCK device_lock;
- UINT device_count;
- struct list device_queue;
- int threads_no;
- HANDLE threads[1];
I think we can make all these _count variables "unsigned int". Relatedly, threads_no -> thread_count.
Matteo Bruni (@Mystral) commented about dlls/d3dx10_43/d3dx10_43_main.c:
{ struct thread_pump *thread_pump = impl_from_ID3DX10ThreadPump(iface); ULONG refcount = InterlockedDecrement(&thread_pump->refcount);
struct work_item *iter, *next;
struct list list;
int i;
TRACE("%p decreasing refcount to %lu.\n", iface, refcount);
if (!refcount)
{
AcquireSRWLockExclusive(&thread_pump->io_lock);thread_pump->io_count = THREAD_PUMP_EXITING;ReleaseSRWLockExclusive(&thread_pump->io_lock);RtlWakeAddressAll((void *)&thread_pump->io_count);
It should be possible to drop the cast here and in all the other RtlWakeAddressAll() calls.
Matteo Bruni (@Mystral) commented about dlls/d3dx10_43/d3dx10_43_main.c:
{ struct thread_pump *thread_pump = impl_from_ID3DX10ThreadPump(iface); ULONG refcount = InterlockedDecrement(&thread_pump->refcount);
- struct work_item *iter, *next;
Not a big deal, here especially, but "iter" could be "item" instead.
Matteo Bruni (@Mystral) commented about dlls/d3dx10_43/d3dx10_43_main.c:
{ struct thread_pump *thread_pump = impl_from_ID3DX10ThreadPump(iface); ULONG refcount = InterlockedDecrement(&thread_pump->refcount);
- struct work_item *iter, *next;
- struct list list;
- int i;
We generally prefer using "unsigned int" for loop counter variables (assuming there is no reason to prefer a signed integer here).
Matteo Bruni (@Mystral) commented about dlls/d3dx10_43/d3dx10_43_main.c:
- {
AcquireSRWLockExclusive(&thread_pump->io_lock);thread_pump->io_count = THREAD_PUMP_EXITING;ReleaseSRWLockExclusive(&thread_pump->io_lock);RtlWakeAddressAll((void *)&thread_pump->io_count);AcquireSRWLockExclusive(&thread_pump->proc_lock);thread_pump->proc_count = THREAD_PUMP_EXITING;ReleaseSRWLockExclusive(&thread_pump->proc_lock);RtlWakeAddressAll((void *)&thread_pump->proc_count);AcquireSRWLockExclusive(&thread_pump->device_lock);thread_pump->device_count = THREAD_PUMP_EXITING;ReleaseSRWLockExclusive(&thread_pump->device_lock);for (i = 0; i < thread_pump->threads_no; i++)
Nitpick, we usually do "++i" in d3d code (obviously when either option works).
Similar for ++io_count and such.
Matteo Bruni (@Mystral) commented about dlls/d3dx10_43/d3dx10_43_main.c:
thread_pump->io_count = THREAD_PUMP_EXITING;ReleaseSRWLockExclusive(&thread_pump->io_lock);RtlWakeAddressAll((void *)&thread_pump->io_count);AcquireSRWLockExclusive(&thread_pump->proc_lock);thread_pump->proc_count = THREAD_PUMP_EXITING;ReleaseSRWLockExclusive(&thread_pump->proc_lock);RtlWakeAddressAll((void *)&thread_pump->proc_count);AcquireSRWLockExclusive(&thread_pump->device_lock);thread_pump->device_count = THREAD_PUMP_EXITING;ReleaseSRWLockExclusive(&thread_pump->device_lock);for (i = 0; i < thread_pump->threads_no; i++){if (!thread_pump->threads[i]) continue;
More nitpick, "if" body on a separate line.
Matteo Bruni (@Mystral) commented about dlls/d3dx10_43/d3dx10_43_main.c:
thread_pump_GetQueueStatus};
+static DWORD WINAPI io_thread(void *arg) +{
- struct thread_pump *thread_pump = arg;
- struct work_item *work_item;
- UINT zero = 0;
- HRESULT hr;
- TRACE("%p thread started.\n", thread_pump);
- while (1)
- {
RtlWaitOnAddress((void *)&thread_pump->io_count, &zero, sizeof(zero), NULL);
Nitpick, we prefer "for (;;)" for infinite loops.
You can drop these casts in RtlWaitOnAddress() calls too, I think.
Matteo Bruni (@Mystral) commented about dlls/d3dx10_43/d3dx10_43_main.c:
- if (!work_item)
return E_OUTOFMEMORY;- work_item->loader = loader;
- work_item->processor = processor;
- work_item->result = result;
- work_item->object = object;
- if (object)
*object = NULL;- AcquireSRWLockExclusive(&thread_pump->io_lock);
- thread_pump->io_count++;
- list_add_tail(&thread_pump->io_queue, &work_item->entry);
- ReleaseSRWLockExclusive(&thread_pump->io_lock);
- RtlWakeAddressAll((void *)&thread_pump->io_count);
Would it make sense to call RtlWakeAddressSingle() here (and in the other places where we add a work item to a queue) instead?
Matteo Bruni (@Mystral) commented about dlls/d3dx10_43/d3dx10_43_main.c:
if (thread_pump->proc_count == THREAD_PUMP_EXITING){ReleaseSRWLockExclusive(&thread_pump->proc_lock);work_item_free(work_item, TRUE);return 0;}list_add_tail(&thread_pump->proc_queue, &work_item->entry);thread_pump->proc_count++;ReleaseSRWLockExclusive(&thread_pump->proc_lock);RtlWakeAddressAll((void *)&thread_pump->proc_count);- }
- return 0;
+}
+static DWORD WINAPI proc_thread(void *arg)
Have you thought about merging the two thread functions into one? They basically do the exact same thing except for the d3dx functions they call and the queue / counter / lock they work on. The generic thread function could take a "role" parameter and key the stage-specific behavior off of it.
I haven't actually tried the idea, it might be too confusing and not be worth it.
Matteo Bruni (@Mystral) commented about dlls/d3dx10_43/d3dx10_43_main.c:
{
- FIXME("iface %p stub!\n", iface);
- return E_NOTIMPL;
- struct thread_pump *thread_pump = impl_from_ID3DX10ThreadPump(iface);
- HRESULT hr;
- LONG v;
- TRACE("iface %p.\n", iface);
- while(1)
- {
if (FAILED((hr = ID3DX10ThreadPump_ProcessDeviceWorkItems(iface, UINT_MAX))))return hr;if (!(v = thread_pump->processing_count))break;
I think there is a theoretical race here with a work item that's being added at the same time. Assuming that's a legitimate use case.
Matteo Bruni (@Mystral) commented about dlls/d3dx10_43/d3dx10_43_main.c:
list_add_tail(&thread_pump->device_queue, &work_item->entry); thread_pump->device_count++; InterlockedDecrement(&thread_pump->processing_count);
RtlWakeAddressAll((void *)&thread_pump->processing_count);
Why isn't this one conditional like the other cases?
Matteo Bruni (@Mystral) commented about dlls/d3dx10_43/d3dx10_43_main.c:
if (!InterlockedDecrement(&thread_pump->processing_count))RtlWakeAddressAll((void *)&thread_pump->processing_count);}if (!(v = thread_pump->processing_count))break;RtlWaitOnAddress((void *)&thread_pump->processing_count, &v, sizeof(v), NULL);- }
- AcquireSRWLockExclusive(&thread_pump->device_lock);
- list_move_tail(&list, &thread_pump->device_queue);
- thread_pump->device_count = 0;
- ReleaseSRWLockExclusive(&thread_pump->device_lock);
- while(!list_empty(&list))
Nitpick, whitespace.
Matteo Bruni (@Mystral) commented about dlls/d3dx10_43/tests/d3dx10.c:
+static DWORD io_tid;
+struct data_object +{
- ID3DX10DataLoader ID3DX10DataLoader_iface;
- ID3DX10DataProcessor ID3DX10DataProcessor_iface;
- HANDLE load_started;
- HANDLE load_done;
- HANDLE decompress_done;
- HRESULT load_ret;
- DWORD process_tid;
+};
+static struct data_object* data_object_from_ID3DX10DataLoader(ID3DX10DataLoader *iface)
Nitpick, '*' placement.
Please give a look at the ok() messages introduced in this patch, a number of them can be improved so that they're more consistent with the usual style "Got unexpected %#x.\n" or similar.
Matteo Bruni (@Mystral) commented about dlls/d3dx10_43/tests/d3dx10.c:
+static struct data_object* data_object_from_ID3DX10DataLoader(ID3DX10DataLoader *iface) +{
- return CONTAINING_RECORD(iface, struct data_object, ID3DX10DataLoader_iface);
+}
+static LONG data_loader_load_count; +static WINAPI HRESULT data_loader_Load(ID3DX10DataLoader *iface) +{
- struct data_object *data_object = data_object_from_ID3DX10DataLoader(iface);
- DWORD ret;
- ok(InterlockedDecrement(&data_loader_load_count) >= 0, "unexpected call\n");
- if (io_tid)
io_tid = GetCurrentThreadId();
Was the condition meant to be reversed?
Again, I generally like the MR. Nevertheless, I've got a bunch of comments...
Josh Simmons (@jsimmons) commented about dlls/d3dx10_43/d3dx10_43_main.c:
- ID3DX10ThreadPump ID3DX10ThreadPump_iface;
- LONG refcount;
- LONG processing_count;
- SRWLOCK io_lock;
- UINT io_count;
- struct list io_queue;
- SRWLOCK proc_lock;
- UINT proc_count;
- struct list proc_queue;
- SRWLOCK device_lock;
- UINT device_count;
- struct list device_queue;
Unsolicited nit, but since these are separately locked queues they should probably be padded out to be on their own cache lines to avoid false sharing.
(No idea whether this API is hot enough for that to be truly important, though)
On Tue Jun 21 08:01:58 2022 +0000, Matteo Bruni wrote:
Would it make sense to call RtlWakeAddressSingle() here (and in the other places where we add a work item to a queue) instead?
Yes, I will change it so only one thread is woken up (I will probably use WakeConditionVariable).
On Tue Jun 21 08:01:59 2022 +0000, Matteo Bruni wrote:
Why isn't this one conditional like the other cases?
It's needed for WaitForAllItems function that should process device work items as soon as they are available (for performance reasons). This is the only place where item was successfully processed and more work needs to be done.
On Tue Jun 21 08:01:58 2022 +0000, Matteo Bruni wrote:
Have you thought about merging the two thread functions into one? They basically do the exact same thing except for the d3dx functions they call and the queue / counter / lock they work on. The generic thread function could take a "role" parameter and key the stage-specific behavior off of it. I haven't actually tried the idea, it might be too confusing and not be worth it.
I think it makes the code much harder to read. While the functions are similar there are many minor differences.
On Tue Jun 21 18:07:44 2022 +0000, **** wrote:
Zebediah Figura replied on the mailing list:
On 6/18/22 14:21, Piotr Caban wrote: > +static DWORD WINAPI io_thread(void *arg) > +{ > + struct thread_pump *thread_pump = arg; > + struct work_item *work_item; > + UINT zero = 0; > + HRESULT hr; > + > + TRACE("%p thread started.\n", thread_pump); > + > + while (1) > + { > + RtlWaitOnAddress((void *)&thread_pump->io_count, &zero, sizeof(zero), NULL); > + AcquireSRWLockExclusive(&thread_pump->io_lock); > + if (!thread_pump->io_count) > + { > + ReleaseSRWLockExclusive(&thread_pump->io_lock); > + continue; > + } This works, but it strikes me as simpler (and more idiomatic) just to use a condition variable. Any reason not to do that? Note also that "zero" can be static const.
I'll change the code to use condition variables.
On Wed Jun 22 18:22:04 2022 +0000, Josh Simmons wrote:
Unsolicited nit, but since these are separately locked queues they should probably be padded out to be on their own cache lines to avoid false sharing. (No idea whether this API is hot enough for that to be truly important, though)
I don't think it will need additional optimizations. The fact that I have used separate locks is just an implementation detail. We can try to optimize it further in future if needed.
-
Francois Gouget (＠fgouget) -
Josh Simmons (＠jsimmons)
-
Matteo Bruni (＠Mystral)
-
Piotr Caban
-
Piotr Caban (＠piotr)
-
Zebediah Figura