[PATCH v2] usp10: Re-use script caches for the same font.

Thomas Faber thomas.faber at reactos.org
Mon Jan 15 09:56:17 CST 2018


v2: use the LOGFONT as the comparison key, not the HDC; add more tests
-------------- next part --------------
From ea0f22a1af76bf648d260057c80be5db5ea2ee61 Mon Sep 17 00:00:00 2001
From: Thomas Faber <thomas.faber at reactos.org>
Date: Fri, 12 Jan 2018 14:30:13 +0100
Subject: usp10: Re-use script caches for the same font.

This makes the caching much more efficient, in particular for cases
like the EDIT control, which calls ScriptStringAnalyse once per line
of text.
Speeds up loading large files in notepad (bug 23193).

Signed-off-by: Thomas Faber <thomas.faber at reactos.org>
---
 dlls/usp10/tests/usp10.c    | 118 ++++++++++++++++++++++++++++++++++++++++++++
 dlls/usp10/usp10.c          |  69 +++++++++++++++++++++++---
 dlls/usp10/usp10_internal.h |   5 ++
 3 files changed, 186 insertions(+), 6 deletions(-)

diff --git a/dlls/usp10/tests/usp10.c b/dlls/usp10/tests/usp10.c
index 35cdec545b88..5c04441e3410 100644
--- a/dlls/usp10/tests/usp10.c
+++ b/dlls/usp10/tests/usp10.c
@@ -1833,6 +1833,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];
@@ -1862,6 +1863,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));
@@ -2087,6 +2092,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];
@@ -2124,6 +2130,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];
@@ -4109,6 +4120,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");
@@ -4182,6 +4299,7 @@ START_TEST(usp10)
     test_ScriptGetLogicalWidths();
 
     test_ScriptIsComplex();
+    test_script_cache_reuse();
 
     ReleaseDC(hwnd, hdc);
     DestroyWindow(hwnd);
diff --git a/dlls/usp10/usp10.c b/dlls/usp10/usp10.c
index 5d257b2c1c0a..17f60a19e1b9 100644
--- a/dlls/usp10/usp10.c
+++ b/dlls/usp10/usp10.c
@@ -675,6 +675,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;
@@ -853,12 +863,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))
     {
@@ -872,18 +904,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;
 }
@@ -1036,6 +1082,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/dlls/usp10/usp10_internal.h b/dlls/usp10/usp10_internal.h
index 2dfca86ca7fe..832d89448f3d 100644
--- a/dlls/usp10/usp10_internal.h
+++ b/dlls/usp10/usp10_internal.h
@@ -18,6 +18,9 @@
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
  *
  */
+
+#include "wine/list.h"
+
 #define MS_MAKE_TAG( _x1, _x2, _x3, _x4 ) \
           ( ( (ULONG)_x4 << 24 ) |     \
             ( (ULONG)_x3 << 16 ) |     \
@@ -174,6 +177,8 @@ typedef struct {
 } CacheGlyphPage;
 
 typedef struct {
+    struct list entry;
+    DWORD refcount;
     LOGFONTW lf;
     TEXTMETRICW tm;
     OUTLINETEXTMETRICW *otm;
-- 
2.15.1



More information about the wine-devel mailing list