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

Various libglvnd fixes/cleanups, and an ABI update #10

Merged
merged 21 commits into from
Nov 27, 2013

Conversation

bnguyen0
Copy link
Contributor

Here is an overview of the changes:

  • Commit 75b1028 attempts to drastically simplify the vendor library ABI, by removing some unnecessary vendor callbacks and unused functionality, and adding exports which allow vendor libraries to make vendor-neutral dispatch functions which closely mirror the internal dispatch functions defined in libglx.c.
  • Commits 71c2395 and a927cdd remove a lot of unused mapi cruft from the driver, which should make the code base significantly easier to understand/maintain.
  • The remaining commits fix various internal libglvnd bugs.

This requires adding ACLOCAL_AMFLAGS variable to the toplevel
Makefile.am which includes the m4 subdirectory since Autotools won't
necessarily check the contents of AC_CONFIG_MACRO_DIR() (see [1] for
more info).

[1] https://www.gnu.org/software/libtool/manual/html_node/Invoking-libtoolize.html

Signed-off-by: Brian Nguyen <[email protected]>
This typedef is already defined in GL/glext.h and GLES/gl.h; including
this redundant typedef may lead to build errors.

Signed-off-by: Brian Nguyen <[email protected]>
These various conditionals are always true or false and hence can be
removed.

Signed-off-by: Brian Nguyen <[email protected]>
In the LDFLAGS primary for libGL.la, add the "-version-info 1" flag so
the library is installed as libGL.so.1 (rather than libGL.so.0).

Signed-off-by: Brian Nguyen <[email protected]>
Build x11glvnd with the Libtool -module flag (since it will be
dlopened), and rename it from libx11glvnd to x11glvnd. Install it
in $(libdir)/xorg/modules/extensions since it will be loaded as an
X11 extension.

Signed-off-by: Brian Nguyen <[email protected]>
Some applications may elect to use the ARB entrypoint instead of
glXGetProcAddress() itself.

Signed-off-by: Brian Nguyen <[email protected]>
Some API-intensive applications may need more than 256 dynamic dispatch
stubs, so bump the number to 4096.  This can be increased further in a
later change if needed, without breaking the ABI.

Signed-off-by: Brian Nguyen <[email protected]>
This removes the following unused files from src/GLdispatch/vnd-glapi:

- Some unused MESA scons, makefile build infrastructure
- various glapi source-code generation scripts in mapi/glapi/gen
- vnd-glapi/mapi/{shared-gl,es1,es2,vg}api
- vnd-glapi/tests
- vnd-glapi/mapi/glapi/glapi_*.c
- vnd-glapi/mapi/glapi/tests
- vnd-glapi/mapi/mapi.c

With this change, the only consumer of vnd-glapi/mapi/makefile.sources is
vnd-glapi/Makefile.am, so move MAPI_{GLAPI,UTIL}_FILES definitions to
that file and delete this file as well.

Signed-off-by: Brian Nguyen <[email protected]>
The vnd-glapi module only builds mapi files with MAPI_MODE_GLAPI. This
change runs:

    unifdef -UMAPI_MODE_BRIDGE -UMAPI_MODE_UTIL -DMAPI_MODE_GLAPI $file

for each file in the mapi subdirectory. It also manually removes usage
of the MAPI_MODE_* macros from u_current.h that didn't get stripped after
running this command.

After this change, -DMAPI_MODE_GLAPI is no longer necessary, so this is
deleted from vnd-glapi/Makefile.am:AM_CPPFLAGS.

Signed-off-by: Brian Nguyen <[email protected]>
The spec mandates that glXGetConfig() should return GLX_BAD_VALUE if
any parameter is invalid, and GLX_BAD_SCREEN if the screen of the
<vis> parameter does not correspond to a screen.

