http://bugs.winehq.org/show_bug.cgi?id=17715
Summary: Incorrect translation of D3D asm instruction "expp" Product: Wine Version: 1.1.17 Platform: PC-x86-64 OS/Version: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: directx-d3d AssignedTo: wine-bugs@winehq.org ReportedBy: liquid.acid@gmx.net CC: stefandoesinger@gmx.at
Hi there,
currently wined3d (I only tested this in ARB mode, but it might also affect GLSL mode) the D3D assembler instruction "expp" is incorrectly translated to ARBvp assembler language.
wined3d takes this D3D asm line "expp r3.y, r3" and translates it into "EXP R3.y, R3;". Well, this isn't working.
EXP is of scalarop type, the first parameter being of "masked destination register" type (masking is used in this case above) and the second parameter (here lies the problem) is of type "scalar source register".
Sadly R3 is a vector :(
Well, let's add there full shader source for completeness: -------------------------------------------- vs_1_0 //D3DX8 Shader Assembler Version 0.91 mov r0, v0 add r1, r0, -c85 dp3 r1, r1, r1 rsq r1.x, r1.y mov r3, r0 dp3 r3.w, r3, r3 rsq r3.w, r3.w mul r7, r3, r3.w mul r1.x, r1.x, r1.y mad r2.x, r1.x, c86.x, c86.y slt r4, r2.x, c0 mul r4, r4, -c83.w max r2.x, r2.x, -r2.x add r2.x, r4.z, r2.x mul r3, r2.x, c83.y expp r3.y, r3 mad r3, r3.y, c83.z, c83.w mul r2, r3.x, r3.x mul r3.y, r2.x, r3.x mul r3.z, r2.x, r3.y mul r3.w, r2.x, r3.z dp4 r2, c82, r3 mul r2, r2, c86.z mul r1.x, r1.x, c86.w add r1.x, c1, -r1.x max r1.x, c0, r1.x mad r0.y, r2, r1.x, r0.y dp4 oPos.x, r0, c2 dp4 oPos.y, r0, c3 dp4 oPos.z, r0, c4 dp4 oPos.w, r0, c5 mov oT0, v3 mov oT1, v3 mov oT2, v3 mov oT3, v3 mov oD0, c1 mov oFog.x, c0 --------------------------------------------
First of all, according to the current MSDN the instruction used above in the source is NOT valid.
See this: http://msdn.microsoft.com/en-us/library/bb173373(VS.85).aspx
They explicitly state "expp dst, src.{x|y|z|w}" as the syntax. They furthermore mention: ------------QUOTE--------------------------- src is a source register. Source register requires explicit use of replicate swizzle, that is, exactly one of the .x, .y, .z, .w swizzle components (or the .r, .g, .b, .a equivalents) must be specified. ------------UNQUOTE--------------------------
Well, this comment about the src reg doesn't seem to be valid at all....
Let's just take a look at the original D3D8 documentation ("D3DX8 Shader Assembler Version 0.91" <- !!!).
To fully quote this: ------------------------------------------------------ expp
Provides exponential 2x partial support.
Syntax: expp vDest, vSrc0
Registers: vDest: Destination register, holding the result of the operation. vSrc0: Source register, specifying the input argument.
Operation: The following code fragment shows the operations performed by the expp instruction to write a result to the destination. SetDestReg(); SetSrcReg(0);
float w = m_Source[0].w; float v = (float)floor(m_Source[0].w);
m_TmpReg.x = (float)pow(2, v); m_TmpReg.y = w - v;
// Reduced precision exponent float tmp = (float)pow(2, w); DWORD tmpd = *(DWORD*)&tmp & 0xffffff00;
m_TmpReg.z = *(float*)&tmpd; m_TmpReg.w = 1;
WriteResult();
Remarks: The expp instruction produces undefined results if fed a negative value for the exponent. This instruction provides exponential base 2 partial precision. It generates an approximate answer in vDest.z and allows for a more accurate determination of vDest.x*function(vDest.y), where function is a user approximation to 2*vDest.y over the limited range (0.0 <= vDest.y < 1.0). This instruction accepts a scalar source, and reduced precision arithmetic is acceptable in evaluating vDest.z. However, the approximation error must be less than 1/(211) the absolute error (10-bit precision) and over the range (0.0 <= t.y < 1.0). Also, expp returns 1.0 in w. The following example illustrates how the expp instruction might be used. expp r5, r0 ------------------------------------------------------
So, the correct translation should be "EXP R3.y, R3.w;"
CCing Stefan Dösinger and Henri Verbeet.
I tried to patch this myself, but it looks like that "expp" has to be moved out of shader_hw_map2gl to be able to do such an adjustement (but maybe not?).
http://bugs.winehq.org/show_bug.cgi?id=17715
Tobias Jakobi liquid.acid@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |hverbeet@gmail.com
--- Comment #1 from Tobias Jakobi liquid.acid@gmx.net 2009-03-13 18:28:09 --- I had success by moving "expp" handling into vshader_hw_rsq_rcp (renaming to vshader_hw_rsq_rcp_expp), but I still wonder if I also have to change the GLSL code and if this patch requieres a testcase.
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #2 from H. Verbeet hverbeet@gmail.com 2009-03-13 22:48:09 --- A test certainly wouldn't hurt, that also makes it easier to verify if GLSL behaves correctly or not.
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #3 from Tobias Jakobi liquid.acid@gmx.net 2009-03-14 10:09:54 --- OK Henri, then I'll take a look at the d3d8 tests and check whether I can derive a test for expp from the existing ones. :)
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #4 from Tobias Jakobi liquid.acid@gmx.net 2009-03-14 12:35:23 --- Created an attachment (id=19941) --> (http://bugs.winehq.org/attachment.cgi?id=19941) expp testcase
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #5 from Tobias Jakobi liquid.acid@gmx.net 2009-03-14 12:37:22 --- Created an attachment (id=19942) --> (http://bugs.winehq.org/attachment.cgi?id=19942) fixes expp handling in ARB mode
GLSL seems to be broken too. Also Stefan mentioned that this expp syntax was only allowed in d3d8 and gets "filtered" in d3d9, however I didn't find the code that does that.
Plus I need someone to run the testcase on a windows system.
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #6 from Rico kgbricola@web.de 2009-03-15 05:15:11 --- Result of the test on winxp sp3 with ati radeon 9600 mobility and on winxp sp2 with geforce 8800GT (both had the same result):
# with expp patch
d3d8_crosstest.exe visual
visual.c:1024: Tests skipped: D3DFMT_P8 textures not supported
visual: 300 tests executed (0 marked as todo, 0 failures), 1 skipped.
# without expp patch
d3d8_crosstest.exe visual
visual.c:934: Tests skipped: D3DFMT_P8 textures not supported
visual: 292 tests executed (0 marked as todo, 0 failures), 1 skipped.
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #7 from Tobias Jakobi liquid.acid@gmx.net 2009-03-15 07:37:55 --- Looks good! Means the test passes fine on native XP.
Now the only thing that's missing is a patch for the GLSL path.
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #8 from Tobias Jakobi liquid.acid@gmx.net 2009-03-15 07:41:46 --- Created an attachment (id=19961) --> (http://bugs.winehq.org/attachment.cgi?id=19961) [wined3d] fix expp shader asm instr also for GLSL
Someone should take a look if this is the right way to emulate the different behaviour for D3D8.
I'm just passing different writemasks (WINED3DSP_WRITEMASK_3 for <vs_2_0 and WINED3DSP_WRITEMASK_0 else) to shader_glsl_add_src_param.
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #9 from H. Verbeet hverbeet@gmail.com 2009-03-15 11:57:53 --- You have some unrelated changes in there, but looks good in principle.
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #10 from Tobias Jakobi liquid.acid@gmx.net 2009-03-15 13:23:38 --- You mean the constification of mask_size and write_mask? I can leave that out, but it looks reasonable to me to const these two (and merge the declaration and initialization).
So, should I submit the patches? Or wait until Stefan gives his OK?
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #11 from Tobias Jakobi liquid.acid@gmx.net 2009-03-17 18:35:32 --- Note to me: Split out the constifications into a separate patch.
Submit the d3d8 expp testcace together with the expp ARB fix and expp GLSL fix when Alexandre has returned from vacation.
Write another d3d9 testcase in the meanwhile that proves that d3d9 doesn't behave like version 8 did (prove this for expp and also rsq + rsq - check if d3d9 makes any differences when expp is used in a vs_1_1 versus a vs_2_0 environment).
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #12 from Tobias Jakobi liquid.acid@gmx.net 2009-03-19 15:14:20 --- Created an attachment (id=20023) --> (http://bugs.winehq.org/attachment.cgi?id=20023) [d3d9] beta testcase for expp instr in vs_1_0 and vs_2_0
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #13 from Tobias Jakobi liquid.acid@gmx.net 2009-03-19 15:17:35 --- @Rico: Would be nice if you could test this on your windows box! :)
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #14 from Rico kgbricola@web.de 2009-03-19 16:18:33 --- Created an attachment (id=20025) --> (http://bugs.winehq.org/attachment.cgi?id=20025) test log
There is at least one error in the test (tested on XP sp2 geforce 8800GT):
visual.c:4394: Test failed: IDirect3DDevice9_CreateVertexShader returned 8876086c
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #15 from Tobias Jakobi liquid.acid@gmx.net 2009-03-19 19:32:19 --- Hmm, strange.... I'll try to figure out what goes wrong there.
I never though this particular test would fail...
http://bugs.winehq.org/show_bug.cgi?id=17715
Tobias Jakobi liquid.acid@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Attachment #19941|0 |1 is obsolete| | Attachment #19942|0 |1 is obsolete| | Attachment #19961|0 |1 is obsolete| | Attachment #20023|0 |1 is obsolete| |
--- Comment #16 from Tobias Jakobi liquid.acid@gmx.net 2009-03-22 10:10:35 --- Created an attachment (id=20063) --> (http://bugs.winehq.org/attachment.cgi?id=20063) [1] [wined3d] ARB: fix handling of expp shader asm instr
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #17 from Tobias Jakobi liquid.acid@gmx.net 2009-03-22 10:11:10 --- Created an attachment (id=20064) --> (http://bugs.winehq.org/attachment.cgi?id=20064) [2] [wined3d] GLSL: fix handling of expp shader asm instr
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #18 from Tobias Jakobi liquid.acid@gmx.net 2009-03-22 10:11:44 --- Created an attachment (id=20065) --> (http://bugs.winehq.org/attachment.cgi?id=20065) [3] [wined3d] constify write_mask and mask_size in shader_glsl_expp
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #19 from Tobias Jakobi liquid.acid@gmx.net 2009-03-22 10:12:15 --- Created an attachment (id=20066) --> (http://bugs.winehq.org/attachment.cgi?id=20066) [4] [d3d8] add testcase from expp asm instruction
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #20 from Tobias Jakobi liquid.acid@gmx.net 2009-03-22 10:13:19 --- Created an attachment (id=20067) --> (http://bugs.winehq.org/attachment.cgi?id=20067) [5] [d3d8] constify some variables in the visual test
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #21 from Tobias Jakobi liquid.acid@gmx.net 2009-03-22 10:13:46 --- Created an attachment (id=20068) --> (http://bugs.winehq.org/attachment.cgi?id=20068) [6] [d3d9] add testcase for expp asm instruction
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #22 from Tobias Jakobi liquid.acid@gmx.net 2009-03-22 15:00:38 --- @Rico: More stuff for you to test! :)
I think this one is final. I already let someone test this on a Vista box and it passes. Will submit it when Alexandre is back to commiting patches to git.
The only thing that remains is doing a small testcase for rsq and rcp.
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #23 from Rico kgbricola@web.de 2009-03-22 16:50:58 --- The d3d9 test is also fine now on my xp box.
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #24 from Tobias Jakobi liquid.acid@gmx.net 2009-03-22 17:32:34 --- Thanks Rico!
Going to submit the patchset then and the remaining tests later.
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #25 from H. Verbeet hverbeet@gmail.com 2009-03-22 17:37:33 --- (In reply to comment #10)
You mean the constification of mask_size and write_mask? I can leave that out, but it looks reasonable to me to const these two (and merge the declaration and initialization).
The usefulness of making non-pointer/array variables const is a bit questionable.
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #26 from Tobias Jakobi liquid.acid@gmx.net 2009-03-22 18:43:09 --- Could you elaborate on that?
How are plain variables and non-array types different from pointers and arrays? I have started with C++ myself and therefore I always try to constify most of my variables, of course when it makes sense to do so.
AFAIK constifying also gives the compiler more hints where to optimize code. Plus it helps against programming errors ("=" against "=="). Of course the programming errors argument doesn't really apply here because the function is very small.
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #27 from Austin English austinenglish@gmail.com 2009-09-22 14:00:32 --- Is this still an issue in current (1.1.29 or newer) wine?
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #28 from Tobias Jakobi liquid.acid@gmx.net 2009-10-03 04:24:29 --- As far as I can see the issue in the ARB path was already fixed by stefand, but the problem still holds for the GLSL path.
I'm currently very busy with my diploma and I'm also moving house, so no time for working on wine, sry :(
http://bugs.winehq.org/show_bug.cgi?id=17715
joaopa jeremielapuree@yahoo.fr changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |jeremielapuree@yahoo.fr
--- Comment #29 from joaopa jeremielapuree@yahoo.fr 2010-12-08 00:07:59 CST --- Is this still an issue in current (1.3.8 or newer) wine?
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #30 from Tobias Jakobi liquid.acid@gmx.net 2010-12-11 15:10:49 CST --- Looking at the sourcecode of a recent git master checkout, this issue is still present. While the code in glsl_shader.c differentiates between 1.x and 2.0 shader mode, the code in arb_program_shader.c still doesn't do this.
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #31 from joaopa jeremielapuree@yahoo.fr 2011-06-09 13:12:26 CDT --- still a bug in current git?
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #32 from Tobias Jakobi liquid.acid@gmx.net 2011-06-09 14:01:10 CDT --- Yes, the issue is still there.
You can also check this yourself. See: (i) shader_glsl_expp in dlls/wined3d/glsl_shader.c Notice the different handling of 1.x and 2.0 shaders.
(ii) shader_hw_scalar_op in dlls/wined3d/arb_program_shader.c There is no such code.
http://bugs.winehq.org/show_bug.cgi?id=17715
Frédéric Delanoy frederic.delanoy@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |frederic.delanoy@gmail.com
--- Comment #33 from Frédéric Delanoy frederic.delanoy@gmail.com 2012-03-24 19:34:22 CDT --- Still a bug in current (1.4 or later) git ?
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #34 from Tobias Jakobi liquid.acid@gmx.net 2012-03-24 20:15:46 CDT --- Read my comment #32
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #35 from Frédéric Delanoy frederic.delanoy@gmail.com 2012-03-25 07:07:43 CDT --- (In reply to comment #34)
Read my comment #32
You can't expect everyone to do the original poster's job of trying to reproduce the issue in different wine versions. Some do, but that's no guarantee. If you want your bug fixed, some cooperation is necessary...
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #36 from Tobias Jakobi liquid.acid@gmx.net 2012-03-25 07:46:10 CDT --- However I can expect something as simple as having a quick look into the respective source files. Which you apparantly haven't bothered to do at all.
It's so much easier to just dump the usual "Still a bug in current (x.y. or later) git ?" message everywhere, and then just move on, isn't it? ;)
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #37 from Frédéric Delanoy frederic.delanoy@gmail.com 2012-03-25 08:25:44 CDT --- (In reply to comment #36)
However I can expect something as simple as having a quick look into the respective source files. Which you apparantly haven't bothered to do at all.
Your bug is not the only one in existence, no the only one I triage. If it's so quick you could have done it yourself, instead of spending time to write two comments.
It's so much easier to just dump the usual "Still a bug in current (x.y. or later) git ?" message everywhere, and then just move on, isn't it? ;)
It would have been far easier to close this bug as abandoned since it hasn't been updated/retested by the OP since 2011-06-09... Or even to let it rot indefinitely...
Those "Still a bug ..." message shouldn't ever be necessary, if people rechecked it regularly themselves, which they should do IMHO
See http://wiki.winehq.org/Bugs for more information E.g. "Do check on your bug every release or two and let us know if the bug is still present. If not, mark it fixed. "
Granted, testing it every two weeks may be overkill, but once every 6 months isn't too much to ask IMO
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #38 from Tobias Jakobi liquid.acid@gmx.net 2012-03-25 08:38:44 CDT --- This bug _is_ already triaged. Maybe you should start actually reading the bugreport in fully.
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #39 from Tobias Jakobi liquid.acid@gmx.net 2012-03-25 08:58:16 CDT --- And to emphasize it again: As long as there is no special-case handling for the EXPP instruction in shader_hw_scalar_op (dlls/wined3d/arb_program_shader.c) this bug is technically still valid.
And checking this doesn't require any special and time-consuming tests, you just take a look at the current tree... http://source.winehq.org/git/wine.git/blob/HEAD:/dlls/wined3d/arb_program_sh...
...search for shader_hw_scalar_op and notice that there's no special-case handling (yet).
The only thing one might argue about is that the GLSL path is now default for the devices that support it. But wine still supports ARB-only GLs.
And I don't bump this bug, since I track changes in wined3d anyway. So if Henri, Stefan or anyone else would add some code, I knew about it and this bug was already closed.
http://bugs.winehq.org/show_bug.cgi?id=17715
--- Comment #40 from Frédéric Delanoy frederic.delanoy@gmail.com 2012-03-25 09:16:24 CDT --- (In reply to comment #38)
This bug _is_ already triaged. Maybe you should start actually reading the bugreport in fully.
OK I shouldn't have used that word. I should have used "check/verify/examine" instead of "triage" in "Your bug is not the only one in existence, nor the only one I triage." Sorry about being imprecise in that sentence.
http://bugs.winehq.org/show_bug.cgi?id=17715
Henri Verbeet hverbeet@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |c8852c3ee337a7ed22a51105dc4 | |c466ca2c8e0b3 Status|UNCONFIRMED |RESOLVED Resolution|--- |FIXED
--- Comment #41 from Henri Verbeet hverbeet@gmail.com --- I think this is fixed by commit c8852c3ee337a7ed22a51105dc4c466ca2c8e0b3.
https://bugs.winehq.org/show_bug.cgi?id=17715
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #42 from Alexandre Julliard julliard@winehq.org --- Closing bugs fixed in 1.7.9.