Bug 6355

Created attachment 5021 [details] [review]
patch for xorg

Created attachment 5022 [details] [review]
patch for mesa

Requires symlinks to files from xorg:

cd Mesa/src/mesa/drivers/x11
ln -s ${XORG_HOME}/GL/mesa/X/xf86glx_util.h xorg_glx_util.h
ln -s ${XORG_HOME}/GL/mesa/X/xf86glx_util.c xorg_glx_util.c

Created attachment 5023 [details] [review]
new files for mesa

Created attachment 5135 [details] [review]
patch for mesa - try 2

actually add indices for extensions.

Again, feel free to ignore the patches and take this as a report that building
GLcore from Mesa seems possible, at least with a bit of ifdef ugliness.

Reassigning to hopefully get feedback from Mesa developers.

The actual Mesa changes are minimal, most of the Mesa patch is:
- updating the XMesa i/f to make xserver/xf86glx.c unaware of Mesa internals
- dropping the old (pre-AIGLX) SI GLcore i/f

If these changes are acceptable, I can update and submit these alone.

I think something like the following can work and solves the problems with the
current situation:

Compile libmesa.a once in Mesa and use it both for Mesa and Xorg, Xorg still
needs source linking but only for a small number of files from drivers/x11 and
glapi (for glx).

The problematic defines are:
- IN_DRI_DRIVER
- USE_XSHM
- XFree86Server
- XFree86LOADER (IN_MODULE, NO_LIBCWRAPPER)
- _XSERVER64
- USE_EXTERNAL_DXTN_LIB

Barring the cases mentioned in comment #1, I think that
- USE_XSHM, XFree86Server, _XSERVER64 only affect drivers/x11 which will still
  be compiled within Xorg.
