KhronosGroup / OpenGL-API

OpenGL and OpenGL ES API Issue Tracker
34 stars 5 forks source link

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

Open erik-kz opened 3 years ago

erik-kz commented 3 years ago

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 commented 3 years ago

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

erik-kz commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

https://gitlab.freedesktop.org/xorg/xserver/-/issues/1106

nwnk commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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:

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

cubanismo commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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.