Regression in patch wined3d: Implement D3DSIO_MOVAin ARB backend.

Aric Cyr Aric.Cyr at gmail.com
Mon Oct 30 03:26:13 CST 2006


Ivan Gyurdiev <ivg231 <at> gmail.com> writes:

> that's because Jason seems to have introduced order requirements on the 
> table that weren't previously there. The mnxn implementation will 
> attempt to access the table as an array, which will break once MOVA is 
> moved before DP3:
> 
> glsl_shader.c:            tmpArg.opcode = 
> &IWineD3DVertexShaderImpl_shader_ins[WINED3DSIO_DP4];
> glsl_shader.c:            tmpArg.opcode = 
> &IWineD3DVertexShaderImpl_shader_ins[WINED3DSIO_DP3];

Yup, seems like the temptation to dereference the array directly was too great.

These should be replaced to use baseshader.c:get_shader_opcode(...), I think,
instead of the array access.  Exact same thing also in arb_program_shader.c too.

To prevent such things in the future, is there any way to remove the enum
WINED3DSHADER_INSTRUCTION_OPCODE_TYPE and place it somewhere less visible that
the included-everywhere wined3d_private_types.h?  A quick glance at the code
seems to suggest it is not feasible right now, but maybe someone has an idea?

Or better yet, since device.c is the only legitimate user of
IWineD3DVertexShaderImpl_shader_ins[], don't extern that array in
wined3d_private.h, but only in device.c (i.e. move the extern declaration from
wined3d_private.h to device.c).  Then glsl_shader.c and arb_program_shader.c and
future source would properly fail to compile on this type of mistake.

In the worst case a comment to the effect of
  /* don't dereference this array directly */ 
certainly seems in order...

- Aric




More information about the wine-devel mailing list