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.froehl...@web.de>

Thanks.

-Brian
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to