01/01: [USP10] Re-use script caches for the same font. CORE-14192

[Thomas Faber]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=cd992d022ff8b6151a592…

commit cd992d022ff8b6151a59214ca0883d8f04bd4569
Author:     Thomas Faber <thomas.faber(a)reactos.org>
AuthorDate: Sat Jan 20 13:41:14 2018 +0100
Commit:     Thomas Faber <thomas.faber(a)reactos.org>
CommitDate: Sat Jan 20 15:55:07 2018 +0100

    [USP10] Re-use script caches for the same font. CORE-14192
    
    This significantly speeds up WM_SETTEXT in multiline edit controls.
---
 dll/win32/usp10/usp10.c                  |  69 ++++++++++++++++--
 dll/win32/usp10/usp10_internal.h         |   3 +
 modules/rostests/winetests/usp10/usp10.c | 118 +++++++++++++++++++++++++++++++
 3 files changed, 184 insertions(+), 6 deletions(-)

diff --git a/dll/win32/usp10/usp10.c b/dll/win32/usp10/usp10.c
index 058fc6f368..abc40fc891 100644
--- a/dll/win32/usp10/usp10.c
+++ b/dll/win32/usp10/usp10.c
@@ -664,6 +664,16 @@ static const SCRIPT_PROPERTIES *script_props[] =
     &scriptInformation[80].props, &scriptInformation[81].props
 };
 
