[PATCH v2 0/1] MR7428: wined3d: Use temporary buffer when stretching a surface to itself with cpu blit

[YuriK7 (＠riyu12383)]

Fixes https://bugs.winehq.org/show_bug.cgi?id=57283.

I believe some sort of smart two pass stretching is possible instead, but not worth it.

[dlls/wined3d/surface.c#L1570](https://gitlab.winehq.org/riyu12383/wine/-/blob/f48645249a3402801f1ca942c35e...) not exactly sure if it's related and should be removed then ?

--
  v2: wined3d: Use temporary buffer when stretching a surface to itself with cpu blit
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7428

[PATCH v2 1/1] wined3d: Use temporary buffer when stretching a surface to itself with cpu blit

[yuri_k7]

From: yuri_k7 <riyu12383(a)gmail.com>

---
 dlls/wined3d/surface.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/dlls/wined3d/surface.c b/dlls/wined3d/surface.c
index 14cfebd7074..a054caba15a 100644
--- a/dlls/wined3d/surface.c
+++ b/dlls/wined3d/surface.c
@@ -616,6 +616,7 @@ static HRESULT surface_cpu_blt(struct wined3d_texture *dst_texture, unsigned int
     const BYTE *sbase;
     const BYTE *sbuf;
     BYTE *dbuf;
+    BYTE *tmp_buffer = NULL;
 
     TRACE("dst_texture %p, dst_sub_resource_idx %u, dst_box %s, src_texture %p, "
             "src_sub_resource_idx %u, src_box %s, flags %#x, fx %p, filter %s.\n",
@@ -808,18 +809,45 @@ static HRESULT surface_cpu_blt(struct wined3d_texture *dst_texture, unsigned int
             else
             {
                 /* Stretching in y direction only. */
+
+                if (same_sub_resource)
+                {
+                    /* Use a temporary buffer if blitting surface to itself */
+                    tmp_buffer = malloc(src_height * src_map.row_pitch);
+                    if (tmp_buffer != NULL)
+                    {
+                        memcpy(tmp_buffer, sbase, src_height * src_map.row_pitch);
+                        sbase = tmp_buffer;
+                    }
+                }
+
                 for (y = sy = 0; y < dst_height; ++y, sy += yinc)
                 {
                     sbuf = sbase + (sy >> 16) * src_map.row_pitch;
                     memcpy(dbuf, sbuf, row_byte_count);
                     dbuf += dst_map.row_pitch;
                 }
+
+                if(same_sub_resource && tmp_buffer != NULL)
+                    free(tmp_buffer);
             }
         }
         else
         {
             /* Stretching in X direction. */
             unsigned int last_sy = ~0u;
+
+            if (same_sub_resource)
+            {
+                /* Use a temporary buffer if blitting surface to itself */
+                tmp_buffer = malloc(src_height * src_map.row_pitch);
+                if (tmp_buffer != NULL)
+                {
+                    memcpy(tmp_buffer, sbase, src_height * src_map.row_pitch);
+                    sbase = tmp_buffer;
+                }
+            }
+
             for (y = sy = 0; y < dst_height; ++y, sy += yinc)
             {
                 sbuf = sbase + (sy >> 16) * src_map.row_pitch;
@@ -878,6 +906,9 @@ do { \
                 dbuf += dst_map.row_pitch;
                 last_sy = sy;
             }
+
+            if(same_sub_resource && tmp_buffer != NULL)
+                free(tmp_buffer);
         }
     }
     else
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/7428

[YuriK7 (＠riyu12383)]

On Thu Feb 27 01:04:11 2025 +0000, Elizabeth Figura wrote:
> Thanks for the patch!
> >
>
> [dlls/wined3d/surface.c#L1570](https://gitlab.winehq.org/riyu12383/wine/-/blob/f48645249a3402801f1ca942c35e...)
> not exactly sure if it's related and should be removed then ?
> It's there because not every case of scaling is implemented in the CPU
> blitter. This patch addresses one case, but notably we still don't
> implement non-point filtering.
> It's also there because (at least at the time) it wasn't clear that
> using the CPU for such a complex blit was better for performance. (It
> may still not be better...)
> With respect to the patch itself, I feel like it should be possible to
> just do something simple like
> ```
> if (same_sub_resource)
> {
>     tmp_buffer = ...
>     memcpy(tmp_buffer, sbase, ...)
>     sbuf = tmp_buffer;
> }
> ```
> and then keep the rest of that branch as-is? That would be a bit simpler.
> A test would be appreciated.
> Ahah yes indeed, I was locked on allocating the minimum needed, but yeah that's also not worth it, I've updated it.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7428#note_96076