Regression in patch wined3d: Implement D3DSIO_MOVA in ARB backend.
Hi, i just installed latest version of wine and now 3DMark 2003 and Flatout 2 is not working anymore, i look at the patches commited yesterday and found this patch which is causing this regression. Mirek
Am Samstag 28 Oktober 2006 10:35 schrieb Mirek:
Hi, i just installed latest version of wine and now 3DMark 2003 and Flatout 2 is not working anymore, i look at the patches commited yesterday and found this patch which is causing this regression. In which way do they not work any more? Is there any debug output, or can you provide screenshots?
Can you test ARB and glsl shaders?
Hi, screen is just black in those two apps, other apps (3DMark 2001, 2005, 2006, NFS MW, Oblivion, TRL, W3, WoW, Prey, Starcraft, Diablo 2, GTA SA, HL 2 EO, HL, NFS 3, NFS 4, Turok, Ignition, Carmageddon 2, Carmageddon 3, Pandemonium 2) works without any problem. Here is screenshot from 3DMark 2003 with GLSL enabled and disabled: http://www.czela.net/pub/Czela_Debian_2.5/mirek/wine/regression/3DMark2003_A... http://www.czela.net/pub/Czela_Debian_2.5/mirek/wine/regression/3DMark2003_G... My software: Debian Sid, kernel 2.6.18.1 SMP, nVidia 9625 drivers. My hardware: Core 2 Extreme, GF 6800 GS, 1GB RAM Do you need anything else? Mirek Stefan Dösinger napsal(a):
Am Samstag 28 Oktober 2006 10:35 schrieb Mirek:
Hi, i just installed latest version of wine and now 3DMark 2003 and Flatout 2 is not working anymore, i look at the patches commited yesterday and found this patch which is causing this regression.
In which way do they not work any more? Is there any debug output, or can you provide screenshots?
Can you test ARB and glsl shaders?
------------------------------------------------------------------------
The MOVA instruction is a 2.0 instruction, which cannot be fully implemented in ARB. However, the patch shouldn't have caused a regression - if you're seeing a black screen with GLSL also, that means another patch is responsible for this.
I tried it three times with same result. Can someone else test if 3DMark 2003 is working with latest wine? Mirek Ivan Gyurdiev napsal(a):
The MOVA instruction is a 2.0 instruction, which cannot be fully implemented in ARB. However, the patch shouldn't have caused a regression - if you're seeing a black screen with GLSL also, that means another patch is responsible for this.
Mirek wrote:
I tried it three times with same result. Can someone else test if 3DMark 2003 is working with latest wine?
You shouldn't test if it's working now - test if it's working before the MOVA patch, and if it's not working after the MOVA patch [ no other patches in between ].
Yes I did that this way. I tried it three times as it is now in CVS (3DMark 2003 and Flatout 2 not working) and three times without this patch (only) (3DMark 2003 and Flatout 2 works just perfect). Mirek Ivan Gyurdiev napsal(a):
Mirek wrote:
I tried it three times with same result. Can someone else test if 3DMark 2003 is working with latest wine?
You shouldn't test if it's working now - test if it's working before the MOVA patch, and if it's not working after the MOVA patch [ no other patches in between ].
On 10/28/06, Mirek <thunder.m(a)czela.net> wrote:
Yes I did that this way. I tried it three times as it is now in CVS (3DMark 2003 and Flatout 2 not working) and three times without this patch (only) (3DMark 2003 and Flatout 2 works just perfect).
Mirek
I see the same thing as Mirek, 3dmark03 is totally broken now. Tom
Tom Wickline wrote:
I see the same thing as Mirek, 3dmark03 is totally broken now.
And yes I tested GLSL and ARB and before and after with the same results as his. So yes, this is a major regression :D When testing with the GLSL backend, can you please do a +d3d_shader trace, find the MOVA instruction, and send me the entire shader that it's part of (most importantly the shader version). The only way I see that this patch could break the GLSL backend is if the instruction version check is wrong. MSDN claims this instruction is available in shaders 2.0 and higher, but it wouldn't be the first time MSDN is dead wrong.
Here is log with 3DMark running for about 3 seconds with patch and without, trace+d3d. http://www.czela.net/pub/Czela_Debian_2.5/mirek/wine/regression/3DMark03.tar... Mirek Ivan Gyurdiev napsal(a):
Tom Wickline wrote:
I see the same thing as Mirek, 3dmark03 is totally broken now.
And yes I tested GLSL and ARB and before and after with the same results as his. So yes, this is a major regression :D
When testing with the GLSL backend, can you please do a +d3d_shader trace, find the MOVA instruction, and send me the entire shader that it's part of (most importantly the shader version). The only way I see that this patch could break the GLSL backend is if the instruction version check is wrong. MSDN claims this instruction is available in shaders 2.0 and higher, but it wouldn't be the first time MSDN is dead wrong.
Here is log with 3DMark running for about 3 seconds with patch and without, trace+d3d.
http://www.czela.net/pub/Czela_Debian_2.5/mirek/wine/regression/3DMark03.tar...
There's no mova instruction in that log.
Ivan Gyurdiev wrote:
Here is log with 3DMark running for about 3 seconds with patch and without, trace+d3d.
http://www.czela.net/pub/Czela_Debian_2.5/mirek/wine/regression/3DMark03.tar...
There's no mova instruction in that log.
...grep for mova, or for "unrecognized" or "Unrecognized" or "unhandled" or "Unhandled". I see the app uses a number of vertex shaders 1.1, so maybe mova is a valid 1.1 instruction.
Ok, i just found why is it not working with this patch, and why is it working without this patch. It is based on position of definition MOVA in vertexshader.c, if MOVA is on "after patch" position it is not working, but if i move line " {WINED3DSIO_MOVA, "mova", NULL, 1, 2, vshader_mova, vshader_hw_map2gl, shader_glsl_mov, WINED3DVS_VERSION(2,0), -1}," to position of definition before patch, evrything is working. Mirek Ivan Gyurdiev napsal(a):
Here is log with 3DMark running for about 3 seconds with patch and without, trace+d3d.
http://www.czela.net/pub/Czela_Debian_2.5/mirek/wine/regression/3DMark03.tar...
There's no mova instruction in that log.
Mirek wrote:
Ok, i just found why is it not working with this patch, and why is it working without this patch. It is based on position of definition MOVA in vertexshader.c, if MOVA is on "after patch" position it is not working, but if i move line " {WINED3DSIO_MOVA, "mova", NULL, 1, 2, vshader_mova, vshader_hw_map2gl, shader_glsl_mov, WINED3DVS_VERSION(2,0), -1}," to position of definition before patch, evrything is working. ... 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]; The instruction table currently does not support this, and it only happens to work for the first few instructions [ which the MOVA change breaks ].
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
participants (5)
-
Aric Cyr -
Ivan Gyurdiev -
Mirek -
Stefan Dösinger -
Tom Wickline