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

[Brian Paul]

On 02/08/2012 11:47 PM, Mathias Fröhlich wrote:
> 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.

This change shouldn't make any difference in perforamcne.  It's really 
just a cleanup I spotted when doing the other changes.


> 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?

I haven't looked at exactly when the tables are recomputed, but I can 
believe there's room for improvement.

I guess I'm not too concerned about that though.  If anything, I'm 
looking at simplifying and getting rid of stuff that doesn't have much 
value nowadays.  In the case of the lighting changes, I stumbled 
across that when I looked at the size of gl_context and found that 35% 
of it was the seldom-used spot light exponent tables.

But if you have some optimizations in mind that'll help real apps, 
that's fine.


> 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>

Thanks.

-Brian