[PATCH v2 0/2] MR2226: gdi32: Track ticks since draw start per window_surface.

[Alexandros Frantzis (＠afrantzis)]

Track ticks since draw start per window_surface, instead of per DC as is
currently the case. This change helps reduce visual glitches caused by
badly timed flushes when drawing to the same window_surface from
multiple DCs, e.g., child windows (see first commit), or in some special
scenarios when the window_surface is updated (see second commit).

(Much) more information about this issue and the rationale for the changes
can be found in the commit messages.

Note that since this is an inherently timing related issue, the visual glitches
depend on the application specific draw patterns, drawing speed (and thus
processor performance and load) etc.

Here is a capture which exhibits the issue with the current implementation:
[resizing-regedit-per-dc-ticks.mkv](/uploads/d7d30d009cc4db1337cbc3266df5ae17/resizing-regedit-per-dc-ticks.mkv)

And here is a capture of the same scenario with the proposed changes applied, which shows the improvement:
[resizing-regedit-per-surface-ticks.mkv](/uploads/6f878568862f5fb5157567341e4e452f/resizing-regedit-per-surface-ticks.mkv)

--
  v2: win32u: Reset draw_start_ticks for new window_surface.
      gdi32: Track ticks since draw start per window_surface.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2226

[PATCH v2 1/2] gdi32: Track ticks since draw start per window_surface.

[Alexandros Frantzis]

From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com>

Track ticks since draw start per window_surface, instead of per DC as is
currently the case. This change helps reduce visual glitches caused by
badly timed flushes when drawing to the same window_surface from
multiple DCs (e.g., for child windows).

This approach is a better fit for the current heuristic for forcing
flushing, which consults the shared window_surface bounds to decide
whether this is the start of a draw in order to reset the (currently per
DC) draw start time.

The problem in the current implementation occurs when a drawing to a DC
begins with an already damaged window_surface, e.g., due to draws from
other DCs targeting that window_surface. In such a case, the DC draw
start time is not reset and refers to the start of some previous draw
sequence using this DC, thus increasing the chances that the 50ms time
flush limit will be eventually exceeded in the middle of the current
draw sequence. In other words, the state of the (shared) window_surface
damage is not a reliable indicator of the beginning (or not) of a draw
to a DC.

An example, assuming DC1 and DC2 target the same window_surface:

DC1.start_ticks = 0
DC2.start_ticks = 0
FLUSH_PERIOD = 50

0 flush
1 draw to DC1 -> DC1.start_ticks = 1
... [no flush] ...
2 draw to DC2 -> DC2.start_ticks remains 0 since surface is damaged
...
50 flush
51 draw to DC1 -> DC1.start_ticks = 51
... [no flush] ...
52 draw to DC2 -> DC2.start_ticks remains 0 since surface is damaged,
                  current - DC2.start_ticks > FLUSH_PERIOD so we are
                  forced to flush in the middle of the drawing
                  sequence => potential glitch

Tracking the draw start per window_surface ameliorates the problem
because the beginning of a draw on a DC targeting an undamaged
window_surface resets the start time for all DCs targeting that
window_surface:

...
50 flush
51 draw to DC1 -> surface.draw_ticks = 51
... [no flush] ...
52 draw to DC2 -> surface.draw_ticks remains 51 since surface is damaged,
                  but current - surface.draw_ticks < FLUSH_PERIOD, so we
                  do not flush

Signed-off-by: Alexandros Frantzis <alexandros.frantzis(a)collabora.com>
---
 dlls/win32u/dibdrv/dc.c   | 6 +++---
 include/wine/gdi_driver.h | 3 ++-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/dlls/win32u/dibdrv/dc.c b/dlls/win32u/dibdrv/dc.c
index 68a009f576e..f96374c1229 100644
--- a/dlls/win32u/dibdrv/dc.c
+++ b/dlls/win32u/dibdrv/dc.c
@@ -731,7 +731,6 @@ struct windrv_physdev
     struct gdi_physdev     dev;
     struct dibdrv_physdev *dibdrv;
     struct window_surface *surface;
