Attachment 91176 Details for Bug 73012 – Attempt to make cairo-win32-font.c thread-safe

[patch] Attempt to make cairo-win32-font.c thread-safe

0001-src-win32-cairo-win32-font.c-Make-Font-HDC-Thread-Sa.patch (text/plain), 14.75 KB, created by Fan, Chun-wei on 2013-12-24 09:35:07 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Fan, Chun-wei

Created: 2013-12-24 09:35:07 UTC

Size: 14.75 KB

patch

obsolete

>From 7dbda141be599376bcd0f231aa148bec13f4b23f Mon Sep 17 00:00:00 2001
>From: Chun-wei Fan <fanchunwei@src.gnome.org>
>Date: Tue, 24 Dec 2013 17:20:47 +0800
>Subject: [PATCH] src/win32/cairo-win32-font.c: Make Font HDC Thread-Safe
>
>This patch is based on Lufy D.Monkey's <lufy2012 at yahoo.ca> patch[1][2]
>that attempts to make cairo-win32-font.c thread-safe.  This is so that we
>can make the creation of the HDC in question protected by mutexes and that
>we can try to make things a bit more efficient by caching the created
>HDC's  by their thread id's so that they don't have to be created and
>deleted from time to time.
>
>https://bugs.freedesktop.org/show_bug.cgi?id=73012
>
>[1]: http://lists.cairographics.org/archives/cairo/2010-July/020336.html
>[2]: http://lists.cairographics.org/archives/cairo/attachments/20100715/8b3a8f05/attachment-0001.obj
>---
> src/win32/cairo-win32-font.c | 230 +++++++++++++++++++++++++++++++++----------
> 1 file changed, 179 insertions(+), 51 deletions(-)
>
>diff --git a/src/win32/cairo-win32-font.c b/src/win32/cairo-win32-font.c
>index 1599b07..af7a71b 100644
>--- a/src/win32/cairo-win32-font.c
>+++ b/src/win32/cairo-win32-font.c
>@@ -159,26 +159,130 @@ _cairo_win32_scaled_font_init_glyph_path (cairo_win32_scaled_font_t *scaled_font
> 
> #define NEARLY_ZERO(d) (fabs(d) < (1. / 65536.))
> 
>-static HDC
>-_get_global_font_dc (void)
>-{
>-    static HDC hdc;
>-
>-    if (!hdc) {
>-	hdc = CreateCompatibleDC (NULL);
>-	if (!hdc) {
>-	    _cairo_win32_print_gdi_error ("_get_global_font_dc");
>+typedef struct
>+{
>+  unsigned int thread_id;
>+  HDC hdc;
>+  HFONT old_font;
>+  int inuse;
>+} hdc_cache_t;
>+
>+#define MAX_HDC_CACHE 4
>+static hdc_cache_t _hdc_cache[MAX_HDC_CACHE] = {0};
>+
>+static HDC _create_hdc()
>+{
>+	HDC hdc = CreateCompatibleDC (NULL);
>+  if (!hdc) {
>+    _cairo_win32_print_gdi_error (__FUNCTION__);
>+    return NULL;
>+  }
>+
>+  if (!SetGraphicsMode (hdc, GM_ADVANCED)) {
>+    _cairo_win32_print_gdi_error (__FUNCTION__);
> 	    return NULL;
>-	}
>+  }
>+  return hdc;
>+}
> 
>-	if (!SetGraphicsMode (hdc, GM_ADVANCED)) {
>-	    _cairo_win32_print_gdi_error ("_get_global_font_dc");
>-	    DeleteDC (hdc);
>-	    return NULL;
>-	}
>-    }
>+static HDC
>+_get_global_font_dc (HFONT *old_font)
>+{
>+  unsigned int thread_id;
>+  int i;
>+  HDC hdc = NULL;
>+
>+  thread_id = GetCurrentThreadId();
>+
>+  CAIRO_MUTEX_LOCK(_cairo_win32_font_face_mutex);
>+
>+  for (i = 0; i < MAX_HDC_CACHE; i++) {
>+    if (_hdc_cache[i].thread_id == thread_id) {
>+      break;
>+    }
>+  }
>+
>+  if (i < MAX_HDC_CACHE) {
>+    hdc = _hdc_cache[i].hdc;
>+    *old_font = _hdc_cache[i].old_font;
>+    _hdc_cache[i].inuse++;
>+  }
>+  else {
>+    /* search for the free item */
>+    for (i = 0; i < MAX_HDC_CACHE; i++) {
>+      if (_hdc_cache[i].thread_id == 0) {
>+        break;
>+      }
>+    }
>+
>+    if (i < MAX_HDC_CACHE) {
>+      hdc = _create_hdc();
>+      if (!hdc)
>+        goto FAIL;
>+
>+
>+      *old_font = GetCurrentObject(hdc, OBJ_FONT);
>+
>+      _hdc_cache[i].hdc = hdc;
>+      _hdc_cache[i].old_font = *old_font;
>+      _hdc_cache[i].thread_id = thread_id;
>+      _hdc_cache[i].inuse++;
>+    }
>+    else {
>+      /* try to find the unused hdc if we can't find any free items */
>+      for (i = 0; i < MAX_HDC_CACHE; i++) {
>+        if (_hdc_cache[i].inuse == 0) {
>+          break;
>+        }
>+      }
>+
>+      if (i < MAX_HDC_CACHE) {
>+        _hdc_cache[i].thread_id = thread_id; /* replace the thread id */
>+        hdc = _hdc_cache[i].hdc; /* reuse the hdc */
>+        *old_font = _hdc_cache[i].old_font;
>+        _hdc_cache[i].inuse++;
>+      }
>+      else
>+      {
>+        hdc = _create_hdc();
>+        *old_font = GetCurrentObject(hdc, OBJ_FONT);
>+      }
>+    }
>+  }
>+
>+FAIL:
>+  CAIRO_MUTEX_UNLOCK(_cairo_win32_font_face_mutex);
>+
>+  return hdc;
>+}
> 
>-    return hdc;
>+static void _release_global_font_dc(HDC hdc, HFONT old_font)
>+{
>+  int i;
>+
>+  if (!hdc)
>+    return;
>+
>+  SelectObject(hdc, old_font);
>+  ModifyWorldTransform(hdc, NULL, MWT_IDENTITY); /* somehow the SetWorldTransform may fail if we don't reset it*/
>+
>+  CAIRO_MUTEX_LOCK(_cairo_win32_font_face_mutex);
>+
>+  for (i = 0; i < MAX_HDC_CACHE; i++) {
>+    if (_hdc_cache[i].hdc == hdc) {
>+      break;
>+    }
>+  }
>+
>+  if (i < MAX_HDC_CACHE) {
>+    assert(_hdc_cache[i].inuse > 0);
>+    _hdc_cache[i].inuse--;
>+  }
>+  else {
>+    DeleteDC(hdc);
>+  }
>+
>+  CAIRO_MUTEX_UNLOCK(_cairo_win32_font_face_mutex);
> }
> 
> static cairo_status_t
>@@ -314,15 +418,10 @@ _win32_scaled_font_create (LOGFONTW                   *logfont,
> 			   const cairo_font_options_t *options,
> 			   cairo_scaled_font_t       **font_out)
> {
>-    HDC hdc;
>     cairo_win32_scaled_font_t *f;
>     cairo_matrix_t scale;
>     cairo_status_t status;
> 
>-    hdc = _get_global_font_dc ();
>-    if (hdc == NULL)
>-	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
>-
>     f = malloc (sizeof(cairo_win32_scaled_font_t));
>     if (f == NULL)
> 	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
>@@ -649,6 +748,7 @@ _cairo_win32_scaled_font_type1_text_to_glyphs (cairo_win32_scaled_font_t *scaled
>     cairo_status_t status;
>     double x_pos, y_pos;
>     HDC hdc = NULL;
>+    HFONT old_font = NULL;
>     cairo_matrix_t mat;
> 
>     status = _cairo_utf8_to_utf16 (utf8, -1, &utf16, &n16);
>@@ -661,7 +761,7 @@ _cairo_win32_scaled_font_type1_text_to_glyphs (cairo_win32_scaled_font_t *scaled
> 	goto FAIL1;
>     }
> 
>-    hdc = _get_global_font_dc ();
>+    hdc = _get_global_font_dc (&old_font);
>     assert (hdc != NULL);
> 
>     status = cairo_win32_scaled_font_select_font (&scaled_font->base, hdc);
>@@ -721,6 +821,7 @@ FAIL2:
>     free (glyph_indices);
> FAIL1:
>     free (utf16);
>+    _release_global_font_dc(hdc, old_font);
> 
>     return status;
> }
>@@ -744,6 +845,7 @@ _cairo_win32_scaled_font_text_to_glyphs (void		*abstract_font,
>     double x_pos, y_pos;
>     double x_incr, y_incr;
>     HDC hdc = NULL;
>+    HFONT old_font = NULL;
> 
>     /* GetCharacterPlacement() returns utf16 instead of glyph indices
>      * for Type 1 fonts. Use GetGlyphIndices for Type 1 fonts. */
>@@ -778,7 +880,7 @@ _cairo_win32_scaled_font_text_to_glyphs (void		*abstract_font,
> 	goto FAIL1;
>     }
> 
>-    hdc = _get_global_font_dc ();
>+    hdc = _get_global_font_dc (&old_font);
>     assert (hdc != NULL);
> 
>     status = cairo_win32_scaled_font_select_font (&scaled_font->base, hdc);
>@@ -850,6 +952,7 @@ _cairo_win32_scaled_font_text_to_glyphs (void		*abstract_font,
> 
>  FAIL1:
>     free (utf16);
>+    _release_global_font_dc(hdc, old_font);
> 
>     return status;
> }
>@@ -862,9 +965,10 @@ _cairo_win32_scaled_font_ucs4_to_index (void		*abstract_font,
>     wchar_t unicode[2];
>     WORD glyph_index;
>     HDC hdc = NULL;
>+    HFONT old_font = NULL;
>     cairo_status_t status;
> 
>-    hdc = _get_global_font_dc ();
>+    hdc = _get_global_font_dc (&old_font);
>     assert (hdc != NULL);
> 
>     status = cairo_win32_scaled_font_select_font (&scaled_font->base, hdc);
>@@ -880,6 +984,8 @@ _cairo_win32_scaled_font_ucs4_to_index (void		*abstract_font,
> 
>     cairo_win32_scaled_font_done_font (&scaled_font->base);
> 
>+    _release_global_font_dc(hdc, old_font);
>+
>     return glyph_index;
> }
> 
>@@ -890,9 +996,10 @@ _cairo_win32_scaled_font_set_metrics (cairo_win32_scaled_font_t *scaled_font)
>     cairo_font_extents_t extents;
> 
>     TEXTMETRIC metrics;
>-    HDC hdc;
>+    HDC hdc = NULL;
>+    HFONT old_font = NULL;
> 
>-    hdc = _get_global_font_dc ();
>+    hdc = _get_global_font_dc (&old_font);
>     assert (hdc != NULL);
> 
>     if (scaled_font->preserve_axes || scaled_font->base.options.hint_metrics == CAIRO_HINT_METRICS_OFF) {
>@@ -900,8 +1007,10 @@ _cairo_win32_scaled_font_set_metrics (cairo_win32_scaled_font_t *scaled_font)
> 	 * from the GDI in logical space, then convert back to font space
> 	 */
> 	status = cairo_win32_scaled_font_select_font (&scaled_font->base, hdc);
>-	if (status)
>+	if (status) {
>+	    _release_global_font_dc(hdc, old_font);
> 	    return status;
>+  }
> 	GetTextMetrics (hdc, &metrics);
> 	cairo_win32_scaled_font_done_font (&scaled_font->base);
> 
>@@ -919,8 +1028,10 @@ _cairo_win32_scaled_font_set_metrics (cairo_win32_scaled_font_t *scaled_font)
> 	 * avoid them.
> 	 */
> 	status = _cairo_win32_scaled_font_select_unscaled_font (&scaled_font->base, hdc);
>-	if (status)
>+	if (status) {
>+	    _release_global_font_dc(hdc, old_font);
> 	    return status;
>+  }
> 	GetTextMetrics (hdc, &metrics);
> 	_cairo_win32_scaled_font_done_unscaled_font (&scaled_font->base);
> 
>@@ -954,6 +1065,7 @@ _cairo_win32_scaled_font_set_metrics (cairo_win32_scaled_font_t *scaled_font)
> 	 }
>     }
> 
>+    _release_global_font_dc(hdc, old_font);
>     return _cairo_scaled_font_set_metrics (&scaled_font->base, &extents);
> }
> 
>@@ -963,11 +1075,12 @@ _cairo_win32_scaled_font_init_glyph_metrics (cairo_win32_scaled_font_t *scaled_f
> {
>     static const MAT2 matrix = { { 0, 1 }, { 0, 0 }, { 0, 0 }, { 0, 1 } };
>     GLYPHMETRICS metrics;
>-    cairo_status_t status;
>+    cairo_status_t status = CAIRO_STATUS_SUCCESS;
>     cairo_text_extents_t extents;
>-    HDC hdc;
>+    HDC hdc = NULL;
>+    HFONT old_font = NULL;
> 
>-    hdc = _get_global_font_dc ();
>+    hdc = _get_global_font_dc (&old_font);
>     assert (hdc != NULL);
> 
>     if (scaled_font->is_bitmap) {
>@@ -980,7 +1093,7 @@ _cairo_win32_scaled_font_init_glyph_metrics (cairo_win32_scaled_font_t *scaled_f
> 
> 	status = cairo_win32_scaled_font_select_font (&scaled_font->base, hdc);
> 	if (status)
>-	    return status;
>+	    goto FAIL;
> 
> 	if (!GetCharWidth32(hdc, charIndex, charIndex, &width)) {
> 	    status = _cairo_win32_print_gdi_error ("_cairo_win32_scaled_font_init_glyph_metrics:GetCharWidth32");
>@@ -988,7 +1101,7 @@ _cairo_win32_scaled_font_init_glyph_metrics (cairo_win32_scaled_font_t *scaled_f
> 	}
> 	cairo_win32_scaled_font_done_font (&scaled_font->base);
> 	if (status)
>-	    return status;
>+	    goto FAIL;
> 
> 	extents.x_bearing = 0;
> 	extents.y_bearing = scaled_font->base.ctm.yy * (-font_extents.ascent / scaled_font->y_scale);
>@@ -1002,7 +1115,7 @@ _cairo_win32_scaled_font_init_glyph_metrics (cairo_win32_scaled_font_t *scaled_f
> 	 */
> 	status = cairo_win32_scaled_font_select_font (&scaled_font->base, hdc);
> 	if (status)
>-	    return status;
>+	    goto FAIL;
> 
> 	if (GetGlyphOutlineW (hdc, _cairo_scaled_glyph_index (scaled_glyph),
> 			      GGO_METRICS | GGO_GLYPH_INDEX,
>@@ -1055,7 +1168,7 @@ _cairo_win32_scaled_font_init_glyph_metrics (cairo_win32_scaled_font_t *scaled_f
> 	 */
> 	status = _cairo_win32_scaled_font_select_unscaled_font (&scaled_font->base, hdc);
> 	if (status)
>-	    return status;
>+	    goto FAIL;
> 
> 	if (GetGlyphOutlineW (hdc, _cairo_scaled_glyph_index (scaled_glyph),
> 	                      GGO_METRICS | GGO_GLYPH_INDEX,
>@@ -1076,7 +1189,9 @@ _cairo_win32_scaled_font_init_glyph_metrics (cairo_win32_scaled_font_t *scaled_f
> 				     &scaled_font->base,
> 				     &extents);
> 
>-    return CAIRO_STATUS_SUCCESS;
>+FAIL:
>+    _release_global_font_dc(hdc, old_font);
>+    return status;;
> }
> 
> /* Not currently used code, but may be useful in the future if we add
>@@ -1096,17 +1211,18 @@ _cairo_win32_scaled_font_glyph_bbox (void		 *abstract_font,
>     int x1 = 0, x2 = 0, y1 = 0, y2 = 0;
> 
>     if (num_glyphs > 0) {
>-	HDC hdc;
>+	HDC hdc = NULL;
>+	HFONT old_font = NULL;
> 	GLYPHMETRICS metrics;
>-	cairo_status_t status;
>+	cairo_status_t status = CAIRO_STATUS_SUCCESS;
> 	int i;
> 
>-	hdc = _get_global_font_dc ();
>+	hdc = _get_global_font_dc (&old_font);
> 	assert (hdc != NULL);
> 
> 	status = cairo_win32_scaled_font_select_font (&scaled_font->base, hdc);
> 	if (status)
>-	    return status;
>+	    goto FAIL;
> 
> 	for (i = 0; i < num_glyphs; i++) {
> 	    int x = _cairo_lround (glyphs[i].x);
>@@ -1133,7 +1249,9 @@ _cairo_win32_scaled_font_glyph_bbox (void		 *abstract_font,
>     bbox->p2.x = _cairo_fixed_from_int (x2);
>     bbox->p2.y = _cairo_fixed_from_int (y2);
> 
>-    return CAIRO_STATUS_SUCCESS;
>+FAIL:
>+    _release_global_font_dc(hdc, old_font);
>+    return status;
> }
> #endif
> 
>@@ -1331,17 +1449,18 @@ _cairo_win32_scaled_font_load_truetype_table (void	       *abstract_font,
>                                              unsigned long     *length)
> {
>     cairo_win32_scaled_font_t *scaled_font = abstract_font;
>-    HDC hdc;
>+    HDC hdc = NULL;
>+    HFONT old_font = NULL;
>     cairo_status_t status;
>     DWORD ret;
> 
>-    hdc = _get_global_font_dc ();
>+    hdc = _get_global_font_dc (&old_font);
>     assert (hdc != NULL);
> 
>     tag = (tag&0x000000ff)<<24 | (tag&0x0000ff00)<<8 | (tag&0x00ff0000)>>8 | (tag&0xff000000)>>24;
>     status = cairo_win32_scaled_font_select_font (&scaled_font->base, hdc);
>     if (status)
>-	return status;
>+	goto FAIL;
> 
>     ret = GetFontData (hdc, tag, offset, buffer, *length);
>     if (ret == GDI_ERROR || (buffer && ret != *length))
>@@ -1351,6 +1470,8 @@ _cairo_win32_scaled_font_load_truetype_table (void	       *abstract_font,
> 
>     cairo_win32_scaled_font_done_font (&scaled_font->base);
> 
>+FAIL:
>+    _release_global_font_dc(hdc, old_font);
>     return status;
> }
> 
>@@ -1364,16 +1485,17 @@ _cairo_win32_scaled_font_index_to_ucs4 (void		*abstract_font,
>     uint16_t *utf16 = NULL;
>     WORD *glyph_indices = NULL;
>     HDC hdc = NULL;
>+    HFONT old_font = NULL;
>     int res;
>     unsigned int i, j, num_glyphs;
>     cairo_status_t status;
> 
>-    hdc = _get_global_font_dc ();
>+    hdc = _get_global_font_dc (&old_font);
>     assert (hdc != NULL);
> 
>     status = cairo_win32_scaled_font_select_font (&scaled_font->base, hdc);
>     if (status)
>-	return status;
>+	goto exit0;
> 
>     res = GetFontUnicodeRanges(hdc, NULL);
>     if (res == 0) {
>@@ -1441,6 +1563,8 @@ exit2:
> exit1:
>     cairo_win32_scaled_font_done_font (&scaled_font->base);
> 
>+exit0:
>+    _release_global_font_dc(hdc, old_font);
>     return status;
> }
> 
>@@ -1651,7 +1775,8 @@ _cairo_win32_scaled_font_init_glyph_path (cairo_win32_scaled_font_t *scaled_font
>     static const MAT2 matrix = { { 0, 1 }, { 0, 0 }, { 0, 0 }, { 0, -1 } };
>     cairo_status_t status;
>     GLYPHMETRICS metrics;
>-    HDC hdc;
>+    HDC hdc = NULL;
>+    HFONT old_font = NULL;
>     DWORD bytesGlyph;
>     unsigned char *buffer, *ptr;
>     cairo_path_fixed_t *path;
>@@ -1661,12 +1786,14 @@ _cairo_win32_scaled_font_init_glyph_path (cairo_win32_scaled_font_t *scaled_font
>     if (scaled_font->is_bitmap)
> 	return CAIRO_INT_STATUS_UNSUPPORTED;
> 
>-    hdc = _get_global_font_dc ();
>+    hdc = _get_global_font_dc (&old_font);
>     assert (hdc != NULL);
> 
>     path = _cairo_path_fixed_create ();
>-    if (!path)
>-	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
>+    if (!path) {
>+      _release_global_font_dc(hdc, old_font);
>+      return _cairo_error (CAIRO_STATUS_NO_MEMORY);
>+    }
> 
>     if (scaled_font->base.options.hint_style == CAIRO_HINT_STYLE_NONE) {
>         status = _cairo_win32_scaled_font_select_unscaled_font (&scaled_font->base, hdc);
>@@ -1812,6 +1939,7 @@ _cairo_win32_scaled_font_init_glyph_path (cairo_win32_scaled_font_t *scaled_font
>     if (status != CAIRO_STATUS_SUCCESS)
> 	_cairo_path_fixed_destroy (path);
> 
>+    _release_global_font_dc(hdc, old_font);
>     return status;
> }
> 
>-- 
>1.8.3.msysgit.0
>

Actions: View | Splinter Review

Attachments on bug 73012: 91176

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.