mesa3d-dev

[Mesa3d-dev] Latest changes

[Keith W. <ke...@va...>]

I'll post a more detailed summary later, but I've committed changes with
fairly sweeping scope.  Fundamentally, core mesa is out of the fallback
business, this is now the job of the drivers and two helper modules 'swrast'
and 'swrast_setup'.  

Swrast is the old software rasterizer wrapped up behind an interface which
removes the need for the driver to care about which statechanges might affect
the rasterizer itself.

Swrast_setup is a layer above the software rasterizer which takes care of
unfilled, offset, two-side lit and flatshaded primitives.  It also provides a
RasterSetup phase for building SWvertices efficiently.

I've modified the FX, X11 and OSmesa drivers to this new regime, though at
this point I've done nothing to ensure performance has been maintained.

In the case of the FX driver, I've taken steps to simplify the codebase in
reflection of it's basically obsolete status.  To this end I've removed lots
of marginal or experimental code, and backported some of the appropriate bits
of a DRI style driver.  I propose that the focus of this driver be providing a
solid Glide2 driver, and that if anyone wants to focus on performance and/or
Glide3, they should backport the new tdfx-3-0-0-branch DRI driver, which
offers very good performance gains on the Glide2 codebase.

Keith

Re: [Mesa3d-dev] Latest changes

[Brian P. <br...@va...>]

Keith Whitwell wrote:
> 
> I'll post a more detailed summary later, but I've committed changes with
> fairly sweeping scope.  Fundamentally, core mesa is out of the fallback
> business, this is now the job of the drivers and two helper modules 'swrast'
> and 'swrast_setup'.
> 
> Swrast is the old software rasterizer wrapped up behind an interface which
> removes the need for the driver to care about which statechanges might affect
> the rasterizer itself.
> 
> Swrast_setup is a layer above the software rasterizer which takes care of
> unfilled, offset, two-side lit and flatshaded primitives.  It also provides a
> RasterSetup phase for building SWvertices efficiently.
> 
> I've modified the FX, X11 and OSmesa drivers to this new regime, though at
> this point I've done nothing to ensure performance has been maintained.
> 
> In the case of the FX driver, I've taken steps to simplify the codebase in
> reflection of it's basically obsolete status.  To this end I've removed lots
> of marginal or experimental code, and backported some of the appropriate bits
> of a DRI style driver.  I propose that the focus of this driver be providing a
> solid Glide2 driver, and that if anyone wants to focus on performance and/or
> Glide3, they should backport the new tdfx-3-0-0-branch DRI driver, which
> offers very good performance gains on the Glide2 codebase.


Good work, Keith.  This is looking really good.

After compiling and taking a quick look over the code I just have a
couple comments:

1. _swsetup_RegisterVB() returns a boolean, but in dd.h, the RegisterVB
   function pointer returns void.  Which should it be?

2. I think the SWvertex pointers passed to the point, line and triangle
   functions could/should be const-qualified.  Right?

-Brian

Re: [Mesa3d-dev] Latest changes

[Keith W. <ke...@va...>]

Brian Paul wrote:
> 

> 
> Good work, Keith.  This is looking really good.
> 
> After compiling and taking a quick look over the code I just have a
> couple comments:
> 
> 1. _swsetup_RegisterVB() returns a boolean, but in dd.h, the RegisterVB
>    function pointer returns void.  Which should it be?

Hmm.  If it does fail (it alloc's memory, typically), I'm not sure we can go
on.  It might as well exit on failure and return void.

> 2. I think the SWvertex pointers passed to the point, line and triangle
>    functions could/should be const-qualified.  Right?

Probably, yes.

Keith

Re: [Mesa3d-dev] Latest changes

[Brian P. <br...@va...>]

Keith Whitwell wrote:
> 
> Brian Paul wrote:
> >
> 
> >
> > Good work, Keith.  This is looking really good.
> >
> > After compiling and taking a quick look over the code I just have a
> > couple comments:
> >
> > 1. _swsetup_RegisterVB() returns a boolean, but in dd.h, the RegisterVB
> >    function pointer returns void.  Which should it be?
> 
> Hmm.  If it does fail (it alloc's memory, typically), I'm not sure we can go
> on.  It might as well exit on failure and return void.
> 
> > 2. I think the SWvertex pointers passed to the point, line and triangle
> >    functions could/should be const-qualified.  Right?
> 
> Probably, yes.


I'm looking at the swrast line functions.  There's no longer a third
provoking vertex parameter like there used to be.  For flat-shaded
lines it looks like your temporarily changing the 0th vertex's color,
calling the line function, then restoring the 0th vertex color.

Why not declare a temporary vertex in the line template function,
set its color/specular/index fields to the provoking vertex's color
and pass it as a third vertex?

I see you do something similar for triangles and for two-sided lighting.

Here's an idea:

Suppose in the SWvertex struct we declare color, specular, and index
to be 2-D arrays where the first dimension is the front/back-side
index:

   GLchan color[2][4];  /* color[0] = front, color[1] = back */

Then we could pass a front/back flag to the triangle function to
tell it which color to use.

Passing a front/back flag might also be useful for culling.

Could you put some comments in the swsetup triangle template about
the edge flag code?  I'm wondering what the 0x1 and 0x2 bits are
for and why they're being modified during rendering.  It seems to
me that the vertex buffer should not be modified during rasterization.

-Brian

Re: [Mesa3d-dev] Latest changes

[Keith W. <ke...@va...>]

Brian Paul wrote:
> 
> I'm looking at the swrast line functions.  There's no longer a third
> provoking vertex parameter like there used to be.  For flat-shaded
> lines it looks like your temporarily changing the 0th vertex's color,
> calling the line function, then restoring the 0th vertex color.
>
> Why not declare a temporary vertex in the line template function,
> set its color/specular/index fields to the provoking vertex's color
> and pass it as a third vertex?

I chose this technique because it is a good match for hardware rasterizers we
already understand.  The copy/restore has proven to be a fast operation on
hardware drivers so I opted for uniformity.  

The software setup code has more copying than the hardware code for 2 reasons:

	- most hardware code doesn't copy specular.  It should do if the hardware
supports it & therefore we have some bugs in the hardware code (mga, i810,
r128).

	- no hardware drivers support color-index rendering.



> I see you do something similar for triangles and for two-sided lighting.
> 
> Here's an idea:
> 
> Suppose in the SWvertex struct we declare color, specular, and index
> to be 2-D arrays where the first dimension is the front/back-side
> index:
> 
>    GLchan color[2][4];  /* color[0] = front, color[1] = back */

I guess it's just a question of where the complexity goes.  I'm pretty happy
with the current breakdown, of a simple triangle rasterizer that just knows
about rasterizing triangles and is a good match for existing hardware
rasterizers.  Above that, a layer which understands the parts of GL state that
go beyond straightforward rasterization.  

I don't think hardware rasterizers are going to change significantly, or at
least not so that we can see - this is probably the last generation of cards
where we'll have to talk to the rasterizer directly.  



> Then we could pass a front/back flag to the triangle function to
> tell it which color to use.
> 
> Passing a front/back flag might also be useful for culling.

If you are going to pass anything it should be area, so that doesn't need
recalculation.  

	_swrast_Triangle( GLcontext *ctx, 
			  SWvertex *v0, SWvertex *v1, SWvertex *v2,
			  SWvertex *pv,
			  GLfloat area )

	_swrast_Line( GLcontext *ctx, 
		      SWvertex *v0, SWvertex *v1,
		      SWvertex *pv,
		      GLfloat area  or GLuint side )

	_swrast_Point( GLcontext *ctx,
		       SWvertex *v0,
		       SWvertex *pv,   /* need this for unfilled, flat-shaded tris */
		       GLfloat area or GLuint side )


This helps to cut out the copying of data, but it's less obvious whats going
on.  Hardware drivers should still be able to use this for fallbacks as long
as the rasterizer is no smarter than the ones we've already seen.  In
'fx_fallback_line', you don't have access to the information to calculate
area, but you can just pretend that the line is frontfacing, and fill in the
color[0] values from the hardware vertices.

Coincidentally, how does current mesa handle GL_POINT mode unfilled,
flatshaded triangles?  How is the provoking vertex passed to the point
routines?
 
In any case I don't get as good a feeling when I look at these routines as I
do when I look at the current ones.  As a stopgap, it is possible to split the
color-copying code to copy either just rgba or (rgba, spec, index) depending
on rendermode, etc. 

As a general summary of my feelings, I guess I'd like to wait & see & find out
more about this code before changing it in new directions...
			  
> Could you put some comments in the swsetup triangle template about
> the edge flag code?  I'm wondering what the 0x1 and 0x2 bits are
> for and why they're being modified during rendering.  It seems to
> me that the vertex buffer should not be modified during rasterization.

There are some old comments in vbrender.c about this.  I'm not entirely happy
with the way I'm doing edgeflags yet -- the SI makes it look really simple by
comparison.

Keith

Re: [Mesa3d-dev] Latest changes

[Brian P. <br...@va...>]

Keith Whitwell wrote:
> 
> Brian Paul wrote:
> >
> > I'm looking at the swrast line functions.  There's no longer a third
> > provoking vertex parameter like there used to be.  For flat-shaded
> > lines it looks like your temporarily changing the 0th vertex's color,
> > calling the line function, then restoring the 0th vertex color.
> >
> > Why not declare a temporary vertex in the line template function,
> > set its color/specular/index fields to the provoking vertex's color
> > and pass it as a third vertex?
> 
> I chose this technique because it is a good match for hardware rasterizers we
> already understand.  The copy/restore has proven to be a fast operation on
> hardware drivers so I opted for uniformity.
> 
> The software setup code has more copying than the hardware code for 2 reasons:
> 
>         - most hardware code doesn't copy specular.  It should do if the hardware
> supports it & therefore we have some bugs in the hardware code (mga, i810,
> r128).
> 
>         - no hardware drivers support color-index rendering.
> 
> > I see you do something similar for triangles and for two-sided lighting.
> >
> > Here's an idea:
> >
> > Suppose in the SWvertex struct we declare color, specular, and index
> > to be 2-D arrays where the first dimension is the front/back-side
> > index:
> >
> >    GLchan color[2][4];  /* color[0] = front, color[1] = back */
> 
> I guess it's just a question of where the complexity goes.  I'm pretty happy
> with the current breakdown, of a simple triangle rasterizer that just knows
> about rasterizing triangles and is a good match for existing hardware
> rasterizers.  Above that, a layer which understands the parts of GL state that
> go beyond straightforward rasterization.
> 
> I don't think hardware rasterizers are going to change significantly, or at
> least not so that we can see - this is probably the last generation of cards
> where we'll have to talk to the rasterizer directly.
> 
> > Then we could pass a front/back flag to the triangle function to
> > tell it which color to use.
> >
> > Passing a front/back flag might also be useful for culling.
> 
> If you are going to pass anything it should be area, so that doesn't need
> recalculation.
> 
>         _swrast_Triangle( GLcontext *ctx,
>                           SWvertex *v0, SWvertex *v1, SWvertex *v2,
>                           SWvertex *pv,
>                           GLfloat area )
> 
>         _swrast_Line( GLcontext *ctx,
>                       SWvertex *v0, SWvertex *v1,
>                       SWvertex *pv,
>                       GLfloat area  or GLuint side )
> 
>         _swrast_Point( GLcontext *ctx,
>                        SWvertex *v0,
>                        SWvertex *pv,   /* need this for unfilled, flat-shaded tris */
>                        GLfloat area or GLuint side )
> 
> This helps to cut out the copying of data, but it's less obvious whats going
> on.  Hardware drivers should still be able to use this for fallbacks as long
> as the rasterizer is no smarter than the ones we've already seen.  In
> 'fx_fallback_line', you don't have access to the information to calculate
> area, but you can just pretend that the line is frontfacing, and fill in the
> color[0] values from the hardware vertices.

Your initial perf numbers seem to confirm that the hit isn't too big.
We can probably optimize things later to make up for any loss, if needed.

 
> Coincidentally, how does current mesa handle GL_POINT mode unfilled,
> flatshaded triangles?  How is the provoking vertex passed to the point
> routines?

It's not.  It's broken.  I guess no one's ever noticed.


> In any case I don't get as good a feeling when I look at these routines as I
> do when I look at the current ones.  As a stopgap, it is possible to split the
> color-copying code to copy either just rgba or (rgba, spec, index) depending
> on rendermode, etc.
> 
> As a general summary of my feelings, I guess I'd like to wait & see & find out
> more about this code before changing it in new directions...
> 
> > Could you put some comments in the swsetup triangle template about
> > the edge flag code?  I'm wondering what the 0x1 and 0x2 bits are
> > for and why they're being modified during rendering.  It seems to
> > me that the vertex buffer should not be modified during rasterization.
> 
> There are some old comments in vbrender.c about this.  I'm not entirely happy
> with the way I'm doing edgeflags yet -- the SI makes it look really simple by
> comparison.

OK.


How or where should we check for feedback and selection mode in
the point/line/triangle selection functions?  Should we test for
that down in the driver's code, or higher up in the _swrast_validate_point()
function?

-Brian

Re: [Mesa3d-dev] Latest changes

[Keith W. <ke...@va...>]

Brian Paul wrote:
> 

> 
> How or where should we check for feedback and selection mode in
> the point/line/triangle selection functions?  Should we test for
> that down in the driver's code, or higher up in the _swrast_validate_point()
> function?

Ahh..  I originally left the feedback/select point/line/tri functions up in
the main directory.  In the last batch of changes I moved them down into
swrast, but I forgot to update the _swrast_choose_line(), etc. functions.  The
tests should be made there...

It'll be necessary to check the two drivers that extend swrast (x11, osmesa)
to make sure that they respect rendermode in their chooser functions, too.

Keith

Re: [Mesa3d-dev] Latest changes

[Brian P. <br...@va...>]

Keith Whitwell wrote:
> 
> Brian Paul wrote:
> >
> 
> >
> > How or where should we check for feedback and selection mode in
> > the point/line/triangle selection functions?  Should we test for
> > that down in the driver's code, or higher up in the _swrast_validate_point()
> > function?
> 
> Ahh..  I originally left the feedback/select point/line/tri functions up in
> the main directory.  In the last batch of changes I moved them down into
> swrast, but I forgot to update the _swrast_choose_line(), etc. functions. 
> The tests should be made there...
> 
> It'll be necessary to check the two drivers that extend swrast (x11, osmesa)
> to make sure that they respect rendermode in their chooser functions, too.


OK, I'm testing for feedback/selection in the XMesa driver now.
I had to tweak _swsetup_choose_rastersetup_func() as well.  As it
was, it was using a multi-texture setup function for selection/
feedback but the MULTITEX test in the setup function examines
the Texture._ReallyEnabled word.  In feedback mode, even if texture
is disabled you still have to return texture coords.
I'll check in my fix soon.

Selection mode with glPolygonMode(GL_POINT) isn't working yet.

-Brian

Re: [Mesa3d-dev] Latest changes

[Keith W. <ke...@va...>]

Brian Paul wrote:
> 

> How or where should we check for feedback and selection mode in
> the point/line/triangle selection functions?  Should we test for
> that down in the driver's code, or higher up in the _swrast_validate_point()
> function?

My first guess wasn't quite right, so I'm not sure what the problem is.  It
looks like we check & install select functions correctly, and the
samples/select demo appears roughly to work.  Can you give more details of
what you're seeing?

If I was to randomly guess again, I'd say something like

	if (ctx->RenderMode != GL_RENDER) return NULL;

was required at the top of get_line_func, get_point_func and get_triangle_func
in xm_line.c and xm_tri.c

Keith

Re: [Mesa3d-dev] Latest changes

[Brian P. <br...@va...>]

Keith Whitwell wrote:

> There are some old comments in vbrender.c about this.  I'm not entirely happy
> with the way I'm doing edgeflags yet -- the SI makes it look really simple by
> comparison.

Selection when polygonmode = point or line isn't working because
the edge flags are always zero.  Perhaps you can look into that.

-Brian

Re: [Mesa3d-dev] Latest changes

[Daryll S. <da...@va...>]

On Sun, Nov 05, 2000 at 02:46:29PM -0700, Keith Whitwell wrote:
> Brian Paul wrote:
> > 1. _swsetup_RegisterVB() returns a boolean, but in dd.h, the RegisterVB
> >    function pointer returns void.  Which should it be?
> 
> Hmm.  If it does fail (it alloc's memory, typically), I'm not sure we can go
> on.  It might as well exit on failure and return void.

Gack no. Libraries should never exit the application. You have to
propigate the failure back to the client application so it has an
opportunity to influence the behavior. 

					- |Daryll