376658 - Enable Xinerama by default

Status: RESOLVED FIXED

(Firefox Build System :: General, enhancement)

Description

[Sylvain Pasche]

Attachment: v1

After some investigations [1], it looks like all the supported distributions [2] have the Xinerama library available, because gtk2 is compiled against it.

This bug proposes that Xinerama is enabled by default. The option --disable-xinerama can be used in case it is not wanted/available.

[1] http://groups.google.com/group/mozilla.dev.builds/browse_frm/thread/ae8c08a4f338ccd4/3e0910693147f42e#3e0910693147f42e
[2] http://developer.mozilla.org/en/docs/Linux_Compatibility_Reference 

I saw another small other bug in configure.in. When using AC_CHECK_HEADER, it will set a variable ac_cv_header_XXX with the value "yes" if the header was found, or "no" otherwise. A check in XShm only tested for non emptiness, I corrected it at the same time.

I removed the old rh7 comment, as it's not on the supported platform list any more.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Given the ifdefs in nsScreenGtk.cpp and nsScreenManagerGtk.cpp, this might actually regress some things, since the panel exclusion code and the code to handle dynamic screen size changes only happens in the non-Xinerama case.  Should that code be improved first, before we do this?

Comment 2

[Sylvain Pasche]

One fact to note is that most distributions are building with --enable-xinerama, so that wouldn't be a regression for users going from their distribution binaries to release builds.

Otherwise, the non-Xinerama code path will be taken in case there's only one screen or Xinerama is not active. So panel exclusion and dynamic resizing will be working in that case. I found out that when I have Xinerama enabled, Xrandr is not available (for testing resizing). I'm wondering if this is the general case or just my configuration.

Comment 3

[Benjamin Smedberg]

Comment on attachment 260766 [details] [diff] [review]
v1

Yeah, since all the distros already do this we should bite the bullet, turn it on for our official builds and see what regressions turn up.

Comment 4

[Benjamin Smedberg]

Comment on attachment 260766 [details] [diff] [review]
v1

Roc should approve this as well, since it's a widget change.

Comment 5

[Benjamin Smedberg]

Fixed on trunk.

Comment 6

[Benjamin Smedberg]

Attachment: fix mac bustage, rev. 1

We should only do this for gtk2, to avoid the mac bustage on tbox.

Comment 7

[Sylvain Pasche]

Oh, I totally didn't think about that happening :-/

Thanks for the fix and check-in.

Comment 8

[Ben Turner (not reading bugmail, use the needinfo flag!)]

This also lit up the linux XULRunner tbox.

Comment 9

[Sylvain Pasche]