Hence, check that the arguments of glXGetConfig() are non-NULL and
return GLX_BAD_VALUE if otherwise.  Furthermore, update the noop
stub to return GLX_BAD_SCREEN, since the vendor-neutral dispatcher
will only choose the no-op stub if it couldn't find a valid vendor
for the given screen number.

Signed-off-by: Brian Nguyen <[email protected]>
When making current, assign the API state's new context and dispatch table
in GLdispatch rather than GLX.  This avoids consistency issues if the API state
is unchanged but the current dispatch or context may have changed.

Update the __glDispatchMakeCurrent() call in
__glXMakeGLDispatchCurrent() to get libglvnd building for now; a later change
will remove libglxgldispatch.c altogether.

Signed-off-by: Brian Nguyen <[email protected]>
…m ScreenFromXID()

Failing to do this results in deadlock since AddScreenXIDMapping()
attempts to acquire the XID screen hash write lock.

Signed-off-by: Brian Nguyen <[email protected]>
Instead of crashing with an assertion, simply return NULL if
__glXLookupVendorByScreen() failed to find a vendor.  Callers should
be able to handle this error gracefully.

The GetProcAddress unit test relies on having a NULL display.  To avoid
breaking this test, only check for a NULL display after first attempting
to look up a preloaded vendor name.

Signed-off-by: Brian Nguyen <[email protected]>
Failing to lookup a mapping via this extension should not cause GLX
to throw X errors.  Hence, instead of returning an error code in
the server-side functions if the lookup fails, write a reply which
indicates the lookup failed.

With this change, ProcGLVQueryXIDScreenMapping() sends a reply with
screen -1 if its lookup failed, and ProcGLVQueryScreenVendorMapping()
sends a reply with a 0-length vendor string if its lookup failed.

Signed-off-by: Brian Nguyen <[email protected]>
If glXDestroyContext() is called on a context that is current to a
thread, the GLX 1.4 spec states that "ctx is not destroyed until
it is no longer current".  Hence, we must assume the context ->
screen mapping is still valid until the context is no longer current
as well.

Add a hashtable to track current contexts in GLX, and add helper
functions {Track,Untrack}CurrentContext() to add/remove contexts from
this hashtable in glXMakeCurrent() and glXMakeContextCurrent().

Instead of calling __glXRemoveScreenContextMapping() directly, call
__glXNotifyContextDestroyed() to look up the context in the current
context hashtable and determine whether the mapping can be removed
immediately or when the context loses current.

Add IsContextCurrentToAnyOtherThread() and fail
glXMake{,Context}Current() early if this returns True, prior to
adding the context to the current context list.

Signed-off-by: Brian Nguyen <[email protected]>
…reads

If a thread is destroyed with a context current, libglvnd needs to be
able to remove the context from the current context list to prevent
failures if another thread attempts to make current to the context.
Define a TSD key which stores a pointer that mirrors the current context
pointer stored in TLS, and update this key in TrackCurrentContext().
Define a destructor for this key which calls UntrackCurrentContext()
on this pointer when the thread is terminated.

Signed-off-by: Brian Nguyen <[email protected]>
Reviewed-by: Aaron Plattner <[email protected]>
…heir own hooks

Use XEXT_GENERATE_CLOSE_DISPLAY() to generate an XCloseDisplay() hook
close_display_internal(); this is necessary for the x11glvnd extension
to clean up its private data structure when the display is closed.  Wrap
this hook with a separate close_display() hook; this hook calls
functions in a callback list.

Add XGLVRegisterCloseDisplayCallback() to allow x11glvnd clients to
register these XCloseDisplay() callbacks with x11glvnd; this allows
clients to hook into XCloseDisplay() without needing to implement
X11 extension shim code.

In libGLX, use XGLVRegisterCloseDisplayCallback() to register a callback
to remove references to a closed display in the API state hashtable.

Bonus: add a missing SyncHandle() in the CHECK_EXTENSION() macro.

