Bug 84324 - cairo quartz font broken in iOS 8 / CGFontGetGlyphPath deprecated in MacOS 10.10 yosemite
Summary: cairo quartz font broken in iOS 8 / CGFontGetGlyphPath deprecated in MacOS 10...
Status: RESOLVED FIXED
Alias: None
Product: cairo
Classification: Unclassified
Component: quartz font backend (show other bugs)
Version: 1.12.16
Hardware: All other
: medium normal
Assignee: Andrea Canciani
QA Contact: cairo-bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-09-25 10:45 UTC by rkdgu+a44zyz19a8hys
Modified: 2015-05-11 04:22 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Simon Cozens' patch (3.34 KB, text/plain)
2014-09-25 10:45 UTC, rkdgu+a44zyz19a8hys
Details
iOS project which works on ipad 3 and crashes on ipad 4, both on latest iOS (2.01 MB, application/octet-stream)
2014-11-28 08:13 UTC, rkdgu+a44zyz19a8hys
Details
patch with minor fix (3.44 KB, text/plain)
2014-11-28 15:36 UTC, rkdgu+a44zyz19a8hys
Details
Wannabe fix (2.78 KB, patch)
2014-12-09 15:31 UTC, Andrea Canciani
Details | Splinter Review
Fix font size handling (561 bytes, patch)
2015-03-17 17:34 UTC, Andrea Canciani
Details | Splinter Review
Screenshots comparing canvas text in Inkscape (68.20 KB, image/png)
2015-05-11 04:22 UTC, ~suv
Details

Description rkdgu+a44zyz19a8hys 2014-09-25 10:45:58 UTC
Created attachment 106845 [details]
Simon Cozens' patch

After updating to iOS 8, quartz_font_ensure_symbols does not set _cairo_quartz_font_symbols_present to TRUE because CGFontGetGlyphPath no longer exists.  This causes _cairo_quartz_font_face_create_for_toy (and others) to error out with CAIRO_STATUS_NO_MEMORY.

