Re: [Mesa-dev] [PATCH] Revert "glx: Fix synthetic error generation in __glXSendError"

2019-05-07 Thread Adam Jackson
On Tue, 2019-05-07 at 20:17 +1000, Timothy Arceri wrote:

> I don't know enough about this code to take responsibility for such 
> changes. I was just trying to revert to the status quo until this could 
> be investigated again.
> 
> My suggestion is we roll back the recent change. Then someone needs to 
> create piglit test for both scenarios before trying to move forward again.
> 
> If you want to try something different then go for it :)

Don't feel bad about not understanding this code. Honestly what libGL
is doing here - fabricating an X11 protocol error out of thin air - is
completely unique to libGL among the X client libraries, and it's not
surprising that attempting to do causes issues.

I'm fine with any amount of revert here. I can't promise I'll have the
time to investigate this in any detail promptly though.

- ajax

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] Revert "glx: Fix synthetic error generation in __glXSendError"

2019-05-07 Thread Timothy Arceri



On 7/5/19 6:27 pm, Michel Dänzer wrote:

On 2019-05-07 5:55 a.m., Timothy Arceri wrote:

This reverts commit e91ee763c378d03883eb88cf0eadd8aa916f7878.

This seems to have broken a number of wine games.

Cc: Adam Jackson 
Cc: Ian Romanick 
Cc: Hal Gentz 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110632
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110590
---
  src/glx/glx_error.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/glx/glx_error.c b/src/glx/glx_error.c
index 712ecf8213d..653cbeb2d2a 100644
--- a/src/glx/glx_error.c
+++ b/src/glx/glx_error.c
@@ -54,7 +54,7 @@ __glXSendError(Display * dpy, int_fast8_t errorCode, 
uint_fast32_t resourceID,
error.errorCode = glx_dpy->codes->first_error + errorCode;
 }
  
-   error.sequenceNumber = dpy->last_request_read;

+   error.sequenceNumber = dpy->request;
 error.resourceID = resourceID;
 error.minorCode = minorCode;
 error.majorCode = glx_dpy->majorOpcode;
@@ -73,7 +73,7 @@ __glXSendErrorForXcb(Display * dpy, const xcb_generic_error_t 
*err)
  
 error.type = X_Error;

 error.errorCode = err->error_code;
-   error.sequenceNumber = dpy->last_request_read;
+   error.sequenceNumber = err->sequence;
 error.resourceID = err->resource_id;
 error.minorCode = err->minor_code;
 error.majorCode = err->major_code;



As-is, this will re-introduce
https://bugs.freedesktop.org/show_bug.cgi?id=99781 . That one was about
__glXSendErrorForXcb, while the regressions are about __glXSendError, so
maybe only revert the __glXSendError hunk for now?




I don't know enough about this code to take responsibility for such 
changes. I was just trying to revert to the status quo until this could 
be investigated again.


My suggestion is we roll back the recent change. Then someone needs to 
create piglit test for both scenarios before trying to move forward again.


If you want to try something different then go for it :)
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] Revert "glx: Fix synthetic error generation in __glXSendError"

2019-05-07 Thread Emil Velikov
On Tue, 7 May 2019 at 09:27, Michel Dänzer  wrote:
>
> On 2019-05-07 5:55 a.m., Timothy Arceri wrote:
> > This reverts commit e91ee763c378d03883eb88cf0eadd8aa916f7878.
> >
> > This seems to have broken a number of wine games.
> >
> > Cc: Adam Jackson 
> > Cc: Ian Romanick 
> > Cc: Hal Gentz 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110632
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110590
> > ---
> >  src/glx/glx_error.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/glx/glx_error.c b/src/glx/glx_error.c
> > index 712ecf8213d..653cbeb2d2a 100644
> > --- a/src/glx/glx_error.c
> > +++ b/src/glx/glx_error.c
> > @@ -54,7 +54,7 @@ __glXSendError(Display * dpy, int_fast8_t errorCode, 
> > uint_fast32_t resourceID,
> >error.errorCode = glx_dpy->codes->first_error + errorCode;
> > }
> >
> > -   error.sequenceNumber = dpy->last_request_read;
> > +   error.sequenceNumber = dpy->request;
> > error.resourceID = resourceID;
> > error.minorCode = minorCode;
> > error.majorCode = glx_dpy->majorOpcode;
> > @@ -73,7 +73,7 @@ __glXSendErrorForXcb(Display * dpy, const 
> > xcb_generic_error_t *err)
> >
> > error.type = X_Error;
> > error.errorCode = err->error_code;
> > -   error.sequenceNumber = dpy->last_request_read;
> > +   error.sequenceNumber = err->sequence;
> > error.resourceID = err->resource_id;
> > error.minorCode = err->minor_code;
> > error.majorCode = err->major_code;
> >
>
> As-is, this will re-introduce
> https://bugs.freedesktop.org/show_bug.cgi?id=99781 . That one was about
> __glXSendErrorForXcb, while the regressions are about __glXSendError, so
> maybe only revert the __glXSendError hunk for now?
>
Could not agree more.