- XFree86LOADER is inactive (and can be dropped, bug #9144)
- IN_DRI_DRIVER can be handled with adding the driInitExtensions() patch to
  xm_api.c and a configure variable in Xorg, enabled by default on linux and
  freebsd.

This approach does not require adding new files to Mesa, only the changes to
xm_api.c and small changes to src/mesa/Makefile.

With regard to the cases in comment #1:

main/context.c
	This one is easy, we can override ctx->Const.CheckArrayBounds in
	XMesaCreateContext() (see bug #9285)

x86/common_x86.c
	This one is actually harmless. IN_DRI_DRIVER actually serves as a
	kernel version check here.

	My reading of the comments is that if we replace the test with just
	a compile-time kernel version check, we lose SSE on pre-2.4.0 kernels
	and mis-configured post-2.4.0 kernels, so I suggest to replace it...

main/texcompress_s3tc.c
	This one is problematic because it needs -ldl and only the xfree86 DDX
	links against libdl by default.

main/execmem.c
	I don't know about this one.

Created attachment 9387 [details] [review]
patch for xorg

Created attachment 9388 [details] [review]
patch for mesa

These are the updated patches. To build:

cd .../mesa/src/mesa/drivers/x11/glcore/
ln -s .../xserver/GL/mesa/X/xf86glx_util.c
ln -s .../xserver/GL/mesa/X/xf86glx_util.h

make linux-dri
make glcore

The patch supports tree sharing with both dri and stand-alone mesa, 
"make glcore" does not switch config files.

There are two open issues with tree sharing and the -ldl in DIX issue.

Hi George, looks good.  One comment: we should consider just moving GL/glx up to the top level and get rid of the GL subdir.  Once mesa is just another out-of-tree provider, there's really nothing else under GL than GL/glx.

Hi Kristian, thanks for reviewing. I should probably mention that out of the issues mentioned in your original mail:

- glapi
  is a non-issue with the patches because the xserver still symlinks those from
  mesa, I think this should be handled separately

- GLcore i/f versioning
  still needs to be looked someway

Created attachment 9512 [details] [review]
patch for xorg

rebase to master and fix some glitches:

- drop --enable-glx-pthreads from previous patch, xserver/glx does not care
  about a PTHREADS mismatch with DRI only about a GLX_USE_TLS mismatch, treat
  GLcore similarly

- add -ldl to GLX extension libs

Created attachment 9513 [details] [review]
patch for mesa

rebase to master and fix some glitches:

- include xorg-server.h in the place of dix-config.h

- mark GLcore interface symbol as exported

- undef 'USE_XSHM' to make it explicit that 'XFree86Server' takes precedence
  it takes now but this can easily break...

- add GLCORE_LIB_DEPS, when sharing tree with DRI

To recap, I think that all lateral issues have been resolved now. Still TBD
are the specifics of the GLcore interface and the dynamic loading of GLcore:

- GLcore interface
  it needs versioning and it maybe cleaner to just drop the GLcore interface
  struct and mark the required XMesa functions as exported, since it's not a
  proper interface anyway...

- dynamic loading of GLcore
  it needs --export-dynamic to LDFLAGS. xfree86 has it and works, dmx has it
  but is untested, kdrive needs this flag which adds ~87KB to the binary size.
  I don't know about other DDX'es

The above issues are masked away if the loader is moved to DIX.

Created attachment 9524 [details] [review]
just export xmesa functions

What about this instead of adding a GLcore interface ?

Created attachment 9604 [details] [review]
patch for xorg

Created attachment 9605 [details] [review]
patch for mesa

I opted for just exporting the xmesa functions, these are rebased patches.

The compilation order issue will also come up with glucose as it is heading towards the glitz repository; glucose is a glitz backend for the internal xserver "window system", similar to xmesa being a mesa backend for the internal xserver "window system".

There are two approaches:
- install libs, install xserver, compile backends from respective libs and
  dlopen from xserver
- install libs, symlink and compile backends from within the xserver using
  pointers to compiled libs (as in comment #7)

The attached patches implement the first approach, but it seems to me that the
second approach is more workable and results in a more streamlined compilation order. I guess this should be discussed somewhere.

Created attachment 9612 [details] [review]
build GLcore partly from Mesa

This implements the "build libmesa.a from Mesa and xmesa from xserver" suggestion. I like this better and would like to commit but after xgl is merged to head.

We discussed this issue a little bit during XDS2007 and people seem to agree that something really needs to be done here. So I am pinging this bug...

I put re-based patches at:
http://people.freedesktop.org/~gsap7/glcore/

George, this is nice. The one thing I'd like to see is that the xorg-server checks on the mesa side are a little more robust. E.g., the pkg-config checks could be made in autoconf. Also, what versions of xorg-server are valid?

(In reply to comment #24)
> George, this is nice. The one thing I'd like to see is that the xorg-server
> checks on the mesa side are a little more robust. E.g., the pkg-config checks
> could be made in autoconf. 

I avoided autoconf, because i did not want to force the glcore dependencies on people building just dri or xlib.

> Also, what versions of xorg-server are valid?
> 

xorg HEAD as of yesterday. Probably a minor version bump is needed for mesa to be able to check the xorg version properly.

(In reply to comment #25)
> (In reply to comment #24)
> > George, this is nice. The one thing I'd like to see is that the xorg-server
> > checks on the mesa side are a little more robust. E.g., the pkg-config checks
> > could be made in autoconf. 
> 
> I avoided autoconf, because i did not want to force the glcore dependencies on
> people building just dri or xlib.

Right, I think it would be best if glcore was a new --with-driver option. And you get different linking options depending on which driver you specified. Is that right? It seems like the libraries glcore links to would always be the same since it doesn't depend on DRI.

For the non-autoconf case, you first have to run `make linux-dri' to get the configs symlink setup. It doesn't work out of the box. I think it would be best to make new linux-glcore, freebsd-glcore, etc., configs files.

If you agree with that plan, I can help write some patches.

There's also nothing that forces libmesa.a to build, so it bombs on a fresh checkout.

I was thinking about making GLcore a DRI driver.  The DRI driver interface now has an extension mechanism that allows us to advertise alternative entry points, which I use for legacy DRI support now.  This mechanism could also be used to provide entry points that doesn't require drm or sareas or the like.  Then the DRI loader in the X server (and libGL even) could look for the sw renderer entry point and call that if the hw entry points fails or if the loader doesn't support them.

The point is that I'd really like to reduce the number of interface we export, and I'd especially like to drive a stake through glcore.h.  The new dri interface is designed to be extensible for this kind of stuff and to hide implementation details as much as possible so it's easy to maintain.

(In reply to comment #26)
> 
> Right, I think it would be best if glcore was a new --with-driver option. And
> you get different linking options depending on which driver you specified. Is
> that right? It seems like the libraries glcore links to would always be the
> same since it doesn't depend on DRI.
> 
> For the non-autoconf case, you first have to run `make linux-dri' to get the
> configs symlink setup. It doesn't work out of the box. I think it would be best
> to make new linux-glcore, freebsd-glcore, etc., configs files.
> 
> If you agree with that plan, I can help write some patches.
> 

I don't pretty much get what you are suggesting but the point of the patches is that glcore is not to be build stand-alone but as a "companion" to either dri or xlib.

You *first* build dri or xlib with whatever built system you like and *then* type:
# make glcore
# make glcore-install

glcore will be built properly in all cases. Otherwise, you'd have to keep a separate tree for glcore ...

(In reply to comment #28)
> I was thinking about making GLcore a DRI driver.  The DRI driver interface now
> has an extension mechanism that allows us to advertise alternative entry
> points, which I use for legacy DRI support now.  This mechanism could also be
> used to provide entry points that doesn't require drm or sareas or the like. 
> Then the DRI loader in the X server (and libGL even) could look for the sw
> renderer entry point and call that if the hw entry points fails or if the
> loader doesn't support them.
> 

I definitely agree with you that this is the right solution, but has not happened for years. Granted the new dri interface makes this easier but the work still remains to be done. With a little imagination __GLcoreModuleRec is pretty close to  __DRIlegacyExtensionRec and can be massaged to the later during the xserver 1.6 timeframe, unless you want to start a new.

I would be glad to see the above finally implemented and the __GLcoreModuleRec ripped off.

Hmm, it's not that hard, I started doing it a while ago but had to put it back on my todo list for a bit.  Let me see if I can find the code...

(In reply to comment #30)
> 
> You *first* build dri or xlib with whatever built system you like and *then*
> type:
> # make glcore
> # make glcore-install
> 
> glcore will be built properly in all cases. Otherwise, you'd have to keep a
> separate tree for glcore ...

I'm thinking a lot more about this from a packaging perspective and not a "developer with a bunch of trees" perspective:

1. Unpack mesa tarball, build DRI libGL, install
2. Unpack xorg-server tarball, build, install
3. Unpack mesa tarball, build glcore, install

So, yeah, I am thinking about it from a separate trees angle because that's essentially what a tarball is. Currently, you also need to build DRI in step 3.

(In reply to comment #33)
> (In reply to comment #30)
> 
> I'm thinking a lot more about this from a packaging perspective and not a
> "developer with a bunch of trees" perspective:
> 
> 1. Unpack mesa tarball, build DRI libGL, install
> 2. Unpack xorg-server tarball, build, install
> 3. Unpack mesa tarball, build glcore, install
> 
> So, yeah, I am thinking about it from a separate trees angle because that's
> essentially what a tarball is. Currently, you also need to build DRI in step 3.
> 

No you don't need to build DRI again, step 3 should be:
3. cd to mesa tree from step 1, build glcore, install (using already built libmesa.a)

I am not saying the situation is ideal, but i think it's towards the right direction. Let's see what Kristian has.

(In reply to comment #34)
> 
> No you don't need to build DRI again, step 3 should be:
> 3. cd to mesa tree from step 1, build glcore, install (using already built
> libmesa.a)

Not if you're a distro or otherwise don't leave already built trees lying around.

> I am not saying the situation is ideal, but i think it's towards the right
> direction. Let's see what Kristian has.

I agree that it's not really that bad. I just think it could be less implicit. In other words, I really want configure to tell me whether I can or cannot build glcore. I don't want the message to be a weird make or compiler error.

But, yeah, let's not get too excited if Kristian's patches refactor things a lot. BTW, I think the patches are in this thread:

http://lists.freedesktop.org/archives/xorg/2006-March/013872.html

Ok, I threw the patch away or something... It wasn't very far along anyway, and I'll try to do it again.

These possibly?

http://lists.freedesktop.org/archives/xorg/2006-March/013878.html

(In reply to comment #37)
> These possibly?
> 
> http://lists.freedesktop.org/archives/xorg/2006-March/013878.html
> 

No, it was more recent... a few weeks back.  They're sitting somewhere on one of my laptops I think.

Created attachment 16183 [details] [review]
Sanitize the variables used for glcore

George, this shuffles around some of the settings so that they are all made from configure or configs/*. I changed GLCORE_LIB_DEPS to always link with -lm and -lpthread (is this right?) and added them to configs/default so they are always inherited. This doesn't hurt anyone as make won't evaluate the variables until they are referenced, which only happens in the glcore Makefile.

What do you think?

Created attachment 16189 [details] [review]
dan replay

Hi Dan,

wrt GLCORE_LIB_DEPS, strictly speaking -lpthread should be added when
-DPTHREADS is defined, but adding it by default should be ok. Also,
I think that -ldl is part of libc in bsd systems, so it should not
be added. Lastly, libGLcore just uses a couple of symbols from xorg core,
so it does not need `pkg-config --libs xorg-server` either.

wrt GLCORE_CFLAGS, what about adding XORG_CFLAGS instead, which is what
PKG_CHECK_MODULES(XORG, xorg-server) would produce ?

lastly, I changed the ifeq dri tests in xorg/Makefile, in case driver xorg
is added.

(In reply to comment #36)
> Ok, I threw the patch away or something... It wasn't very far along anyway, and
> I'll try to do it again.
> 

Hi Kristian,

I will have some time next week, so I will be able to help or get started if you provide some details. Is your patch based on fb_dri.c ? does it support the dithering stuff ? does it need any driver extensions ? what load extensions does it need ? swap_buffers ? buffer_resize ?

Mass version move, cvs -> git