Re: [Mesa-dev] [PATCH 1/7] egl: Simplify queries for EGL_RENDER_BUFFER
Reviewed-by: Tapani Pälli
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(>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
[Mesa-dev] [PATCH 1/7] egl: Simplify queries for EGL_RENDER_BUFFER
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(>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
[Mesa-dev] [PATCH 1/7] egl: Simplify queries for EGL_RENDER_BUFFER
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(>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
