On 11/4/21 8:17 AM, Henri Verbeet wrote:
On Wed, 3 Nov 2021 at 21:47, Zebediah Figura zfigura@codeweavers.com wrote:
On 11/3/21 12:11 PM, Henri Verbeet wrote:
On Wed, 3 Nov 2021 at 00:37, Zebediah Figura zfigura@codeweavers.com wrote:
+static bool adapter_vk_alloc_bo(struct wined3d_device *device, struct wined3d_resource *resource,
unsigned int sub_resource_idx, struct wined3d_bo_address *addr)+{
- wined3d_not_from_cs(device->cs);
- if (resource->type == WINED3D_RTYPE_BUFFER)
- {
struct wined3d_buffer_vk *buffer_vk = wined3d_buffer_vk(buffer_from_resource(resource));struct wined3d_client_bo_vk *client_bo;if (!(client_bo = heap_alloc(sizeof(*client_bo))))return false;if (!wined3d_buffer_vk_create_buffer_object(buffer_vk, NULL, &client_bo->bo)){heap_free(client_bo);return false;}if (!client_bo->bo.b.map_ptr){struct wined3d_client_bo_vk_map_ctx ctx = {.device = device, .client_bo = client_bo};WARN_(d3d_perf)("BO %p (chunk %p, slab %p) is not persistently mapped.\n", &client_bo->bo,client_bo->bo.memory ? client_bo->bo.memory->chunk : NULL, client_bo->bo.slab);wined3d_cs_map_object(device->cs, wined3d_client_bo_vk_map_cs, &ctx);wined3d_cs_finish(device->cs, WINED3D_CS_QUEUE_MAP);}wined3d_cs_map_object() almost sounds like it would emit a WINED3D_CS_OP_MAP, somewhat like wined3d_device_context_map() back when that was still called wined3d_cs_map()...
Indeed, except that WINED3D_CS_OP_MAP takes the resource. I don't think we want to change that, either, although we could create a new CS op instead of using WINED3D_CS_OP_CALLBACK.
Yeah, I'm not suggesting we change WINED3D_CS_OP_MAP. It's somewhat moot for the Vulkan backend now, but I would argue that if we would have to go through the CS to map the bo anyway, we might as well either give up and just map the resource through the CS, or allocate the staging memory on the CPU.
I think the motivation for not just doing that was to make sure that we actually did allocate new memory, instead of repeatedly going through the slow path to use the same mapped memory. I don't remember if this happened in practice, but there's a comment to similar effect on one of Matteo's patches...
(Now that I think about it, *is* there any sort of a rule as to what deserves WINED3D_CS_OP_CALLBACK and what doesn't? I know that currently it's just a matter of init and destroy, but it seeems a bit arbitrary on reflection.)
For the most part, it's things that I don't think should have their own wined3d_cs_op. WINED3D_CS_OP_CALLBACK is a little ugly, but it works well enough for the moment. I suppose we could do something along the lines of the following:
struct wined3d_cs_resource_ops { void (*cs_resource_init)(struct wined3d_cs_resource *resource); void (*cs_resource_cleanup)(struct wined3d_cs_resource *resource); }; struct wined3d_cs_resource { struct wined3d_cs_resource_ops *ops; };and then
struct wined3d_cs_init_resource { enum wined3d_cs_op opcode; struct wined3d_cs_resource *resource; }; static void wined3d_cs_exec_init_resource(struct wined3d_cs *cs,const void *data) { const struct wined3d_cs_init_resource *op = data;
op->resource->ops->cs_resource_init(op->resource); }and equivalent cleanup variants. Something like wined3d_sampler_vk would then include a wined3d_cs_resource structure, and call "wined3d_cs_init_object(device->cs, &sampler_vk->cs_resource);". That ends up being largely equivalent to the current scheme, although perhaps a little more structured.
Seems reasonable.