[Mesa-dev] [PATCH 3/3] i965/dri: Support R8G8B8A8 and R8G8B8X8 configs

[Chad Versace]

The Android framework requires support for EGLConfigs with
HAL_PIXEL_FORMAT_RGBX_8888 and HAL_PIXEL_FORMAT_RGBA_8888.

Even though all RGBX formats are disabled on gen9 by
brw_surface_formats.c, the new configs work correctly on Broxton thanks
to _mesa_format_fallback_rgbx_to_rgba().

On GLX, this creates no new configs, and therefore breaks no existing
apps. See in-patch comments for explanation. I tested with glxinfo and
glxgears on Skylake.

On Wayland, this also creates no new configs, and therfore breaks no
existing apps. (I tested with mesa-demos' eglinfo and es2gears_wayland
on Skylake). The reason differs from GLX, though. In
dri2_wl_add_configs_for_visual(), the format table contains only
B8G8R8X8, B8G8R8A8, and B5G6B5; and dri2_add_config() correctly matches
EGLConfig to format by inspecting channel masks.

On Android, in Chrome OS, I tested this on a Broxton device. I confirmed
that the Google Play Store's EGLSurface used HAL_PIXEL_FORMAT_RGBA_8888,
and that an Asteroid game's EGLSurface used HAL_PIXEL_FORMAT_RGBX_8888.
Both apps worked well. (Disclaimer: I didn't test this patch on Android
with Mesa master. I backported this patch series to an older Android
branch).
---
 src/mesa/drivers/dri/i965/intel_fbo.c    | 23 +++++++++++++++++++++--
 src/mesa/drivers/dri/i965/intel_screen.c | 23 ++++++++++++++++++++++-
 2 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/intel_fbo.c b/src/mesa/drivers/dri/i965/intel_fbo.c
index caf182ca8b7..b59fc1fc7d0 100644
--- a/src/mesa/drivers/dri/i965/intel_fbo.c
+++ b/src/mesa/drivers/dri/i965/intel_fbo.c
@@ -450,10 +450,29 @@ intel_create_winsys_renderbuffer(struct intel_screen *screen,
 
    _mesa_init_renderbuffer(rb, 0);
    rb->ClassID = INTEL_RB_CLASS;
+   rb->NumSamples = num_samples;
+
+   /* The base format and internal format must be derived from the user-visible
+    * format (that is, the gl_config's format), even if we internally use
+    * choose a different format for the renderbuffer. Otherwise, rendering may
+    * use incorrect channel write masks.
+    */
    rb->_BaseFormat = _mesa_get_format_base_format(format);
-   rb->Format = format;
    rb->InternalFormat = rb->_BaseFormat;
-   rb->NumSamples = num_samples;
+
+   rb->Format = format;
+   if (!screen->mesa_format_supports_render[rb->Format]) {
+      /* The glRenderbufferStorage paths in core Mesa detect if the driver
+       * does not support the user-requested format, and then searches for
+       * a falback format. The DRI code bypasses core Mesa, though. So we do
+       * the fallbacks here.
+       *
+       * We must support MESA_FORMAT_R8G8B8X8 on Android because the Android
+       * framework requires HAL_PIXEL_FORMAT_RGBX8888 winsys surfaces.
+       */
+      rb->Format = _mesa_format_fallback_rgbx_to_rgba(rb->Format);
+      assert(screen->mesa_format_supports_render[rb->Format]);
+   }
 
    /* intel-specific methods */
    rb->Delete = intel_delete_renderbuffer;
diff --git a/src/mesa/drivers/dri/i965/intel_screen.c b/src/mesa/drivers/dri/i965/intel_screen.c
index 8621af26378..0e03c56cf88 100644
--- a/src/mesa/drivers/dri/i965/intel_screen.c
+++ b/src/mesa/drivers/dri/i965/intel_screen.c
@@ -1717,7 +1717,28 @@ intel_screen_make_configs(__DRIscreen *dri_screen)
    static const mesa_format formats[] = {
       MESA_FORMAT_B5G6R5_UNORM,
       MESA_FORMAT_B8G8R8A8_UNORM,
-      MESA_FORMAT_B8G8R8X8_UNORM
+      MESA_FORMAT_B8G8R8X8_UNORM,
+
+      /* The 32-bit RGBA format must not precede the 32-bit BGRA format.
+       * Likewise for RGBX and BGRX.  Otherwise, the GLX client and the GLX
+       * server may disagree on which format the GLXFBConfig represents,
+       * resulting in swapped color channels.
+       *
+       * The problem, as of 2017-05-30:
+       * When matching a GLXFBConfig to a __DRIconfig, GLX ignores the channel
+       * order and chooses the first __DRIconfig with the expected channel
+       * sizes. Specifically, GLX compares the GLXFBConfig's and __DRIconfig's
+       * __DRI_ATTRIB_{CHANNEL}_SIZE but ignores __DRI_ATTRIB_{CHANNEL}_MASK.
+       *
+       * EGL does not suffer from this problem. It correctly compares the
+       * channel masks when matching EGLConfig to __DRIconfig.
+       */
+
+      /* Required by Android, for HAL_PIXEL_FORMAT_RGBA_8888. */
+      MESA_FORMAT_R8G8B8A8_UNORM,
+
+      /* Required by Android, for HAL_PIXEL_FORMAT_RGBX_8888. */
+      MESA_FORMAT_R8G8B8X8_UNORM,
    };
 
    /* GLX_SWAP_COPY_OML is not supported due to page flipping. */
-- 
2.13.0