scaled_font lifecycle issue

Submitted by Fred Beca on April 6, 2016, 8:59 a.m.

Details

Message ID CAKTSDKbiTfYGGp8fwhYyJWQc1RpU+BPCyrNGw5aLNAbOenozTg@mail.gmail.com
State New
Headers show
Series "scaled_font lifecycle issue" ( rev: 1 ) in Cairo

Not browsing as part of any series.

Commit Message

Fred Beca April 6, 2016, 8:59 a.m.
Hi,

With the current implementation of scaled fonts, I am encountering
lifecycle issues when using system fonts (HFONT on windows or
CGFontRef on Mac) that are already managed by third party components.

Typically, I would expect that after the following code, the system
font can be fully released (cairo not holding any reference to it
anymore):, but it is not the case

// win32 example but same problem with other backends
cairo_font_face_t* font_face=cairo_win32_font_face_create_for_hfont(my_hfont);
cairo_set_font_face(ctx,font_face);
cairo_destroy(ctx);
// I want to be able to destroy the my_hfont object here - if I do it,
subsequent calls to cairo using the same font face with same size will
fail because the hfont has been kept in cache.

The problem lies in the _cairo_scaled_font_map cache that holds
references to unused scaled fonts in the holdovers list, and in the
mru_scaled_font. So even if the font is not used anymore, it will
remain in this extra cache forever.

It is of course easy to fix this by completely removing the holdovers
and mru cache (it works very well with the attached patch) but given
the impact this is probably not a long term solution. I know that
there are callbacks to be notified when fonts are actually destroyed,
but it is not an option when font management is done by an external
component (and anyway in this case it only happens when calling the
cairo_debug_reset_static_data, so it will just leak in production
code).

So would it be possible to add something to disable this cache of
unused items at runtime or buildtime? For example, a macro or a global
function that would be called on initialization would be nice.

Patch hide | download patch | download mbox

Index: cairo-scaled-font.c
===================================================================
--- cairo-scaled-font.c (cairo-1.14.6)
+++ cairo-scaled-font.c (Working Copy)
@@ -351,12 +351,13 @@ 
 /* This defines the size of the holdover array ... that is, the number
  * of scaled fonts we keep around even when not otherwise referenced
  */
-#define CAIRO_SCALED_FONT_MAX_HOLDOVERS 256
+#define CAIRO_SCALED_FONT_MAX_HOLDOVERS 0
+#define CAIRO_SCALED_FONT_MAX_HOLDOVERS_ARRAY_SIZE 1

 typedef struct _cairo_scaled_font_map {
     cairo_scaled_font_t *mru_scaled_font;
     cairo_hash_table_t *hash_table;
-    cairo_scaled_font_t *holdovers[CAIRO_SCALED_FONT_MAX_HOLDOVERS];
+    cairo_scaled_font_t *holdovers[CAIRO_SCALED_FONT_MAX_HOLDOVERS_ARRAY_SIZE];
     int num_holdovers;
 } cairo_scaled_font_map_t;

@@ -1099,9 +1100,11 @@ 
      * held. */

     old = font_map->mru_scaled_font;
+#if CAIRO_SCALED_FONT_MAX_HOLDOVERS>0
     font_map->mru_scaled_font = scaled_font;
     /* increment reference count for the mru cache */
     _cairo_reference_count_inc (&scaled_font->ref_count);
+#endif
     /* and increment for the returned reference */
     _cairo_reference_count_inc (&scaled_font->ref_count);
     _cairo_scaled_font_map_unlock ();
@@ -1175,8 +1178,10 @@ 
        &scaled_font->hash_entry);
     if (likely (status == CAIRO_STATUS_SUCCESS)) {
  old = font_map->mru_scaled_font;
+#if CAIRO_SCALED_FONT_MAX_HOLDOVERS>0
  font_map->mru_scaled_font = scaled_font;
  _cairo_reference_count_inc (&scaled_font->ref_count);
+#endif
     }

     _cairo_scaled_font_map_unlock ();
@@ -1335,7 +1340,7 @@ 
      * the reference count). To make room for it, we do actually
      * destroy the least-recently-used holdover.
      */
-
+        #if CAIRO_SCALED_FONT_MAX_HOLDOVERS>0
     if (font_map->num_holdovers == CAIRO_SCALED_FONT_MAX_HOLDOVERS) {
  lru = font_map->holdovers[0];
  assert (! CAIRO_REFERENCE_COUNT_HAS_REFERENCE (&lru->ref_count));
@@ -1351,6 +1356,11 @@ 

     font_map->holdovers[font_map->num_holdovers++] = scaled_font;
     scaled_font->holdover = TRUE;
+#else
+        lru = scaled_font;
+        _cairo_hash_table_remove (font_map->hash_table,
+  &lru->hash_entry);
+#endif
  } else
     lru = scaled_font;
     }

Comments

