Closed Bug 1432751 Opened 2 years ago Closed 2 years ago

build failure with system cairo since bug 1427641

Categories

(Core :: Graphics: Text, defect)

59 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: gaston, Assigned: jfkthame)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Per bug 1427641 comment 16, build fails when using system-cairo because the public cairo api has been modified in https://hg.mozilla.org/integration/mozilla-inbound/rev/54896137200d

/usr/obj/ports/firefox-59.0beta3/mozilla-beta-c94e1fd9fe7a47c51c421db2a7052292ac411e3a/gfx/thebes/gfxFcPlatformFontList.cpp:731:9: error: 
      no matching function for call to 'cairo_ft_font_face_create_for_pattern'                                                            
        cairo_ft_font_face_create_for_pattern(aRenderPattern,
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/include/cairo/cairo-ft.h:104:1: note: candidate function not viable: requires single argument 'pattern', but 3 arguments were  
      provided
cairo_ft_font_face_create_for_pattern (FcPattern *pattern);

/usr/obj/ports/firefox-59.0beta3/mozilla-beta-c94e1fd9fe7a47c51c421db2a7052292ac411e3a/gfx/2d/ScaledFontFontconfig.cpp:400:29: error:     
      no matching function for call to 'cairo_ft_font_face_create_for_pattern'
  cairo_font_face_t* font = cairo_ft_font_face_create_for_pattern(pattern, nullptr, 0);
                            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/local/include/cairo/cairo-ft.h:104:1: note: candidate function not viable: requires single argument 'pattern', but 3
      arguments were provided
cairo_ft_font_face_create_for_pattern (FcPattern *pattern);

removing the two extra args from those two cairo_ft_font_face_create_for_pattern calls allows me to build again, but this is ugly as f.
Blocks: 1427641
(In reply to Landry Breuil (:gaston) from comment #0)
> removing the two extra args from those two
> cairo_ft_font_face_create_for_pattern calls allows me to build again, but
> this is ugly as f.

And presumably means variation-font functionality is broken (is it?) in the resulting build. Maybe we should explicitly disable variation fonts completely when system-cairo is used; or maybe it'll work if the system cairo is sufficiently new (I believe current cairo includes some support for variation fonts, but our in-tree one is far too old for that, which is why bug 1427641 needed to hack its internals).

If you have time to propose a patch here, that'd be great; otherwise, I'll keep it in mind but not sure how soon I'll have an opportunity to really look into it.
I have no idea what variation-font is, nor how to check if it's broken or not. going to https://www.axis-praxis.org/specimens/__DEFAULT__ with 59.0b3 yields 'Your browser appears to be incompatible with variable fonts.'

No time nor knowledge to dive into this madness either, i just hope 'someone' responsible for the gfx stack will take the decisions to explicitely state that building with system cairo is insupported (and deal with the fallout), or fixes the build failure by wrapping the corresponding bits within the necessary #ifdefs... locally, ive done the simple thing: https://cgit.rhaalovely.net/mozilla-firefox/commit/?h=beta&id=e1ac53d1d283a3df093fe69283bd0aadc51f106b
FWIW, we haven't really supported building with system Cairo in truth since quite a while ago, and trying to do will result in breakage with other features that, while it might seem to compile, will be unstable at a minimum and nonfunctional at worst. Sorry.
Then.. take responsability, and deprecate/remove the option so that it's a hard failure, so that everyone gets to build against the local horror in gfx/cairo/cairo ?
(In reply to Landry Breuil (:gaston) from comment #2)
> I have no idea what variation-font is, nor how to check if it's broken or
> not. going to https://www.axis-praxis.org/specimens/__DEFAULT__ with 59.0b3
> yields 'Your browser appears to be incompatible with variable fonts.'

The support is currently behind a couple of runtime prefs. If you set

  layout.css.font-variations.enabled;true
  gfx.downloadable_fonts.keep_variation_tables;true
  gfx.downloadable_fonts.otl_validation;false

and then try https://www.axis-praxis.org/specimens/__DEFAULT__, it should no longer complain. But my guess is that the variation sliders on the right of the page (may need to zoom out to see them, if you're on a smallish screen) will not work (or at least not work reliably) without the added cairo support.

(In reply to Landry Breuil (:gaston) from comment #4)
> Then.. take responsability, and deprecate/remove the option so that it's a
> hard failure, so that everyone gets to build against the local horror in
> gfx/cairo/cairo ?

This just came up a few months back in bug 1390317, too. Maybe it's time to do that? I guess that'd be a decision for Jeff as Graphics module owner to make.
Flags: needinfo?(jmuizelaar)
I'm in favour of dropping the build option to use system cairo.
Flags: needinfo?(jmuizelaar)
This should disable the system-cairo option, and make it a configure-time error for anyone who tries to use it. (That seems more useful than silently ignoring it.) For simplicity, I left the MOZ_TREE_CAIRO definition in place for now, just hard-coded to 1; as a followup, we can remove all the related #ifdef checks scattered around the codebase.
Attachment #8945413 - Flags: review?(jmuizelaar)
Attachment #8945413 - Flags: review?(jmuizelaar) → review+
Pushed by jkew@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/933304b9909e
Building with system cairo is no longer supported; make --enable-system-cairo a configure-time error. r=jrmuizel
Blocks: 1433429
https://hg.mozilla.org/mozilla-central/rev/933304b9909e
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Assignee: nobody → jfkthame
You need to log in before you can comment on or make changes to this bug.