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: *** [gfxASurface.o] Error 1 make: 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.
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.
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?
Created attachment 372770 [details] [diff] [review] Update Fabian's patch Also use MOZ_X11 instead of CAIRO_HAS_XLIB_SURFACE for cairo-gtk2-dfb.
Bug confirmed on Ubuntu 9.04. Hiroyuki's update to Fabien's patch allows build without gfxASurface error.
This patch isn't sufficient to build comm-central, but that may be a different bug.
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?
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?
Comment on attachment 372770 [details] [diff] [review] Update Fabian's patch This is clearly right...
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.
Comment on attachment 372770 [details] [diff] [review] Update Fabian's patch per comment #11.
Comment on attachment 372770 [details] [diff] [review] Update Fabian's patch Shifting approval flag onto the next 188.8.131.52 build so we can hopefully pick this up for all apps.
Comment on attachment 372770 [details] [diff] [review] Update Fabian's patch a1912=beltzner
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 184.108.40.206.
(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 220.127.116.11.
Comment on attachment 372770 [details] [diff] [review] Update Fabian's patch Approved for 18.104.22.168, a=dveditz for release-drivers Let's try to get this one landed this time.
Comment on attachment 372770 [details] [diff] [review] Update Fabian's patch past code-freeze for 22.214.171.124, removing non-blocker approval.
Comment on attachment 372770 [details] [diff] [review] Update Fabian's patch If noone objects, I'll take care of the checkin for 126.96.36.199, I'd like to get this patch in, its a pity it hasn't made it before.
Comment on attachment 372770 [details] [diff] [review] Update Fabian's patch Approved for 188.8.131.52, a=dveditz for release-drivers
Pushed to mozilla 1.9.1 <http://hg.mozilla.org/releases/mozilla-1.9.1/rev/55b533191e5c> status 1.9.1 -> .5-fixed