[PATCH weston v2] gl-renderer: If an XRGB format is requested but unavailable, try ARGB

[Dima Ryazanov]

I don't know if the "alloca" change is a good idea. Some people consider it bad
practice
<http://stackoverflow.com/questions/1018853/why-is-alloca-not-considered-good-practice>,
and looks like it's not used anywhere in wayland/weston code - but someone
else should make that call.

Otherwise, though, looks good.

On Tue, May 5, 2015 at 2:22 PM, Derek Foreman <derekf at osg.samsung.com>
wrote:

> Recent versions of Mesa have stopped exposing XRGB visuals for gl on
> some Intel GPUs.  While this may be changed in Mesa eventually, it's
> not impossible that some other hardware in the future won't provide
> XRGB visuals either.
>
> Let's try again with an ARGB visual if XRGB is unavailable.  Since
> we're not changing the scanout buffer format, and our current
> rendering loop always results in saturated alpha in the frame buffer,
> it should be Just Fine(tm) - and probably better than just exiting.
>
> Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
> ---
>
> Dima Ryazanov called me out for fallback_format() not working on all
> endians,
> and I've reworked the loop to be a single loop instead of two, similar to
> Daniel's suggestion (but still keeping the NULL visual_id means "give me
> the
> first match" semantic of the original)
>
> I've also replaced calloc with aloca so I don't have to worry about freeing
> the array on return, which simplifies things a little bit.  I didn't bother
> zeroing out the array, since it's filled in by eglChooseConfig.
>
> It's still an ugly function. :)
>
>  src/gl-renderer.c | 57
> +++++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 43 insertions(+), 14 deletions(-)
>
> diff --git a/src/gl-renderer.c b/src/gl-renderer.c
> index ae3122f..a4fd3a0 100644
> --- a/src/gl-renderer.c
> +++ b/src/gl-renderer.c
> @@ -934,6 +934,14 @@ output_rotate_damage(struct weston_output *output,
>         go->border_damage[go->buffer_damage_index] = border_status;
>  }
>
> +/* NOTE: We now allow falling back to ARGB  gl visual ids when XRGB is
> + * unavailable, so we're assuming the background has no transparency
> + * and that everything with a blend, like drop shadows, will have
> something
> + * opaque (like the background) drawn underneath it.
> + *
> + * Depending on the underlying hardware, violating that assumption could
> + * result in seeing through to another display plane.
> + */
>  static void
>  gl_renderer_repaint_output(struct weston_output *output,
>                               pixman_region32_t *output_damage)
> @@ -1897,6 +1905,15 @@ log_egl_config_info(EGLDisplay egldpy, EGLConfig
> eglconfig)
>                 weston_log_continue(" unknown\n");
>  }
>
> +static EGLint
> +fallback_format(EGLint format)
> +{
> +       if ((format & 0xFF) != 'X')
> +               return 0;
> +
> +       return (format & 0xFFFFFF00) | 'A';
> +}
> +
>  static int
>  egl_choose_config(struct gl_renderer *gr, const EGLint *attribs,
>                   const EGLint *visual_id,
> @@ -1904,41 +1921,53 @@ egl_choose_config(struct gl_renderer *gr, const
> EGLint *attribs,
>  {
>         EGLint count = 0;
>         EGLint matched = 0;
> +       EGLint fallback_id;
>         EGLConfig *configs;
> +       EGLConfig fallback_config = 0;
>         int i;
>
>         if (!eglGetConfigs(gr->egl_display, NULL, 0, &count) || count < 1)
>                 return -1;
>
> -       configs = calloc(count, sizeof *configs);
> +       configs = alloca(count * sizeof *configs);
>         if (!configs)
>                 return -1;
>
>         if (!eglChooseConfig(gr->egl_display, attribs, configs,
>                               count, &matched))
> -               goto out;
> +               return -1;
> +
> +       if (!matched)
> +               return -1;
>
> +       if (!visual_id) {
> +               *config_out = configs[0];
> +               return 0;
> +       }
> +
> +       fallback_id = fallback_format(*visual_id);
>         for (i = 0; i < matched; ++i) {
>                 EGLint id;
>
> -               if (visual_id) {
> -                       if (!eglGetConfigAttrib(gr->egl_display,
> -                                       configs[i], EGL_NATIVE_VISUAL_ID,
> -                                       &id))
> -                               continue;
> +               if (!eglGetConfigAttrib(gr->egl_display,
> +                               configs[i], EGL_NATIVE_VISUAL_ID,
> +                               &id))
> +                       continue;
>
> -                       if (id != 0 && id != *visual_id)
> -                               continue;
> +               if (id == *visual_id) {
> +                       *config_out = configs[i];
> +                       return 0;
>                 }
> +               if (id == fallback_id)
> +                       fallback_config = configs[i];
> +       }
>
> -               *config_out = configs[i];
> -
> -               free(configs);
> +       if (fallback_config) {
> +               weston_log("Falling back to ARGB visual\n");
> +               *config_out = fallback_config;
>                 return 0;
>         }
>
> -out:
> -       free(configs);
>         return -1;
>  }
>
> --
> 2.1.4
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20150506/31fbae18/attachment.html>