build failure for cairo-gtk2 build with a directfb cairo

RESOLVED FIXED

Status

()

Core
Graphics
RESOLVED FIXED
9 years ago
7 years ago

People

(Reporter: Fabien Tassin, Assigned: hiro)

Tracking

Trunk
x86
Linux
Points:
---

Firefox Tracking Flags

(status1.9.1 .6-fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

1.74 KB, patch
roc
: review+
Samuel Sidler (old account; do not CC)
: approval1.9.1.2-
Details | Diff | Splinter Review
(Reporter)

Description

9 years ago
Created attachment 349521 [details] [diff] [review]
fix v1

When building trunk with a system cairo that is directfb enabled, it fails with:

g++ -o gfxASurface.o -c -I../../../dist/include/system_wrappers -include ../../../config/gcc_hidden.h -DIMPL_THEBES -DMOZILLA_INTERNAL_API -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_NS_COM_OBSOLETE -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES  -DOSTYPE=\"Linux2.6\" -DOSARCH=Linux  -I. -I. -I../../../dist/include/cairo -I../../../dist/include/string -I../../../dist/include/pref -I../../../dist/include/xpcom -I../../../dist/include/unicharutil -I../../../dist/include/lcms -I../../../dist/include/locale -I../../../dist/include   -I../../../dist/include/thebes -I/usr/include/nspr   -I/usr/include  -I/build/buildd/xulrunner-1.9.1-1.9.1~b2~hg20081121r21862+nobinonly/build-tree/mozilla/dist/sdk/include    -fPIC   -fno-rtti -fno-exceptions -Wall -Wpointer-arith -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wcast-align -Wno-invalid-offsetof -Wno-long-long -pedantic -g -fno-strict-aliasing -fshort-wchar -pthread -pipe  -DNDEBUG -DTRIMMED -Os -freorder-blocks -fno-reorder-functions  -D_REENTRANT -I/usr/include/cairo -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/directfb -I/usr/include/libpng12   -D_REENTRANT -I/usr/include/cairo -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/directfb -I/usr/include/libpng12   -D_REENTRANT -I/usr/include/gtk-2.0 -I/usr/lib/gtk-2.0/include -I/usr/include/atk-1.0 -I/usr/include/cairo -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/pixman-1 -I/usr/include/freetype2 -I/usr/include/directfb -I/usr/include/libpng12 -I/usr/include/gtk-unix-print-2.0   -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/freetype2     -DMOZILLA_CLIENT -include ../../../mozilla-config.h -Wp,-MD,.deps/gfxASurface.pp gfxASurface.cpp
gfxASurface.cpp:59:32: error: gfxDirectFBSurface.h: No such file or directory
gfxASurface.cpp: In static member function 'static already_AddRefed<gfxASurface> gfxASurface::Wrap(cairo_surface_t*)':
gfxASurface.cpp:176: error: expected type-specifier before 'gfxDirectFBSurface'
gfxASurface.cpp:176: error: cannot convert 'int*' to 'gfxASurface*' in assignment
gfxASurface.cpp:176: error: expected `;' before 'gfxDirectFBSurface'
make[6]: *** [gfxASurface.o] Error 1
make[6]: Leaving directory `/build/buildd/xulrunner/mozilla/gfx/thebes/src'

One option could be to protect the code with MOZ_DFB as in the patch attached.
Another could be to fix the CFLAGS to include -I../public.
Attachment #349521 - Flags: review?(vladimir)

Comment 1

8 years ago
The CFLAGS already include -I../../../dist/include/thebes, which is where /src/gfx/themes/public/ gets copied. The file is not there because you are compiling MOZ_DEFAULT_TOOLKIT="cairo-gtk2", not "cairo-gtk2-dfb".

When mozilla is compiled with the bundled cairo with
 MOZ_DEFAULT_TOOLKIT="cairo-gtk2", cairo is configured to have
 CAIRO_HAS_XLIB_SURFACE but not CAIRO_HAS_DIRECTFB_SURFACE.
Which is great.
In src/gfx/thebes/public/Makefile.in, the varius surfaces are included in
the exported dist include directory depending on MOZ_WIDGET_TOOLKIT
and MOZ_X11 with MOZ_DFB. These are set by the main configure script, when
MOZ_DEFAULT_TOOLKIT is set. So with cairo-gtk2, MOZ_X11 is set but MOZ_DFB
is unset. gfxDirectFBSurface.h is not exported. This is also great.

When compiling mozilla/src/gfx/thebes/src/gfxASurface.cpp, it includes the
various surfaces depending on whether cairo supports them (as defined in
cairo-features.h, included from cairo.h), rather than depending on
MOZ_WIDGET_TOOLKIT and MOZ_X11 with MOZ_DFB, i.e. the configuration.
Ans so, when compiled with "ac_add_options --enable-system-cairo", if the system
cairo has been compiled with DFB support (for example on Ubuntu), gfxASurface.cpp
tries to include gfxDirectFBSurface.h, which is not in dist/include/thebes,
and errors out.

I think gfxASurface.cpp should really use mozilla's configuration macros rather
than cairo's to determine which surface to use. Maybe check both, though
it's the configure script's job to determine that cairo has the required support
for the toolkit we're requesting.

Comment 2

8 years ago
Same error when building Thunderbird 3.0 beta 2 on Ubuntu 9.04 beta.  It's picking up CAIRO_HAS_DIRECTFB_SURFACE=1 from /usr/include/cairo/cairo-features.h, but MOZ_DFB=0 from the configure scripts.

This may or may not actually be causing the problem, but Thunderbird/comm-central's configure script does not have a toolkit option of 'cairo-gtk2-dfb', as mozilla-central's configure script does.  I can see that causing some inconsistencies as the comm-central sources build and rely on mozilla-central.  Can the same modifications that vladimir made to add DirectFB support to mozilla-central (http://people.mozilla.com/~vladimir/misc/1-thebes-directfb.patch) be added to comm-central?
(Assignee)

Comment 3

8 years ago
Created attachment 372770 [details] [diff] [review]
Update Fabian's patch

Also use MOZ_X11 instead of CAIRO_HAS_XLIB_SURFACE for cairo-gtk2-dfb.
Attachment #349521 - Attachment is obsolete: true
Attachment #349521 - Flags: review?(vladimir)

Comment 4

8 years ago
Bug confirmed on Ubuntu 9.04. Hiroyuki's update to Fabien's patch allows build without gfxASurface error.
(Assignee)

Updated

8 years ago
Attachment #372770 - Flags: review?(vladimir)
This patch isn't sufficient to build comm-central, but that may be a different bug.
(Assignee)

Comment 6

8 years ago
Which bug?

Comment 7

8 years ago
Philipp, please try again.  I was also having a different problem building a working Thunderbird even earlier this week, due to something in the Mozilla 1.9.1 source, but that issue seems to have been resolved recently.

Building under an environment where cairo has directfb support is still broken.  This patch still fixes it with the current source (it is the only patch I need).  Could you (or someone) please review and commit?

Comment 8

8 years ago
This patch has been ready for review for two months.  It is marked for review by vladimir.  Is he available for review or can someone else do it?
(Assignee)

Updated

8 years ago
Attachment #372770 - Flags: review?(vladimir) → review?(roc)
Attachment #372770 - Flags: review?(roc) → review+
Comment on attachment 372770 [details] [diff] [review]
Update Fabian's patch

This is clearly right...
(Assignee)

Updated

8 years ago
Assignee: nobody → ikezoe
Status: NEW → ASSIGNED
Keywords: checkin-needed
(Assignee)

Comment 10

8 years ago
Pushed.

http://hg.mozilla.org/mozilla-central/rev/9eca240fc4b8
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED

Updated

8 years ago
Keywords: checkin-needed

Comment 11

8 years ago
Thanks for applying the patch to mozilla-central.  However this patch also needs to be applied to mozilla-1.9.1 in order for comm-central (Thunderbird 3.0 and Sunbird 1.0) to build correctly.
(Assignee)

Comment 12

8 years ago
Comment on attachment 372770 [details] [diff] [review]
Update Fabian's patch

per comment #11.
Attachment #372770 - Flags: approval1.9.1?
Duplicate of this bug: 505078
Comment on attachment 372770 [details] [diff] [review]
Update Fabian's patch

Shifting approval flag onto the next 1.9.1.2 build so we can hopefully pick this up for all apps.
Attachment #372770 - Flags: approval1.9.1? → approval1.9.1.2?
Comment on attachment 372770 [details] [diff] [review]
Update Fabian's patch

a1912=beltzner
Attachment #372770 - Flags: approval1.9.1.2? → approval1.9.1.2+

Comment 16

8 years ago
Thunderbird 3.0 beta 3 pulls from mozilla-central using the tag 'GECKO_1_9_1_BASE', so it is not getting this patch.  (See 'client.py' in comm-central.)  I'd like to see Thunderbird 3.0 (and Sunbird/Lightning 1.0) not be affected by this bug out of the box.  So does Thunderbird 3.0 need to start pulling a different tag from mozilla-central ... or does this patch need to be checked in under 'GECKO_1_9_1_BASE' ... or basically what needs to occur to make that happen?
Hiroyuki Ikezoe: When you check this into mozilla-1.9.1, be sure to use ".2-fixed" option in the "status1.9.1" field to mark this as fixed for 1.9.1.2.
(In reply to comment #16)
> Thunderbird 3.0 beta 3 pulls from mozilla-central using the tag
> 'GECKO_1_9_1_BASE', so it is not getting this patch.  (See 'client.py' in
> comm-central.) I'd like to see Thunderbird 3.0 (and Sunbird/Lightning 1.0) not
> be affected by this bug out of the box.  So does Thunderbird 3.0 need to start
> pulling a different tag from mozilla-central ... or does this patch need to be
> checked in under 'GECKO_1_9_1_BASE' ... or basically what needs to occur to
> make that happen?

The Thunderbird 3 builds don't pull from mozilla-central they pull from mozilla-1.9.1 which is getting this fix. So nothing no changes are needed to make this happen.
Comment on attachment 372770 [details] [diff] [review]
Update Fabian's patch

This didn't land in time for 1.9.1.2.
Attachment #372770 - Flags: approval1.9.1.3?
Attachment #372770 - Flags: approval1.9.1.2-
Attachment #372770 - Flags: approval1.9.1.2+
Comment on attachment 372770 [details] [diff] [review]
Update Fabian's patch

Approved for 1.9.1.4, a=dveditz for release-drivers

Let's try to get this one landed this time.
Attachment #372770 - Flags: approval1.9.1.3? → approval1.9.1.4+
Comment on attachment 372770 [details] [diff] [review]
Update Fabian's patch

past code-freeze for 1.9.1.4, removing non-blocker approval.
Attachment #372770 - Flags: approval1.9.1.4+ → approval1.9.1.4-
Comment on attachment 372770 [details] [diff] [review]
Update Fabian's patch

If noone objects, I'll take care of the checkin for 1.9.1.5, I'd like to get this patch in, its a pity it hasn't made it before.
Attachment #372770 - Flags: approval1.9.1.5?
Comment on attachment 372770 [details] [diff] [review]
Update Fabian's patch

Approved for 1.9.1.5, a=dveditz for release-drivers
Attachment #372770 - Flags: approval1.9.1.5? → approval1.9.1.5+
Pushed to mozilla 1.9.1
<http://hg.mozilla.org/releases/mozilla-1.9.1/rev/55b533191e5c>

status 1.9.1 -> .5-fixed
status1.9.1: --- → .5-fixed
Duplicate of this bug: 501239
You need to log in before you can comment on or make changes to this bug.