(In reply to comment #8)
> This also lit up the linux XULRunner tbox.

Error context:

c++ -I/usr/X11R6/include -fno-rtti -fno-exceptions -Wall -Wconversion -Wpointer-arith -Wcast-align -Woverloaded-virtual -Wsynth -Wno-ctor-dtor-privacy -Wno-non-virtual-dtor -Wno-long-long -pedantic -fshort-wchar -pthread -pipe  -DNDEBUG -DTRIMMED -O -fPIC -shared -Wl,-z,defs -Wl,-h,libxul.so -o libxul.so  nsStaticXULComponents.o nsUnicharUtils.o nsCompressedCharMap.o nsRDFResource.o    -lpthread    -Wl,--whole-archive 
[..skipped .a files ..]
 -Wl,--no-whole-archive   -Wl,-rpath-lin!
 k,../../dis
t/bin -L../../dist/bin -L../../dist/lib -L../../jpeg -lmozjpeg -L../../modules/libimg/png -lmozpng -L../../dist/bin -lmozjs -L../../dist/bin -L../../dist/lib -lcrmf -lsmime3 -lssl3 -lnss3 -lsoftokn3  -L../../modules/zlib/src -lmozz -lpangoxft-1.0 -lpangoft2-1.0 -lpango-1.0 -lgobject-2.0 -lgmodule-2.0 -ldl -lglib-2.0   -L../../gfx/cairo/cairo/src -lmozcairo -L../../gfx/cairo/libpixman/src -lmozlibpixman   -L/usr/X11R6/lib -lXrender -lfreetype -lfontconfig -L../../dist/lib -lplds4 -lplc4 -lnspr4 -lpthread -ldl -lXinerama -L/usr/X11R6/lib -lX11   -L/usr/X11R6/lib -lXft -lX11 -lfreetype -lXrender -lfontconfig   -lgtk-x11-2.0 -latk-1.0 -lgdk-x11-2.0 -lgdk_pixbuf-2.0 -lm -lpangoxft-1.0 -lpangox-1.0 -lpangoft2-1.0 -lpango-1.0 -lgmodule-2.0 -ldl -lgobject-2.0 -lglib-2.0   -lXt -lfreetype -lz -ldl -lm     
/usr/X11R6/lib/libXinerama.a(Xinerama.o)(.text+0x25): In function `find_display':
: undefined reference to `XextCreateExtension'
/usr/X11R6/lib/libXinerama.a(Xinerama.o)(.text+0x3d): In function `find_display':
: undefined reference to `XextFindDisplay'
/usr/X11R6/lib/libXinerama.a(Xinerama.o)(.text+0x85): In function `find_display':
: undefined reference to `XextAddDisplay'
/usr/X11R6/lib/libXinerama.a(Xinerama.o)(.text+0xb5): In function `close_display':
: undefined reference to `XextRemoveDisplay'
/usr/X11R6/lib/libXinerama.a(Xinerama.o)(.text+0x150): In function `XPanoramiXQueryVersion':
: undefined reference to `XMissingExtension'
/usr/X11R6/lib/libXinerama.a(Xinerama.o)(.text+0x2c0): In function `XPanoramiXGetState':
: undefined reference to `XMissingExtension'
/usr/X11R6/lib/libXinerama.a(Xinerama.o)(.text+0x400): In function `XPanoramiXGetScreenCount':
: undefined reference to `XMissingExtension'
/usr/X11R6/lib/libXinerama.a(Xinerama.o)(.text+0x53a): In function `XPanoramiXGetScreenSize':
: undefined reference to `XMissingExtension'
/usr/X11R6/lib/libXinerama.a(Xinerama.o)(.text+0x801): In function `XineramaQueryScreens':
: undefined reference to `XMissingExtension'
collect2: ld returned 1 exit status
gmake[4]: *** [libxul.so] Error 1
gmake[4]: Leaving directory `/builds/tinderbox/XR-Trunk/Linux_2.4.21-32.0.1.ELsmp_Depend/mozilla/toolkit/library'



I didn't manage to reproduce the failure while building xulrunner on a fc4 box, which is the nearest I have from "Red Hat Enterprise Linux AS release 3 (Taroon Update 5)".

Linking libxul.so succeeds even that -lXext is not given on the command line. libXext is where the missing symbols can be found.

My guess would be to try setting MOZ_XINERAMA_LIBS to "-lXinerama -lXext" instead of "-lXinerama" only.

Comment 10

[Sylvain Pasche]

Attachment: xr bustage fix proposal

That should be safe for recent distribs. Unfortunately I couldn't test it where it fails.

What Centos version matches the RHEL of the tinderbox? I wanted to try with Centos server 3.8, but pango is outdated there (1.2.5 where 1.6 required).

Comment 11

[Benjamin Smedberg]

Comment on attachment 261277 [details] [diff] [review]
xr bustage fix proposal

I've landed this on trunk.

Comment 12

[dolphinling]

This caused firefox to not start on distros that don't include Xinerama support (by default). See bug 377259.

Comment 13

[Sylvain Pasche]

(In reply to comment #12)
> This caused firefox to not start on distros that don't include Xinerama support
> (by default). See bug 377259.

Yes, this is expected. However, the supported set of distributions all have Xinerama support, so we don't break for them.