[PATCH v2 1/5] wined3d: Introduce a prepare_upload_bo device context operation and use it to upload sub-resource data.
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/wined3d/buffer.c | 2 +- dlls/wined3d/cs.c | 54 ++++++++++++++++++++++++++++++++++ dlls/wined3d/device.c | 3 +- dlls/wined3d/texture.c | 2 +- dlls/wined3d/wined3d_private.h | 6 ++++ 5 files changed, 64 insertions(+), 3 deletions(-) diff --git a/dlls/wined3d/buffer.c b/dlls/wined3d/buffer.c index 9d22a909fb6..4c5e2054fe0 100644 --- a/dlls/wined3d/buffer.c +++ b/dlls/wined3d/buffer.c @@ -1063,7 +1063,7 @@ static void wined3d_buffer_init_data(struct wined3d_buffer *buffer, if (buffer->flags & WINED3D_BUFFER_USE_BO) { wined3d_box_set(&box, 0, 0, resource->size, 1, 0, 1); - device->cs->c.ops->update_sub_resource(&device->cs->c, resource, + wined3d_device_context_emit_update_sub_resource(&device->cs->c, resource, 0, &box, data->data, data->row_pitch, data->slice_pitch); } else diff --git a/dlls/wined3d/cs.c b/dlls/wined3d/cs.c index 47bc36d4cc0..a3124a229e1 100644 --- a/dlls/wined3d/cs.c +++ b/dlls/wined3d/cs.c @@ -2649,6 +2649,40 @@ done: wined3d_resource_release(resource); } +void wined3d_device_context_emit_update_sub_resource(struct wined3d_device_context *context, + struct wined3d_resource *resource, unsigned int sub_resource_idx, const struct wined3d_box *box, + const void *data, unsigned int row_pitch, unsigned int slice_pitch) +{ + struct wined3d_const_bo_address src_addr; + void *map_ptr; + + if ((map_ptr = context->ops->prepare_upload_bo(context, resource, sub_resource_idx, box, + row_pitch, slice_pitch, WINED3D_MAP_WRITE, &src_addr))) + { + struct wined3d_cs_update_sub_resource *op; + + wined3d_format_copy_data(resource->format, data, row_pitch, slice_pitch, map_ptr, row_pitch, slice_pitch, + box->right - box->left, box->bottom - box->top, box->back - box->front); + + op = wined3d_device_context_require_space(context, sizeof(*op), WINED3D_CS_QUEUE_DEFAULT); + op->opcode = WINED3D_CS_OP_UPDATE_SUB_RESOURCE; + op->resource = resource; + op->sub_resource_idx = sub_resource_idx; + op->box = *box; + op->addr = src_addr; + op->row_pitch = row_pitch; + op->slice_pitch = slice_pitch; + + wined3d_device_context_acquire_resource(context, resource); + + wined3d_device_context_submit(context, WINED3D_CS_QUEUE_DEFAULT); + } + else + { + context->ops->update_sub_resource(context, resource, sub_resource_idx, box, data, row_pitch, slice_pitch); + } +} + static void wined3d_cs_update_sub_resource(struct wined3d_device_context *context, struct wined3d_resource *resource, unsigned int sub_resource_idx, const struct wined3d_box *box, const void *data, unsigned int row_pitch, unsigned int slice_pitch) @@ -2946,12 +2980,21 @@ static void wined3d_cs_st_finish(struct wined3d_device_context *context, enum wi { } +static void *wined3d_cs_prepare_upload_bo(struct wined3d_device_context *context, struct wined3d_resource *resource, + unsigned int sub_resource_idx, const struct wined3d_box *box, unsigned int row_pitch, + unsigned int slice_pitch, uint32_t flags, struct wined3d_const_bo_address *address) +{ + /* FIXME: We would like to return mapped or newly allocated memory here. */ + return NULL; +} + static const struct wined3d_device_context_ops wined3d_cs_st_ops = { wined3d_cs_st_require_space, wined3d_cs_st_submit, wined3d_cs_st_finish, wined3d_cs_st_push_constants, + wined3d_cs_prepare_upload_bo, wined3d_cs_map, wined3d_cs_unmap, wined3d_cs_update_sub_resource, @@ -3080,6 +3123,7 @@ static const struct wined3d_device_context_ops wined3d_cs_mt_ops = wined3d_cs_mt_submit, wined3d_cs_mt_finish, wined3d_cs_mt_push_constants, + wined3d_cs_prepare_upload_bo, wined3d_cs_map, wined3d_cs_unmap, wined3d_cs_update_sub_resource, @@ -3353,6 +3397,15 @@ static void wined3d_deferred_context_push_constants(struct wined3d_device_contex FIXME("context %p, p %#x, start_idx %u, count %u, constants %p, stub!\n", context, p, start_idx, count, constants); } +static void *wined3d_deferred_context_prepare_upload_bo(struct wined3d_device_context *context, + struct wined3d_resource *resource, unsigned int sub_resource_idx, const struct wined3d_box *box, + unsigned int row_pitch, unsigned int slice_pitch, uint32_t flags, struct wined3d_const_bo_address *address) +{ + FIXME("context %p, resource %p, sub_resource_idx %u, box %p, flags %#x, address %p, stub!\n", + context, resource, sub_resource_idx, box, flags, address); + return false; +} + static HRESULT wined3d_deferred_context_map(struct wined3d_device_context *context, struct wined3d_resource *resource, unsigned int sub_resource_idx, void **map_ptr, const struct wined3d_box *box, unsigned int flags) { @@ -3430,6 +3483,7 @@ static const struct wined3d_device_context_ops wined3d_deferred_context_ops = wined3d_deferred_context_submit, wined3d_deferred_context_finish, wined3d_deferred_context_push_constants, + wined3d_deferred_context_prepare_upload_bo, wined3d_deferred_context_map, wined3d_deferred_context_unmap, wined3d_deferred_context_update_sub_resource, diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c index 96657141419..e37bf46594c 100644 --- a/dlls/wined3d/device.c +++ b/dlls/wined3d/device.c @@ -4679,7 +4679,8 @@ void CDECL wined3d_device_context_update_sub_resource(struct wined3d_device_cont return; } - context->ops->update_sub_resource(context, resource, sub_resource_idx, box, data, row_pitch, depth_pitch); + wined3d_device_context_emit_update_sub_resource(context, resource, + sub_resource_idx, box, data, row_pitch, depth_pitch); } void CDECL wined3d_device_context_resolve_sub_resource(struct wined3d_device_context *context, diff --git a/dlls/wined3d/texture.c b/dlls/wined3d/texture.c index 62f38a1d592..c1247fbc56b 100644 --- a/dlls/wined3d/texture.c +++ b/dlls/wined3d/texture.c @@ -4350,7 +4350,7 @@ HRESULT CDECL wined3d_texture_create(struct wined3d_device *device, const struct for (i = 0; i < sub_count; ++i) { wined3d_texture_get_level_box(*texture, i % (*texture)->level_count, &box); - device->cs->c.ops->update_sub_resource(&device->cs->c, &(*texture)->resource, + wined3d_device_context_emit_update_sub_resource(&device->cs->c, &(*texture)->resource, i, &box, data[i].data, data[i].row_pitch, data[i].slice_pitch); } } diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index c70b0f7c338..83a0a48471b 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -4704,6 +4704,9 @@ struct wined3d_device_context_ops void (*finish)(struct wined3d_device_context *context, enum wined3d_cs_queue_id queue_id); void (*push_constants)(struct wined3d_device_context *context, enum wined3d_push_constants p, unsigned int start_idx, unsigned int count, const void *constants); + void *(*prepare_upload_bo)(struct wined3d_device_context *context, struct wined3d_resource *resource, + unsigned int sub_resource_idx, const struct wined3d_box *box, unsigned int row_pitch, + unsigned int slice_pitch, uint32_t flags, struct wined3d_const_bo_address *address); HRESULT (*map)(struct wined3d_device_context *context, struct wined3d_resource *resource, unsigned int sub_resource_idx, void **map_ptr, const struct wined3d_box *box, unsigned int flags); HRESULT (*unmap)(struct wined3d_device_context *context, struct wined3d_resource *resource, @@ -4855,6 +4858,9 @@ void wined3d_device_context_emit_set_vertex_declaration(struct wined3d_device_co struct wined3d_vertex_declaration *declaration) DECLSPEC_HIDDEN; void wined3d_device_context_emit_set_viewports(struct wined3d_device_context *context, unsigned int viewport_count, const struct wined3d_viewport *viewports) DECLSPEC_HIDDEN; +void wined3d_device_context_emit_update_sub_resource(struct wined3d_device_context *context, + struct wined3d_resource *resource, unsigned int sub_resource_idx, const struct wined3d_box *box, + const void *data, unsigned int row_pitch, unsigned int slice_pitch) DECLSPEC_HIDDEN; static inline void wined3d_resource_wait_idle(struct wined3d_resource *resource) { -- 2.30.2
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/wined3d/cs.c | 64 ++++++++++++++++++++++++++++++++-- dlls/wined3d/resource.c | 30 +++++++++------- dlls/wined3d/wined3d_private.h | 3 ++ 3 files changed, 81 insertions(+), 16 deletions(-) diff --git a/dlls/wined3d/cs.c b/dlls/wined3d/cs.c index a3124a229e1..80636943069 100644 --- a/dlls/wined3d/cs.c +++ b/dlls/wined3d/cs.c @@ -26,6 +26,13 @@ WINE_DECLARE_DEBUG_CHANNEL(fps); #define WINED3D_INITIAL_CS_SIZE 4096 +struct wined3d_deferred_upload +{ + struct wined3d_resource *resource; + unsigned int sub_resource_idx; + void *sysmem; +}; + struct wined3d_command_list { LONG refcount; @@ -38,6 +45,9 @@ struct wined3d_command_list SIZE_T resource_count; struct wined3d_resource **resources; + SIZE_T upload_count; + struct wined3d_deferred_upload *uploads; + /* List of command lists queued for execution on this command list. We might * be the only thing holding a pointer to another command list, so we need * to hold a reference here (and in wined3d_deferred_context) as well. */ @@ -48,9 +58,13 @@ struct wined3d_command_list static void wined3d_command_list_destroy_object(void *object) { struct wined3d_command_list *list = object; + SIZE_T i; TRACE("list %p.\n", list); + for (i = 0; i < list->upload_count; ++i) + wined3d_free_sysmem(list->uploads[i].sysmem); + heap_free(list->resources); heap_free(list->data); heap_free(list); @@ -3343,6 +3357,9 @@ struct wined3d_deferred_context SIZE_T resource_count, resources_capacity; struct wined3d_resource **resources; + SIZE_T upload_count, uploads_capacity; + struct wined3d_deferred_upload *uploads; + /* List of command lists queued for execution on this context. A command * list can be the only thing holding a pointer to another command list, so * we need to hold a reference here and in wined3d_command_list as well. */ @@ -3401,9 +3418,44 @@ static void *wined3d_deferred_context_prepare_upload_bo(struct wined3d_device_co struct wined3d_resource *resource, unsigned int sub_resource_idx, const struct wined3d_box *box, unsigned int row_pitch, unsigned int slice_pitch, uint32_t flags, struct wined3d_const_bo_address *address) { - FIXME("context %p, resource %p, sub_resource_idx %u, box %p, flags %#x, address %p, stub!\n", - context, resource, sub_resource_idx, box, flags, address); - return false; + struct wined3d_deferred_context *deferred = wined3d_deferred_context_from_context(context); + const struct wined3d_format *format = resource->format; + struct wined3d_deferred_upload *upload; + void *sysmem; + size_t size; + + size = (box->back - box->front - 1) * slice_pitch + + ((box->bottom - box->top - 1) / format->block_height) * row_pitch + + ((box->right - box->left + format->block_width - 1) / format->block_width) * format->block_byte_count; + + if (!(flags & WINED3D_MAP_WRITE)) + { + WARN("Flags %#x are not valid on a deferred context.\n", flags); + return NULL; + } + + if (flags & ~(WINED3D_MAP_WRITE | WINED3D_MAP_DISCARD)) + { + FIXME("Unhandled flags %#x.\n", flags); + return NULL; + } + + if (!wined3d_array_reserve((void **)&deferred->uploads, &deferred->uploads_capacity, + deferred->upload_count + 1, sizeof(*deferred->uploads))) + return NULL; + + if (!(sysmem = wined3d_allocate_sysmem(size))) + return NULL; + + upload = &deferred->uploads[deferred->upload_count++]; + upload->resource = resource; + wined3d_resource_incref(resource); + upload->sub_resource_idx = sub_resource_idx; + upload->sysmem = sysmem; + + address->buffer_object = 0; + address->addr = sysmem; + return sysmem; } static HRESULT wined3d_deferred_context_map(struct wined3d_device_context *context, struct wined3d_resource *resource, @@ -3527,6 +3579,12 @@ void CDECL wined3d_deferred_context_destroy(struct wined3d_device_context *conte for (i = 0; i < deferred->resource_count; ++i) wined3d_resource_decref(deferred->resources[i]); + + for (i = 0; i < deferred->upload_count; ++i) + { + wined3d_resource_decref(deferred->uploads[i].resource); + wined3d_free_sysmem(deferred->uploads[i].sysmem); + } heap_free(deferred->resources); wined3d_state_destroy(deferred->c.state); diff --git a/dlls/wined3d/resource.c b/dlls/wined3d/resource.c index f37c313e023..8e4be0e0315 100644 --- a/dlls/wined3d/resource.c +++ b/dlls/wined3d/resource.c @@ -334,24 +334,33 @@ void CDECL wined3d_resource_preload(struct wined3d_resource *resource) wined3d_cs_emit_preload_resource(resource->device->cs, resource); } -static BOOL wined3d_resource_allocate_sysmem(struct wined3d_resource *resource) +void *wined3d_allocate_sysmem(SIZE_T size) { void **p; - SIZE_T align = RESOURCE_ALIGNMENT - 1 + sizeof(*p); + static const SIZE_T align = RESOURCE_ALIGNMENT - 1 + sizeof(*p); void *mem; - if (!(mem = heap_alloc_zero(resource->size + align))) + if (!(mem = heap_alloc_zero(size + align))) { ERR("Failed to allocate system memory.\n"); - return FALSE; + return NULL; } + TRACE("Allocated %lu bytes at %p.\n", size, mem); p = (void **)(((ULONG_PTR)mem + align) & ~(RESOURCE_ALIGNMENT - 1)) - 1; *p = mem; - resource->heap_memory = ++p; + return ++p; +} - return TRUE; +void wined3d_free_sysmem(void *mem) +{ + void **p = mem; + + if (!p) + return; + + heap_free(*(--p)); } BOOL wined3d_resource_prepare_sysmem(struct wined3d_resource *resource) @@ -359,17 +368,12 @@ BOOL wined3d_resource_prepare_sysmem(struct wined3d_resource *resource) if (resource->heap_memory) return TRUE; - return wined3d_resource_allocate_sysmem(resource); + return !!(resource->heap_memory = wined3d_allocate_sysmem(resource->size)); } void wined3d_resource_free_sysmem(struct wined3d_resource *resource) { - void **p = resource->heap_memory; - - if (!p) - return; - - heap_free(*(--p)); + wined3d_free_sysmem(resource->heap_memory); resource->heap_memory = NULL; } diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index 83a0a48471b..93ec378e06b 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -4155,6 +4155,9 @@ void wined3d_resource_update_draw_binding(struct wined3d_resource *resource) DEC #define RESOURCE_ALIGNMENT 16 #define WINED3D_CONSTANT_BUFFER_ALIGNMENT 16 +void *wined3d_allocate_sysmem(SIZE_T size) DECLSPEC_HIDDEN; +void wined3d_free_sysmem(void *mem) DECLSPEC_HIDDEN; + #define WINED3D_LOCATION_DISCARDED 0x00000001 #define WINED3D_LOCATION_SYSMEM 0x00000002 #define WINED3D_LOCATION_BUFFER 0x00000008 -- 2.30.2
On Wed, 30 Jun 2021 at 06:33, Zebediah Figura <z.figura12(a)gmail.com> wrote:
@@ -3401,9 +3418,44 @@ static void *wined3d_deferred_context_prepare_upload_bo(struct wined3d_device_co struct wined3d_resource *resource, unsigned int sub_resource_idx, const struct wined3d_box *box, unsigned int row_pitch, unsigned int slice_pitch, uint32_t flags, struct wined3d_const_bo_address *address) { - FIXME("context %p, resource %p, sub_resource_idx %u, box %p, flags %#x, address %p, stub!\n", - context, resource, sub_resource_idx, box, flags, address); - return false; + struct wined3d_deferred_context *deferred = wined3d_deferred_context_from_context(context); + const struct wined3d_format *format = resource->format; + struct wined3d_deferred_upload *upload; + void *sysmem; + size_t size; + + size = (box->back - box->front - 1) * slice_pitch + + ((box->bottom - box->top - 1) / format->block_height) * row_pitch + + ((box->right - box->left + format->block_width - 1) / format->block_width) * format->block_byte_count; + + if (!(flags & WINED3D_MAP_WRITE)) + { + WARN("Flags %#x are not valid on a deferred context.\n", flags); + return NULL; + } + + if (flags & ~(WINED3D_MAP_WRITE | WINED3D_MAP_DISCARD)) + { + FIXME("Unhandled flags %#x.\n", flags); + return NULL; + } + + if (!wined3d_array_reserve((void **)&deferred->uploads, &deferred->uploads_capacity, + deferred->upload_count + 1, sizeof(*deferred->uploads))) + return NULL; + + if (!(sysmem = wined3d_allocate_sysmem(size))) + return NULL; + + upload = &deferred->uploads[deferred->upload_count++]; + upload->resource = resource; + wined3d_resource_incref(resource); + upload->sub_resource_idx = sub_resource_idx; + upload->sysmem = sysmem; + + address->buffer_object = 0; + address->addr = sysmem; + return sysmem; }
Do we need to use wined3d_allocate_sysmem() here? We could also do something like the following: upload->sysmem = heap_alloc(size + alignment - 1); [...] return address->addr = upload->sysmem & ~(alignment - 1) (And note that not all callers of wined3d_deferred_context_prepare_upload_bo() will necessarily need an allocation aligned to RESOURCE_ALIGNMENT.)
-static BOOL wined3d_resource_allocate_sysmem(struct wined3d_resource *resource) +void *wined3d_allocate_sysmem(SIZE_T size) { void **p; - SIZE_T align = RESOURCE_ALIGNMENT - 1 + sizeof(*p); + static const SIZE_T align = RESOURCE_ALIGNMENT - 1 + sizeof(*p); void *mem;
- if (!(mem = heap_alloc_zero(resource->size + align))) + if (!(mem = heap_alloc_zero(size + align))) { ERR("Failed to allocate system memory.\n"); - return FALSE; + return NULL; }
+ TRACE("Allocated %lu bytes at %p.\n", size, mem); p = (void **)(((ULONG_PTR)mem + align) & ~(RESOURCE_ALIGNMENT - 1)) - 1; *p = mem;
- resource->heap_memory = ++p; + return ++p; +}
If we're turning this into a generic helper, perhaps it would be more appropriate in utils.c, and we may want to consider naming it something like e.g. wined3d_allocate_aligned().
On 6/30/21 7:14 AM, Henri Verbeet wrote:
On Wed, 30 Jun 2021 at 06:33, Zebediah Figura <z.figura12(a)gmail.com> wrote:
@@ -3401,9 +3418,44 @@ static void *wined3d_deferred_context_prepare_upload_bo(struct wined3d_device_co struct wined3d_resource *resource, unsigned int sub_resource_idx, const struct wined3d_box *box, unsigned int row_pitch, unsigned int slice_pitch, uint32_t flags, struct wined3d_const_bo_address *address) { - FIXME("context %p, resource %p, sub_resource_idx %u, box %p, flags %#x, address %p, stub!\n", - context, resource, sub_resource_idx, box, flags, address); - return false; + struct wined3d_deferred_context *deferred = wined3d_deferred_context_from_context(context); + const struct wined3d_format *format = resource->format; + struct wined3d_deferred_upload *upload; + void *sysmem; + size_t size; + + size = (box->back - box->front - 1) * slice_pitch + + ((box->bottom - box->top - 1) / format->block_height) * row_pitch + + ((box->right - box->left + format->block_width - 1) / format->block_width) * format->block_byte_count; + + if (!(flags & WINED3D_MAP_WRITE)) + { + WARN("Flags %#x are not valid on a deferred context.\n", flags); + return NULL; + } + + if (flags & ~(WINED3D_MAP_WRITE | WINED3D_MAP_DISCARD)) + { + FIXME("Unhandled flags %#x.\n", flags); + return NULL; + } + + if (!wined3d_array_reserve((void **)&deferred->uploads, &deferred->uploads_capacity, + deferred->upload_count + 1, sizeof(*deferred->uploads))) + return NULL; + + if (!(sysmem = wined3d_allocate_sysmem(size))) + return NULL; + + upload = &deferred->uploads[deferred->upload_count++]; + upload->resource = resource; + wined3d_resource_incref(resource); + upload->sub_resource_idx = sub_resource_idx; + upload->sysmem = sysmem; + + address->buffer_object = 0; + address->addr = sysmem; + return sysmem; }
Do we need to use wined3d_allocate_sysmem() here? We could also do something like the following:
upload->sysmem = heap_alloc(size + alignment - 1); [...] return address->addr = upload->sysmem & ~(alignment - 1)
(And note that not all callers of wined3d_deferred_context_prepare_upload_bo() will necessarily need an allocation aligned to RESOURCE_ALIGNMENT.)
No, I suppose not, and that'll save an extra pointer...
-static BOOL wined3d_resource_allocate_sysmem(struct wined3d_resource *resource) +void *wined3d_allocate_sysmem(SIZE_T size) { void **p; - SIZE_T align = RESOURCE_ALIGNMENT - 1 + sizeof(*p); + static const SIZE_T align = RESOURCE_ALIGNMENT - 1 + sizeof(*p); void *mem;
- if (!(mem = heap_alloc_zero(resource->size + align))) + if (!(mem = heap_alloc_zero(size + align))) { ERR("Failed to allocate system memory.\n"); - return FALSE; + return NULL; }
+ TRACE("Allocated %lu bytes at %p.\n", size, mem); p = (void **)(((ULONG_PTR)mem + align) & ~(RESOURCE_ALIGNMENT - 1)) - 1; *p = mem;
- resource->heap_memory = ++p; + return ++p; +}
If we're turning this into a generic helper, perhaps it would be more appropriate in utils.c, and we may want to consider naming it something like e.g. wined3d_allocate_aligned().
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/d3d11/device.c | 4 ++++ dlls/d3d11/tests/d3d11.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/dlls/d3d11/device.c b/dlls/d3d11/device.c index 56f34c70ea1..d043af7051a 100644 --- a/dlls/d3d11/device.c +++ b/dlls/d3d11/device.c @@ -744,6 +744,10 @@ static HRESULT STDMETHODCALLTYPE d3d11_device_context_Map(ID3D11DeviceContext1 * if (map_flags) FIXME("Ignoring map_flags %#x.\n", map_flags); + if (context->type != D3D11_DEVICE_CONTEXT_IMMEDIATE + && map_type != D3D11_MAP_WRITE_DISCARD && map_type != D3D11_MAP_WRITE_NO_OVERWRITE) + return E_INVALIDARG; + wined3d_resource = wined3d_resource_from_d3d11_resource(resource); wined3d_mutex_lock(); diff --git a/dlls/d3d11/tests/d3d11.c b/dlls/d3d11/tests/d3d11.c index b81f1f652fe..cf51e79b084 100644 --- a/dlls/d3d11/tests/d3d11.c +++ b/dlls/d3d11/tests/d3d11.c @@ -33059,7 +33059,7 @@ static void test_deferred_context_map(void) ok(hr == E_INVALIDARG, "Got unexpected hr %#x.\n", hr); hr = ID3D11DeviceContext_Map(deferred, (ID3D11Resource *)buffer, 0, D3D11_MAP_WRITE, 0, &map_desc); - todo_wine ok(hr == E_INVALIDARG, "Got unexpected hr %#x.\n", hr); + ok(hr == E_INVALIDARG, "Got unexpected hr %#x.\n", hr); hr = ID3D11DeviceContext_Map(deferred, (ID3D11Resource *)buffer, 0, D3D11_MAP_WRITE_NO_OVERWRITE, 0, &map_desc); todo_wine ok(hr == D3D11_ERROR_DEFERRED_CONTEXT_MAP_WITHOUT_INITIAL_DISCARD, "Got unexpected hr %#x.\n", hr); -- 2.30.2
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=93326 Your paranoid android. === w10pro64 (32 bit report) === d3d11: d3d11.c:5763: Test failed: Got unexpected query result 0x0000000000000000. d3d11.c:5916: Test failed: Got unexpected IAVertices count: 0. d3d11.c:5917: Test failed: Got unexpected IAPrimitives count: 0. d3d11.c:5918: Test failed: Got unexpected VSInvocations count: 0. d3d11.c:5921: Test failed: Got unexpected CInvocations count: 0. d3d11.c:5922: Test failed: Got unexpected CPrimitives count: 0. === wvistau64 (64 bit report) === d3d11: d3d11.c:5916: Test failed: Got unexpected IAVertices count: 0. d3d11.c:5917: Test failed: Got unexpected IAPrimitives count: 0. d3d11.c:5918: Test failed: Got unexpected VSInvocations count: 0. d3d11.c:5921: Test failed: Got unexpected CInvocations count: 0. d3d11.c:5922: Test failed: Got unexpected CPrimitives count: 0. === debiant2 (32 bit French report) === d3d11: d3d11.c:5922: Test failed: d3d11.c:5998: Test marked todo: Got unexpected CPrimitives count: 3. d3d11.c:9766: Test failed: d3d11.c:15127: Test marked todo: Got hr 0 for WRITE.
This has the notable effect of implementing maps on deferred contexts. Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/d3d11/tests/d3d11.c | 55 +++++++++++++------------ dlls/wined3d/cs.c | 73 +++++++++++++++++++++++++++------- dlls/wined3d/device.c | 38 ++++++++++++++++-- dlls/wined3d/wined3d_private.h | 5 +++ 4 files changed, 125 insertions(+), 46 deletions(-) diff --git a/dlls/d3d11/tests/d3d11.c b/dlls/d3d11/tests/d3d11.c index cf51e79b084..df1d5a35c6e 100644 --- a/dlls/d3d11/tests/d3d11.c +++ b/dlls/d3d11/tests/d3d11.c @@ -33065,15 +33065,7 @@ static void test_deferred_context_map(void) todo_wine ok(hr == D3D11_ERROR_DEFERRED_CONTEXT_MAP_WITHOUT_INITIAL_DISCARD, "Got unexpected hr %#x.\n", hr); hr = ID3D11DeviceContext_Map(deferred, (ID3D11Resource *)buffer, 0, D3D11_MAP_WRITE_DISCARD, 0, &map_desc); - todo_wine ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); - if (hr != S_OK) - { - ID3D11Buffer_Release(buffer2); - ID3D11Buffer_Release(buffer); - ID3D11DeviceContext_Release(deferred); - release_test_context(&test_context); - return; - } + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); map_data = map_desc.pData; /* The previous contents of map_data are undefined and may in practice be * uninitialized garbage. */ @@ -33122,13 +33114,14 @@ static void test_deferred_context_map(void) ID3D11DeviceContext_Unmap(immediate, (ID3D11Resource *)buffer, 0); hr = ID3D11DeviceContext_Map(deferred, (ID3D11Resource *)buffer, 0, D3D11_MAP_WRITE_NO_OVERWRITE, 0, &map_desc); - ok(hr == D3D11_ERROR_DEFERRED_CONTEXT_MAP_WITHOUT_INITIAL_DISCARD, "Got unexpected hr %#x.\n", hr); + todo_wine ok(hr == D3D11_ERROR_DEFERRED_CONTEXT_MAP_WITHOUT_INITIAL_DISCARD, "Got unexpected hr %#x.\n", hr); hr = ID3D11DeviceContext_Map(deferred, (ID3D11Resource *)buffer, 0, D3D11_MAP_WRITE_DISCARD, 0, &map_desc); ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); map_data = map_desc.pData; for (i = 0; i < ARRAY_SIZE(data); ++i) map_data[i] = 2 * i; + memcpy(data, map_data, sizeof(data)); ID3D11DeviceContext_Unmap(deferred, (ID3D11Resource *)buffer, 0); hr = ID3D11DeviceContext_Map(deferred, (ID3D11Resource *)buffer, 0, D3D11_MAP_READ, 0, &map_desc); @@ -33141,32 +33134,38 @@ static void test_deferred_context_map(void) ok(hr == E_INVALIDARG, "Got unexpected hr %#x.\n", hr); hr = ID3D11DeviceContext_Map(deferred, (ID3D11Resource *)buffer, 0, D3D11_MAP_WRITE_NO_OVERWRITE, 0, &map_desc); - ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + todo_wine ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); - map_data = map_desc.pData; - for (i = 0; i < ARRAY_SIZE(data); ++i) + if (hr == S_OK) { - ok(map_data[i] == 2 * i, "Got unexpected value %.8e at %u.\n", map_data[i], i); - if (i % 2) - map_data[i] = 3 * i; - } - memcpy(data, map_data, sizeof(data)); + map_data = map_desc.pData; + for (i = 0; i < ARRAY_SIZE(data); ++i) + { + ok(map_data[i] == 2 * i, "Got unexpected value %.8e at %u.\n", map_data[i], i); + if (i % 2) + map_data[i] = 3 * i; + } + memcpy(data, map_data, sizeof(data)); - ID3D11DeviceContext_Unmap(deferred, (ID3D11Resource *)buffer, 0); + ID3D11DeviceContext_Unmap(deferred, (ID3D11Resource *)buffer, 0); + } hr = ID3D11DeviceContext_Map(deferred, (ID3D11Resource *)buffer, 0, D3D11_MAP_WRITE_NO_OVERWRITE, 0, &map_desc); - ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + todo_wine ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); - map_data = map_desc.pData; - for (i = 0; i < ARRAY_SIZE(data); ++i) + if (hr == S_OK) { - ok(map_data[i] == data[i], "Got unexpected value %.8e at %u.\n", map_data[i], i); - if (i % 3) - map_data[i] = 4 * i; - } - memcpy(data, map_data, sizeof(data)); + map_data = map_desc.pData; + for (i = 0; i < ARRAY_SIZE(data); ++i) + { + ok(map_data[i] == data[i], "Got unexpected value %.8e at %u.\n", map_data[i], i); + if (i % 3) + map_data[i] = 4 * i; + } + memcpy(data, map_data, sizeof(data)); - ID3D11DeviceContext_Unmap(deferred, (ID3D11Resource *)buffer, 0); + ID3D11DeviceContext_Unmap(deferred, (ID3D11Resource *)buffer, 0); + } hr = ID3D11DeviceContext_FinishCommandList(deferred, FALSE, &list); ok(hr == S_OK, "Failed to create command list, hr %#x.\n", hr); diff --git a/dlls/wined3d/cs.c b/dlls/wined3d/cs.c index 80636943069..c70606ad036 100644 --- a/dlls/wined3d/cs.c +++ b/dlls/wined3d/cs.c @@ -31,6 +31,7 @@ struct wined3d_deferred_upload struct wined3d_resource *resource; unsigned int sub_resource_idx; void *sysmem; + struct wined3d_box box; }; struct wined3d_command_list @@ -2663,6 +2664,29 @@ done: wined3d_resource_release(resource); } +void wined3d_device_context_upload_bo(struct wined3d_device_context *context, + struct wined3d_resource *resource, unsigned int sub_resource_idx, const struct wined3d_box *box, + const struct wined3d_const_bo_address *addr, unsigned int row_pitch, unsigned int slice_pitch) +{ + struct wined3d_cs_update_sub_resource *op; + + TRACE("context %p, resource %p, sub_resource_idx %u, box %s, addr %s, row_pitch %u, slice_pitch %u.\n", + context, resource, sub_resource_idx, debug_box(box), debug_const_bo_address(addr), row_pitch, slice_pitch); + + op = wined3d_device_context_require_space(context, sizeof(*op), WINED3D_CS_QUEUE_DEFAULT); + op->opcode = WINED3D_CS_OP_UPDATE_SUB_RESOURCE; + op->resource = resource; + op->sub_resource_idx = sub_resource_idx; + op->box = *box; + op->addr = *addr; + op->row_pitch = row_pitch; + op->slice_pitch = slice_pitch; + + wined3d_device_context_acquire_resource(context, resource); + + wined3d_device_context_submit(context, WINED3D_CS_QUEUE_DEFAULT); +} + void wined3d_device_context_emit_update_sub_resource(struct wined3d_device_context *context, struct wined3d_resource *resource, unsigned int sub_resource_idx, const struct wined3d_box *box, const void *data, unsigned int row_pitch, unsigned int slice_pitch) @@ -2673,23 +2697,9 @@ void wined3d_device_context_emit_update_sub_resource(struct wined3d_device_conte if ((map_ptr = context->ops->prepare_upload_bo(context, resource, sub_resource_idx, box, row_pitch, slice_pitch, WINED3D_MAP_WRITE, &src_addr))) { - struct wined3d_cs_update_sub_resource *op; - wined3d_format_copy_data(resource->format, data, row_pitch, slice_pitch, map_ptr, row_pitch, slice_pitch, box->right - box->left, box->bottom - box->top, box->back - box->front); - - op = wined3d_device_context_require_space(context, sizeof(*op), WINED3D_CS_QUEUE_DEFAULT); - op->opcode = WINED3D_CS_OP_UPDATE_SUB_RESOURCE; - op->resource = resource; - op->sub_resource_idx = sub_resource_idx; - op->box = *box; - op->addr = src_addr; - op->row_pitch = row_pitch; - op->slice_pitch = slice_pitch; - - wined3d_device_context_acquire_resource(context, resource); - - wined3d_device_context_submit(context, WINED3D_CS_QUEUE_DEFAULT); + wined3d_device_context_upload_bo(context, resource, sub_resource_idx, box, &src_addr, row_pitch, slice_pitch); } else { @@ -3002,6 +3012,12 @@ static void *wined3d_cs_prepare_upload_bo(struct wined3d_device_context *context return NULL; } +static bool wined3d_cs_get_upload_bo(struct wined3d_device_context *context, struct wined3d_resource *resource, + unsigned int sub_resource_idx, struct wined3d_box *box, struct wined3d_const_bo_address *address) +{ + return false; +} + static const struct wined3d_device_context_ops wined3d_cs_st_ops = { wined3d_cs_st_require_space, @@ -3009,6 +3025,7 @@ static const struct wined3d_device_context_ops wined3d_cs_st_ops = wined3d_cs_st_finish, wined3d_cs_st_push_constants, wined3d_cs_prepare_upload_bo, + wined3d_cs_get_upload_bo, wined3d_cs_map, wined3d_cs_unmap, wined3d_cs_update_sub_resource, @@ -3138,6 +3155,7 @@ static const struct wined3d_device_context_ops wined3d_cs_mt_ops = wined3d_cs_mt_finish, wined3d_cs_mt_push_constants, wined3d_cs_prepare_upload_bo, + wined3d_cs_get_upload_bo, wined3d_cs_map, wined3d_cs_unmap, wined3d_cs_update_sub_resource, @@ -3452,12 +3470,36 @@ static void *wined3d_deferred_context_prepare_upload_bo(struct wined3d_device_co wined3d_resource_incref(resource); upload->sub_resource_idx = sub_resource_idx; upload->sysmem = sysmem; + upload->box = *box; address->buffer_object = 0; address->addr = sysmem; return sysmem; } +static bool wined3d_deferred_context_get_upload_bo(struct wined3d_device_context *context, + struct wined3d_resource *resource, unsigned int sub_resource_idx, + struct wined3d_box *box, struct wined3d_const_bo_address *address) +{ + struct wined3d_deferred_context *deferred = wined3d_deferred_context_from_context(context); + int i = deferred->upload_count; + + while (i--) + { + struct wined3d_deferred_upload *upload = &deferred->uploads[i]; + + if (upload->resource == resource && upload->sub_resource_idx == sub_resource_idx) + { + *box = upload->box; + address->buffer_object = 0; + address->addr = upload->sysmem; + return true; + } + } + + return false; +} + static HRESULT wined3d_deferred_context_map(struct wined3d_device_context *context, struct wined3d_resource *resource, unsigned int sub_resource_idx, void **map_ptr, const struct wined3d_box *box, unsigned int flags) { @@ -3536,6 +3578,7 @@ static const struct wined3d_device_context_ops wined3d_deferred_context_ops = wined3d_deferred_context_finish, wined3d_deferred_context_push_constants, wined3d_deferred_context_prepare_upload_bo, + wined3d_deferred_context_get_upload_bo, wined3d_deferred_context_map, wined3d_deferred_context_unmap, wined3d_deferred_context_update_sub_resource, diff --git a/dlls/wined3d/device.c b/dlls/wined3d/device.c index e37bf46594c..774c77d1374 100644 --- a/dlls/wined3d/device.c +++ b/dlls/wined3d/device.c @@ -4833,7 +4833,10 @@ HRESULT CDECL wined3d_device_context_map(struct wined3d_device_context *context, struct wined3d_map_desc *map_desc, const struct wined3d_box *box, unsigned int flags) { struct wined3d_sub_resource_desc desc; + struct wined3d_const_bo_address addr; + unsigned int row_pitch, slice_pitch; struct wined3d_box b; + void *map_ptr; HRESULT hr; TRACE("context %p, resource %p, sub_resource_idx %u, map_desc %p, box %s, flags %#x.\n", @@ -4878,18 +4881,47 @@ HRESULT CDECL wined3d_device_context_map(struct wined3d_device_context *context, return WINED3DERR_INVALIDCALL; } + wined3d_resource_get_sub_resource_map_pitch(resource, sub_resource_idx, &row_pitch, &slice_pitch); + + if ((map_ptr = context->ops->prepare_upload_bo(context, resource, + sub_resource_idx, box, row_pitch, slice_pitch, flags, &addr))) + { + TRACE("Returning upload bo %s, data %p, row pitch %u, slice pitch %u.\n", + debug_const_bo_address(&addr), map_ptr, row_pitch, slice_pitch); + map_desc->data = map_ptr; + map_desc->row_pitch = row_pitch; + map_desc->slice_pitch = slice_pitch; + return WINED3D_OK; + } + if (SUCCEEDED(hr = context->ops->map(context, resource, sub_resource_idx, &map_desc->data, box, flags))) - wined3d_resource_get_sub_resource_map_pitch(resource, sub_resource_idx, - &map_desc->row_pitch, &map_desc->slice_pitch); + { + map_desc->row_pitch = row_pitch; + map_desc->slice_pitch = slice_pitch; + } return hr; } HRESULT CDECL wined3d_device_context_unmap(struct wined3d_device_context *context, struct wined3d_resource *resource, unsigned int sub_resource_idx) { + struct wined3d_const_bo_address addr; + struct wined3d_box box; + TRACE("context %p, resource %p, sub_resource_idx %u.\n", context, resource, sub_resource_idx); - return context->ops->unmap(context, resource, sub_resource_idx); + if (context->ops->get_upload_bo(context, resource, sub_resource_idx, &box, &addr)) + { + unsigned int row_pitch, slice_pitch; + + wined3d_resource_get_sub_resource_map_pitch(resource, sub_resource_idx, &row_pitch, &slice_pitch); + wined3d_device_context_upload_bo(context, resource, sub_resource_idx, &box, &addr, row_pitch, slice_pitch); + return WINED3D_OK; + } + else + { + return context->ops->unmap(context, resource, sub_resource_idx); + } } void CDECL wined3d_device_context_issue_query(struct wined3d_device_context *context, diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index 93ec378e06b..2c8cc5771e3 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -4710,6 +4710,8 @@ struct wined3d_device_context_ops void *(*prepare_upload_bo)(struct wined3d_device_context *context, struct wined3d_resource *resource, unsigned int sub_resource_idx, const struct wined3d_box *box, unsigned int row_pitch, unsigned int slice_pitch, uint32_t flags, struct wined3d_const_bo_address *address); + bool (*get_upload_bo)(struct wined3d_device_context *context, struct wined3d_resource *resource, + unsigned int sub_resource_idx, struct wined3d_box *box, struct wined3d_const_bo_address *address); HRESULT (*map)(struct wined3d_device_context *context, struct wined3d_resource *resource, unsigned int sub_resource_idx, void **map_ptr, const struct wined3d_box *box, unsigned int flags); HRESULT (*unmap)(struct wined3d_device_context *context, struct wined3d_resource *resource, @@ -4864,6 +4866,9 @@ void wined3d_device_context_emit_set_viewports(struct wined3d_device_context *co void wined3d_device_context_emit_update_sub_resource(struct wined3d_device_context *context, struct wined3d_resource *resource, unsigned int sub_resource_idx, const struct wined3d_box *box, const void *data, unsigned int row_pitch, unsigned int slice_pitch) DECLSPEC_HIDDEN; +void wined3d_device_context_upload_bo(struct wined3d_device_context *context, + struct wined3d_resource *resource, unsigned int sub_resource_idx, const struct wined3d_box *box, + const struct wined3d_const_bo_address *addr, unsigned int row_pitch, unsigned int slice_pitch); static inline void wined3d_resource_wait_idle(struct wined3d_resource *resource) { -- 2.30.2
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=93327 Your paranoid android. === w1064v1809 (32 bit report) === d3d11: d3d11.c:5916: Test failed: Got unexpected IAVertices count: 0. d3d11.c:5917: Test failed: Got unexpected IAPrimitives count: 0. d3d11.c:5918: Test failed: Got unexpected VSInvocations count: 0. d3d11.c:5921: Test failed: Got unexpected CInvocations count: 0. d3d11.c:5922: Test failed: Got unexpected CPrimitives count: 0. === debiant2 (32 bit German report) === d3d11: d3d11.c:9766: Test failed: d3d11.c:15218: Test marked todo: Got hr 0 for WRITE_NO_OVERWRITE. === debiant2 (32 bit French report) === d3d11: d3d11.c:5922: Test failed: d3d11.c:6236: Test marked todo: Got unexpected hr 0x1.
On Wed, 30 Jun 2021 at 06:34, Zebediah Figura <z.figura12(a)gmail.com> wrote:
+static bool wined3d_deferred_context_get_upload_bo(struct wined3d_device_context *context, + struct wined3d_resource *resource, unsigned int sub_resource_idx, + struct wined3d_box *box, struct wined3d_const_bo_address *address) +{ + struct wined3d_deferred_context *deferred = wined3d_deferred_context_from_context(context); + int i = deferred->upload_count; + + while (i--) + { + struct wined3d_deferred_upload *upload = &deferred->uploads[i]; + + if (upload->resource == resource && upload->sub_resource_idx == sub_resource_idx) + { + *box = upload->box; + address->buffer_object = 0; + address->addr = upload->sysmem; + return true; + } + } + + return false; +} + "SIZE_T i;", or at least "unsigned int i;"
HRESULT CDECL wined3d_device_context_unmap(struct wined3d_device_context *context, struct wined3d_resource *resource, unsigned int sub_resource_idx) { + struct wined3d_const_bo_address addr; + struct wined3d_box box; + TRACE("context %p, resource %p, sub_resource_idx %u.\n", context, resource, sub_resource_idx);
- return context->ops->unmap(context, resource, sub_resource_idx); + if (context->ops->get_upload_bo(context, resource, sub_resource_idx, &box, &addr)) + { + unsigned int row_pitch, slice_pitch; + + wined3d_resource_get_sub_resource_map_pitch(resource, sub_resource_idx, &row_pitch, &slice_pitch); + wined3d_device_context_upload_bo(context, resource, sub_resource_idx, &box, &addr, row_pitch, slice_pitch); + return WINED3D_OK; + } + else + { + return context->ops->unmap(context, resource, sub_resource_idx); + } }
Would we ever use ops->get_upload_bo() and ops->unmap() independently of each other? If not, we may as well merge those together.
On 6/30/21 7:14 AM, Henri Verbeet wrote:
On Wed, 30 Jun 2021 at 06:34, Zebediah Figura <z.figura12(a)gmail.com> wrote:
+static bool wined3d_deferred_context_get_upload_bo(struct wined3d_device_context *context, + struct wined3d_resource *resource, unsigned int sub_resource_idx, + struct wined3d_box *box, struct wined3d_const_bo_address *address) +{ + struct wined3d_deferred_context *deferred = wined3d_deferred_context_from_context(context); + int i = deferred->upload_count; + + while (i--) + { + struct wined3d_deferred_upload *upload = &deferred->uploads[i]; + + if (upload->resource == resource && upload->sub_resource_idx == sub_resource_idx) + { + *box = upload->box; + address->buffer_object = 0; + address->addr = upload->sysmem; + return true; + } + } + + return false; +} + "SIZE_T i;", or at least "unsigned int i;"
Oh, right, "while (i--)" does work with unsigned integers. Reverse iteration is annoying :-/
HRESULT CDECL wined3d_device_context_unmap(struct wined3d_device_context *context, struct wined3d_resource *resource, unsigned int sub_resource_idx) { + struct wined3d_const_bo_address addr; + struct wined3d_box box; + TRACE("context %p, resource %p, sub_resource_idx %u.\n", context, resource, sub_resource_idx);
- return context->ops->unmap(context, resource, sub_resource_idx); + if (context->ops->get_upload_bo(context, resource, sub_resource_idx, &box, &addr)) + { + unsigned int row_pitch, slice_pitch; + + wined3d_resource_get_sub_resource_map_pitch(resource, sub_resource_idx, &row_pitch, &slice_pitch); + wined3d_device_context_upload_bo(context, resource, sub_resource_idx, &box, &addr, row_pitch, slice_pitch); + return WINED3D_OK; + } + else + { + return context->ops->unmap(context, resource, sub_resource_idx); + } }
Would we ever use ops->get_upload_bo() and ops->unmap() independently of each other? If not, we may as well merge those together.
No, but they're conceptually pretty different, and merging them strikes me as ugly... I mean, the nice way to merge them is to also put the prepare_upload_bo bits into map(), but that defeats the purpose of having the helper in the first place.
On Wed, 30 Jun 2021 at 18:45, Zebediah Figura (she/her) <zfigura(a)codeweavers.com> wrote:
On 6/30/21 7:14 AM, Henri Verbeet wrote:
Would we ever use ops->get_upload_bo() and ops->unmap() independently of each other? If not, we may as well merge those together.
No, but they're conceptually pretty different, and merging them strikes me as ugly...
Well, wined3d_device_context_unmap() seems conceptually similar enough to wined3d_deferred_context_unmap(). I.e., we could implement wined3d_deferred_context_unmap() like this: ... if (!wined3d_deferred_context_get_upload_bo(...)) return E_NOTIMPL; wined3d_resource_get_sub_resource_map_pitch(...); wined3d_device_context_upload_bo(...); return WINED3D_OK; which doesn't seem terrible, and then just call ops->unmap() in wined3d_device_context_unmap() like we do now. Of course, wined3d_cs_unmap() would be different, but it would also retrieve the upload bo in a different way. Alternatively, we could do something like this in wined3d_device_context_unmap(): ... if (context->ops->get_upload_bo(...)) { ... wined3d_device_context_upload_bo(...); return WINED3D_OK; } ... return wined3d_device_context_emit_unmap(...); which should also work, and may be preferable, because we could implement wined3d_device_context_map() and wined3d_device_context_update_sub_resource() in a similar way. I.e., I think we can make this work with either prepare_upload_bo()/get_upload_bo() or update_sub_resource()/map()/unmap() in the device context ops, but we probably don't need both sets of ops.
On 6/30/21 2:46 PM, Henri Verbeet wrote:
On Wed, 30 Jun 2021 at 18:45, Zebediah Figura (she/her) <zfigura(a)codeweavers.com> wrote:
On 6/30/21 7:14 AM, Henri Verbeet wrote:
Would we ever use ops->get_upload_bo() and ops->unmap() independently of each other? If not, we may as well merge those together.
No, but they're conceptually pretty different, and merging them strikes me as ugly...
Well, wined3d_device_context_unmap() seems conceptually similar enough to wined3d_deferred_context_unmap(). I.e., we could implement wined3d_deferred_context_unmap() like this:
... if (!wined3d_deferred_context_get_upload_bo(...)) return E_NOTIMPL;
wined3d_resource_get_sub_resource_map_pitch(...); wined3d_device_context_upload_bo(...);
return WINED3D_OK;
which doesn't seem terrible, and then just call ops->unmap() in wined3d_device_context_unmap() like we do now. Of course, wined3d_cs_unmap() would be different, but it would also retrieve the upload bo in a different way.
Alternatively, we could do something like this in wined3d_device_context_unmap():
... if (context->ops->get_upload_bo(...)) { ... wined3d_device_context_upload_bo(...); return WINED3D_OK; } ... return wined3d_device_context_emit_unmap(...);
Where I'm guessing wined3d_device_context_emit_unmap() contains pretty much the entirety of wined3d_cs_unmap()? That had occurred to me as well, although I didn't implement it. I probably would leave it for a separate patch, though I'm not opposed to rolling it into this one.
which should also work, and may be preferable, because we could implement wined3d_device_context_map() and wined3d_device_context_update_sub_resource() in a similar way. I.e., I think we can make this work with either prepare_upload_bo()/get_upload_bo() or update_sub_resource()/map()/unmap() in the device context ops, but we probably don't need both sets of ops.
I'd agree the second approach outlined in this email looks clearer. That, or to just use update_sub_resource/map/unmap. I don't have a preference between those two, so I'll leave that decision to the maintainer ;-)
On Wed, 30 Jun 2021 at 22:07, Zebediah Figura (she/her) <zfigura(a)codeweavers.com> wrote:
On 6/30/21 2:46 PM, Henri Verbeet wrote:
Alternatively, we could do something like this in wined3d_device_context_unmap():
... if (context->ops->get_upload_bo(...)) { ... wined3d_device_context_upload_bo(...); return WINED3D_OK; } ... return wined3d_device_context_emit_unmap(...);
Where I'm guessing wined3d_device_context_emit_unmap() contains pretty much the entirety of wined3d_cs_unmap()?
Right.
That had occurred to me as well, although I didn't implement it. I probably would leave it for a separate patch, though I'm not opposed to rolling it into this one.
Sure, we could do that in a subsequent patch.
which should also work, and may be preferable, because we could implement wined3d_device_context_map() and wined3d_device_context_update_sub_resource() in a similar way. I.e., I think we can make this work with either prepare_upload_bo()/get_upload_bo() or update_sub_resource()/map()/unmap() in the device context ops, but we probably don't need both sets of ops.
I'd agree the second approach outlined in this email looks clearer. That, or to just use update_sub_resource/map/unmap. I don't have a preference between those two, so I'll leave that decision to the maintainer ;-)
I'm leaning towards prepare_upload_bo()/get_upload_bo() because it's the smaller device context ops interface, and results in fewer levels of map/unmap API, but it's not a strong preference.
Signed-off-by: Zebediah Figura <z.figura12(a)gmail.com> --- dlls/d3d11/tests/d3d11.c | 42 +++++++++++++++++----------------------- dlls/wined3d/cs.c | 21 +++++++++++++++++++- 2 files changed, 38 insertions(+), 25 deletions(-) diff --git a/dlls/d3d11/tests/d3d11.c b/dlls/d3d11/tests/d3d11.c index df1d5a35c6e..fcbe4f4a740 100644 --- a/dlls/d3d11/tests/d3d11.c +++ b/dlls/d3d11/tests/d3d11.c @@ -33134,38 +33134,32 @@ static void test_deferred_context_map(void) ok(hr == E_INVALIDARG, "Got unexpected hr %#x.\n", hr); hr = ID3D11DeviceContext_Map(deferred, (ID3D11Resource *)buffer, 0, D3D11_MAP_WRITE_NO_OVERWRITE, 0, &map_desc); - todo_wine ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); - if (hr == S_OK) + map_data = map_desc.pData; + for (i = 0; i < ARRAY_SIZE(data); ++i) { - map_data = map_desc.pData; - for (i = 0; i < ARRAY_SIZE(data); ++i) - { - ok(map_data[i] == 2 * i, "Got unexpected value %.8e at %u.\n", map_data[i], i); - if (i % 2) - map_data[i] = 3 * i; - } - memcpy(data, map_data, sizeof(data)); - - ID3D11DeviceContext_Unmap(deferred, (ID3D11Resource *)buffer, 0); + ok(map_data[i] == 2 * i, "Got unexpected value %.8e at %u.\n", map_data[i], i); + if (i % 2) + map_data[i] = 3 * i; } + memcpy(data, map_data, sizeof(data)); + + ID3D11DeviceContext_Unmap(deferred, (ID3D11Resource *)buffer, 0); hr = ID3D11DeviceContext_Map(deferred, (ID3D11Resource *)buffer, 0, D3D11_MAP_WRITE_NO_OVERWRITE, 0, &map_desc); - todo_wine ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); + ok(hr == S_OK, "Got unexpected hr %#x.\n", hr); - if (hr == S_OK) + map_data = map_desc.pData; + for (i = 0; i < ARRAY_SIZE(data); ++i) { - map_data = map_desc.pData; - for (i = 0; i < ARRAY_SIZE(data); ++i) - { - ok(map_data[i] == data[i], "Got unexpected value %.8e at %u.\n", map_data[i], i); - if (i % 3) - map_data[i] = 4 * i; - } - memcpy(data, map_data, sizeof(data)); - - ID3D11DeviceContext_Unmap(deferred, (ID3D11Resource *)buffer, 0); + ok(map_data[i] == data[i], "Got unexpected value %.8e at %u.\n", map_data[i], i); + if (i % 3) + map_data[i] = 4 * i; } + memcpy(data, map_data, sizeof(data)); + + ID3D11DeviceContext_Unmap(deferred, (ID3D11Resource *)buffer, 0); hr = ID3D11DeviceContext_FinishCommandList(deferred, FALSE, &list); ok(hr == S_OK, "Failed to create command list, hr %#x.\n", hr); diff --git a/dlls/wined3d/cs.c b/dlls/wined3d/cs.c index c70606ad036..51fb4eefcd9 100644 --- a/dlls/wined3d/cs.c +++ b/dlls/wined3d/cs.c @@ -3452,12 +3452,31 @@ static void *wined3d_deferred_context_prepare_upload_bo(struct wined3d_device_co return NULL; } - if (flags & ~(WINED3D_MAP_WRITE | WINED3D_MAP_DISCARD)) + if (flags & ~(WINED3D_MAP_WRITE | WINED3D_MAP_DISCARD | WINED3D_MAP_NOOVERWRITE)) { FIXME("Unhandled flags %#x.\n", flags); return NULL; } + if (flags & WINED3D_MAP_NOOVERWRITE) + { + int i = deferred->upload_count; + + while (i--) + { + struct wined3d_deferred_upload *upload = &deferred->uploads[i]; + + if (upload->resource == resource && upload->sub_resource_idx == sub_resource_idx) + { + address->buffer_object = 0; + address->addr = upload->sysmem; + return upload->sysmem; + } + } + + return NULL; + } + if (!wined3d_array_reserve((void **)&deferred->uploads, &deferred->uploads_capacity, deferred->upload_count + 1, sizeof(*deferred->uploads))) return NULL; -- 2.30.2
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=93328 Your paranoid android. === w1064v1809 (32 bit report) === d3d11: d3d11.c:5916: Test failed: Got unexpected IAVertices count: 0. d3d11.c:5917: Test failed: Got unexpected IAPrimitives count: 0. d3d11.c:5918: Test failed: Got unexpected VSInvocations count: 0. d3d11.c:5921: Test failed: Got unexpected CInvocations count: 0. d3d11.c:5922: Test failed: Got unexpected CPrimitives count: 0. d3d11.c:5763: Test failed: Got unexpected query result 0x0000000000000000. === w1064 (32 bit report) === d3d11: d3d11.c:5916: Test failed: Got unexpected IAVertices count: 0. d3d11.c:5917: Test failed: Got unexpected IAPrimitives count: 0. d3d11.c:5918: Test failed: Got unexpected VSInvocations count: 0. d3d11.c:5921: Test failed: Got unexpected CInvocations count: 0. d3d11.c:5922: Test failed: Got unexpected CPrimitives count: 0. d3d11.c:5763: Test failed: Got unexpected query result 0x0000000000000000. === w10pro64 (32 bit report) === d3d11: d3d11.c:5763: Test failed: Got unexpected query result 0x0000000000000000. d3d11.c:5916: Test failed: Got unexpected IAVertices count: 0. d3d11.c:5917: Test failed: Got unexpected IAPrimitives count: 0. d3d11.c:5918: Test failed: Got unexpected VSInvocations count: 0. d3d11.c:5921: Test failed: Got unexpected CInvocations count: 0. d3d11.c:5922: Test failed: Got unexpected CPrimitives count: 0. === debiant2 (32 bit Hebrew:Israel report) === d3d11: d3d11.c:20083: Test failed: d3d11.c:16407: Test marked todo: Got {0x80000000, 0x00000000, 0x00000000, 0x00000000}, expected {0x00000000, 0xffffffff, 0x00000000, 0x00000000} at (0, 0), sub-resource 0.
On Wed, 30 Jun 2021 at 06:34, Zebediah Figura <z.figura12(a)gmail.com> wrote:
@@ -3452,12 +3452,31 @@ static void *wined3d_deferred_context_prepare_upload_bo(struct wined3d_device_co return NULL; }
- if (flags & ~(WINED3D_MAP_WRITE | WINED3D_MAP_DISCARD)) + if (flags & ~(WINED3D_MAP_WRITE | WINED3D_MAP_DISCARD | WINED3D_MAP_NOOVERWRITE)) { FIXME("Unhandled flags %#x.\n", flags); return NULL; }
+ if (flags & WINED3D_MAP_NOOVERWRITE) + { + int i = deferred->upload_count; + + while (i--) + { + struct wined3d_deferred_upload *upload = &deferred->uploads[i]; + + if (upload->resource == resource && upload->sub_resource_idx == sub_resource_idx) + { + address->buffer_object = 0; + address->addr = upload->sysmem; + return upload->sysmem; + } + } + + return NULL; + } + I.e., wined3d_deferred_context_get_upload_bo(), right?
On 6/30/21 7:15 AM, Henri Verbeet wrote:
On Wed, 30 Jun 2021 at 06:34, Zebediah Figura <z.figura12(a)gmail.com> wrote:
@@ -3452,12 +3452,31 @@ static void *wined3d_deferred_context_prepare_upload_bo(struct wined3d_device_co return NULL; }
- if (flags & ~(WINED3D_MAP_WRITE | WINED3D_MAP_DISCARD)) + if (flags & ~(WINED3D_MAP_WRITE | WINED3D_MAP_DISCARD | WINED3D_MAP_NOOVERWRITE)) { FIXME("Unhandled flags %#x.\n", flags); return NULL; }
+ if (flags & WINED3D_MAP_NOOVERWRITE) + { + int i = deferred->upload_count; + + while (i--) + { + struct wined3d_deferred_upload *upload = &deferred->uploads[i]; + + if (upload->resource == resource && upload->sub_resource_idx == sub_resource_idx) + { + address->buffer_object = 0; + address->addr = upload->sysmem; + return upload->sysmem; + } + } + + return NULL; + } + I.e., wined3d_deferred_context_get_upload_bo(), right?
Y...es. Except it's a bit conceptually awkward because of box sizes. Perhaps a common helper would be better.
On Wed, 30 Jun 2021 at 06:33, Zebediah Figura <z.figura12(a)gmail.com> wrote:
+static void *wined3d_deferred_context_prepare_upload_bo(struct wined3d_device_context *context, + struct wined3d_resource *resource, unsigned int sub_resource_idx, const struct wined3d_box *box, + unsigned int row_pitch, unsigned int slice_pitch, uint32_t flags, struct wined3d_const_bo_address *address) +{ + FIXME("context %p, resource %p, sub_resource_idx %u, box %p, flags %#x, address %p, stub!\n", + context, resource, sub_resource_idx, box, flags, address); + return false; +} + That works, but should probably be "return NULL;" instead.
participants (4)
-
Henri Verbeet -
Marvin -
Zebediah Figura -
Zebediah Figura (she/her)