[Mesa-dev] [PATCH 3/3] i965/dri: Support R8G8B8A8 and R8G8B8X8 configs
Kenneth Graunke
kenneth at whitecape.org
Tue Jun 27 19:43:53 UTC 2017
On Tuesday, June 27, 2017 11:00:48 AM PDT Chad Versace wrote:
> 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.
The series is:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
But, I still think this should be fixed. This seems fragile - we're
essentially relying on a bug in the GLX code.
I suppose fixing it would cause GLX to begin exposing new RGBA visuals
(in addition to BGRA). I don't think we want to do that - I don't see much
point outside of Android. It's mostly harmless, except that changing the
visuals ends up being absurdly painful for driver developers, since the X
server and clients need to agree on lists. That means you can't move across
the change point without updating the Mesa your X server uses and restarting
the X server.
If we do fix GLX, and want to avoid exposing visuals, I think we could
easily add an #ifdef ANDROID. Is there some reason to not go ahead and
do that now?
Also...did we fix the visual ordering problem? Earlier you mentioned that
this patch wouldn't work for Android, because we needed to move the RGBA
formats above the BGRA ones (but that wouldn't work for X). But I think
you said you had EGL patches which fixed that, and I'm pretty sure those
landed. So...we're good now?
> + *
> + * 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. */
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170627/ffb1347b/attachment.sig>
More information about the mesa-dev
mailing list