Re: [WINED3D] Improve pshader code structure

[Vitaliy Margolen]

Wednesday, April 12, 2006, 4:02:58 PM, Ivan Gyurdiev wrote:
> The point of this patch is to pull out things like modifier processing 
> (write masks, saturate, swizzle) from different case statements, and put
> them in a more common path, which simplifies code, and makes sure we 
> don't miss cases. Also, reorder things as appropriate to get rid of the
> crazy continue/switch flow control that's confusing me. Dead code paths
> were also if0'ed.
> Patch introduces undesired effect that now input and output modifiers 
> are allowed on all instructions, which is definitely not valid according
> to MSDN. However, I don't think we should care at this point, since 
> processing of valid shaders, and simplifying the code path should take 
> precedence. We can add validation later in a more appropriate place [ 
> the current "validation" is to completely disable or enable modifiers on
> certain instructions, which is not correct either - and it should fail,
> not just print a fixme on error ].
> Patch will enable processing _bx2 modifier on certain texture 
> instructions, which is valid in shaders 1.2/1.3. Also, is negation and 
> swizzling valid on texture instructions in 3.0? If it is, this will 
> process it. Also will enable write mask processing in some texture 
> instructions where it previously wasn't, generating more correct shader.

I'm afraid this page changes too much at one go. You should split it into
separate pieces. For example:
>                  TRACE("Found opcode D3D:%s GL:%s, PARAMS:%d, \n",
> -
>                curOpcode->name, curOpcode->glname, curOpcode->num_params);
> -
>                curOpcode->name, curOpcode->glname, curOpcode->num_params - 1);

should be one patch. Btw why are you adding -1 here?

This could be turned into one big patch that is no-op:
>                  /* Cubemap textures will be more used than 3D ones. */
> -
>                sprintf(tmpLine, "TEX T%lu, TMP, texture[%lu], CUBE;\n", reg, reg);
> -
>                sprintf(tmpLine, "TEX T%lu, TMP, texture[%lu], CUBE;\n", output, output);

Vitaliy Margolen

[WINED3D] Improve pshader code structure

[Ivan Gyurdiev]

> I'm afraid this page changes too much at one go. You should split it into
> separate pieces. For example:

I can break out some unrelated pieces of it as you point out, but once 
we start moving code around it seems like all of it should be moved in 
the right place, or the result wouldn't make any sense [ i.e if you 
process tokens at the beginning, then code that depends on looking at 
the individual tokens should go into the beginning ]. Does the patch 
cause a regression for you?
> >                  TRACE("Found opcode D3D:%s GL:%s, PARAMS:%d, \n",
> > -
> >                curOpcode->name, curOpcode->glname, curOpcode->num_params);
> > -
> >                curOpcode->name, curOpcode->glname, curOpcode->num_params - 1);
>
> should be one patch. Btw why are you adding -1 here?

That's fixing a minor bug I introduced w/ last patch when I moved the 
trace. numParams includes the destination register, it seems better not 
to print it out, esp since later when we loop over the parameters we 
don't list it, so it looks like a parameter is missing.
> This could be turned into one big patch that is no-op:
> >                  /* Cubemap textures will be more used than 3D ones. */
> > -
> >                sprintf(tmpLine, "TEX T%lu, TMP, texture[%lu], CUBE;\n", reg, reg);
> > -
> >                sprintf(tmpLine, "TEX T%lu, TMP, texture[%lu], CUBE;\n", output, output);

What was the question here?