Bug 75299

Summary: Use refcounting in hb-ft.cc
Product: HarfBuzz Reporter: Behdad Esfahbod <freedesktop>
Component: srcAssignee: Behdad Esfahbod <freedesktop>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium CC: freedesktop
Version: unspecified   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:

Description Behdad Esfahbod 2014-02-21 06:18:03 UTC
FT added refcounting to faces and libraries a while back.  We should ref faces we reference.
Comment 1 Behdad Esfahbod 2014-12-29 02:18:18 UTC
commit 350f3a02ce225e5d78db8ac96de1351ff9f96dd5
Author: Behdad Esfahbod <behdad@behdad.org>
Date:   Sun Dec 28 17:44:26 2014 -0800

    [ft] Add hb_ft_face_create_referenced() and hb_ft_font_create_referenced()
    
    When I originally wrote hb-ft, FreeType objects did not support reference
    counting.  As such, hb_ft_face_create() and hb_ft_font_create() had a
    "destroy" callback and client was responsible for making sure FT_Face is
    kept around as long as the hb-font/face are alive.
    
    However, since this was not clearly documented, some clienets didn't
    correctly did that.  In particular, some clients assumed that it's safe
    to destroy FT_Face and then hb_face_t.  This, indeed, used to work, until
    45fd9424c723f115ca98995b8f8a25185a6fc71d, which make face destroy access
    font tables.
    
    Now, I fixed that issue in 395b35903e052aecc97d0807e4f813c64c0d2b0b since
    the access was not needed, but the problem remains that not all clients
    handle this correctly.  See:
    
      https://bugs.freedesktop.org/show_bug.cgi?id=86300
    
    Fortunately, FT_Reference_Face() was added to FreeType in 2010, and so we
    can use it now.  Originally I wanted to change hb_ft_face_create() and
    hb_ft_font_create() to reference the face if destroy==NULL was passed in.
    That would improve pretty much all clients, with little undesired effects.
    Except that FreeType itself, when compiled with HarfBuzz support, calls
    hb_ft_font_create() with destroy==NULL and saves the resulting hb-font on
    the ft-face (why does it not free it immediately?).  Making hb-face
    reference ft-face causes a cycling reference there.  At least, that's my
    current understanding.
    
    At any rate, a cleaner approach, even if it means all clients will need a
    change, is to introduce brand new API.  Which this commit does.
    
    Some comments added to hb-ft.h, hoping to make future clients make better
    choices.
    
    Fixes https://bugs.freedesktop.org/show_bug.cgi?id=75299

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.