Can I suggest adding inline comment + even a bugzilla link? Otherwise
we're bound to copy/paste and break it again.

HTH
-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] Revert "glx: Fix synthetic error generation in __glXSendError"

2019-05-07 Thread Michel Dänzer
On 2019-05-07 5:55 a.m., Timothy Arceri wrote:
> This reverts commit e91ee763c378d03883eb88cf0eadd8aa916f7878.
> 
> This seems to have broken a number of wine games.
> 
> Cc: Adam Jackson 
> Cc: Ian Romanick 
> Cc: Hal Gentz 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110632
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110590
> ---
>  src/glx/glx_error.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/glx/glx_error.c b/src/glx/glx_error.c
> index 712ecf8213d..653cbeb2d2a 100644
> --- a/src/glx/glx_error.c
> +++ b/src/glx/glx_error.c
> @@ -54,7 +54,7 @@ __glXSendError(Display * dpy, int_fast8_t errorCode, 
> uint_fast32_t resourceID,
>error.errorCode = glx_dpy->codes->first_error + errorCode;
> }
>  
> -   error.sequenceNumber = dpy->last_request_read;
> +   error.sequenceNumber = dpy->request;
> error.resourceID = resourceID;
> error.minorCode = minorCode;
> error.majorCode = glx_dpy->majorOpcode;
> @@ -73,7 +73,7 @@ __glXSendErrorForXcb(Display * dpy, const 
> xcb_generic_error_t *err)
>  
> error.type = X_Error;
> error.errorCode = err->error_code;
> -   error.sequenceNumber = dpy->last_request_read;
> +   error.sequenceNumber = err->sequence;
> error.resourceID = err->resource_id;
> error.minorCode = err->minor_code;
> error.majorCode = err->major_code;
> 

As-is, this will re-introduce
https://bugs.freedesktop.org/show_bug.cgi?id=99781 . That one was about
__glXSendErrorForXcb, while the regressions are about __glXSendError, so
maybe only revert the __glXSendError hunk for now?


-- 
Earthling Michel Dänzer   |  https://www.amd.com
Libre software enthusiast | Mesa and X developer
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] Revert "glx: Fix synthetic error generation in __glXSendError"

2019-05-06 Thread Timothy Arceri
This reverts commit e91ee763c378d03883eb88cf0eadd8aa916f7878.

This seems to have broken a number of wine games.

Cc: Adam Jackson 
Cc: Ian Romanick 
Cc: Hal Gentz 
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110632
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110590
---
 src/glx/glx_error.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/glx/glx_error.c b/src/glx/glx_error.c
index 712ecf8213d..653cbeb2d2a 100644
--- a/src/glx/glx_error.c
+++ b/src/glx/glx_error.c
@@ -54,7 +54,7 @@ __glXSendError(Display * dpy, int_fast8_t errorCode, 
uint_fast32_t resourceID,
   error.errorCode = glx_dpy->codes->first_error + errorCode;
}
 
-   error.sequenceNumber = dpy->last_request_read;
+   error.sequenceNumber = dpy->request;
error.resourceID = resourceID;
error.minorCode = minorCode;
error.majorCode = glx_dpy->majorOpcode;
@@ -73,7 +73,7 @@ __glXSendErrorForXcb(Display * dpy, const xcb_generic_error_t 
*err)
 
error.type = X_Error;
error.errorCode = err->error_code;
-   error.sequenceNumber = dpy->last_request_read;
+   error.sequenceNumber = err->sequence;
error.resourceID = err->resource_id;
error.minorCode = err->minor_code;
error.majorCode = err->major_code;
-- 
2.20.1

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev