290835 - cairo linkage problems

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[tor]

We're about to get into a situation where both mozilla's SVG and canvas support
require cairo.  SVG currently links cairo into a component which is always a
seperate library, so that if appropriate versions of freetype/fontconfig are
missing the component can fail and mozilla will still run.

canvas takes the approach of linking cairo directly to gklayout, so any failure
to load the cairo library will cause mozilla to fail to start.

Additionally, with the patch in bug 290518 we're going to linking cairo
statically into both the svg renderer component and gklayout, which is a waste.

Are we willing to assume that freetype/fontconfig are around, in which case we
can link the svg renderer into gklayout?  Or does canvas need to be reengineered
to be in a seperate component or dynamically load cairo?

Comment 1

[Stuart Parmenter]

I think we need to just depend on freetype and fontconfig always being around,
although that may mean updating our lowest supported system config (redhat8
currently?)

More and more libraries in Mozilla are going to start depending on cairo
directly or indirectly so trying to move things in to seperate libraries isn't
going to work for long.

Comment 2

[tor]

RH8's Xrender isn't enough to deal with cairo - there's a patch in bug 286422 to
add that library to the tree (about 20k of code), if we keep that as the base
configuration.

Comment 3

[Benjamin Smedberg]

What's the story on win32? Are we using cairo there, which (I presume) has a DLL
dependency on GDI+?

Comment 4

[tor]

For SVG on win32 we use gdi+ directly, with a shim component checking for the
library before pulling in the renderer code.

Cairo doesn't use gdi+ on win32, so canvas doesn't have that dependency.

Comment 5

[Darin Fisher]

Doesn't our GTK2+XFT build require fontconfig already?

If we really want a build that fails gracefully when fontconfig and freetype
aren't around, maybe we could hack cairo-ft-font.c since it seems to be the only
module that uses those libraries.  In fact, that module has very few extern
functions, so we could probably turn that module into a shim DSO if needed. 
But, it seems to me that we could probably just require those libraries and be
done with it.

Comment 6

[tor]

Attachment: link cairo svg renderer into gklayout

Since we're turning on <canvas> and linking cairo directly into gklayout, do
the same for the svg cairo renderer so we don't end up carrying around two
copies of the cairo code.

Comment 7

[tor]

*** Bug 291086 has been marked as a duplicate of this bug. ***

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I talked this through with tor on IRC and we don't want to statically link Cairo
into layout after all, because gfx/src/cairo needs it and we don't want two
copies. So we want something like my patch in bug 291086 to make gklayout link
with -lmozcairo.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: dynamically link libmozcairo/libmozlibpixman from everywhere

This patch works for me. Canvas and SVG both work, and they're both dynamically
linking to libmozcairo.so.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

tor did mention that we might need to do something to make this work with static
builds. I haven't tested that.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: safer fix

Perhaps this fix is safer ... force Cairo to be a shared lib so that we know
the dynamic gksvgcairo can get to it. Pick one of these patches :-)

Comment 12

[Benjamin Smedberg]

Since we're building cairo ourself, gksvgcairo doesn't need to be dynamic. In a
static build these should all be statically linked.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: uber-fix

This patch merges the other patches together so that
-- the Cairo SVG renderer is no longer a component. It is statically linked
into gklayout when enabled.
-- libmozcairo and libmozpixman are treated like other gfx libraries ... they
are dynamic libraries in nonstatic builds, and static libraries in static
builds.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: uber-fix #2

Add MOZ_CAIRO_LIBS if either MOZ_ENABLE_CAIRO or MOZ_SVG_RENDERER_CAIRO.

(MOZ_ENABLE_CAIRO is misleading ... either it should be renamed
MOZ_ENABLE_CANVAS, or it should be set when MOZ_SVG_RENDERER_CAIRO is
selected.)

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: uber-fix #3

all of the above, plus links libpixman statically into libmozcairo.so.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Unfortunately this approach doesn't work because cairo.h doesn't have any
"export" annotations, so on gcc with visibility pragmas, and presumably on
Windows, the public API functions don't get exported in the shared library.

So for now I think we just have to go with tor's original fix. That means
regular 1.8 builds will have just one copy of Cairo, statically linked into
gklayout. cairo-gtk2 builds will have two copies of Cairo, but that's acceptable
because they're experimental for the forseeable future. (Here's hoping Cairo
doesn't have any global state!) Post-1.8 we'll need to revisit this situation.
Probably we should push the Cairo people to have export annotations on their
public APIs. I will send email to the list.

Comment 17

[Darin Fisher]

You should be thinking about making this optimal in the case of a static firefox
build or in the case of a xulrunner libxul build (one big honking DLL).  The
multi-DLL build is really a thing of the past.  Making that efficient should be
low priority.

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

It appears that Cairo does have some global state (global shared caches), so
it's not just a question of efficiency, it's also going to be necessary if
multi-DLL builds are going to work at all without exploding in surprising ways.
My machine may be studly enough to handle building XULRunner debug every time I
make a change during development, but that may not be true for everyone.

Comment 19

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Sorry if that last comment was a bit testy. It's been a frustrating afternoon.

Comment 20

[Brendan Eich [:brendan]]

Darin was no doubt excluding developer builds in putting emphasis on static
builds ;-).

Why don't we make a deal to do the macro-ized declspec decoration of cairo.h,
and contribute it?

/be

Comment 21

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I've sent mail to the cairo list asking if they'd be interested in such a patch.

Comment 22

[Darin Fisher]

i totally agree that multi-DLL developer builds should work.  i just meant that
if we have to have two copies of cairo in the developer build, then so be it.
but, i guess that's totally a moot point now ;-)

we need to support a multi-DLL, developer friendly build for xulrunner or else
it won't be any fun for people developing gecko in the new world of libxul.

Comment 23

[Benjamin Smedberg]

Comment on attachment 181282 [details] [diff] [review]
link cairo svg renderer into gklayout

I'm assuming this patch is obsolete by now?

Comment 24

[tor]

Comment on attachment 181282 [details] [diff] [review]
link cairo svg renderer into gklayout

No, this is the patch that roc and I found to be the best way forwards at this
point.	See comment 16.

It's post bryner's cairo landing, if that's what you were thinking of.

Comment 25

[Benjamin Smedberg]

Comment on attachment 181282 [details] [diff] [review]
link cairo svg renderer into gklayout

layout/svg/renderer/src/cairo/Makefile.in
Please change MOZILLA_INTERNAL_API to LIBXUL_LIBRARY

Comment 26

[tor]

Comment on attachment 181282 [details] [diff] [review]
link cairo svg renderer into gklayout

Allows us to build with both canvas and svg, stops us from carrying around two
copies of the cairo code in builds.

Comment 27

[chris hofmann]

Comment on attachment 181282 [details] [diff] [review]
link cairo svg renderer into gklayout

a=chofmann

Comment 28

[tor]

Checked in with LIBXUL_LIBRARY.