Two nits. On 02/25/2013 10:51 AM, Ian Romanick wrote: > From: Ian Romanick <ian.d.roman...@intel.com> > > Previously only the 32-bit X visual would match the 32-bit RGBA8888 > configs. This resulted in every config with alpha getting the "magic" > visual whose alpha is used by the compositor. This also resulted in no > multisample visuals being advertised. How many ways could we lose? > > This patch inverts the problem... now you can't get the visual with > alpha used by the compositor even if you want it. I think we need to > invent a new value for EGL_TRANSPARENT_TYPE that apps can use to get > this. I'm surprised that there isn't already a choice for > EGL_TRANSPARENT_ALPHA. > > Signed-off-by: Ian Romanick <ian.d.roman...@intel.com> > Tested-by: Tian Ye <yex.t...@intel.com> > Cc: Kristian Høgsberg <k...@bitplanet.net> > Cc: Chad Versace <chad.vers...@linux.intel.com> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=59783 > --- > src/egl/drivers/dri2/egl_dri2.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c > index ae842d7..5d00e4f 100644 > --- a/src/egl/drivers/dri2/egl_dri2.c > +++ b/src/egl/drivers/dri2/egl_dri2.c > @@ -195,7 +195,14 @@ dri2_add_config(_EGLDisplay *disp, const __DRIconfig > *dri_config, int id, > for (i = 0; attr_list[i] != EGL_NONE; i += 2) > _eglSetConfigKey(&base, attr_list[i], attr_list[i+1]); > > - if (depth > 0 && depth != base.BufferSize) > + /* Allow a 24-bit RGB visual to match a 32-bit RGBA fbconfig. Otherwise > it
fbconfig occurs nowhere in this code, and encountering it here confused me. The code is dealing with an EGLConfig, so let's call it what it is. > + * will only match a 32-bit RGBA visual. On a composited window manager > on > + * X11, this will make all of the fbconfigs with destination alpha get > + * blended by the compositor. This is probably not what the application > + * wants... especially on drivers that only have 32-bit RGBA fbconfigs! > + */ > + if (depth > 0 && > + !(depth == base.BufferSize || (depth == 24 && base.BufferSize == 32))) > return NULL; DeMorgan could simplify this to: depth > 0 && depth != base.BufferSize && !(depth == 24 && base.BufferSize == 32) > > if (rgba_masks && memcmp(rgba_masks, dri_masks, sizeof(dri_masks))) > The code's correct, so feel free to ignore my nits. And THANK YOU for fixing this bug. It's been bothering me for months. Reviewed-by: Chad Versace <chad.vers...@linux.intel.com> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
