[PATCH 0/1] MR2463: dwmapi: Sleep in DwmFlush().

[Paul Gofman (＠gofman)]

wine-gecko started calling dwmapi since dwmapi stubs were improved between Wine 7 and 8. It loops in gfxWindowsPlatform.cpp:VBlankLoop() in a tight loop now because DwmFlush() is a no-op. Besides unneeded CPU load that also causes hangs on exit in Final Fantasy XIV launcher. The launcher can't get WM_QUIT message in its message loop because it constantly gets the messages queued from VBlankLoop() while processing previous "nsAppShell:EventID" message.

DwmFlush() is supposed to wait until next VBlank. My supplied test only tests that the wait is not alertable, but I also ad-hoc tested that the delay time introduced by DmwFlush() is between 0 and display refresh frequency.

[PATCH 1/1] dwmapi: Sleep in DwmFlush().

[Paul Gofman]

From: Paul Gofman

---
 dlls/dwmapi/dwmapi_main.c  | 37 +++++++++++++++++++++++++------------
 dlls/dwmapi/tests/dwmapi.c | 31 +++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 12 deletions(-)

diff --git a/dlls/dwmapi/dwmapi_main.c b/dlls/dwmapi/dwmapi_main.c
index 9ec03185f36..1ee1cc0ad4f 100644
--- a/dlls/dwmapi/dwmapi_main.c
+++ b/dlls/dwmapi/dwmapi_main.c
@@ -83,18 +83,6 @@ HRESULT WINAPI DwmGetColorizationColor(DWORD *colorization, BOOL *opaque_blend)
     return E_NOTIMPL;
 }

-/**********************************************************************
- *                  DwmFlush              (DWMAPI.@)
- */
-HRESULT WINAPI DwmFlush(void)
-{
-    static BOOL once;
-
-    if (!once++) FIXME("() stub\n");
-
-    return S_OK;
-}
-
 /**********************************************************************
  *        DwmInvalidateIconicBitmaps      (DWMAPI.@)
  */
@@ -264,6 +252,31 @@ HRESULT WINAPI DwmGetCompositionTimingInfo(HWND hwnd, DWM_TIMING_INFO *info)
     return S_OK;
 }

+/**********************************************************************
+ *                  DwmFlush              (DWMAPI.@)
+ */
+HRESULT WINAPI DwmFlush(void)
+{
+    LARGE_INTEGER qpf, qpc, delay;
+    LONG64 qpc_refresh_period;
+    int display_frequency;
+    static BOOL once;
+
+    if (!once++)
+        FIXME("() stub\n");
+    else
+        TRACE(".\n");
+
+    display_frequency = get_display_frequency();
+    NtQueryPerformanceCounter(&qpc, &qpf);
+    qpc_refresh_period = qpf.QuadPart / display_frequency;
+    delay.QuadPart = (qpc.QuadPart - ((qpc.QuadPart + qpc_refresh_period - 1) / qpc_refresh_period) * qpc_refresh_period)
+            * 10000000 / qpf.QuadPart;
+    NtDelayExecution(FALSE, &delay);
+
+    return S_OK;
+}
+
 /**********************************************************************
  *           DwmAttachMilContent         (DWMAPI.@)
  */
diff --git a/dlls/dwmapi/tests/dwmapi.c b/dlls/dwmapi/tests/dwmapi.c
index 29dbcbe74bd..7ae47f25ab7 100644
--- a/dlls/dwmapi/tests/dwmapi.c
+++ b/dlls/dwmapi/tests/dwmapi.c
@@ -83,8 +83,39 @@ static void test_DwmGetCompositionTimingInfo(void)
             "Got wrong monitor refresh period %s.\n", wine_dbgstr_longlong(timing_info.qpcRefreshPeriod));
 }

+static void WINAPI apc_func(ULONG_PTR apc_count)
+{
+    ++*(unsigned int *)apc_count;
+}
+
+static void test_DwmFlush(void)
+{
+    unsigned int apc_count;
+    BOOL enabled = FALSE;
+    HRESULT hr;
+    BOOL ret;
+
+    hr = DwmIsCompositionEnabled(&enabled);
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+
+    if (!enabled)
+    {
+        skip("DWM is disabled.\n");
+        return;
+    }
+    ret = QueueUserAPC(apc_func, GetCurrentThread(), (ULONG_PTR)&apc_count);
+    ok(ret, "QueueUserAPC returned %d\n", ret);
+    apc_count = 0;
+    hr = DwmFlush();
+    ok(hr == S_OK, "Got hr %#lx.\n", hr);
+    ok(!apc_count, "got apc_count %d.\n", apc_count);
+    SleepEx(0, TRUE);
+    ok(apc_count == 1, "got apc_count %d.\n", apc_count);
+}
+
 START_TEST(dwmapi)
 {
     test_DwmIsCompositionEnabled();
     test_DwmGetCompositionTimingInfo();
+    test_DwmFlush();
 }

[Alex Henrie (＠alexhenrie)]

Alex Henrie (@alexhenrie) commented about dlls/dwmapi/dwmapi_main.c:
> }
>
> +/**********************************************************************
> -
> -
>              DwmFlush              (DWMAPI.@)
> - */
>
> +HRESULT WINAPI DwmFlush(void)
> +{
> - LARGE_INTEGER qpf, qpc, delay;
> - LONG64 qpc_refresh_period;
> - int display_frequency;
> - static BOOL once;
> -
> - if (!once++)
> -
>    FIXME("() stub\n");
> - else
> -
>    TRACE(".\n");

It doesn't matter much, but why print "." here instead of "()"?

[Paul Gofman (＠gofman)]

On Wed Mar 22 17:28:59 2023 +0000, Alex Henrie wrote:
> It doesn't matter much, but why print "." here instead of "()"?

Eh, yeah, it is definitely better to have it consistent but I'd rather change () to . in the FIXME.

[Alex Henrie (＠alexhenrie)]

On Wed Mar 22 17:30:44 2023 +0000, Paul Gofman wrote:
> Eh, yeah, it is definitely better to have it consistent but I'd rather
> change () to . in the FIXME.

Keep in mind that a space is automatically added between the function name and the message, so users will see `DwmFlush .`. Personally I'd prefer to see `DwmFlush ()` or even `DwmFlush `, but again, it doesn't really matter; whatever you choose is fine.