I wouldn’t know of course :) So I just checked, for Copy or Create functions you own the returned object, for Get functions you don’t, so according to that only the CGFont's need to be released.
On Fri, Nov 29, 2013 at 06:19:36PM -0500, Behdad Esfahbod wrote: > Shouldn't we CFRelease these: > > + CTFontRef run_ct_font = static_cast<CTFontRef>(CFDictionaryGetValue > (attributes, kCTFontAttributeName)); > + CGFontRef run_cg_font = CTFontCopyGraphicsFont (run_ct_font, 0); > + CGFontRef cg_font = CTFontCopyGraphicsFont (font_data->ct_font, 0); > > > On 13-11-29 06:01 PM, Khaled Hosny wrote: > > On Fri, Nov 29, 2013 at 05:38:06PM -0500, Behdad Esfahbod wrote: > >> On 13-11-29 05:10 PM, Khaled Hosny wrote: > >>> We should compare the CGFont's then. Updated patch attached. > >> > >> Ok, I just studied this. I think the work should be done at the very > >> beginning of the look (line 653), > > > > It had to be after the CTRunGetStringIndices() call, but the reworked > > patch does not need this, so I moved it up now. > > > >> and produce one notdef per input character. The number of glyphs in > >> the current run is dependent on the chosen fallback font and not what > >> we should be using. > > > > Right, I overlooked that. > > > > Regards, > > Khaled > > > > > >> I can rework this myself eventually, but don't have my Mac around right > >> now. > >> Feel free to send an updated patch though. > >> > >> Thanks, > >> behdad > >> > >> > >>> Regards, > >>> Khaled > >>> > >>> On Fri, Nov 29, 2013 at 03:13:19PM -0500, Behdad Esfahbod wrote: > >>>> Your patch doesn't work with user features. For each user feature we > >>>> use a > >>>> sub-font of ct_font: > >>>> > >>>> range->font = CTFontCreateCopyWithAttributes (font_data->ct_font, 0.0, > >>>> NULL, > >>>> font_desc); > >>>> > >>>> > >>>> On 13-11-25 08:42 AM, Khaled Hosny wrote: > >>>>> Resending a patch that actually applies! > >>>>> > >>>>> > >>>>> > >>>>> _______________________________________________ > >>>>> HarfBuzz mailing list > >>>>> [email protected] > >>>>> http://lists.freedesktop.org/mailman/listinfo/harfbuzz > >>>>> > >>>> > >>>> -- > >>>> behdad > >>>> http://behdad.org/ > >> > >> -- > >> behdad > >> http://behdad.org/ > > -- > behdad > http://behdad.org/
>From e3b7085c31aeb8efb3a53da6f3b1e3c486e81f9d Mon Sep 17 00:00:00 2001 From: Khaled Hosny <[email protected]> Date: Mon, 25 Nov 2013 15:28:10 +0200 Subject: [PATCH] Avoid font fallback with CoreText shaper CoreText does automatic font fallback (AKA "cascading") for characters not supported by the requested font, and provides no way to turn it off, so detect if the returned run uses a font other than the requested one and fill in the buffer with .notdef glyphs instead of random indices glyph from a different font. --- src/hb-coretext.cc | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/hb-coretext.cc b/src/hb-coretext.cc index ba80136..951fc82 100644 --- a/src/hb-coretext.cc +++ b/src/hb-coretext.cc @@ -651,6 +651,41 @@ _hb_coretext_shape (hb_shape_plan_t *shape_plan, for (unsigned int i = 0; i < num_runs; i++) { CTRunRef run = (CTRunRef) CFArrayGetValueAtIndex (glyph_runs, i); + /* CoreText does automatic font fallback (AKA "cascading") for characters + * not supported by the requested font, and provides no way to turn it off, + * so we detect if the returned run uses a font other than the requested + * one and fill in the buffer with .notdef glyphs instead of random glyph + * indices from a different font. + */ + CFRange range = CTRunGetStringRange (run); + CFDictionaryRef attributes = CTRunGetAttributes (run); + CTFontRef run_ct_font = static_cast<CTFontRef>(CFDictionaryGetValue (attributes, kCTFontAttributeName)); + CGFontRef run_cg_font = CTFontCopyGraphicsFont (run_ct_font, 0); + CGFontRef cg_font = CTFontCopyGraphicsFont (font_data->ct_font, 0); + if (!CFEqual (run_cg_font, cg_font)) { + CFRelease (run_cg_font); + CFRelease (cg_font); + for (CFIndex j = 0; j < range.length; j++) { + CGGlyph notdef = 0; + double advance = CTFontGetAdvancesForGlyphs (font_data->ct_font, kCTFontHorizontalOrientation, ¬def, NULL, 1); + + hb_glyph_info_t *info = &buffer->info[buffer->len]; + + info->codepoint = notdef; + info->cluster = range.location + j; + + info->mask = advance; + info->var1.u32 = 0; + info->var2.u32 = 0; + + buffer->len++; + } + continue; + } + + CFRelease (run_cg_font); + CFRelease (cg_font); + unsigned int num_glyphs = CTRunGetGlyphCount (run); if (num_glyphs == 0) continue; -- 1.8.4.1
_______________________________________________ HarfBuzz mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/harfbuzz