Signed-off-by: Brian Nguyen <[email protected]>
The aliasing functionality designed to reduce the size of the static
dispatch table in glapi interacts badly with forward-compatible contexts
as defined in GLX_ARB_create_context.  For example, a forward-compatible
OpenGL ES 1.0 context will not support calling into glGetTexGeniv(), but
may support calling into glGetTexGenivOES().  These two functions are aliases
in glapi, so a call to the latter may end up dispatching into the former,
which may be implemented as a stub in the vendor library.

Hence, when generating the static dispatch functions via gl_XML.py,
don't use the "alias" attribute if it is present in the "function" tag.

Signed-off-by: Brian Nguyen <[email protected]>
- Pull GL dispatch callback typedefs into GLdispatchABI.h.  This is
  a common header which will be shared between the libGLX and libEGL
  ABIs.

- Add extern "C" blocks to libglxabi.h and GLdispatchABI.h so these
  headers may be used with vendor libraries that are built C++.

- Remove support for auxiliary dispatch table management
  (libglxgldispatch.[ch] and __GLdispatchExports).  This can be added
  back in later on if it proves useful.

- In libglxmappings.c, replace static initialization of glxExportsTable
  with one-time runtime initialization of the table.

- In libglxmappings.c, use __glDispatchCreateTable() directly to create
  the initial dispatch table.

- In libglxnoopdefs.h, remove unnecessary includes of libglxabipriv.h
  and libglxnoop.h.

- Add screen mapping functions to the GLX exports table.  Some extension
  functions not implemented by libglvnd, e.g. glXCreateContextAttribsARB()
  from GLX_ARB_create_context, may need to update the mappings from
  context or drawable to screen that are managed by libglvnd in order to
  work properly.  Additionally, dispatch functions implemented by vendor
  libraries for GLX extension functions may want to query these mappings
  in order to properly dispatch the call.

- Remove the getDispatchProto() vendor callback.  This is not needed by
  libglvnd since libglvnd does not support mapping more than one
  entrypoint to a single dispatch stub.  See GLdispatch.c for the
  rationale behind this decision.

- Remove redundant typedefs to prevent build errors.

- Delete the vendorData parameter from the getProcAddress() vendor
  callback, which isn't useful if there is no support for more than one
  libglvnd dispatch table per context.  Add the isClientAPI parameter,
  which can be used to distinguish between getProcAddress() requests for
  GLX functions versus getProcAddress() requests for client API
  functions.

- Remove the "screen" argument from Remove.*Mapping() functions.  This
  is unnecessary, as the hash key used to lookup the mapping object in
  libglvnd's hashtables doesn't depend on the screen (which is the hash
  value).  It's problematic since in some cases we don't actually know
  the value of the screen mapped to by a key.

Signed-off-by: Brian Nguyen <[email protected]>
When we lose current we need to also update the TSD copy of the current
context to keep the current state consistent.

Signed-off-by: Brian Nguyen <[email protected]>
- Do some renaming to reduce confusion:

  threadDestroyKey -> tsdContextKey
  threadDestroyKeyInitialized -> tsdContextKeyInitialized
  threadInitOnceControl -> threadCreateTSDContextOnceControl
  ThreadInitOnce -> ThreadCreateTSDContextOnce

- Delete __glXInitThreads().  Instead the TSD context key is initialized
  when TrackCurrentContext() is first called.

- XGLVRegisterCloseDisplayCallback(DisplayClosed) is called by
  __glXInit() now instead of ThreadInitOnce().

Signed-off-by: Brian Nguyen <[email protected]>
Reviewed-by: Aaron Plattner <[email protected]>
bnguyen0 pushed a commit that referenced this pull request Nov 27, 2013
Various libglvnd fixes/cleanups, and an ABI update
@bnguyen0 bnguyen0 merged commit 46e8cc9 into NVIDIA:master Nov 27, 2013
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

Successfully merging this pull request may close these issues.

2 participants