Merge lp:~albaguirre/mir/fix-1475571 into lp:mir

[Alberto Aguirre]

Don't throw when gbm_device_is_format_supported returns false.

gbm_device_is_format_supported reports false on formats that work just fine with gbm_bo_create.

Don't throw when gbm_device_is_format_supported returns false.

gbm_device_is_format_supported reports false on formats that work just fine with gbm_bo_create.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2755
http://jenkins.qa.ubuntu.com/job/mir-ci/4350/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-vivid-i386-build/3205
    FAILURE: http://s-jenkins.ubuntu-ci:8080/job/mir-clang-ts-wily-amd64-build/58/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-wily-amd64-build/730/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-vivid-touch/3153
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-wily-amd64-ci/499/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3153
        deb: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-builder-vivid-armhf/3153/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-mediumtests-runner-mako/5957
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/21965

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/mir-ci/4350/rebuild

review: Needs Fixing (continuous-integration)

[Daniel van Vugt (vanvugt)]

That check is correct and should probably not be deleted.

I suspect the issue is now we're more careful with pixel format semantics, an invalid format used screencast is being detected. This could be a simple matter of accidentally requesting RGBA for an RGBX framebuffer or mixed up byte ordering. I'm somewhat confident the bug lies in mirscreencast but have not looked yet. Will do so today...

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

Oh I see. It's probably the bo_flags getting GBM_BO_USE_SCANOUT due to the size of the surface (which is a very ugly heuristic). With the SCANOUT flag, gbm will refuse RGBA and only allow RGBX. That's spurious because we actually have no desire for scan-out support in screencasting and there might be a real alpha channel that we want.

I think if gbm_bo_create throws adequate errors still then this change is fine. Providing it passes CI.

review: Approve

[Daniel van Vugt (vanvugt)]

An alternate fix could be to change:
  gbm_device_is_format_supported(device, gbm_format, bo_flags)
to:
  gbm_device_is_format_supported(device, gbm_format, 0)

[Chris Halse Rogers (raof)]

If it really is the GBM_BO_USE_SCANOUT flag making the difference, then wouldn't it be a better idea to fix the allocator to not try to allocate a scanout buffer for screencasts (which doesn't make sense anyway).

I'd be concerned that just dropping the check will instead bite us later when we try and scanout from a BO that's not scanout-capable.

review: Needs Information

=== modified file 'src/platforms/mesa/server/common/buffer_allocator.cpp'
--- src/platforms/mesa/server/common/buffer_allocator.cpp	2015-07-16 07:03:19 +0000
+++ src/platforms/mesa/server/common/buffer_allocator.cpp	2015-07-17 22:53:13 +0000
@@ -157,12 +157,6 @@
         bo_flags |= GBM_BO_USE_SCANOUT;
     }
 
-    if (!gbm_device_is_format_supported(device, gbm_format, bo_flags))
-    {
-        BOOST_THROW_EXCEPTION(
-            std::runtime_error("Trying to create GBM buffer with unsupported pixel format"));
-    }
-
     gbm_bo *bo_raw = gbm_bo_create(
         device,
         buffer_properties.size.width.as_uint32_t(),