On 06/04/16 18:29, Fred Bca wrote:
> Hi,
> 
> With the current implementation of scaled fonts, I am encountering
> lifecycle issues when using system fonts (HFONT on windows or
> CGFontRef on Mac) that are already managed by third party components.
> 
> Typically, I would expect that after the following code, the system
> font can be fully released (cairo not holding any reference to it
> anymore):, but it is not the case
> 
> // win32 example but same problem with other backends
> cairo_font_face_t* font_face=cairo_win32_font_face_create_for_hfont(my_hfont);
> cairo_set_font_face(ctx,font_face);
> cairo_destroy(ctx);
> // I want to be able to destroy the my_hfont object here - if I do it,
> subsequent calls to cairo using the same font face with same size will
> fail because the hfont has been kept in cache.
> 
> The problem lies in the _cairo_scaled_font_map cache that holds
> references to unused scaled fonts in the holdovers list, and in the
> mru_scaled_font. So even if the font is not used anymore, it will
> remain in this extra cache forever.
> 
> It is of course easy to fix this by completely removing the holdovers
> and mru cache (it works very well with the attached patch) but given
> the impact this is probably not a long term solution. I know that
> there are callbacks to be notified when fonts are actually destroyed,
> but it is not an option when font management is done by an external
> component (and anyway in this case it only happens when calling the
> cairo_debug_reset_static_data, so it will just leak in production
> code).

Maybe we need a font_face API function to make cairo drop a font from
the cache once the application drops all references to the font_face and
scaled fonts?
On Sat, 9 Apr 2016 19:30:01 +0930, Adrian Johnson wrote:

> Maybe we need a font_face API function to make cairo drop a font from
> the cache once the application drops all references to the font_face
> and scaled fonts?

A more general invalidate-cache-for-specified-font-face function
would be very useful. It would help with use of multiple-master-type
font technologies. I hit this situation when implementing my wrapper
for the Hershey fonts <https://github.com/ldo/hersheypy> for Cairo: to
use different line widths, I had to load multiple copies of the same
font to get around Cairo’s caching.
On 10/04/16 09:08, Lawrence D'Oliveiro wrote:
> On Sat, 9 Apr 2016 19:30:01 +0930, Adrian Johnson wrote:
> 
>> Maybe we need a font_face API function to make cairo drop a font from
>> the cache once the application drops all references to the font_face
>> and scaled fonts?
> 
> A more general invalidate-cache-for-specified-font-face function
> would be very useful. It would help with use of multiple-master-type
> font technologies. I hit this situation when implementing my wrapper
> for the Hershey fonts <https://github.com/ldo/hersheypy> for Cairo: to
> use different line widths, I had to load multiple copies of the same
> font to get around Cairo’s caching.

I'm not sure how well this would work for PDF/PS/SVG as these backends
hold on to the font_face of every font used until after all pages have
been emitted before all the fonts are emitted.
On Sun, 10 Apr 2016 11:27:31 +0930, Adrian Johnson wrote:

> On 10/04/16 09:08, Lawrence D'Oliveiro wrote:
>
>> A more general invalidate-cache-for-specified-font-face function
>> would be very useful. It would help with use of multiple-master-type
>> font technologies.  
> 
> I'm not sure how well this would work for PDF/PS/SVG as these backends
> hold on to the font_face of every font used until after all pages have
> been emitted before all the fonts are emitted.

I guess the only answer is to do what we already do now, and that is
have separate font_face objects, so that Cairo maintains a separate
cache for each. Just it would be nice if these could share underlying
font data somehow, instead of having to load multiple copies.
My problem here is not with the cache, which is properly implemented,
removing scaled fonts when not in use anymore (refcount=0), which also
removes the associated font_face from the cache if not used elsewhere.

The issue comes from the additional optimization that consists in
storing UNUSED fonts in the hold_overs list as well as in the
mru_scaled_font item. I am just proposing an API to configure cairo
not to use these additional lists, so that fonts that are not used
anymore are released as expected when using external system fonts.


On Sun, Apr 10, 2016 at 4:20 AM, Lawrence D'Oliveiro
<ldo@geek-central.gen.nz> wrote:
> On Sun, 10 Apr 2016 11:27:31 +0930, Adrian Johnson wrote:
>
>> On 10/04/16 09:08, Lawrence D'Oliveiro wrote:
>>
>>> A more general invalidate-cache-for-specified-font-face function
>>> would be very useful. It would help with use of multiple-master-type
>>> font technologies.
>>
>> I'm not sure how well this would work for PDF/PS/SVG as these backends
>> hold on to the font_face of every font used until after all pages have
>> been emitted before all the fonts are emitted.
>
> I guess the only answer is to do what we already do now, and that is
> have separate font_face objects, so that Cairo maintains a separate
> cache for each. Just it would be nice if these could share underlying
> font data somehow, instead of having to load multiple copies.
> --
> cairo mailing list
> cairo@cairographics.org
> https://lists.cairographics.org/mailman/listinfo/cairo