1390317 - gfx/thebes/gfxFont.cpp:846:5: error: 'cairo_scaled_font_get_hint_metrics' was not declared in this scope

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect, P3)

Description

[Jan Beich]

$ pkg info -x cairo
cairo-1.14.10,2
py27-cairo-1.10.0_2

$ echo "ac_add_options --enable-system-cairo" >>.mozconfig
$ ./mach build
[...]
In file included from objdir/gfx/thebes/Unified_cpp_gfx_thebes0.cpp:65:
gfx/thebes/gfxFont.cpp:846:5: error: use of undeclared identifier
      'cairo_scaled_font_get_hint_metrics'; did you mean 'cairo_scaled_font_get_font_face'?
    cairo_scaled_font_get_hint_metrics(scaled_font);
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    cairo_scaled_font_get_font_face
/usr/local/include/cairo/cairo.h:1628:1: note: 'cairo_scaled_font_get_font_face' declared here
cairo_scaled_font_get_font_face (cairo_scaled_font_t *scaled_font);
^
In file included from objdir/gfx/thebes/Unified_cpp_gfx_thebes0.cpp:65:
gfx/thebes/gfxFont.cpp:845:24: error: cannot initialize a variable of type
      'cairo_hint_metrics_t' (aka '_cairo_hint_metrics') with an rvalue of type 'cairo_font_face_t *'
      (aka '_cairo_font_face *')
  cairo_hint_metrics_t hint_metrics =
                       ^
2 errors generated.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: Don't try to use the mozilla-specific cairo extension from bug 1377257 if we're building with system cairo

Hmm, I didn't know we still supported building with --enable-system-cairo. OK, for that situation we can't use the new mozilla-specific cairo api that was added in bug 1377257, so I guess we can just fall back on the old code here.

Comment 2

[Jan Beich]

Comment on attachment 8897179 [details] [diff] [review]
Don't try to use the mozilla-specific cairo extension from bug 1377257 if we're building with system cairo

Thanks. With the patch applied --enable-system-cairo builds fine and runs as before.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

Building with system cairo isn't really supported anymore. Without a really compelling reason I think it would probably be better to just remove the --enable-system-cairo flag.

Comment 4

[Jonathan Kew [:jfkthame]]

Jan, do you have "a really compelling reason" (see comment 3) to use --enable-system-cairo? Or can we just remove it?

Comment 5

[Jan Beich]

(In reply to Jonathan Kew (:jfkthame) from comment #4)
> Jan, do you have "a really compelling reason" (see comment 3) to use
> --enable-system-cairo? Or can we just remove it?

No. What about Gentoo?

Comment 6

[Jory A. Pratt [:anarchy]]

We do not support building with system-cairo.

Comment 7

[Jan Beich]

Can you either land the fix or remove the option? Leaving --enable-system-cairo half-broken on FF57 is going to confuse users. Maybe add AC_MSG_ERROR to old-configure.in if you're not ready to remove the ifdefs e.g., need to compare old vs. new Cairo sometimes.

Comment 8

[Jan Beich]

FWIW, I've used --enable-system-cairo for years without issues (except circa FF40 due to OMTC) and didn't notice it was broken. e10s was disabled by extensions.

Comment 9

[Hussam Al-Tayeb]

Same here. Apart from the very rare breakage, I've been compiling with --enable-system-cairo since firefox 2.0 days.
I use cairo 1.15.8 at the moment.

Comment 10

[Jonathan Kew [:jfkthame]]

Jeff: while I have no particular attachment to --enable-system-cairo, some people are accustomed to using it, and removing it altogether looks like a lot more upheaval than the patch here. Given that this is such a simple fix (and doesn't affect functionality at all, it's just disabling a minor optimization), I'd be inclined to take this patch for now.

Removing build support for --enable-system-cairo (and the associated MOZ_TREE_CAIRO #ifdefs, etc) should be a separate followup, IMO, that we can consider at leisure.

Comment 11

[Landry Breuil (:gaston)]

We've also been using --with-system-cairo since more or less forever (1.14.10 right now), when it was possible. It wasnt possible for a while in the 40-something days, but since the switch to Gtk3 i reenabled it (my notes point to bug 983843 for the actual reason; memory is fuzzy).

If it's officially unsupported, then someone has to take the responsability to remove it.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ff5a050f7eb7
Don't try to use the mozilla-specific cairo extension from bug 1377257 if we're building with system cairo. r=jrmuizel

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/ff5a050f7eb7

Comment 14

[Jan Beich]

Can you request merge to Firefox 57 to avoid regression there as well?

Comment 15

[Jonathan Kew [:jfkthame]]

Comment on attachment 8897179 [details] [diff] [review]
Don't try to use the mozilla-specific cairo extension from bug 1377257 if we're building with system cairo

We can request merging to 57, but I don't know if it will be accepted; the bar for patches to land there is pretty strict, I believe. But as this has no effect on mozilla builds, maybe it'll be OK.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1377257
[User impact if declined]: build failure for downstream consumers using --enable-system-cairo
[Is this code covered by automated tests?]: n/a, #ifdef'd code that is relevant only to non-mozilla --enable-system-cairo builds
[Has the fix been verified in Nightly?]: n/a
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: no impact on mozilla builds, #ifdef'd alternative just reverts to previous code (dropping a micro-optimization) for downstream builds
[String changes made/needed]: none

Comment 16

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8897179 [details] [diff] [review]
Don't try to use the mozilla-specific cairo extension from bug 1377257 if we're building with system cairo

NPOTB, Beta57+

Comment 17

[Ryan VanderMeulen (PTO, back July 27)]

https://hg.mozilla.org/releases/mozilla-beta/rev/c8d3b27528f7