328082 - rename cairo functions

Status: RESOLVED FIXED

(Core :: Graphics, defect, P1)

Description

[Christian :Biesinger (don't email me, ping me on IRC)]

to avoid conflicts with the system cairo, the functions of the in-tree cairo should be renamed, because if they aren't, I crash:

#0  0xb78cf4c8 in cairo_surface_reference () from /usr/lib/libcairo.so.2
#1  0xb5dc378f in _cairo_gstate_init_copy (gstate=0x828a240, other=0x82ab4d0) at /home/chb/mozilla/gfx/cairo/cairo/src/cairo-gstate.c:173
#2  0xb5dc39ad in _cairo_gstate_clone (other=0x82ab4d0) at /home/chb/mozilla/gfx/cairo/cairo/src/cairo-gstate.c:246
...

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

this was generated by looking at the exported functions from my libthebes. it therefore doesn't include win- or mac-specific functions. but as there is not usually a system cairo there, this should be fine.

Comment 2

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 212629 [details] [diff] [review]
patch

So we should only rename if we're building with a system cairo, IMO, or we should rename all public functions (including win32, etc.)... otherwise debugging will be a pain, since some functions will be cairo_* and some will be _moz_cairo_*.  I'd prefer just renaming everything always.

Comment 3

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch v2

Rename all cairo_public functions.

Comment 4

[Darin Fisher]

why does libthebes export cairo symbols at all?  why aren't they all statically linked as hidden symbols into libthebes?  you should be able to write a linker script to prevent them from being visible at least.  is renaming really necessary? 

Comment 5

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to comment #4)
> why does libthebes export cairo symbols at all?  why aren't they all statically
> linked as hidden symbols into libthebes?  you should be able to write a linker
> script to prevent them from being visible at least.  is renaming really
> necessary? 

It exports them because of SVG and Canvas -- canvas has already been rewritten for thebes, but I can't land it until we make the switch on linux at least (because landing it would mean disabling canvas on non-thebes builds).  I'm not sure what SVG's plans are for switching to thebes, but they'll have to come along for the ride soon...

Doing the renaming seems like a reasonable interim fix, and we'll need it on the 1.8.1 branch anyway, for linux -- pango/gtk will be using cairo, and so will svg/canvas, and we don't want to run into symbol issues there.

Comment 6

[Darin Fisher]

ah, makes perfect sense.  sorry for the drive-by.

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch v2

includes a makefile target to regenerate the file

Comment 8

[Rinaldi J. Montessi]

Patch v2 can no longer be applied to pixman-remap.h.  Current version 1.6, patched version 1.3

Comment 9

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 215182 [details] [diff] [review]
patch v2

Updated with discussion from IRC; I'll check in with those.

Comment 10

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Checked in.