Hi Jeremy,
First of all, let me say that I'm sorry I didn't review your work
before it was committed, that would have been the right time to
address this. Anyway, my problem is that the addition of even more
#ifdef's to the glx client code makes an already tangled code base
harder to read and work with. Even worse, a lot of the new #ifndef
GLX_USE_APPLEGL is of the form
PUBLIC void
glXCopyContext(Display * dpy, GLXContext source,
GLXContext dest, unsigned long mask)
{
#ifdef GLX_USE_APPLEGL
/* Apple GL code */
#else
/* What was there before */
#endif
}
which is just screaming out for a vtable implementation. There are
places where code is shared, but it's often just a small snippet like
if (!dpy) return NULL; or it's a bigger piece of code that can be put
in a common function and called from the different implementations.
Of course, the existing code already did this to some extent with the
direct/indirect code paths. Most of these cases look like
#if defined(GLX_DIRECT_RENDERING)
if (gc->driContext) {
/* direct rendering case */
}
/* fall through to indirect case */
#endif
/* indirect case */
which would be handled just fine by the vtable approach as well: at
context creation time, we set the context vtable pointer to the
direct, indirect or applegl functions and this will just work. I
wanted to do some of this in the past, but I didn't feel like sinking
too much work into the glx code before the license change.
Now, I'm not saying that we need to do this refactoring now - it's
certainly not a priority for me right now. But as a minimum, I'd like
us to be on the same page with where this code should be, and
hopefully we can work towards a cleaner code base. FIguring out
what's going on in any of the glx entrypoints is just too hard right
now with all the #ifdef spaghetti.
Kristian
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
