On Sun, 21 Mar 2021 at 22:06, Matteo Bruni matteo.mystral@gmail.com wrote:
On Thu, Mar 18, 2021 at 2:33 PM Henri Verbeet hverbeet@gmail.com wrote:
On Wed, 17 Mar 2021 at 13:35, Matteo Bruni mbruni@codeweavers.com wrote:
@@ -1666,6 +1666,16 @@ HRESULT texture2d_blt(struct wined3d_texture *dst_texture, unsigned int dst_sub_ TRACE("Not doing download because of partial download (src).\n"); else if (!wined3d_texture_is_full_rect(dst_texture, dst_sub_resource_idx % dst_texture->level_count, &dst_rect)) TRACE("Not doing download because of partial download (dst).\n");
else if (src_sub_resource->locations == WINED3D_LOCATION_DRAWABLE){context = context_acquire(device, src_texture, src_sub_resource_idx);texture2d_read_from_framebuffer(src_texture, src_sub_resource_idx, context,WINED3D_LOCATION_DRAWABLE, dst_texture->resource.map_binding);wined3d_texture_validate_location(src_texture, src_sub_resource_idx, dst_texture->resource.map_binding);context_release(context);return texture2d_blt(dst_texture, dst_sub_resource_idx, dst_box,src_texture, src_sub_resource_idx, src_box, flags, fx, filter);}It's perhaps not immediately obvious from the function name, but texture2d_read_from_framebuffer() is specific to the GL backend. Calling it from common code like texture2d_blt() is problematic.
You're right :/ For some background, this patch was also related to backbuffer ORM: IIRC after this the d3d tests start to give sensible results (mostly broken, of course, but at least backbuffer readback works so SOME tests pass).
I didn't write the patch, but my guess would be that the issue was that wined3d_texture_download_from_texture() doesn't handle resources in WINED3D_LOCATION_DRAWABLE. Perhaps the most straightforward way to address that would be to simply implement downloading from WINED3D_LOCATION_DRAWABLE in wined3d_texture_download_from_texture() and wined3d_texture_gl_download_data(). Alternatively: - We could detect that case here, skip wined3d_texture_download_from_texture(), and let the blitter figure it out. That should work, although it may introduce more copies between the DRAWABLE/TEXTURE/SYSMEM locations than desirable. - Instead of skipping wined3d_texture_download_from_texture(), we could call wined3d_texture_load_location() before calling it. The main disadvantage of that approach is that for on-screen resources like the back-buffer that ends up doing a flip in system memory first. In that context, it may be worth considering using the "AlwaysOffscreen" approach for backbuffer ORM as well. That would introduce an extra blit for presents, but would simplify some other, potentially more expensive things, like the issue we're discussing here.