Discussion:
[PATCH] glx: GLX_MESA_multithread_makecurrent is direct-only
(too old to reply)
Adam Jackson
2017-12-05 16:10:09 UTC
Permalink
This extension is not defined for indirect contexts. Marking it as
"client only", as the old code did here, would make the extension
available in indirect contexts, even though the server would certainly
not have it in its extension list.

Cc: <mesa-***@lists.freedesktop.org>
Signed-off-by: Adam Jackson <***@redhat.com>
---
src/glx/glxextensions.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/glx/glxextensions.c b/src/glx/glxextensions.c
index af6ffbf660..638d8bcbbe 100644
--- a/src/glx/glxextensions.c
+++ b/src/glx/glxextensions.c
@@ -152,7 +152,7 @@ static const struct extension_info known_glx_extensions[] = {
{ GLX(ATI_pixel_format_float), VER(0,0), N, N, N, N },
{ GLX(INTEL_swap_event), VER(0,0), Y, N, N, N },
{ GLX(MESA_copy_sub_buffer), VER(0,0), Y, N, N, N },
- { GLX(MESA_multithread_makecurrent),VER(0,0), Y, N, Y, N },
+ { GLX(MESA_multithread_makecurrent),VER(0,0), Y, N, N, Y },
{ GLX(MESA_query_renderer), VER(0,0), Y, N, N, Y },
{ GLX(MESA_swap_control), VER(0,0), Y, N, N, Y },
{ GLX(NV_float_buffer), VER(0,0), N, N, N, N },
--
2.14.3
Nicolai Hähnle
2017-12-06 10:29:37 UTC
Permalink
Post by Adam Jackson
This extension is not defined for indirect contexts. Marking it as
"client only", as the old code did here, would make the extension
available in indirect contexts, even though the server would certainly
not have it in its extension list.
Makes sense.
Post by Adam Jackson
---
src/glx/glxextensions.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/glx/glxextensions.c b/src/glx/glxextensions.c
index af6ffbf660..638d8bcbbe 100644
--- a/src/glx/glxextensions.c
+++ b/src/glx/glxextensions.c
@@ -152,7 +152,7 @@ static const struct extension_info known_glx_extensions[] = {
{ GLX(ATI_pixel_format_float), VER(0,0), N, N, N, N },
{ GLX(INTEL_swap_event), VER(0,0), Y, N, N, N },
{ GLX(MESA_copy_sub_buffer), VER(0,0), Y, N, N, N },
- { GLX(MESA_multithread_makecurrent),VER(0,0), Y, N, Y, N },
+ { GLX(MESA_multithread_makecurrent),VER(0,0), Y, N, N, Y },
{ GLX(MESA_query_renderer), VER(0,0), Y, N, N, Y },
{ GLX(MESA_swap_control), VER(0,0), Y, N, N, Y },
{ GLX(NV_float_buffer), VER(0,0), N, N, N, N },
--
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.
Emil Velikov
2017-12-06 15:32:19 UTC
Permalink
Post by Adam Jackson
This extension is not defined for indirect contexts. Marking it as
"client only", as the old code did here, would make the extension
available in indirect contexts, even though the server would certainly
not have it in its extension list.
Reviewed-by: Emil Velikov <***@collabora.com>

Unrelated: reportedly only cairo is using the extension, so could we
consider the extension obsolete?
Alternatively how about making it an official Khronos extension ;-)

-Emil
Ian Romanick
2017-12-06 20:01:50 UTC
Permalink
Post by Emil Velikov
Post by Adam Jackson
This extension is not defined for indirect contexts. Marking it as
"client only", as the old code did here, would make the extension
available in indirect contexts, even though the server would certainly
not have it in its extension list.
Unrelated: reportedly only cairo is using the extension, so could we
consider the extension obsolete?
It's not too surprising that only Cairo is using it. IIRC, Eric
specifically made this extension for Cairo, and it was a pretty big perf
win at the time.

I had wanted to test this patch, but... does LIBGL_ALWAYS_INDIRECT=y
just not work any more? With the distro Mesa I get:

name of display: :0
X Error of failed request: GLXBadContext
Major opcode of failed request: 151 (GLX)
Minor opcode of failed request: 6 (X_GLXIsDirect)
Serial number of failed request: 37
Current serial number in output stream: 36

And with fairly recent master I get direct rendering.
Post by Emil Velikov
Alternatively how about making it an official Khronos extension ;-)
-Emil
_______________________________________________
mesa-stable mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-stable
Adam Jackson
2017-12-06 21:13:13 UTC
Permalink
Post by Ian Romanick
Post by Emil Velikov
Post by Adam Jackson
This extension is not defined for indirect contexts. Marking it as
"client only", as the old code did here, would make the extension
available in indirect contexts, even though the server would certainly
not have it in its extension list.
Unrelated: reportedly only cairo is using the extension, so could we
consider the extension obsolete?
It's not too surprising that only Cairo is using it. IIRC, Eric
specifically made this extension for Cairo, and it was a pretty big perf
win at the time.
I think at this point most of the effect could be achieved with no-
flush contexts, but yeah.
Post by Ian Romanick
I had wanted to test this patch, but... does LIBGL_ALWAYS_INDIRECT=y
It works, that's the server telling you it doesn't support indirect
rendering. We turned that off by default a few releases ago as being
slow and underfeatured and CVE-prone. Start your server with +iglx or
with this in xorg.conf:

Section "ServerFlags"
Option "IndirectGLX" "true"
EndSection

- ajax
Andres Gomez
2018-08-09 12:54:49 UTC
Permalink
Adam, which is the status of this patch? Is this effectively dropped?
Post by Adam Jackson
Post by Ian Romanick
Post by Emil Velikov
Post by Adam Jackson
This extension is not defined for indirect contexts. Marking it as
"client only", as the old code did here, would make the extension
available in indirect contexts, even though the server would certainly
not have it in its extension list.
Unrelated: reportedly only cairo is using the extension, so could we
consider the extension obsolete?
It's not too surprising that only Cairo is using it. IIRC, Eric
specifically made this extension for Cairo, and it was a pretty big perf
win at the time.
I think at this point most of the effect could be achieved with no-
flush contexts, but yeah.
Post by Ian Romanick
I had wanted to test this patch, but... does LIBGL_ALWAYS_INDIRECT=y
It works, that's the server telling you it doesn't support indirect
rendering. We turned that off by default a few releases ago as being
slow and underfeatured and CVE-prone. Start your server with +iglx or
Section "ServerFlags"
Option "IndirectGLX" "true"
EndSection
- ajax
_______________________________________________
mesa-stable mailing list
https://lists.freedesktop.org/mailman/listinfo/mesa-stable
--
Br,

Andres
Adam Jackson
2018-08-09 16:34:33 UTC
Permalink
Post by Andres Gomez
Adam, which is the status of this patch? Is this effectively dropped?
It's still valid, and still not very important. I'd just forgotten it.
Merged now, thanks for the reminder:

remote: remote: I: patch #191858 updated using rev 63a6b719d98fb1ad58ae93c2de859e6d4bfa8b8b
remote: remote: I: 1 patch(es) updated to state Accepted.
remote: remote: % Total % Received % Xferd Average Speed Time Time Time Current
remote: remote: Dload Upload Total Spent Left Speed
remote: remote: 0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
remote: To ssh://kemper.freedesktop.org/git/mesa/mesa
remote: fcf259ef97..63a6b719d9 63a6b719d98fb1ad58ae93c2de859e6d4bfa8b8b -> master
To gitlab.freedesktop.org:mesa/mesa.git
fcf259ef97f..63a6b719d98 master -> master

- ajax

Loading...