-    DWORD                  start_ticks;
 };
 
 static const struct gdi_dc_funcs window_driver;
@@ -745,13 +744,14 @@ static inline void lock_surface( struct windrv_physdev *dev )
 {
     /* gdi_lock should not be locked */
     dev->surface->funcs->lock( dev->surface );
-    if (IsRectEmpty( dev->dibdrv->bounds )) dev->start_ticks = NtGetTickCount();
+    if (IsRectEmpty( dev->dibdrv->bounds )) dev->surface->draw_start_ticks = NtGetTickCount();
 }
 
 static inline void unlock_surface( struct windrv_physdev *dev )
 {
+    BOOL should_flush = NtGetTickCount() - dev->surface->draw_start_ticks > FLUSH_PERIOD;
     dev->surface->funcs->unlock( dev->surface );
-    if (NtGetTickCount() - dev->start_ticks > FLUSH_PERIOD) dev->surface->funcs->flush( dev->surface );
+    if (should_flush) dev->surface->funcs->flush( dev->surface );
 }
 
 static void CDECL unlock_bits_surface( struct gdi_image_bits *bits )
diff --git a/include/wine/gdi_driver.h b/include/wine/gdi_driver.h
index 3db3d0d6af3..d7adaad1e35 100644
--- a/include/wine/gdi_driver.h
+++ b/include/wine/gdi_driver.h
@@ -168,7 +168,7 @@ struct gdi_dc_funcs
 };
 
 /* increment this when you change the DC function table */
-#define WINE_GDI_DRIVER_VERSION 81
+#define WINE_GDI_DRIVER_VERSION 82
 
 #define GDI_PRIORITY_NULL_DRV        0  /* null driver */
 #define GDI_PRIORITY_FONT_DRV      100  /* any font driver */
@@ -217,6 +217,7 @@ struct window_surface
     struct list                        entry; /* entry in global list managed by user32 */
     LONG                               ref;   /* reference count */
     RECT                               rect;  /* constant, no locking needed */
+    DWORD                              draw_start_ticks; /* start ticks of fresh draw */
     /* driver-specific fields here */
 };
 
-- 
GitLab


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

[PATCH v2 2/2] win32u: Reset draw_start_ticks for new window_surface.

[Alexandros Frantzis]

From: Alexandros Frantzis <alexandros.frantzis(a)collabora.com>

Normally, a new window_surface has no damage so its draw_start_ticks
value is reset at the time of the first lock_surface.

However, if contents from the old window_surface are copied to the new
one when the window_surface for a window is updated, the new
window_surface will be initially damaged. We want to reset the
draw_start_ticks value in this case too, to avoid flushes occuring soon
after such updates, during the initial redraw of the window_surface.

Signed-off-by: Alexandros Frantzis <alexandros.frantzis(a)collabora.com>
---
 dlls/win32u/dibdrv/dc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/dlls/win32u/dibdrv/dc.c b/dlls/win32u/dibdrv/dc.c
index f96374c1229..23ef7f64390 100644
--- a/dlls/win32u/dibdrv/dc.c
+++ b/dlls/win32u/dibdrv/dc.c
@@ -744,7 +744,8 @@ static inline void lock_surface( struct windrv_physdev *dev )
 {
     /* gdi_lock should not be locked */
     dev->surface->funcs->lock( dev->surface );
-    if (IsRectEmpty( dev->dibdrv->bounds )) dev->surface->draw_start_ticks = NtGetTickCount();
+    if (IsRectEmpty( dev->dibdrv->bounds ) || dev->surface->draw_start_ticks == 0)
+        dev->surface->draw_start_ticks = NtGetTickCount();
 }
 
 static inline void unlock_surface( struct windrv_physdev *dev )
-- 
GitLab

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