[Mesa-dev] [PATCH 18/20] mesa: Rewrite the way uniforms are tracked and handled

[Eric Anholt]

On Thu, 5 Jan 2012 01:31:22 +0100, Marek Olšák <maraeo at gmail.com> wrote:
> On Thu, Jan 5, 2012 at 12:45 AM, Eric Anholt <eric at anholt.net> wrote:
> > On Wed, 04 Jan 2012 03:28:12 -0800, Kenneth Graunke <kenneth at whitecape.org> wrote:
> >> On 01/03/2012 09:43 PM, Marek Olšák wrote:
> >> > This ProgramStringNotify call causes excessive shader recompilations
> >> > in the game Cogs (from some Humble Bundle). Approximately 25% of
> >> > in-game CPU time is spent on translating shaders and register
> >> > allocation. If the shaders were larger, it would be much worse.
> >> > Moreover, I have realized calling ProgramStringNotify is not needed
> >> > for st/mesa in this case at all (I just comment it out if I want
> >> > higher frame rates), because it doesn't use the prog->SamplerUnits
> >> > array when translating shaders. The array is only used when we set
> >> > samplers, so setting _NEW_TEXTURE is sufficient. It seems that i965
> >> > could do the same thing. Another option would be to cache precompiled
> >> > shaders, but that seems more complicated than doing what st/mesa does.
> >> >
> >> > Marek
> >>
> >> Yeah, I noticed that a while back too, but we never arrived at a
> >> solution and it slipped off my radar.  I agree completely,
> >> ProgramStringNotify is completely overkill and bogus.
> >>
> >> We actually already rely on _NEW_TEXTURE for all shader programs.  Since
> >> the uniform code already flags _NEW_TEXTURE, we should be able to just
> >> add the SamplerUnits array to the program key (specifically,
> >> brw_sampler_prog_key_data), and that should take care of any state
> >> dependent recompiles.  Then we wouldn't need ProgramStringNotify at all.
> >>
> >> Right?
> >
> > Well, since the data stored is part of the program, and when just
> > changing programs _NEW_TEXTURE wouldn't get set, I think _NEW_PROGRAM is
> > the flag you mean.  That would make a ton of sense to me -- if you
> > depend on SamplerUnits for your program handling, go look at it when
> > _NEW_PROGRAM is flagged.
> 
> I don't think this is about what makes sense in theory, but how each
> driver handles SamplerUnits. st/mesa just rebinds textures, so it
> expects _NEW_TEXTURE. i965 recompiles the fragment program, so it
> expects _NEW_PROGRAM. The worst option would be to set both, but at
> the same time it's the only option which would work for everybody now.

So when someone changes from a program using samplerunits bound one way
to a program using samplerunits bound another way, are you expecting
glUseProgram to flag _NEW_TEXTURE?  Because it doesn't.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20120105/a0d116de/attachment-0001.pgp>