Simon Cozens on the mailing list saw that this function was deprecated in OS X 10.10 in June and submitted a patch.  Applying the patch fixed the issue.
Comment 1 Behdad Esfahbod 2014-10-06 01:03:55 UTC
Looks good to me.  Anyone can commit this please?
Comment 2 Andrea Canciani 2014-10-06 17:14:40 UTC
(In reply to Behdad Esfahbod from comment #1)
> Looks good to me.  Anyone can commit this please?

I raised some concerns about the patch in the discussion on the mailing list.
Are we ok with dropping support for 10.4? (This would also enable some more cleanup in quartz, but at the very least we should mention it)

Should we go for a CoreText font backend?

For a handle to the thread in the mailing list, see http://lists.cairographics.org/archives/cairo/2014-July/025348.html
Comment 3 Behdad Esfahbod 2014-10-06 17:21:41 UTC
(In reply to Andrea Canciani from comment #2)
> (In reply to Behdad Esfahbod from comment #1)
> > Looks good to me.  Anyone can commit this please?
> 
> I raised some concerns about the patch in the discussion on the mailing list.
> Are we ok with dropping support for 10.4? (This would also enable some more
> cleanup in quartz, but at the very least we should mention it)

I wouldn't mind if we don't support 10.5 or 10.6 either.  10.4 is definitely a no-brainer to me.


> Should we go for a CoreText font backend?

Not quite sure.  Only if we have to.  Remove the old ATSU stuff.  Start adding CoreText when it makes sense.  Right now the backend does without CoreText because what we need is very simple to do with CGFont (scale by em-size).  This works because OS X almost fully supports subpixel-positioned text, so there's little hinting to worry about.

> For a handle to the thread in the mailing list, see
> http://lists.cairographics.org/archives/cairo/2014-July/025348.html
Comment 4 Friedrich Beckmann 2014-11-07 10:47:07 UTC
Hi, are there any conclusions regarding the introduction of the patch to remove the call to CGFontGetGlyphPath?

We currently have a problem to run pspp on yosemite because of that problem:

https://savannah.gnu.org/bugs/?43469
Comment 5 Friedrich Beckmann 2014-11-07 12:39:44 UTC
Here is the bug tracker for macports that discusses the problem with 
CGFontGetGlyphPath and MacOS yosemite. 

https://trac.macports.org/ticket/45599
Comment 6 Friedrich Beckmann 2014-11-07 12:42:09 UTC
Here is a bug entry in macports regarding MacOS 10.10 yosemite and the deprecated CGFontGetGlyphPath function.

https://trac.macports.org/ticket/45599
Comment 7 Friedrich Beckmann 2014-11-08 05:25:09 UTC
Andrea wants to provide an updated patch which removes 10.4 specific code. 10.4 will not be supported in the future. See cairo mailing list:

http://lists.cairographics.org/archives/cairo/2014-November/025792.html
Comment 8 rkdgu+a44zyz19a8hys 2014-11-28 08:13:10 UTC
Created attachment 110157 [details]
iOS project which works on ipad 3 and crashes on ipad 4, both on latest iOS
Comment 9 rkdgu+a44zyz19a8hys 2014-11-28 08:14:10 UTC
There seems to be a problem with this patch (at least as far as iOS goes).  It works on ipad 3 but crashes on ipad 4 and later.

I am investigating further, but see attached iOS test project PDFCrashTest.zip if interested (includes cairo and pixman and renders and displays a PDF).  It works on ipad 3 and crashes on ipad 4 (both running ios 8.1.1).

It crashes here (only after drawing text and calling cairo_show_page):

ctFont = CTFontCreateWithGraphicsFontPtr (font_face->cgFont);

with EXC_BAD_ACCESS.  The call stack looks like:

#0	0x234bc750 in CTFontDescriptorCopyAttribute ()
#1	0x234af26c in TFont::SetExtras(__CTFontDescriptor const*) ()
#2	0x234aed12 in TFont::TFont(CGFont*, float, CGAffineTransform const*, __CTFontDescriptor const*) ()
#3	0x234aec06 in CTFontCreateWithGraphicsFont ()
#4	0x000f2c40 in _cairo_quartz_init_glyph_path at /Users/rkdgu/PDFCrashTest/PDFCrashTest/cairo/cairo-quartz-font.c:575
#5	0x000f2736 in _cairo_quartz_scaled_glyph_init at /Users/rkdgu/PDFCrashTest/PDFCrashTest/cairo/cairo-quartz-font.c:748
#6	0x00128044 in _cairo_scaled_glyph_lookup at /Users/rkdgu/PDFCrashTest/PDFCrashTest/cairo/cairo-scaled-font.c:3004
...
#20	0x0023cd28 in cairo_show_page at /Users/rkdgu/PDFCrashTest/PDFCrashTest/cairo/cairo.c:2284
Comment 10 rkdgu+a44zyz19a8hys 2014-11-28 15:36:26 UTC
Created attachment 110168 [details]
patch with minor fix

I fixed the crash by calling CTFontCreateWithGraphicsFont with all of its documented parameters.  Strange that it crashes on ipad 4 and not ipad 3.  See attached updated patch.
Comment 11 Andrea Canciani 2014-12-09 15:31:13 UTC
Created attachment 110634 [details] [review]
Wannabe fix

This could be a fix.
I tried it on Yosemite, but I don't have iOS devices.
Could somebody check if it works?

The main differences from the original and the previously attached patches are:
 - CFRelease() on the CTFont instance (it should be needed to avoid memleaks)
 - no explicit linking of CoreText (AFAICT it might be already pulled in indirectly by CoreGraphics)
Comment 12 Andrea Canciani 2015-02-05 16:36:45 UTC
Should be fixed by:

commit 70cc8f250b5669e757b4f044571ba0f71e3dea9e
Author: Andrea Canciani <ranma42@gmail.com>
Date:   Tue Dec 9 16:13:00 2014 +0100

    quartz: Remove call to obsolete CGFontGetGlyphPath
    
    CGFontGetGlyphPath was not public and is not available anymore on
    modern OSX/iOS systems. The same functionality is available through
    the CoreText API since OSX 10.5.
    
    Based on a patch by Simon Cozens.
    
    Fixes https://bugs.freedesktop.org/show_bug.cgi?id=84324
Comment 13 ~suv 2015-03-16 23:54:27 UTC
This commit breaks canvas text (SPCanvasText) in Inkscape (used e.g. to display snap indicators and measurements (new measure tool) on-canvas) if compiled with the Quartz backend. The messages display as expected if running the same Inkscape builds with a recompiled cairo 1.14.2 which has this commit reverted. Is there anything that cairo clients need to watch out for to adapt to this commit?

Build env:
Mac OS X 10.7.5, dependencies installed via MacPorts (local port tree), tested with Inkscape 0.48.5, 0.91 and recent trunk (r14007).

I can't offer a reduced test case (code), but do have two osx application bundles of Inkscape available - the older one includes cairo 1.14.0, the newer one 1.14.2 - if this would be of any help [1].

Please let me know if you prefer to have a separate report filed for this regression. I'd also be glad for any info I could forward to inkscape devs if canvas text needs to be changed when compiled with GTK+/Quartz [2].

-- 
[1] download links for bundled apps:
with cairo 1.14.0: https://inkscape.org/en/gallery/6542/
with cairo 1.14.2: https://inkscape.org/en/gallery/6796/

Steps to reproduce:
1) draw a rectangle
2) switch to the measure tool, and measure (click+drag) across the rectangle

[2] AFAICT relevant source in inkscape (trunk):
https://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/view/head:/src/display/drawing-text.cpp
Comment 14 Andrea Canciani 2015-03-17 17:34:17 UTC
Created attachment 114401 [details] [review]
Fix font size handling
Comment 15 Andrea Canciani 2015-03-17 17:36:57 UTC
(In reply to ~suv from comment #13)
> This commit breaks canvas text (SPCanvasText) in Inkscape (used e.g. to
> display snap indicators and measurements (new measure tool) on-canvas) if
> compiled with the Quartz backend. The messages display as expected if
> running the same Inkscape builds with a recompiled cairo 1.14.2 which has
> this commit reverted. Is there anything that cairo clients need to watch out
> for to adapt to this commit?

No, I'm afraid I'm at fault here; I investigated it a bit and it looks like the issue comes from missing handling of the font size. I created the patch I just attached, which in my env it seems to work and fix Inkscape.
Can you confirm if it fixes the issue in your env (when applied on top of 1.14.2)?

Sorry for the breakage :(

> 
> Build env:
> Mac OS X 10.7.5, dependencies installed via MacPorts (local port tree),
> tested with Inkscape 0.48.5, 0.91 and recent trunk (r14007).
> 
> I can't offer a reduced test case (code), but do have two osx application
> bundles of Inkscape available - the older one includes cairo 1.14.0, the
> newer one 1.14.2 - if this would be of any help [1].
> 
> Please let me know if you prefer to have a separate report filed for this
> regression. I'd also be glad for any info I could forward to inkscape devs
> if canvas text needs to be changed when compiled with GTK+/Quartz [2].
> 
> -- 
> [1] download links for bundled apps:
> with cairo 1.14.0: https://inkscape.org/en/gallery/6542/
> with cairo 1.14.2: https://inkscape.org/en/gallery/6796/
> 
> Steps to reproduce:
> 1) draw a rectangle
> 2) switch to the measure tool, and measure (click+drag) across the rectangle
> 
> [2] AFAICT relevant source in inkscape (trunk):
> https://bazaar.launchpad.net/~inkscape.dev/inkscape/trunk/view/head:/src/
> display/drawing-text.cpp
Comment 16 ~suv 2015-03-17 18:03:48 UTC
(In reply to Andrea Canciani from comment #15)
> (...) it looks like the issue comes from missing handling of the font
> size. I created the patch I just attached, which in my env it seems
> to work and fix Inkscape. Can you confirm if it fixes the issue in
> your env (when applied on top of 1.14.2)?

Confirmed - patch from comment #15 applied to cairo 1.14.2 does fix the canvas text rendering in local Inkscape builds too (tested with Inkscape 0.91 and latest trunk r14010).

> Sorry for the breakage :(

No problem - many thanks for looking into this, and providing a fix so promptly!
Comment 17 Bryce Harrington 2015-04-25 01:28:01 UTC
Thanks, patch applied.

   d5ffe67..8b798c3  master -> master
Comment 18 ~suv 2015-05-11 04:22:26 UTC
Created attachment 115677 [details]
Screenshots comparing canvas text in Inkscape

Attaching missing screenshots [1] to illustrate the regression as observed with Inkscape's canvas text and latest stable cairo 1.14.2:
- left: Inkscape 0.91+devel, cairo 1.14.2
- right: Inkscape 0.91+devel, cairo 1.14.2 + patch from comment #14

-- 
[1] http://lists.freedesktop.org/archives/cairo/2015-May/026263.html


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.