+static CRITICAL_SECTION cs_script_cache;
+static CRITICAL_SECTION_DEBUG cs_script_cache_dbg =
+{
+    0, 0, &cs_script_cache,
+    { &cs_script_cache_dbg.ProcessLocksList,
&cs_script_cache_dbg.ProcessLocksList },
+      0, 0, { (DWORD_PTR)(__FILE__ ": script_cache") }
+};
+static CRITICAL_SECTION cs_script_cache = { &cs_script_cache_dbg, -1, 0, 0, 0, 0 };
+static struct list script_cache_list = LIST_INIT(script_cache_list);
+
 typedef struct {
     ScriptCache *sc;
     int numGlyphs;
@@ -842,12 +852,34 @@ static inline BOOL set_cache_glyph_widths(SCRIPT_CACHE *psc, WORD
glyph, ABC *ab
 static HRESULT init_script_cache(const HDC hdc, SCRIPT_CACHE *psc)
 {
     ScriptCache *sc;
-    int size;
+    unsigned size;
+    LOGFONTW lf;
 
     if (!psc) return E_INVALIDARG;
     if (*psc) return S_OK;
     if (!hdc) return E_PENDING;
 
+    if (!GetObjectW(GetCurrentObject(hdc, OBJ_FONT), sizeof(lf), &lf))
+    {
+        return E_INVALIDARG;
+    }
+    /* Ensure canonical result by zeroing extra space in lfFaceName */
+    size = strlenW(lf.lfFaceName);
+    memset(lf.lfFaceName + size, 0, sizeof(lf.lfFaceName) - size * sizeof(WCHAR));
+
+    EnterCriticalSection(&cs_script_cache);
+    LIST_FOR_EACH_ENTRY(sc, &script_cache_list, ScriptCache, entry)
+    {
+        if (!memcmp(&sc->lf, &lf, sizeof(lf)))
+        {
+            sc->refcount++;
+            LeaveCriticalSection(&cs_script_cache);
+            *psc = sc;
+            return S_OK;
+        }
+    }
+    LeaveCriticalSection(&cs_script_cache);
+
     if (!(sc = heap_alloc_zero(sizeof(ScriptCache)))) return E_OUTOFMEMORY;
     if (!GetTextMetricsW(hdc, &sc->tm))
     {
@@ -861,18 +893,32 @@ static HRESULT init_script_cache(const HDC hdc, SCRIPT_CACHE *psc)
         sc->otm->otmSize = size;
         GetOutlineTextMetricsW(hdc, size, sc->otm);
     }
-    if (!GetObjectW(GetCurrentObject(hdc, OBJ_FONT), sizeof(LOGFONTW), &sc->lf))
-    {
-        heap_free(sc);
-        return E_INVALIDARG;
-    }
     sc->sfnt = (GetFontData(hdc,
MS_MAKE_TAG('h','e','a','d'), 0, NULL, 0)!=GDI_ERROR);
     if (!set_cache_font_properties(hdc, sc))
     {
         heap_free(sc);
         return E_INVALIDARG;
     }
+    sc->lf = lf;
+    sc->refcount = 1;
     *psc = sc;
+
+    EnterCriticalSection(&cs_script_cache);
+    list_add_head(&script_cache_list, &sc->entry);
+    LIST_FOR_EACH_ENTRY(sc, &script_cache_list, ScriptCache, entry)
+    {
+        if (sc != *psc && !memcmp(&sc->lf, &lf, sizeof(lf)))
+        {
+            /* Another thread won the race. Use their cache instead of ours */
+            list_remove(&sc->entry);
+            sc->refcount++;
+            LeaveCriticalSection(&cs_script_cache);
+            heap_free(*psc);
+            *psc = sc;
+            return S_OK;
+        }
+    }
+    LeaveCriticalSection(&cs_script_cache);
     TRACE("<- %p\n", sc);
     return S_OK;
 }
@@ -1025,6 +1071,17 @@ HRESULT WINAPI ScriptFreeCache(SCRIPT_CACHE *psc)
     {
         unsigned int i;
         INT n;
+
+        EnterCriticalSection(&cs_script_cache);
+        if (--((ScriptCache *)*psc)->refcount > 0)
+        {
+            LeaveCriticalSection(&cs_script_cache);
+            *psc = NULL;
+            return S_OK;
+        }
+        list_remove(&((ScriptCache *)*psc)->entry);
+        LeaveCriticalSection(&cs_script_cache);
+
         for (i = 0; i < GLYPH_MAX / GLYPH_BLOCK_SIZE; i++)
         {
             heap_free(((ScriptCache *)*psc)->widths[i]);
diff --git a/dll/win32/usp10/usp10_internal.h b/dll/win32/usp10/usp10_internal.h
index da5f3fe8eb..c0e28d542c 100644
--- a/dll/win32/usp10/usp10_internal.h
+++ b/dll/win32/usp10/usp10_internal.h
@@ -37,6 +37,7 @@
 
 #include <wine/debug.h>
 #include <wine/unicode.h>
+#include <wine/list.h>
 
 #define MS_MAKE_TAG( _x1, _x2, _x3, _x4 ) \
           ( ( (ULONG)_x4 << 24 ) |     \
@@ -194,6 +195,8 @@ typedef struct {
 } CacheGlyphPage;
 
 typedef struct {
+    struct list entry;
+    DWORD refcount;
     LOGFONTW lf;
     TEXTMETRICW tm;
     OUTLINETEXTMETRICW *otm;
diff --git a/modules/rostests/winetests/usp10/usp10.c
b/modules/rostests/winetests/usp10/usp10.c
index 417d0b8888..1ea873786a 100644
--- a/modules/rostests/winetests/usp10/usp10.c
+++ b/modules/rostests/winetests/usp10/usp10.c
@@ -1836,6 +1836,7 @@ static void test_ScriptShape(HDC hdc)
     static const WCHAR test3[] = {0x30b7};
     HRESULT hr;
     SCRIPT_CACHE sc = NULL;
+    SCRIPT_CACHE sc2 = NULL;
     WORD glyphs[4], glyphs2[4], logclust[4], glyphs3[4];
     SCRIPT_VISATTR attrs[4];
     SCRIPT_ITEM items[4];
@@ -1865,6 +1866,10 @@ static void test_ScriptShape(HDC hdc)
     ok(hr == S_OK, "ScriptShape should return S_OK not %08x\n", hr);
     ok(items[0].a.fNoGlyphIndex == FALSE, "fNoGlyphIndex TRUE\n");
 
+    hr = ScriptShape(hdc, &sc2, test1, 4, 4, &items[0].a, glyphs, logclust,
attrs, &nb);
+    ok(hr == S_OK, "ScriptShape should return S_OK not %08x\n", hr);
+    ok(sc2 == sc, "caches %p, %p not identical\n", sc, sc2);
+    ScriptFreeCache(&sc2);
 
     memset(glyphs,-1,sizeof(glyphs));
     memset(logclust,-1,sizeof(logclust));
@@ -2090,6 +2095,7 @@ static void test_ScriptPlace(HDC hdc)
     BOOL ret;
     HRESULT hr;
     SCRIPT_CACHE sc = NULL;
+    SCRIPT_CACHE sc2 = NULL;
     WORD glyphs[4], logclust[4];
     SCRIPT_VISATTR attrs[4];
     SCRIPT_ITEM items[2];
@@ -2127,6 +2133,11 @@ static void test_ScriptPlace(HDC hdc)
     ok(hr == S_OK, "ScriptPlace should return S_OK not %08x\n", hr);
     ok(items[0].a.fNoGlyphIndex == FALSE, "fNoGlyphIndex TRUE\n");
 
+    hr = ScriptPlace(hdc, &sc2, glyphs, 4, attrs, &items[0].a, widths, offset,
NULL);
+    ok(hr == S_OK, "ScriptPlace should return S_OK not %08x\n", hr);
+    ok(sc2 == sc, "caches %p, %p not identical\n", sc, sc2);
+    ScriptFreeCache(&sc2);
+
     if (widths[0] != 0)
     {
         int old_width = widths[0];
@@ -4112,6 +4123,112 @@ static void test_ScriptString_pSize(HDC hdc)
     ok(hr == S_OK, "Failed to free ssa, hr %#x.\n", hr);
 }
 
+static void test_script_cache_reuse(void)
+{
+    HRESULT hr;
+    HWND hwnd1, hwnd2;
+    HDC hdc1, hdc2;
+    LOGFONTA lf;
+    HFONT hfont1, hfont2;
+    HFONT prev_hfont1, prev_hfont2;
+    SCRIPT_CACHE sc = NULL;
+    SCRIPT_CACHE sc2;
+    LONG height;
+
+    hwnd1 = create_test_window();
+    hwnd2 = create_test_window();
+
+    hdc1 = GetDC(hwnd1);
+    hdc2 = GetDC(hwnd2);
+    ok(hdc1 != NULL && hdc2 != NULL, "Failed to get window dc.\n");
+
+    memset(&lf, 0, sizeof(LOGFONTA));
+    lstrcpyA(lf.lfFaceName, "Tahoma");
+
+    lf.lfHeight = 10;
+    hfont1 = CreateFontIndirectA(&lf);
+    ok(hfont1 != NULL, "CreateFontIndirectA failed\n");
+    hfont2 = CreateFontIndirectA(&lf);
+    ok(hfont2 != NULL, "CreateFontIndirectA failed\n");
+    ok(hfont1 != hfont2, "Expected fonts %p and %p to differ\n", hfont1,
hfont2);
+
+    prev_hfont1 = SelectObject(hdc1, hfont1);
+    ok(prev_hfont1 != NULL, "SelectObject failed: %p\n", prev_hfont1);
+    prev_hfont2 = SelectObject(hdc2, hfont1);
+    ok(prev_hfont2 != NULL, "SelectObject failed: %p\n", prev_hfont2);
+
+    /* Get a script cache */
+    hr = ScriptCacheGetHeight(hdc1, &sc, &height);
+    ok(hr == S_OK, "expected S_OK, got 0x%08x\n", hr);
+    ok(sc != NULL, "Script cache is NULL\n");
+
+    /* Same font, same DC -> same SCRIPT_CACHE */
+    sc2 = NULL;
+    hr = ScriptCacheGetHeight(hdc1, &sc2, &height);
+    ok(hr == S_OK, "expected S_OK, got 0x%08x\n", hr);
+    ok(sc2 != NULL, "Script cache is NULL\n");
+    ok(sc == sc2, "Expected caches %p, %p to be identical\n", sc, sc2);
+    ScriptFreeCache(&sc2);
+
+    /* Same font in different DC -> same SCRIPT_CACHE */
+    sc2 = NULL;
+    hr = ScriptCacheGetHeight(hdc2, &sc2, &height);
+    ok(hr == S_OK, "expected S_OK, got 0x%08x\n", hr);
+    ok(sc2 != NULL, "Script cache is NULL\n");
+    ok(sc == sc2, "Expected caches %p, %p to be identical\n", sc, sc2);
+    ScriptFreeCache(&sc2);
+
+    /* Same font face & size, but different font handle */
+    ok(SelectObject(hdc1, hfont2) != NULL, "SelectObject failed\n");
+    ok(SelectObject(hdc2, hfont2) != NULL, "SelectObject failed\n");
+
+    sc2 = NULL;
+    hr = ScriptCacheGetHeight(hdc1, &sc2, &height);
+    ok(hr == S_OK, "expected S_OK, got 0x%08x\n", hr);
+    ok(sc2 != NULL, "Script cache is NULL\n");
+    ok(sc == sc2, "Expected caches %p, %p to be identical\n", sc, sc2);
+    ScriptFreeCache(&sc2);
+
+    sc2 = NULL;
+    hr = ScriptCacheGetHeight(hdc2, &sc2, &height);
+    ok(hr == S_OK, "expected S_OK, got 0x%08x\n", hr);
+    ok(sc2 != NULL, "Script cache is NULL\n");
+    ok(sc == sc2, "Expected caches %p, %p to be identical\n", sc, sc2);
+    ScriptFreeCache(&sc2);
+
+    /* Different font size -- now we get a different SCRIPT_CACHE */
+    SelectObject(hdc1, prev_hfont1);
+    SelectObject(hdc2, prev_hfont2);
+    DeleteObject(hfont2);
+    lf.lfHeight = 20;
+    hfont2 = CreateFontIndirectA(&lf);
+    ok(hfont2 != NULL, "CreateFontIndirectA failed\n");
+    ok(SelectObject(hdc1, hfont2) != NULL, "SelectObject failed\n");
+    ok(SelectObject(hdc2, hfont2) != NULL, "SelectObject failed\n");
+
+    sc2 = NULL;
+    hr = ScriptCacheGetHeight(hdc1, &sc2, &height);
+    ok(hr == S_OK, "expected S_OK, got 0x%08x\n", hr);
+    ok(sc2 != NULL, "Script cache is NULL\n");
+    ok(sc != sc2, "Expected caches %p, %p to be different\n", sc, sc2);
+    ScriptFreeCache(&sc2);
+
+    sc2 = NULL;
+    hr = ScriptCacheGetHeight(hdc2, &sc2, &height);
+    ok(hr == S_OK, "expected S_OK, got 0x%08x\n", hr);
+    ok(sc2 != NULL, "Script cache is NULL\n");
+    ok(sc != sc2, "Expected caches %p, %p to be different\n", sc, sc2);
+    ScriptFreeCache(&sc2);
+
+    ScriptFreeCache(&sc);
+    SelectObject(hdc1, prev_hfont1);
+    SelectObject(hdc2, prev_hfont2);
+    DeleteObject(hfont1);
+    DeleteObject(hfont2);
+    DestroyWindow(hwnd1);
+    DestroyWindow(hwnd2);
+}
+
 static void init_tests(void)
 {
     HMODULE module = GetModuleHandleA("usp10.dll");
@@ -4185,6 +4302,7 @@ START_TEST(usp10)
     test_ScriptGetLogicalWidths();
 
     test_ScriptIsComplex();
+    test_script_cache_reuse();
 
     ReleaseDC(hwnd, hdc);
     DestroyWindow(hwnd);