On Fri, Mar 21, 2003 at 08:29:02PM +0000, Keith Whitwell wrote:
> José Fonseca wrote:
> >That is, instead of Mesa acting as the middle man, it should act more as
> >a library. This specificaly means that, instead of (phony names):
> >
> >userapp.c: glEnable(GL_TEXTURE);
> >
> >mesa.c: _mesa_Enable(enum) {
> > // Do its thing here
> > if(ctx->Driver.Enable)
> > ctx->Driver.Enable(ctx, enum);
> > }
> >
> >driver.c: RadeonEnable(ctx, enum) {
> > // Do its thing here
> > }
> >
> >It would be:
> >
> >userapp.c: glEnable(GL_TEXTURE);
> >
> >driver.c: RadeonEnable(enum) {
> > // Do its thing here
> > _mesa_Enable(ctx, enum)
> > // ... or here
> > }
> >
> >mesa.c: _mesa_Enable(enum) {
> > // Do its thing here
> > }
> >
>
> This was the initial idea for the mesa 3.5 rework (which resulted in
> things like the swrast,tnl,etc. modules). I didn't go this far as I felt
> the 3.5 structure was enough to bite off in a single chunk...
Were you thinking going the full way now for the vtxfmt rework? (Sorry
for asking and not looking to the code myself, but I don't know exactly
what's its state and where to start, and I'm also with my hands full now
as you know)
> Anyway, the slight trouble you get with this is error checking. Who does
> it?
That's a choice that RadeonEnable has: if it builds upon _mesa_Enable
then _mesa_Enable can do that for him. If _mesa_Enable is never called
then is its responsability to set the error.
> How does RadeonEnable check if _mesa_Enable succeeded?
In many cases you can simply check the error status. If ctx->ErrorValue
hasn't enough (or updated) info about the error, we can extend it
elsewhere (e.g., ctx->ReturnValue which would be set on zero on success).
> Also, error checking is tied in with the general structure of the
> function a lot of the time. Anyway, the code in api_validate.c is a
> fragment of my original thoughts on this, but won't work so well where
> error checking can't be easily seperated from the rest of the
> functionality.
I see. In some cases most of the code in a API is validation, and the
solution you show in api_validate is the most apropriate, but others are
which aren't that simple.
> Continuing with the Enable() example, it would be nice to duplicating
> avoid the 'switch' on the enum in both RadeonEnable and _mesa_Enable.
NOTE: I'm thinking out loud in the next paragraphs, so you may want to
skim quickly through them and read only the last code snippets.
The only way I see to avoid the duplication of the switch is to break
the glEnable in glEnableTexture, glEnableFoo, ... which is not a bad
thing. We could have a function table (in the context or passed as and
argument to a function _mesa_do_enable which would do the 'switch')
whose the index would be 'enum', and there would only by one
implementation of glEnable. RadeonEnableTexture would call
_mesa_enable_texture, and so on...
On other places this is way too much trouble for a small 'switch', and
we could simply duplicate the code in Mesa's 'case' statements and
eliminate the call to the Mesa API completly. Note that the state is
stored in the context already is exported to the drivers, so everytime
we change the state we need to check if the drivers aren't broken, but I
confess that I'm still not very confortable doing this, since if we add
derived state, things will silently be broken.
Things would be easier if all these glEnableFoo were inlined and defined
in a header, and _mesa_Enable and RadeonEnable would use them as
suitable. There would be duplication of the switch statement in the
code, but only one of them would be executed at run-time.
Overall, if we consider that we need to support new OpenGL extensions
everytime, and that we want to do that without having to modify each
driver, then the only viable solutions are either in the line of
api_validate.c or breaking glEnable in smaller functions. The others
suck badily. Passing functions tables in C++ is a PITA if we want to
model these functions as methods... so I think api_validate is the way
to. Still, in many cases is enough just to check the error code and let
Mesa do the validation by itself. That is:
-----------
userapp.c: glEnable(GL_TEXTURE);
driver.c: RadeonEnable(enum) {
_mesa_Enable(ctx, enum)
if ( !ctx->ReturnValue )
switch() {
case GL_ONLY_A_SUBSET_OF_THE_POSSIBLE_ENUMS:
// Do its thing here
break;
}
}
mesa.c: _mesa_Enable(ctx, enum) {
// Do its thing here
ctx->ReturnValue = error_condition ? 1 : 0;
}
-----------
Or when we need pre-validation:
-----------
userapp.c: glEnable(GL_TEXTURE);
driver.c: RadeonEnable(enum) {
if ( !_mesa_validate_Enable(ctx, enum) ) {
switch() {
case GL_ONLY_A_SUBSET_OF_THE_POSSIBLE_ENUMS:
// Do its thing here
break;
}
_mesa_Enable_novalidate(ctx, enum)
}
}
mesa.c: _mesa_Enable(ctx, enum) {
if ( !_mesa_validate_Enable(ctx, enum) ) {
// Do its thing here
_mesa_Enable_novalidate(ctx, enum)
}
}
_mesa_Enable_novalidate(ctx, enum) {
// No need to validate here
switch () {
// Do its thing here
}
}
-----------
José Fonseca
-------------------------------------------------------
This SF.net email is sponsored by:Crypto Challenge is now open!
Get cracking and register here for some mind boggling fun and
the chance of winning an Apple iPod:
http://ads.sourceforge.net/cgi-bin/redirect.pl?thaw0031en
_______________________________________________
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel
