Hi,
I've sent a request to revert this commit in 17.2. I'll keep it in
master, but I'll add a fix not to expose the new formats for GLX.
Marek
On Tue, Jul 11, 2017 at 10:34 PM, Rob Herring <r...@kernel.org> wrote:
> From: Marek Olšák <marek.ol...@amd.com>
>
> Add support for 32-bit RGBX/RGBA formats which are required for Android.
>
> The original patch (commit ccdcf91104a5) was reverted (commit
> c0c6ca40a25e) in mesa as it broke GLX resulting in swapped colors. Based
> on further investigation by Chad Versace, moving the RGBX/RGBA configs
> to the end is enough to prevent breaking GLX.
>
> The handling of RGBA/RGBX in dri_fill_st_visual is a fix from Marek
> Olšák.
>
> Cc: Eric Anholt <e...@anholt.net>
> Cc: Chad Versace <chadvers...@chromium.org>
> Cc: Mauro Rossi <issor.or...@gmail.com>
> Reviewed-by: Marek Olšák <marek.ol...@amd.com>
> Signed-off-by: Rob Herring <r...@kernel.org>
> ---
> v2:
> - Incorporated dri_fill_st_visual RGBA/X handling from Marek
> - Handle RGBA/X in dri2_drawable_get_buffers for completeness
>
> src/gallium/state_trackers/dri/dri2.c | 8 ++++
> src/gallium/state_trackers/dri/dri_screen.c | 69
> ++++++++++++++++++++++++-----
> 2 files changed, 65 insertions(+), 12 deletions(-)
>
> diff --git a/src/gallium/state_trackers/dri/dri2.c
> b/src/gallium/state_trackers/dri/dri2.c
> index 60ec38d8e445..e20b2c075361 100644
> --- a/src/gallium/state_trackers/dri/dri2.c
> +++ b/src/gallium/state_trackers/dri/dri2.c
> @@ -186,6 +186,9 @@ static enum pipe_format dri2_format_to_pipe_format (int
> format)
> case __DRI_IMAGE_FORMAT_ARGB8888:
> pf = PIPE_FORMAT_BGRA8888_UNORM;
> break;
> + case __DRI_IMAGE_FORMAT_XBGR8888:
> + pf = PIPE_FORMAT_RGBX8888_UNORM;
> + break;
> case __DRI_IMAGE_FORMAT_ABGR8888:
> pf = PIPE_FORMAT_RGBA8888_UNORM;
> break;
> @@ -356,9 +359,11 @@ dri2_drawable_get_buffers(struct dri_drawable *drawable,
> */
> switch(format) {
> case PIPE_FORMAT_BGRA8888_UNORM:
> + case PIPE_FORMAT_RGBA8888_UNORM:
> depth = 32;
> break;
> case PIPE_FORMAT_BGRX8888_UNORM:
> + case PIPE_FORMAT_RGBX8888_UNORM:
> depth = 24;
> break;
> case PIPE_FORMAT_B5G6R5_UNORM:
> @@ -434,6 +439,9 @@ dri_image_drawable_get_buffers(struct dri_drawable
> *drawable,
> case PIPE_FORMAT_BGRA8888_UNORM:
> image_format = __DRI_IMAGE_FORMAT_ARGB8888;
> break;
> + case PIPE_FORMAT_RGBX8888_UNORM:
> + image_format = __DRI_IMAGE_FORMAT_XBGR8888;
> + break;
> case PIPE_FORMAT_RGBA8888_UNORM:
> image_format = __DRI_IMAGE_FORMAT_ABGR8888;
> break;
> diff --git a/src/gallium/state_trackers/dri/dri_screen.c
> b/src/gallium/state_trackers/dri/dri_screen.c
> index 6b58830e0b42..a0d9b34d667b 100644
> --- a/src/gallium/state_trackers/dri/dri_screen.c
> +++ b/src/gallium/state_trackers/dri/dri_screen.c
> @@ -132,6 +132,27 @@ dri_fill_in_modes(struct dri_screen *screen)
> MESA_FORMAT_B8G8R8A8_SRGB,
> MESA_FORMAT_B8G8R8X8_SRGB,
> MESA_FORMAT_B5G6R5_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,
> };
> static const enum pipe_format pipe_formats[] = {
> PIPE_FORMAT_BGRA8888_UNORM,
> @@ -139,6 +160,8 @@ dri_fill_in_modes(struct dri_screen *screen)
> PIPE_FORMAT_BGRA8888_SRGB,
> PIPE_FORMAT_BGRX8888_SRGB,
> PIPE_FORMAT_B5G6R5_UNORM,
> + PIPE_FORMAT_RGBA8888_UNORM,
> + PIPE_FORMAT_RGBX8888_UNORM,
> };
> mesa_format format;
> __DRIconfig **configs = NULL;
> @@ -275,19 +298,41 @@ dri_fill_st_visual(struct st_visual *stvis, struct
> dri_screen *screen,
> if (!mode)
> return;
>
> - if (mode->redBits == 8) {
> - if (mode->alphaBits == 8)
> - if (mode->sRGBCapable)
> - stvis->color_format = PIPE_FORMAT_BGRA8888_SRGB;
> - else
> - stvis->color_format = PIPE_FORMAT_BGRA8888_UNORM;
> - else
> - if (mode->sRGBCapable)
> - stvis->color_format = PIPE_FORMAT_BGRX8888_SRGB;
> - else
> - stvis->color_format = PIPE_FORMAT_BGRX8888_UNORM;
> - } else {
> + /* Deduce the color format. */
> + switch (mode->redMask) {
> + case 0x00FF0000:
> + if (mode->alphaMask) {
> + assert(mode->alphaMask == 0xFF000000);
> + stvis->color_format = mode->sRGBCapable ?
> + PIPE_FORMAT_BGRA8888_SRGB :
> + PIPE_FORMAT_BGRA8888_UNORM;
> + } else {
> + stvis->color_format = mode->sRGBCapable ?
> + PIPE_FORMAT_BGRX8888_SRGB :
> + PIPE_FORMAT_BGRX8888_UNORM;
> + }
> + break;
> +
> + case 0x000000FF:
> + if (mode->alphaMask) {
> + assert(mode->alphaMask == 0xFF000000);
> + stvis->color_format = mode->sRGBCapable ?
> + PIPE_FORMAT_RGBA8888_SRGB :
> + PIPE_FORMAT_RGBA8888_UNORM;
> + } else {
> + stvis->color_format = mode->sRGBCapable ?
> + PIPE_FORMAT_RGBX8888_SRGB :
> + PIPE_FORMAT_RGBX8888_UNORM;
> + }
> + break;
> +
> + case 0x0000F800:
> stvis->color_format = PIPE_FORMAT_B5G6R5_UNORM;
> + break;
> +
> + default:
> + assert(!"unsupported visual: invalid red mask");
> + return;
> }
>
> if (mode->sampleBuffers) {
> --
> 2.11.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
