On Mon, Oct 19, 2009 at 05:15:31PM -0700, Ian Romanick wrote:
> > I've managed to clean up the code and created a patch series. As it
> > touches glapi, the file size exceeds the limit the list easily. I have
> > to upload it to somewhere else
> I looked over the patches, and they look good to me. It's hard to give
> a thorough review due to the volume of changes. I only noticed a couple
> minor issues.
> - remap.c has no license.
> - remap.h has an incorrect license. At least the Mesa version should
> be changed.
Thanks. I have added/corrected the license and uploaded a new version
to
http://0xlab.org/~olv/mesa-remap-table-v2.tar.gz
There is no code change.
> > The second patch switches the use of dri remap table and
> > extension_helper.h to the in core remap table and remap_helper.h. As
> > the helper headers provide the same source level API, the switch here is
> > simple. driInitExtensions is also updated to call the in core
> > functions.
> > It also solves the chicken-egg problem that can be seen in all hw dri
> > drivers. The remap table is now automatically initialized in
> > one_time_init(). I think this is an important change, and it is good.
> I agree. This was the one thing that always bothered me about the remap
> stuff. I wanted to do it differently, but I didn't see a good way.
My way of doing it is to think of libmesa.a as a library of its own. It
should not rely on symbols from applications (that is, drivers), and
SET/GET/CALL macros should work without special care because they are
part of libmesa.a API.
> > As for libglapi.a, there will be some missing symbols, like
> > glGenTexturesEXT, when GLX_INDIRECT_RENDERING is defined. It is
> > possible to modify gl_apitemp.py to generate the stubs. But it is
> > skipped in this patch series.
> The issue here was that we want some functions to alias (such as
> glGenTexturesEXT and glGenTextures), but they have different GLX protocol.
Yeah. To minimize the changes needed, I am thinking re-organizing
glapitemp.h in a way that it can provide _only_ those symbols affected
by GLX_INDIRECT_RENDERING. drivers/x11/ or gallium/winsys/xlib/ can
then include it to provide the missing symbols.
But it is still weird that libglapi.a has missing symbols, if one views
it as a library. I would also like to change this while at it.
--
Regards,
olv
|