Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[GLX] Error generated on failure to create an indirect GLX context is unspecified #76

Open
erik-kz opened this issue Nov 4, 2020 · 12 comments
Assignees

Comments

@erik-kz
Copy link

erik-kz commented Nov 4, 2020

glXCreateContext, glXCreateNewContext, and glXCreateContextAttribsARB will generate an error if the "direct" argument is False but the server does does not allow indirect GLX contexts, which has been X.org's default behavior for some time. However the GLX specification does not mention what the error should be in this case. For example, Mesa's server-side GLX implementation will generate a different error than NVIDIA's.

@pdaniell-nv
Copy link

@cubanismo is this easy to fix? I'm not sure what the process is for GLX.

@erik-kz
Copy link
Author

erik-kz commented Nov 11, 2020

To provide some additional details, in the situation described Mesa will generate a GLXBadContext error, while NVIDIA will generate a BadValue error, as of the latest version of both implementations. This is the case for glXCreateContext and glXCreateNewContext.

The following paragraphs from sections 3.5 and 3.3.7 of the spec respectively describe what errors these functions might generate. Notice that neither mention anything about attempting to create an indirect context when that isn't supported...

glXCreateContext can generate the following errors: GLXBadContext if
share list is neither zero nor a valid GLX rendering context; BadValue if visual
is not a valid X Visual or if GLX does not support it; BadMatch if share list
defines an address space that cannot be shared with the newly created context or
if share list was created on a different screen than the one referenced by visual;
BadAlloc if the server does not have enough resources to allocate the new context.

glXCreateNewContext can generate the following errors: GLXBadContext
if share list is neither zero nor a valid GLX rendering context;
GLXBadFBConfig if config is not a valid GLXFBConfig; BadMatch if
the server context state for share list exists in an address space that cannot be
shared with the newly created context or if share list was created on a different
screen than the one referenced by config; BadAlloc if the server does not have
enough resources to allocate the new context; BadValue if render type does not
refer to a valid rendering type.

@ianromanick
Copy link

I suspect both implementations just pick an error at random. Looking at the list of error condition in the extension, I could come up with weak justification for using any of BadValue, GLXBadContext, or GLXBadFBConfig. My gut tells me we should just pick an error for this condition, and both implementations should update to using it.

I'll try to get some other relevant people from the Mesa community to comment.

@ianromanick
Copy link

@nwnk
Copy link

nwnk commented Nov 26, 2020

I think this is a bug in Mesa's client side. xserver's stock GLX should generate BadValue for both the legacy path:

https://gitlab.freedesktop.org/xorg/xserver/-/blob/master/glx/glxcmds.c#L301

and the CreateContextAttribs path:

https://gitlab.freedesktop.org/xorg/xserver/-/blob/master/glx/createcontext.c#L324

Which, to be clear, I think is the most appropriate error. If you're seeing GLXBadContext I think it's because of this bit of Mesa's libGL:

https://gitlab.freedesktop.org/mesa/mesa/-/blob/master/src/glx/glxcmds.c#L416

In which we force a GLXIsDirect request to the server in order to verify that creating the context actually worked; if it didn't, then the XID we just created will indeed not name a GLX context, and you'd expect GLXBadContext. But Mesa should attempt to recognize that case, silently absorb the failure from IsDirect, and make sure BadValue is thrown.

@cubanismo
Copy link

Hehe, from our server-side GLX code:

"Returning BadValue here is a bit strange, but this is what X.Org does."

So we tried. I assume the IsDirect request was added after we coded that up to match X.org.

I tend to agree with @nwnk. We also force a round trip for similar reasons, but our version doesn't have the same side effect. If it's not too much work to eat that extra error code in the Mesa client, that'd probably be the cleanest solution from an app POV.

@nwnk
Copy link

nwnk commented Dec 1, 2020

I've opened a merge request for this that fixes things for me:

haldol:~/git/mesa% LD_LIBRARY_PATH=/home/ajax/git/mesa/build/src/glx DISPLAY=:8 LIBGL_ALWAYS_INDIRECT=1 glxinfo
name of display: :8
X Error of failed request:  BadValue (integer parameter out of range for operation)
  Major opcode of failed request:  149 (GLX)
  Minor opcode of failed request:  24 (X_GLXCreateNewContext)
  Value in failed request:  0x0
  Serial number of failed request:  35
  Current serial number in output stream:  36

https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/7859

Vaguely related, xserver will now not advertise GLX_EXT_import_context if indirect rendering is turned off, since it can't possibly work:

https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/269

I don't have an NVIDIA config handy to check, but since the GLX extension string in libglxserver_nvidia.so is one solid string I don't think NVIDIA is doing this yet. It might be nice to align that, since given the age of the extension it's a quite unambiguous signal that indirect contexts are disabled.

@cubanismo
Copy link

We do dynamically build our server extension string actually, but that one is statically included. Importing contexts across processes certainly won't work for direct-rendering contexts, but there are other aspects of the extension that could remain useful (Querying its Visual, mostly), so I'm inclined to leave it enabled.

I think at some point we had considered adding a way to query for indirect rendering support directly. Perhaps that's worth revisiting.

@nwnk
Copy link

nwnk commented Dec 2, 2020

Importing contexts across processes certainly won't work for direct-rendering contexts, but there are other aspects of the extension that could remain useful (Querying its Visual, mostly), so I'm inclined to leave it enabled.

I think when I looked at this I concluded all the useful bits were already core features:

  • glXGetCurrentDisplayEXT is glXGetCurrentDisplay in GLX 1.2
  • glXQueryContextInfoEXT is glXQueryContext in GLX 1.3
  • glXGetContextIDEXT is only really useful to get the XID to pass to glXImportContextEXT
  • glXFreeContextEXT is only useful to destroy the client-side state of an imported context

Meh, up to you. I agree that an explicit way to advertise indirect context support would be better.

@cubanismo
Copy link

glXQueryContextInfoEXT is glXQueryContext in GLX 1.3

This one specifically lacks the ability to query the visual, providing an FBConfig query instead, at least in our implementation's interpretation. Maybe you can walk the steps and use that to query an FBConfig even from a 1.2-style context and it does something sensible, and then you could query a visual from the FBConfig as a separate step, but it's easier to just leave it than verify that works as expected.

And there's also the off-chance risk that some legacy app that will never be updated is relying on the old extension. Again, easier to just leave it than investigate.

We have some code in our internal tests that detects indirect support by attempting an indirect glXCreateContext() and looking for BadValue once at startup, then caching the result. It's a little awkward, but it isn't that bad for a test harness.

@erik-kz
Copy link
Author

erik-kz commented Dec 2, 2020

We have some code in our internal tests that detects indirect support by attempting an indirect glXCreateContext() and looking for BadValue once at startup, then caching the result. It's a little awkward, but it isn't that bad for a test harness.

Incidentally, that was what prompted the original bug. In the case of Xwayland we'd be using mesa's server-side GLX implementation which was causing this to fail.

@nwnk
Copy link

nwnk commented Dec 3, 2020

Hah, right you are, GLX 1.4 only names three context properties and the visual ID is not among them. Mesa's QueryContext has always supported it since the initial GLX 1.3 work 16 years ago (my goodness), because it aliased together the entrypoints for EXT_import_context and 1.3.

Well. Now we know. The Mesa MR is landed now, so I think this issue is now just to document the BadValue convention.

@pdaniell-nv pdaniell-nv added this to the Needs Action/PR milestone Jan 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants