[Mesa-dev] [PATCH 3/3] mesa: replace GET_SHINE_TAB_ENTRY() macro with an inline function

[Mathias Fröhlich]

Hi Brian,

On Wednesday, February 08, 2012 20:13:44 Brian Paul wrote:
> ---
>  src/mesa/main/light.h        |   32 +++++++++++++++++---------------
>  src/mesa/tnl/t_rasterpos.c   |    3 +--
>  src/mesa/tnl/t_vb_lighttmp.h |   27 +++++++--------------------
>  3 files changed, 25 insertions(+), 37 deletions(-)

I could see revalidating those tables high in some profiles on r600g.
So, this is really good to see.

Anyway, I can see some further improovement:
Given the above file list you can see that the table lookup is only used from 
rasterpos and the classic tnl pipeline.
Gallium still needs the rasterpos api function. So, we cannot push the shine 
table just into the tnl code.
But, the tables are almost never used in a typical fixed function OpenGL 
program using gallium. Even though they are recomputed on normal state 
validation if the shininess has changed, which can happen quite often in 
typical traditional OpenGL program.

For that it would help even more for execution time if computation of the 
shine table is delayed until we really make use of the tables.
I think revalidating and may be recomputation of the tables could be delayed 
until we call rasterpos, where the extra check for a table validity should not 
hurt given this functions complexity. Or until the the classic tnl pipeline is 
really run, where shine table revalidation could be put somewhere into tnl 
state validation?

Other than the above possible improovement, and if I am allowed to say that:
For the series
Reviewed-by: Mathias Fröhlich <Mathias.Froehlich at web.de>

Mathias