On 8/17/22 07:34, Henri Verbeet (@hverbeet) wrote:
From patch 2/5:
+bool wined3d_texture_validate_sub_resource_idx(const struct wined3d_texture *texture, unsigned int sub_resource_idx) +{ + if (sub_resource_idx < texture->level_count * texture->layer_count) + return true; + + WARN("Invalid sub-resource index %u.\n", sub_resource_idx); + return false; +}
It's fairly inconsequential, but `wined3d_texture_validate_sub_resource_idx()` sounds like something that modifies the texture. (Compare e.g. `wined3d_texture_validate_location()`.)
Perhaps, although I'm not sure how to distinguish "make valid" from "verify validity", and in the absence of other code I would typically assume the latter. Perhaps "verify" rather than "validate"?
From patch 3/5:
@@ -5521,8 +5521,23 @@ void CDECL wined3d_device_evict_managed_resources(struct wined3d_device *device) if ((resource->usage & WINED3DUSAGE_MANAGED) && !resource->map_count) { - TRACE("Evicting %p.\n", resource); - wined3d_cs_emit_unload_resource(device->cs, resource); + if (resource->access & WINED3D_RESOURCE_ACCESS_GPU) + { + TRACE("Evicting %p.\n", resource); + wined3d_cs_emit_unload_resource(device->cs, resource); + }
Doesn't `WINED3DUSAGE_MANAGED` imply `WINED3D_RESOURCE_ACCESS_GPU`?
At the moment, but the plan is to separate the texture into CPU and GPU copies, and at the moment at least it's more convenient to mark both copies with the MANAGED flag.
[In particular: d3d8_surface currently holds a reference to the sysmem texture, and this lets GetDesc() work without any extra work. Also, we currently check the MANAGED flag to determine whether to pin sysmem, but we should probably be pinning sysmem for all CPU-accessible textures.]
The above diff probably shouldn't have been part of this patch, though, and probably was combined with it by mistake.