[Mesa-dev] [PATCH 1/7] egl: Simplify queries for EGL_RENDER_BUFFER
Tapani Pälli
tapani.palli at intel.com
Fri Aug 3 11:08:09 UTC 2018
Reviewed-by: Tapani Pälli <tapani.palli at intel.com>
On 07/31/2018 09:18 PM, Chad Versace wrote:
> There exist *two* queryable EGL_RENDER_BUFFER states in EGL:
> eglQuerySurface(EGL_RENDER_BUFFER) and
> eglQueryContext(EGL_RENDER_BUFFER).
>
> These changes eliminate potentially very fragile code in the upcoming
> EGL_KHR_mutable_render_buffer implementation.
>
> * eglQuerySurface(EGL_RENDER_BUFFER)
>
> The implementation of eglQuerySurface(EGL_RENDER_BUFFER) contained
> abstruse logic which required comprehending the specification
> complexities of how the two EGL_RENDER_BUFFER states interact. The
> function sometimes returned _EGLContext::WindowRenderBuffer, sometimes
> _EGLSurface::RenderBuffer. Why? The function tried to encode the
> actual logic from the EGL spec. When did the function return which
> variable? Go study the EGL spec, hope you understand it, then hope
> Mesa mutated the EGL_RENDER_BUFFER state in all the correct places.
> Have fun.
>
> To simplify eglQuerySurface(EGL_RENDER_BUFFER), and to improve
> confidence in its correctness, flatten its indirect logic. For pixmap
> and pbuffer surfaces, simply return a hard-coded literal value, as the
> spec suggests. For window surfaces, simply return
> _EGLSurface::RequestedRenderBuffer. Nothing difficult here.
>
> * eglQueryContext(EGL_RENDER_BUFFER)
>
> The implementation of this suffered from the same issues as
> eglQuerySurface, and the solution is the same. confidence in its
> correctness, flatten its indirect logic. For pixmap and pbuffer
> surfaces, simply return a hard-coded literal value, as the spec
> suggests. For window surfaces, simply return
> _EGLSurface::ActiveRenderBuffer.
> ---
> src/egl/drivers/dri2/egl_dri2.c | 6 -----
> src/egl/main/eglcontext.c | 40 +++++++++++++++++++++++++++------
> src/egl/main/eglcontext.h | 3 ---
> src/egl/main/eglsurface.c | 28 ++++++++++++++++++++---
> src/egl/main/eglsurface.h | 28 ++++++++++++++++++++++-
> 5 files changed, 85 insertions(+), 20 deletions(-)
>
> diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
> index c3024795a10..e109ad37f55 100644
> --- a/src/egl/drivers/dri2/egl_dri2.c
> +++ b/src/egl/drivers/dri2/egl_dri2.c
> @@ -1289,12 +1289,6 @@ dri2_create_context(_EGLDriver *drv, _EGLDisplay *disp, _EGLConfig *conf,
> dri_config = dri2_config->dri_config[1][0];
> else
> dri_config = dri2_config->dri_config[0][0];
> -
> - /* EGL_WINDOW_BIT is set only when there is a double-buffered dri_config.
> - * This makes sure the back buffer will always be used.
> - */
> - if (conf->SurfaceType & EGL_WINDOW_BIT)
> - dri2_ctx->base.WindowRenderBuffer = EGL_BACK_BUFFER;
> }
> else
> dri_config = NULL;
> diff --git a/src/egl/main/eglcontext.c b/src/egl/main/eglcontext.c
> index 3e5d8ad97bf..ecc546e1132 100644
> --- a/src/egl/main/eglcontext.c
> +++ b/src/egl/main/eglcontext.c
> @@ -588,7 +588,6 @@ _eglInitContext(_EGLContext *ctx, _EGLDisplay *dpy, _EGLConfig *conf,
> _eglInitResource(&ctx->Resource, sizeof(*ctx), dpy);
> ctx->ClientAPI = api;
> ctx->Config = conf;
> - ctx->WindowRenderBuffer = EGL_NONE;
> ctx->Profile = EGL_CONTEXT_OPENGL_CORE_PROFILE_BIT_KHR;
>
> ctx->ClientMajorVersion = 1; /* the default, per EGL spec */
> @@ -620,15 +619,42 @@ static EGLint
> _eglQueryContextRenderBuffer(_EGLContext *ctx)
> {
> _EGLSurface *surf = ctx->DrawSurface;
> - EGLint rb;
>
> + /* From the EGL 1.5 spec:
> + *
> + * - If the context is not bound to a surface, then EGL_NONE will be
> + * returned.
> + */
> if (!surf)
> return EGL_NONE;
> - if (surf->Type == EGL_WINDOW_BIT && ctx->WindowRenderBuffer != EGL_NONE)
> - rb = ctx->WindowRenderBuffer;
> - else
> - rb = surf->RenderBuffer;
> - return rb;
> +
> + switch (surf->Type) {
> + default:
> + unreachable("bad EGLSurface type");
> + case EGL_PIXMAP_BIT:
> + /* - If the context is bound to a pixmap surface, then EGL_SINGLE_BUFFER
> + * will be returned.
> + */
> + return EGL_SINGLE_BUFFER;
> + case EGL_PBUFFER_BIT:
> + /* - If the context is bound to a pbuffer surface, then EGL_BACK_BUFFER
> + * will be returned.
> + */
> + return EGL_BACK_BUFFER;
> + case EGL_WINDOW_BIT:
> + /* - If the context is bound to a window surface, then either
> + * EGL_BACK_BUFFER or EGL_SINGLE_BUFFER may be returned. The value
> + * returned depends on both the buffer requested by the setting of the
> + * EGL_RENDER_BUFFER property of the surface [...], and on the client
> + * API (not all client APIs support single-buffer Rendering to window
> + * surfaces). Some client APIs allow control of whether rendering goes
> + * to the front or back buffer. This client API-specific choice is not
> + * reflected in the returned value, which only describes the buffer
> + * that will be rendered to by default if not overridden by the client
> + * API.
> + */
> + return surf->ActiveRenderBuffer;
> + }
> }
>
>
> diff --git a/src/egl/main/eglcontext.h b/src/egl/main/eglcontext.h
> index 8d97ef9eab9..deddefdcaba 100644
> --- a/src/egl/main/eglcontext.h
> +++ b/src/egl/main/eglcontext.h
> @@ -65,9 +65,6 @@ struct _egl_context
> EGLint ContextPriority;
> EGLBoolean NoError;
> EGLint ReleaseBehavior;
> -
> - /* The real render buffer when a window surface is bound */
> - EGLint WindowRenderBuffer;
> };
>
>
> diff --git a/src/egl/main/eglsurface.c b/src/egl/main/eglsurface.c
> index 3bd14a8cd03..73c31e11588 100644
> --- a/src/egl/main/eglsurface.c
> +++ b/src/egl/main/eglsurface.c
> @@ -122,7 +122,7 @@ _eglParseSurfaceAttribList(_EGLSurface *surf, const EGLint *attrib_list)
> err = EGL_BAD_ATTRIBUTE;
> break;
> }
> - surf->RenderBuffer = val;
> + surf->RequestedRenderBuffer = val;
> break;
> case EGL_POST_SUB_BUFFER_SUPPORTED_NV:
> if (!dpy->Extensions.NV_post_sub_buffer ||
> @@ -285,7 +285,8 @@ _eglInitSurface(_EGLSurface *surf, _EGLDisplay *dpy, EGLint type,
> surf->TextureTarget = EGL_NO_TEXTURE;
> surf->MipmapTexture = EGL_FALSE;
> surf->LargestPbuffer = EGL_FALSE;
> - surf->RenderBuffer = renderBuffer;
> + surf->RequestedRenderBuffer = renderBuffer;
> + surf->ActiveRenderBuffer = renderBuffer;
> surf->VGAlphaFormat = EGL_VG_ALPHA_FORMAT_NONPRE;
> surf->VGColorspace = EGL_VG_COLORSPACE_sRGB;
> surf->GLColorspace = EGL_GL_COLORSPACE_LINEAR_KHR;
> @@ -358,7 +359,28 @@ _eglQuerySurface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surface,
> *value = surface->SwapBehavior;
> break;
> case EGL_RENDER_BUFFER:
> - *value = surface->RenderBuffer;
> + /* From the EGL 1.5 spec (2014.08.27):
> + *
> + * Querying EGL_RENDER_BUFFER returns the buffer which client API
> + * rendering is requested to use. For a window surface, this is the
> + * same attribute value specified when the surface was created. For
> + * a pbuffer surface, it is always EGL_BACK_BUFFER . For a pixmap
> + * surface, it is always EGL_SINGLE_BUFFER . To determine the actual
> + * buffer being rendered to by a context, call eglQueryContext.
> + */
> + switch (surface->Type) {
> + default:
> + unreachable("bad EGLSurface type");
> + case EGL_WINDOW_BIT:
> + *value = surface->RequestedRenderBuffer;
> + break;
> + case EGL_PBUFFER_BIT:
> + *value = EGL_BACK_BUFFER;
> + break;
> + case EGL_PIXMAP_BIT:
> + *value = EGL_SINGLE_BUFFER;
> + break;
> + }
> break;
> case EGL_PIXEL_ASPECT_RATIO:
> *value = surface->AspectRatio;
> diff --git a/src/egl/main/eglsurface.h b/src/egl/main/eglsurface.h
> index c53e8d00671..06e3b5b1b81 100644
> --- a/src/egl/main/eglsurface.h
> +++ b/src/egl/main/eglsurface.h
> @@ -67,7 +67,33 @@ struct _egl_surface
> EGLenum TextureTarget;
> EGLBoolean MipmapTexture;
> EGLBoolean LargestPbuffer;
> - EGLenum RenderBuffer;
> +
> + /**
> + * Value of EGL_RENDER_BUFFER selected at creation.
> + *
> + * The user may select, for window surfaces, the EGL_RENDER_BUFFER through
> + * the attribute list of eglCreateWindowSurface(). The EGL spec allows the
> + * implementation to ignore request, though; hence why we maintain both
> + * RequestedRenderBuffer and ActiveRenderBuffer. For pbuffer and pixmap
> + * surfaces, the EGL spec hard-codes the EGL_RENDER_BUFFER value and the
> + * user must not provide it in the attribute list.
> + *
> + * Refer to eglQuerySurface() in the EGL spec.
> + * eglQueryContext(EGL_RENDER_BUFFER) ignores this.
> + */
> + EGLenum RequestedRenderBuffer;
> +
> + /**
> + * The EGL_RENDER_BUFFER in use by the context.
> + *
> + * This is valid only when bound as the draw surface. This may differ from
> + * the RequestedRenderBuffer.
> + *
> + * Refer to eglQueryContext(EGL_RENDER_BUFFER) in the EGL spec.
> + * eglQuerySurface(EGL_RENDER_BUFFER) ignores this.
> + */
> + EGLenum ActiveRenderBuffer;
> +
> EGLenum VGAlphaFormat;
> EGLenum VGColorspace;
> EGLenum GLColorspace;
>
More information about the mesa-dev
mailing list