Merge lp:~brandontschaefer/libsdl/enable-mir-support-ffe into lp:ubuntu/trusty/libsdl2

[Brandon Schaefer]

Enable Mir support by default.

Enable Mir support by default.

[Colin Watson (cjwatson)]

This is going to make libsdl2 not build on some architectures it
currently builds on, which will fail proposed-migration (and for good
reason). You need to restrict this change to only architectures where
libmirclient-dev is currently available, not just to linux-any.

[Brandon Schaefer (brandontschaefer)]

This is very true. libmirclient-dev is in trusty, which is what my target for this patch really is. How would I go about ensuring the restriction to just trusty?

[Colin Watson (cjwatson)]

trusty is not the relevant thing. libmirclient-dev is only available on
amd64, armhf, and i386 right now, so your changes need to be restricted
to those architectures.

[Brandon Schaefer (brandontschaefer)]

I see, that makes much more sense. Sorry! All pushed and tested as well! So now only mir depends will be added iff the correct archs are selected. As well as for the rules, will only enable mir with the correct archs.

[Dmitry Shachnev (mitya57)]

Marking as Work in Progress as it "needs FFe approval first".

[Colin Watson (cjwatson)]

A clearer and more robust way to spell the debian/rules test would be:

  DEB_HOST_ARCH ?= $(shell dpkg-architecture -qDEB_HOST_ARCH)
(put this together with the other similar variable settings, if not already set; you want DEB_HOST_ARCH for this and not DEB_HOST_ARCH_CPU, because e.g. you've missed that DEB_HOST_ARCH_CPU is "arm" on the armhf architecture)

  ifneq (,$(filter $(MIR_SUPPORTED_ARCH),$(DEB_HOST_ARCH)))

(Needs Fixing because of the arm bit if nothing else.)

Also, why did you delete the --disable-altivec configuration for powerpc? That looks like a mistake.

review: Needs Fixing

[Brandon Schaefer (brandontschaefer)]

Very correct on all! Thanks for reviewing

[tshirtman (gabriel-pettier)]

Not sure this is the right place for this, i built with this patch on my ubuntu-touch nexus4, and tried to run a simple program (for example https://raw.githubusercontent.com/crust/sdl2-examples/master/createwindow.cpp), while this one runs without errors (though nothing gets displayed in ubuntu), if i add | SDL_WINDOW_OPENGL to the flags, i get:

Could not create window: Failed to created a window surface 0x1

when running.

Any input welcome.

[Colin Watson (cjwatson)]

@tshirtman: Probably best to file a separate bug for this - merge proposals tend not to be a good place for support / bug tracking.

My main concern here is to ensure that existing behaviour doesn't regress, as we can always fix up new behaviour later. To that end I've tested this on a desktop system, and mpv still seems to work sensibly (and may be my new favourite lightweight video player ...); and indeed Brandon's changes have addressed my earlier code-review concerns. So this looks good to me.

review: Approve

[Brandon Schaefer (brandontschaefer)]

The good thing about this patch, is all it does is allow SDL to pick Mir as a video driver backend. X11 will still default if running on X11. The only time this patch will make a difference is if you try to run an SDL2 app directly on a Mir server. Otherwise, nothing changes :).

=== modified file 'debian/changelog'
--- debian/changelog	2014-03-19 20:22:29 +0000
+++ debian/changelog	2014-03-25 19:14:23 +0000
@@ -1,3 +1,9 @@
+libsdl2 (2.0.2+dfsg1-3ubuntu1) trusty; urgency=medium
+
+  * Enable Mir video support.
+
+ -- Brandon Schaefer <[e-mail]>  Thu, 20 Mar 2014 14:55:50 -0700
+
 libsdl2 (2.0.2+dfsg1-3) unstable; urgency=medium
 
   * Actually change Build-Depends libegl1-mesa-dev to be conditionally

=== modified file 'debian/control'
--- debian/control	2014-03-19 20:22:29 +0000
+++ debian/control	2014-03-25 19:14:23 +0000
@@ -15,6 +15,7 @@
                libgl1-mesa-dev,
                libgles2-mesa-dev [!hurd-any],
                libglu1-mesa-dev,
+               libmirclient-dev [i386 amd64 armhf],
                libpulse-dev,
                libts-dev [linux-any],
                libudev-dev [linux-any],
@@ -62,6 +63,7 @@
          libgl1-mesa-dev,
          libgles2-mesa-dev [!hurd-any],
          libglu1-mesa-dev,
+         libmirclient-dev [i386 amd64 armhf],
          libpulse-dev,
          libts-dev [linux-any],
          libudev-dev [linux-any],

=== modified file 'debian/rules'
--- debian/rules	2014-03-18 20:17:10 +0000
+++ debian/rules	2014-03-25 19:14:23 +0000
@@ -2,8 +2,10 @@
 
 SHLIBVER = 2.0.0
 
-DEB_HOST_ARCH_CPU ?= $(shell dpkg-architecture -qDEB_HOST_ARCH_CPU)
+DEB_HOST_ARCH      ?= $(shell dpkg-architecture -qDEB_HOST_ARCH)
+DEB_HOST_ARCH_CPU  ?= $(shell dpkg-architecture -qDEB_HOST_ARCH_CPU)
 DEB_HOST_MULTIARCH ?= $(shell dpkg-architecture -qDEB_HOST_MULTIARCH)
+MIR_SUPPORTED_ARCH := i386 amd64 armhf
 
 confflags = --disable-rpath --enable-sdl-dlopen --disable-loadso \
             --disable-nas --disable-esd --disable-arts \
@@ -29,6 +31,9 @@
   confflags += --disable--video-opengles
 endif
 
+ifneq (,$(filter $(MIR_SUPPORTED_ARCH),$(DEB_HOST_ARCH)))
+  confflags += --enable-video-mir --enable-mir-shared
+endif
 
 %:
 	dh $@ --with autoreconf --parallel