On Mon, 11 Oct 2021 at 05:53, Zebediah Figura zfigura@codeweavers.com wrote:
+static void invalidate_persistent_map(struct wined3d_resource *resource, unsigned int sub_resource_idx) +{
- struct wined3d_client_sub_resource *sub_resource;
- sub_resource = wined3d_resource_get_client_sub_resource(resource, sub_resource_idx);
- memset(&sub_resource->addr, 0, sizeof(sub_resource->addr));
+}
Is this function correctly named? It doesn't quite do what I'd expect something called invalidate_persistent_map() to do.
I suppose your expectation is that it invalidates the map contents, which it does not, but rather it invalidates the map pointer. I don't know how better to name it to clarify that difference. (Shove a _ptr onto the end, maybe?)
Essentially it means "the CS thread might reallocate the BO behind our back, thereby invalidating bo->map_ptr, which means we can't treat this one as canonical anymore". It's definitely awkward, but I don't see a better way to achieve this...
Part of the issue is that it operates on a wined3d_resource, I think. You can tell it actually modifies the client sub-resource by looking at the implementation, but ideally you wouldn't have to. On the other hand, the fact that it's a persistent map is somewhat immaterial. I guess I would have expected something like invalidate_client_address(), or perhaps wined3d_client_sub_resource_invalidate_address().
@@ -2554,7 +2585,7 @@ HRESULT wined3d_device_context_emit_unmap(struct wined3d_device_context *context unsigned int row_pitch, slice_pitch;
wined3d_resource_get_sub_resource_map_pitch(resource, sub_resource_idx, &row_pitch, &slice_pitch);
if (bo.flags & UPLOAD_BO_UPLOAD_ON_UNMAP)
if (bo.flags & (UPLOAD_BO_UPLOAD_ON_UNMAP | UPLOAD_BO_FLUSH_ON_UNMAP)) wined3d_device_context_upload_bo(context, resource, sub_resource_idx, &box, &bo, row_pitch, slice_pitch); return WINED3D_OK; }Uploading when FLUSH_ON_UNMAP is set seems odd. I.e., should there ever be a case where FLUSH_ON_UNMAP is set, UPLOAD_ON_UNMAP is not set, and we still want to call wined3d_device_context_upload_bo()?
As it stands this patch never has both FLUSH_ON_UNMAP and UPLOAD_ON_UNMAP set at the same time. I essentially was thinking of them as different things to do on an unmap. I guess in that sense "UPLOAD" should really have been "COPY" or "BLIT" or something.
That probably works. Alternatively, could we just drop FLUSH_ON_UNMAP and set UPLOAD_ON_UNMAP any time we want to call wined3d_device_context_upload_bo()?