Re: [PATCH xserver 06/13] glx: Enable GLX_SGI_make_current_read in the core

2016-03-30 Thread Emil Velikov
On 30 March 2016 at 14:58, Adam Jackson  wrote:
> On Wed, 2016-03-30 at 12:12 +0100, Emil Velikov wrote:
>> On 23 March 2016 at 22:46, Adam Jackson  wrote:
>> > diff --git a/glx/glxdri2.c b/glx/glxdri2.c
>> > index c56a376..71dab2a 100644
>> > --- a/glx/glxdri2.c
>> > +++ b/glx/glxdri2.c
>> > @@ -900,13 +900,6 @@ initializeExtensions(__GLXDRIscreen * screen)
>> >  }
>> >
>> >  for (i = 0; extensions[i]; i++) {
>> > -if (strcmp(extensions[i]->name, __DRI_READ_DRAWABLE) == 0) {
>> > -__glXEnableExtension(screen->glx_enable_bits,
>> > - "GLX_SGI_make_current_read");
>> > -
>> > -LogMessage(X_INFO, "AIGLX: enabled 
>> > GLX_SGI_make_current_read\n");
>> > -}
>> > -
>> Afaics we never had a DRI2 based dri module that provided this
>> extension. Which brings the question if this has ever been tested and
>> confirmed working. Can we have a small note about this in the commit
>> message ?
>
> Not quite. I am dismayed to report that piglit does not appear to
> contain any tests for separate drawable and readable, so whether this
> has been tested, who knows. And you are correct (and I'm somewhat
> surprised to discover) that Mesa currently does not expose this
> extension. But it used to:
>
> https://cgit.freedesktop.org/mesa/mesa/commit/?id=ad3221587164c10ae16d85db514484b717cabc6f
>
Must have butchered something... the local tree does not go that far in history.

> GLX 1.3 implies equivalent functionality, spelled glXMakeContextCurrent
> instead of glXMakeCurrentReadSGI. The dispatch for both the extension
> and 1.3 versions of the interface has remained wired up in xserver, and
> note that the bindContext call down to the driver _always_ takes both
> drawable and readable arguments regardless of which Make*Current*
> request triggered it.
>
> So I don't think this is making anything any worse.
>
Definitely - make_context_current (and bindContext) have been using
separate readable/drawable since forever. Can you please add some of
the above explanation in the commit history - for posterity.

Thanks
Emil
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 06/13] glx: Enable GLX_SGI_make_current_read in the core

2016-03-30 Thread Adam Jackson
On Wed, 2016-03-30 at 12:12 +0100, Emil Velikov wrote:
> On 23 March 2016 at 22:46, Adam Jackson  wrote:
> > diff --git a/glx/glxdri2.c b/glx/glxdri2.c
> > index c56a376..71dab2a 100644
> > --- a/glx/glxdri2.c
> > +++ b/glx/glxdri2.c
> > @@ -900,13 +900,6 @@ initializeExtensions(__GLXDRIscreen * screen)
> >  }
> > 
> >  for (i = 0; extensions[i]; i++) {
> > -if (strcmp(extensions[i]->name, __DRI_READ_DRAWABLE) == 0) {
> > -__glXEnableExtension(screen->glx_enable_bits,
> > - "GLX_SGI_make_current_read");
> > -
> > -LogMessage(X_INFO, "AIGLX: enabled 
> > GLX_SGI_make_current_read\n");
> > -}
> > -
> Afaics we never had a DRI2 based dri module that provided this
> extension. Which brings the question if this has ever been tested and
> confirmed working. Can we have a small note about this in the commit
> message ?

Not quite. I am dismayed to report that piglit does not appear to
contain any tests for separate drawable and readable, so whether this
has been tested, who knows. And you are correct (and I'm somewhat
surprised to discover) that Mesa currently does not expose this
extension. But it used to:

https://cgit.freedesktop.org/mesa/mesa/commit/?id=ad3221587164c10ae16d85db514484b717cabc6f

GLX 1.3 implies equivalent functionality, spelled glXMakeContextCurrent
instead of glXMakeCurrentReadSGI. The dispatch for both the extension
and 1.3 versions of the interface has remained wired up in xserver, and
note that the bindContext call down to the driver _always_ takes both
drawable and readable arguments regardless of which Make*Current*
request triggered it.

So I don't think this is making anything any worse.

- ajax
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH xserver 06/13] glx: Enable GLX_SGI_make_current_read in the core

2016-03-30 Thread Emil Velikov
On 23 March 2016 at 22:46, Adam Jackson  wrote:
> Signed-off-by: Adam Jackson 
> ---
>  glx/extension_string.c | 2 +-
>  glx/glxdri2.c  | 7 ---
>  glx/glxdriswrast.c | 1 -
>  3 files changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/glx/extension_string.c b/glx/extension_string.c
> index 30c3416..7e74090 100644
> --- a/glx/extension_string.c
> +++ b/glx/extension_string.c
> @@ -92,7 +92,7 @@ static const struct extension_info known_glx_extensions[] = 
> {
>
>  { GLX(MESA_copy_sub_buffer),VER(0,0), N, },
>  { GLX(OML_swap_method), VER(0,0), Y, },
> -{ GLX(SGI_make_current_read),   VER(1,3), N, },
> +{ GLX(SGI_make_current_read),   VER(1,3), Y, },
>  { GLX(SGI_swap_control),VER(0,0), N, },
>  { GLX(SGIS_multisample),VER(0,0), Y, },
>  { GLX(SGIX_fbconfig),   VER(1,3), Y, },
> diff --git a/glx/glxdri2.c b/glx/glxdri2.c
> index c56a376..71dab2a 100644
> --- a/glx/glxdri2.c
> +++ b/glx/glxdri2.c
> @@ -900,13 +900,6 @@ initializeExtensions(__GLXDRIscreen * screen)
>  }
>
>  for (i = 0; extensions[i]; i++) {
> -if (strcmp(extensions[i]->name, __DRI_READ_DRAWABLE) == 0) {
> -__glXEnableExtension(screen->glx_enable_bits,
> - "GLX_SGI_make_current_read");
> -
> -LogMessage(X_INFO, "AIGLX: enabled GLX_SGI_make_current_read\n");
> -}
> -
Afaics we never had a DRI2 based dri module that provided this
extension. Which brings the question if this has ever been tested and
confirmed working. Can we have a small note about this in the commit
message ?

-Emil
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[PATCH xserver 06/13] glx: Enable GLX_SGI_make_current_read in the core

2016-03-23 Thread Adam Jackson
Signed-off-by: Adam Jackson 
---
 glx/extension_string.c | 2 +-
 glx/glxdri2.c  | 7 ---
 glx/glxdriswrast.c | 1 -
 3 files changed, 1 insertion(+), 9 deletions(-)

diff --git a/glx/extension_string.c b/glx/extension_string.c
index 30c3416..7e74090 100644
--- a/glx/extension_string.c
+++ b/glx/extension_string.c
@@ -92,7 +92,7 @@ static const struct extension_info known_glx_extensions[] = {
 
 { GLX(MESA_copy_sub_buffer),VER(0,0), N, },
 { GLX(OML_swap_method), VER(0,0), Y, },
-{ GLX(SGI_make_current_read),   VER(1,3), N, },
+{ GLX(SGI_make_current_read),   VER(1,3), Y, },
 { GLX(SGI_swap_control),VER(0,0), N, },
 { GLX(SGIS_multisample),VER(0,0), Y, },
 { GLX(SGIX_fbconfig),   VER(1,3), Y, },
diff --git a/glx/glxdri2.c b/glx/glxdri2.c
index c56a376..71dab2a 100644
--- a/glx/glxdri2.c
+++ b/glx/glxdri2.c
@@ -900,13 +900,6 @@ initializeExtensions(__GLXDRIscreen * screen)
 }
 
 for (i = 0; extensions[i]; i++) {
-if (strcmp(extensions[i]->name, __DRI_READ_DRAWABLE) == 0) {
-__glXEnableExtension(screen->glx_enable_bits,
- "GLX_SGI_make_current_read");
-
-LogMessage(X_INFO, "AIGLX: enabled GLX_SGI_make_current_read\n");
-}
-
 if (strcmp(extensions[i]->name, __DRI_TEX_BUFFER) == 0) {
 screen->texBuffer = (const __DRItexBufferExtension *) 
extensions[i];
 __glXEnableExtension(screen->glx_enable_bits,
diff --git a/glx/glxdriswrast.c b/glx/glxdriswrast.c
index 84fd3a1..aaf55ca 100644
--- a/glx/glxdriswrast.c
+++ b/glx/glxdriswrast.c
@@ -414,7 +414,6 @@ initializeExtensions(__GLXDRIscreen * screen)
 __glXEnableExtension(screen->glx_enable_bits, "GLX_EXT_framebuffer_sRGB");
 __glXEnableExtension(screen->glx_enable_bits, "GLX_ARB_fbconfig_float");
 __glXEnableExtension(screen->glx_enable_bits, 
"GLX_EXT_fbconfig_packed_float");
-__glXEnableExtension(screen->glx_enable_bits, "GLX_SGI_make_current_read");
 
 extensions = screen->core->getExtensions(screen->driScreen);
 
-- 
2.5.0

___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel