Re: [Mesa-dev] [PATCH] swrast: Fix eglMakeCurrent(dpy, NULL, NULL, ctx) (v2)
Adam Jackson writes: > Fixes 14 piglits, mostly in egl_khr_create_context. > > v2: Also short-circuit the same-context-no-drawables case (Eric Anholt) > > Fixes: https://github.com/anholt/libepoxy/issues/177 > Signed-off-by: Adam Jackson r-b signature.asc Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] swrast: Fix eglMakeCurrent(dpy, NULL, NULL, ctx) (v2)
Fixes 14 piglits, mostly in egl_khr_create_context.
v2: Also short-circuit the same-context-no-drawables case (Eric Anholt)
Fixes: https://github.com/anholt/libepoxy/issues/177
Signed-off-by: Adam Jackson
---
src/mesa/drivers/dri/swrast/swrast.c | 37 ++--
1 file changed, 18 insertions(+), 19 deletions(-)
diff --git a/src/mesa/drivers/dri/swrast/swrast.c
b/src/mesa/drivers/dri/swrast/swrast.c
index ae5874f5927..a88ece97f3b 100644
--- a/src/mesa/drivers/dri/swrast/swrast.c
+++ b/src/mesa/drivers/dri/swrast/swrast.c
@@ -675,6 +675,9 @@ swrast_check_and_update_window_size( struct gl_context
*ctx, struct gl_framebuff
{
GLsizei width, height;
+if (!fb)
+return;
+
get_window_size(fb, , );
if (fb->Width != width || fb->Height != height) {
_mesa_resize_framebuffer(ctx, fb, width, height);
@@ -857,30 +860,26 @@ dri_make_current(__DRIcontext * cPriv,
__DRIdrawable * driReadPriv)
{
struct gl_context *mesaCtx;
-struct gl_framebuffer *mesaDraw;
-struct gl_framebuffer *mesaRead;
+struct gl_framebuffer *mesaDraw = NULL;
+struct gl_framebuffer *mesaRead = NULL;
TRACE;
if (cPriv) {
- struct dri_context *ctx = dri_context(cPriv);
- struct dri_drawable *draw;
- struct dri_drawable *read;
+mesaCtx = _context(cPriv)->Base;
- if (!driDrawPriv || !driReadPriv)
- return GL_FALSE;
+ if (driDrawPriv && driReadPriv) {
+ struct dri_drawable *draw = dri_drawable(driDrawPriv);
+ struct dri_drawable *read = dri_drawable(driReadPriv);
+ mesaDraw = >Base;
+ mesaRead = >Base;
+}
- draw = dri_drawable(driDrawPriv);
- read = dri_drawable(driReadPriv);
- mesaCtx = >Base;
- mesaDraw = >Base;
- mesaRead = >Base;
-
- /* check for same context and buffer */
- if (mesaCtx == _mesa_get_current_context()
- && mesaCtx->DrawBuffer == mesaDraw
- && mesaCtx->ReadBuffer == mesaRead) {
- return GL_TRUE;
- }
+/* check for same context and buffer */
+if (mesaCtx == _mesa_get_current_context()
+&& mesaCtx->DrawBuffer == mesaDraw
+&& mesaCtx->ReadBuffer == mesaRead) {
+return GL_TRUE;
+}
_glapi_check_multithread();
--
2.17.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] swrast: Fix eglMakeCurrent(dpy, NULL, NULL, ctx)
On 07/06/2018 08:45 AM, Adam Jackson wrote: > On Fri, 2018-07-06 at 10:29 +0100, Emil Velikov wrote: > >> Skimming through the egl_khr_create_context text, this relaxes some >> parts which are meant to be an error. >> For example >> >> * If either of or is a valid surface and the other >> is EGL_NO_SURFACE, an EGL_BAD_MATCH error is generated. > > That seems like an invariant EGL promises for the driver, not something > the driver validates. eglMakeCurrent already says: > > if ((!draw_surf && draw != EGL_NO_SURFACE) || > (!read_surf && read != EGL_NO_SURFACE)) > RETURN_EGL_ERROR(disp, EGL_BAD_SURFACE, EGL_FALSE); > > glXMakeCurrent doesn't seem to have code to generate an error in that > case, anymore. The server would throw an error for the GLXMakeCurrent > request, but we don't always emit those for... bad reasons. Regardless > I'd say that check belongs in MakeContextCurrent, not the driver. That is how we have typically divided up work. The libEGL / libGLX layer handles the API validation, and the driver tries not to crash. :) > - ajax > ___ > 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
Re: [Mesa-dev] [PATCH] swrast: Fix eglMakeCurrent(dpy, NULL, NULL, ctx)
On Fri, 2018-07-06 at 10:29 +0100, Emil Velikov wrote: > Skimming through the egl_khr_create_context text, this relaxes some > parts which are meant to be an error. > For example > > * If either of or is a valid surface and the other > is EGL_NO_SURFACE, an EGL_BAD_MATCH error is generated. That seems like an invariant EGL promises for the driver, not something the driver validates. eglMakeCurrent already says: if ((!draw_surf && draw != EGL_NO_SURFACE) || (!read_surf && read != EGL_NO_SURFACE)) RETURN_EGL_ERROR(disp, EGL_BAD_SURFACE, EGL_FALSE); glXMakeCurrent doesn't seem to have code to generate an error in that case, anymore. The server would throw an error for the GLXMakeCurrent request, but we don't always emit those for... bad reasons. Regardless I'd say that check belongs in MakeContextCurrent, not the driver. - ajax ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] swrast: Fix eglMakeCurrent(dpy, NULL, NULL, ctx)
On 5 July 2018 at 20:35, Adam Jackson wrote:
> Fixes 14 piglits, mostly in egl_khr_create_context.
>
> Fixes: https://github.com/anholt/libepoxy/issues/177
> Signed-off-by: Adam Jackson
> ---
> src/mesa/drivers/dri/swrast/swrast.c | 34 +++-
> 1 file changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/swrast/swrast.c
> b/src/mesa/drivers/dri/swrast/swrast.c
> index ae5874f5927..7f08107c24f 100644
> --- a/src/mesa/drivers/dri/swrast/swrast.c
> +++ b/src/mesa/drivers/dri/swrast/swrast.c
> @@ -675,6 +675,9 @@ swrast_check_and_update_window_size( struct gl_context
> *ctx, struct gl_framebuff
> {
> GLsizei width, height;
>
> +if (!fb)
> +return;
> +
> get_window_size(fb, , );
> if (fb->Width != width || fb->Height != height) {
> _mesa_resize_framebuffer(ctx, fb, width, height);
> @@ -857,30 +860,29 @@ dri_make_current(__DRIcontext * cPriv,
> __DRIdrawable * driReadPriv)
> {
> struct gl_context *mesaCtx;
> -struct gl_framebuffer *mesaDraw;
> -struct gl_framebuffer *mesaRead;
> +struct gl_framebuffer *mesaDraw = NULL;
> +struct gl_framebuffer *mesaRead = NULL;
> TRACE;
>
> if (cPriv) {
> - struct dri_context *ctx = dri_context(cPriv);
> struct dri_drawable *draw;
> struct dri_drawable *read;
>
> - if (!driDrawPriv || !driReadPriv)
> - return GL_FALSE;
> +mesaCtx = _context(cPriv)->Base;
>
> - draw = dri_drawable(driDrawPriv);
> - read = dri_drawable(driReadPriv);
> - mesaCtx = >Base;
> - mesaDraw = >Base;
> - mesaRead = >Base;
> + if (driDrawPriv && driReadPriv) {
> + draw = dri_drawable(driDrawPriv);
> + read = dri_drawable(driReadPriv);
> + mesaDraw = >Base;
> + mesaRead = >Base;
>
> - /* check for same context and buffer */
> - if (mesaCtx == _mesa_get_current_context()
> - && mesaCtx->DrawBuffer == mesaDraw
> - && mesaCtx->ReadBuffer == mesaRead) {
> - return GL_TRUE;
> - }
> + /* check for same context and buffer */
> + if (mesaCtx == _mesa_get_current_context()
> + && mesaCtx->DrawBuffer == mesaDraw
> + && mesaCtx->ReadBuffer == mesaRead) {
> + return GL_TRUE;
> + }
> +}
>
Skimming through the egl_khr_create_context text, this relaxes some
parts which are meant to be an error.
For example
* If either of or is a valid surface and the other
is EGL_NO_SURFACE, an EGL_BAD_MATCH error is generated.
Something like the below should handle the case you're thinking of, right?
HTH
Emil
diff --git a/src/mesa/drivers/dri/swrast/swrast.c
b/src/mesa/drivers/dri/swrast/swrast.c
index ae5874f5927..3fa2dd83f27 100644
--- a/src/mesa/drivers/dri/swrast/swrast.c
+++ b/src/mesa/drivers/dri/swrast/swrast.c
@@ -866,6 +866,12 @@ dri_make_current(__DRIcontext * cPriv,
struct dri_drawable *draw;
struct dri_drawable *read;
+/* GL 3.0 allows *MakeCurrent(dpy, NULL, NULL, ctx) */
+if (!driDrawPriv && !driReadPriv) {
+_mesa_make_current(ctx, NULL, NULL);
+return GL_TRUE;
+}
+
if (!driDrawPriv || !driReadPriv)
return GL_FALSE;
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] swrast: Fix eglMakeCurrent(dpy, NULL, NULL, ctx)
Adam Jackson writes:
> Fixes 14 piglits, mostly in egl_khr_create_context.
>
> Fixes: https://github.com/anholt/libepoxy/issues/177
> Signed-off-by: Adam Jackson
> ---
> src/mesa/drivers/dri/swrast/swrast.c | 34 +++-
> 1 file changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/swrast/swrast.c
> b/src/mesa/drivers/dri/swrast/swrast.c
> index ae5874f5927..7f08107c24f 100644
> --- a/src/mesa/drivers/dri/swrast/swrast.c
> +++ b/src/mesa/drivers/dri/swrast/swrast.c
> @@ -675,6 +675,9 @@ swrast_check_and_update_window_size( struct gl_context
> *ctx, struct gl_framebuff
> {
> GLsizei width, height;
>
> +if (!fb)
> +return;
> +
> get_window_size(fb, , );
> if (fb->Width != width || fb->Height != height) {
> _mesa_resize_framebuffer(ctx, fb, width, height);
> @@ -857,30 +860,29 @@ dri_make_current(__DRIcontext * cPriv,
>__DRIdrawable * driReadPriv)
> {
> struct gl_context *mesaCtx;
> -struct gl_framebuffer *mesaDraw;
> -struct gl_framebuffer *mesaRead;
> +struct gl_framebuffer *mesaDraw = NULL;
> +struct gl_framebuffer *mesaRead = NULL;
> TRACE;
>
> if (cPriv) {
> - struct dri_context *ctx = dri_context(cPriv);
> struct dri_drawable *draw;
> struct dri_drawable *read;
>
> - if (!driDrawPriv || !driReadPriv)
> - return GL_FALSE;
> +mesaCtx = _context(cPriv)->Base;
>
> - draw = dri_drawable(driDrawPriv);
> - read = dri_drawable(driReadPriv);
> - mesaCtx = >Base;
> - mesaDraw = >Base;
> - mesaRead = >Base;
> + if (driDrawPriv && driReadPriv) {
> + draw = dri_drawable(driDrawPriv);
> + read = dri_drawable(driReadPriv);
> + mesaDraw = >Base;
> + mesaRead = >Base;
>
> - /* check for same context and buffer */
> - if (mesaCtx == _mesa_get_current_context()
> - && mesaCtx->DrawBuffer == mesaDraw
> - && mesaCtx->ReadBuffer == mesaRead) {
> - return GL_TRUE;
> - }
> + /* check for same context and buffer */
> + if (mesaCtx == _mesa_get_current_context()
> + && mesaCtx->DrawBuffer == mesaDraw
> + && mesaCtx->ReadBuffer == mesaRead) {
> + return GL_TRUE;
> + }
> +}
Didn't you mean for this block to be outside of the driDrawPriv &&
driReadPriv condition? That way you can get the short-circuit in the
no-drawable case, too.
Other than that,
Reviewed-by: Eric Anholt
signature.asc
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] swrast: Fix eglMakeCurrent(dpy, NULL, NULL, ctx)
Fixes 14 piglits, mostly in egl_khr_create_context.
Fixes: https://github.com/anholt/libepoxy/issues/177
Signed-off-by: Adam Jackson
---
src/mesa/drivers/dri/swrast/swrast.c | 34 +++-
1 file changed, 18 insertions(+), 16 deletions(-)
diff --git a/src/mesa/drivers/dri/swrast/swrast.c
b/src/mesa/drivers/dri/swrast/swrast.c
index ae5874f5927..7f08107c24f 100644
--- a/src/mesa/drivers/dri/swrast/swrast.c
+++ b/src/mesa/drivers/dri/swrast/swrast.c
@@ -675,6 +675,9 @@ swrast_check_and_update_window_size( struct gl_context
*ctx, struct gl_framebuff
{
GLsizei width, height;
+if (!fb)
+return;
+
get_window_size(fb, , );
if (fb->Width != width || fb->Height != height) {
_mesa_resize_framebuffer(ctx, fb, width, height);
@@ -857,30 +860,29 @@ dri_make_current(__DRIcontext * cPriv,
__DRIdrawable * driReadPriv)
{
struct gl_context *mesaCtx;
-struct gl_framebuffer *mesaDraw;
-struct gl_framebuffer *mesaRead;
+struct gl_framebuffer *mesaDraw = NULL;
+struct gl_framebuffer *mesaRead = NULL;
TRACE;
if (cPriv) {
- struct dri_context *ctx = dri_context(cPriv);
struct dri_drawable *draw;
struct dri_drawable *read;
- if (!driDrawPriv || !driReadPriv)
- return GL_FALSE;
+mesaCtx = _context(cPriv)->Base;
- draw = dri_drawable(driDrawPriv);
- read = dri_drawable(driReadPriv);
- mesaCtx = >Base;
- mesaDraw = >Base;
- mesaRead = >Base;
+ if (driDrawPriv && driReadPriv) {
+ draw = dri_drawable(driDrawPriv);
+ read = dri_drawable(driReadPriv);
+ mesaDraw = >Base;
+ mesaRead = >Base;
- /* check for same context and buffer */
- if (mesaCtx == _mesa_get_current_context()
- && mesaCtx->DrawBuffer == mesaDraw
- && mesaCtx->ReadBuffer == mesaRead) {
- return GL_TRUE;
- }
+ /* check for same context and buffer */
+ if (mesaCtx == _mesa_get_current_context()
+ && mesaCtx->DrawBuffer == mesaDraw
+ && mesaCtx->ReadBuffer == mesaRead) {
+ return GL_TRUE;
+ }
+}
_glapi_check_multithread();
--
2.17.0
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
