<div dir="ltr">On 10 January 2014 11:41, Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>> wrote: <div class="gmail_extra"><div class="gmail_quote"> <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class=""><div class="h5">On 01/09/2014 06:19 PM, Paul Berry wrote: > This is possible now that ctx->Shader.CurrentProgram is an array. > --- > src/mesa/main/texstate.c | 75 +++++++++++++++++++----------------------------- > 1 file changed, 29 insertions(+), 46 deletions(-) > > diff --git a/src/mesa/main/texstate.c b/src/mesa/main/texstate.c > index b9c76da..905a9d5 100644 > --- a/src/mesa/main/texstate.c > +++ b/src/mesa/main/texstate.c > @@ -526,27 +526,20 @@ static void > update_texture_state( struct gl_context *ctx ) > { > GLuint unit; > - struct gl_program *fprog = NULL; > - struct gl_program *vprog = NULL; > - struct gl_program *gprog = NULL; > + struct gl_program *prog[MESA_SHADER_STAGES]; > GLbitfield enabledFragUnits = 0x0; > - > - if (ctx->Shader.CurrentProgram[MESA_SHADER_VERTEX] && > - ctx->Shader.CurrentProgram[MESA_SHADER_VERTEX]->LinkStatus) { > - vprog = ctx->Shader.CurrentProgram[MESA_SHADER_VERTEX]->_LinkedShaders[MESA_SHADER_VERTEX]->Program; > - } > - > - if (ctx->Shader.CurrentProgram[MESA_SHADER_GEOMETRY] && > - ctx->Shader.CurrentProgram[MESA_SHADER_GEOMETRY]->LinkStatus) { > - gprog = ctx->Shader.CurrentProgram[MESA_SHADER_GEOMETRY]->_LinkedShaders[MESA_SHADER_GEOMETRY]->Program; > - } > - > - if (ctx->Shader.CurrentProgram[MESA_SHADER_FRAGMENT] && > - ctx->Shader.CurrentProgram[MESA_SHADER_FRAGMENT]->LinkStatus) { > - fprog = ctx->Shader.CurrentProgram[MESA_SHADER_FRAGMENT]->_LinkedShaders[MESA_SHADER_FRAGMENT]->Program; > - } > - else if (ctx->FragmentProgram._Enabled) { > - fprog = &ctx->FragmentProgram.Current->Base; > + int i; > + > + for (i = 0; i < MESA_SHADER_STAGES; i++) { > + if (ctx->Shader.CurrentProgram[i] && > + ctx->Shader.CurrentProgram[i]->LinkStatus) { > + prog[i] = ctx->Shader.CurrentProgram[i]->_LinkedShaders[i]->Program; > + } else { > + if (i == MESA_SHADER_FRAGMENT && ctx->FragmentProgram._Enabled) > + prog[i] = &ctx->FragmentProgram.Current->Base; > + else > + prog[i] = NULL; > + } > } > > /* TODO: only set this if there are actual changes */ > @@ -562,9 +555,7 @@ update_texture_state( struct gl_context *ctx ) > */ > for (unit = 0; unit < ctx->Const.MaxCombinedTextureImageUnits; unit++) { > struct gl_texture_unit *texUnit = &ctx->Texture.Unit[unit]; > - GLbitfield enabledVertTargets = 0x0; > - GLbitfield enabledFragTargets = 0x0; > - GLbitfield enabledGeomTargets = 0x0; > + GLbitfield enabledTargetsByStage[MESA_SHADER_STAGES]; > GLbitfield enabledTargets = 0x0; > GLuint texIndex; > > @@ -574,25 +565,16 @@ update_texture_state( struct gl_context *ctx ) > * by a fragment program/program. When multiple flags are set, we'll > * settle on the one with highest priority (see below). > */ > - if (vprog) { > - enabledVertTargets |= vprog->TexturesUsed[unit]; > + for (i = 0; i < MESA_SHADER_STAGES; i++) { > + if (prog[i]) > + enabledTargetsByStage[i] = prog[i]->TexturesUsed[unit]; > + else if (i == MESA_SHADER_FRAGMENT) > + enabledTargetsByStage[i] = texUnit->Enabled; > + else > + enabledTargetsByStage[i] = 0; > + enabledTargets |= enabledTargetsByStage[i]; </div></div>This doesn't look equivalent. Previously, enabled{Vert,Frag,Geom}Targets would accumulate bits over every iteration through units. Now, enabledTargetsByStage only reflects the most recent unit. enabledTargets still accumulates properly, but you use enabledTargetsByStage below, so I think it needs to as well. You just need to use |= in both places, like the old code. </blockquote><div> </div><div>It's definitely possible that I got something wrong in this patch--I didn't completely grok what the code was doing and instead strived to just preserve the existing semantics. But I'm pretty sure I got the enabledTargetsByStage part right. Here's what the code used to look like--the enabled{Vert,Frag,Geom}Targets variables were all reset to 0 every time through the loop: for (unit = 0; unit < ctx->Const.MaxCombinedTextureImageUnits; unit++) { struct gl_texture_unit *texUnit = &ctx->Texture.Unit[unit]; GLbitfield enabledVertTargets = 0x0; GLbitfield enabledFragTargets = 0x0; GLbitfield enabledGeomTargets = 0x0; GLbitfield enabledTargets = 0x0; GLuint texIndex; /* Get the bitmask of texture target enables. * enableBits will be a mask of the TEXTURE_*_BIT flags indicating * which texture targets are enabled (fixed function) or referenced * by a fragment program/program. When multiple flags are set, we'll * settle on the one with highest priority (see below). */ if (vprog) { enabledVertTargets |= vprog->TexturesUsed[unit]; } if (gprog) { enabledGeomTargets |= gprog->TexturesUsed[unit]; } if (fprog) { enabledFragTargets |= fprog->TexturesUsed[unit]; } else { /* fixed-function fragment program */ enabledFragTargets |= texUnit->Enabled; } enabledTargets = enabledVertTargets | enabledFragTargets | enabledGeomTargets; ... } </div><div>I don't know if that was intentional or a bug, but I'm pretty sure my patch preserves the behaviour. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> Also, I just want to say - this code is absurdly confusing. Always has been. I don't envy you having to touch it. :) </blockquote><div> </div><div>You can say that again :) Hopefully my patch doesn't make the situation any worse. </div></div></div></div>