On 10/13/21 4:03 PM, Henri Verbeet wrote:
On Tue, 12 Oct 2021 at 23:31, Zebediah Figura zfigura@codeweavers.com wrote:
+static void adapter_gl_flush_bo_address(struct wined3d_context *context,
const struct wined3d_const_bo_address *data, size_t size)+{
- FIXME("context %p, data %s, size %zu, stub!\n", context, debug_const_bo_address(data), size);
+}
Note that we do have an implementation for this in wined3d_context_gl_unmap_bo_address().
I left it out of this patch for simplicity (this patch is already large), but I'm certainly open to adding it in.
+static void flush_buffer_range(struct wined3d_context_vk *context_vk,
struct wined3d_bo_vk *bo, unsigned int offset, unsigned int size)+{
- struct wined3d_device_vk *device_vk = wined3d_device_vk(context_vk->c.device);
- const struct wined3d_vk_info *vk_info = context_vk->vk_info;
- VkMappedMemoryRange range;
- range.sType = VK_STRUCTURE_TYPE_MAPPED_MEMORY_RANGE;
- range.pNext = NULL;
- range.memory = bo->vk_memory;
- range.offset = bo->b.memory_offset + offset;
- range.size = size;
- VK_CALL(vkFlushMappedMemoryRanges(device_vk->vk_device, 1, &range));
+}
flush_buffer_range() doesn't actually operate on a buffer though...
Fair enough; I'll rename it ;-)
@@ -938,6 +938,7 @@ static HRESULT buffer_resource_sub_resource_map(struct wined3d_resource *resourc addr.buffer_object = buffer->buffer_object; addr.addr = 0; buffer->map_ptr = wined3d_context_map_bo_address(context, &addr, resource->size, flags);
buffer->resource.client.addr = addr;Does that update need to be atomic? I.e., is there any risk of the application thread seeing e.g. a new "addr.buffer_object" in combination with an old "addr.addr"? I think the idea is that we're protected from that by needing to go through the CS if the buffer isn't mapped yet, right?
Right, the client thread is waiting for this operation to complete and holding a mutex until it does, so it should be thread-safe.
I can add a comment to that effect if it'd help.
static bool wined3d_cs_map_upload_bo(struct wined3d_device_context *context, struct wined3d_resource *resource, unsigned int sub_resource_idx, struct wined3d_map_desc *map_desc, const struct wined3d_box *box, uint32_t flags) {
- /* FIXME: We would like to return mapped or newly allocated memory here. */
- /* Limit NOOVERWRITE maps to buffers for now; there are too many ways that
* a texture can be invalidated to even count. */- if (wined3d_map_persistent() && resource->type == WINED3D_RTYPE_BUFFER && (flags & WINED3D_MAP_NOOVERWRITE))
- {
struct wined3d_client_resource *client = &resource->client;const struct wined3d_bo *bo;uint8_t *map_ptr;bo = (const struct wined3d_bo *)client->addr.buffer_object;map_ptr = bo ? bo->map_ptr : NULL;map_ptr += (uintptr_t)client->addr.addr;if (!map_ptr){TRACE("Sub-resource is not persistently mapped.\n");return false;}So if "bo" is NULL, "map_ptr" can in theory end up being non-NULL here. I guess that in practice "client->addr.addr" is always going to be NULL if "bo" is, but in that case there's not much point in adding the offset before checking "map_ptr" either.
wined3d_resource_get_sub_resource_map_pitch(resource, sub_resource_idx,&map_desc->row_pitch, &map_desc->slice_pitch);client->mapped_upload.addr = *wined3d_const_bo_address(&client->addr);client->mapped_upload.flags = 0;if (bo)map_ptr += bo->memory_offset;Then we account for "bo" potentially being NULL here,
map_desc->data = resource_offset_map_pointer(resource, sub_resource_idx, map_ptr, box);if (!bo->coherent)client->mapped_upload.flags |= UPLOAD_BO_UPLOAD_ON_UNMAP;but not here.
I tried to account for the possibility that the persistent address for a buffer could be sysmem (which isn't done yet, but I'm inclined to understand is a reasonable potentiality), in which case the "!map_ptr" check should be correct as-is, but the "!bo->coherent" is clearly wrong. I'll fix the latter...