crosire / d3d8to9

A D3D8 pseudo-driver which converts API calls and bytecode shaders to equivalent D3D9 ones.
BSD 2-Clause "Simplified" License
880 stars 78 forks source link

Pixel shader translation does not work as expected (Star Wars Republic Commando) #31

Closed metallicafan212 closed 6 years ago

metallicafan212 commented 7 years ago

Continuing from the previous issue:

(Recap) d3d8to9 is changing modifiers for pixel shaders and not restoring their effect. Rather, it's just stripping them off on pixel shaders.

The pixel shader for DOT3DiffSpec (HardwareShaders.Bump.DOT3DiffSpec) is:

;; PC

ps.1.3

tex t0 ; base map tex t1 ; bump map tex t2 ; light vector from normalizer cube map tex t3 ; half angle vector

; N.L dp3_sat r0, t1_bx2, t2_bx2

; (N.H) dp3_sat r1, t1_bx2, t3_bx2

; Multiply by bump color ; Add baked + vertex lighting mad r0.rgb, r0, v1, v0

; approximate (N.H)^16 ; [(N.H)^2 - 0.75] * 4 == (N.H)^16 mad_x2_sat r1, r1.a, r1.a, -c0

; Set the color of the specular mul_sat r1.rgb, r1, v1

; Mulitply specular by specular mask ;;;mul r1, r1, t1.a

; Self Illumination ;;;lrp r0, t0.a, t0, r0

; [(N.L) * base] + (N.H)^16 ; Diffuse and Spec mad_sat r0, r0, t0, r1

The important line is mad_x2_sat r1, r1.a, r1.a, -c0

d3d8to9 is producing:

ps_1_3
tex t0
tex t1
tex t2
tex t3
dp3_sat r0, t1_bx2, t2_bx2
dp3_sat r1, t1_bx2, t3_bx2
mad r0.xyz, r0, v1, v0
mad_x2_sat r1, r1.w, r1.w, c0 /* removed modifier - */
mul_sat r1.xyz, r1, v1
mad_sat r0, r0, t0, r1

Now the line is mad_x2_sat r1, r1.w, r1.w, c0 /* removed modifier - */

It removes the modifier and does not restore the effect of it. Removing the - sign from the pixel shader and running the game without d3d8to9 produces the same effect.

Commenting out lines 1733-1734 causes the editor to crash upon loading the shader and a error produced in the log.

..\Star Wars Republic Commando\GameData\System\memory(9,5): error X5539: (third source param) Modifiers are not allowed on constants for ps_1_x.

What it's supposed to look like: proper spec

What it looks like with d3d8to9: d3d8spec

What it looks like without d3d8to9 and a - sign removed from one shader: well whaddya know

(I am using my own mod that does not change shaders, only textures; the problem is universal)

If anyone needs more info feel free to ask.

HerMajestyDrMona commented 7 years ago

Seems like another regex problem, somewhere near this line? https://github.com/crosire/d3d8to9/blob/master/source/d3d8to9_device.cpp#L1733

I don't like regex, but maybe replacing: SourceCode = std::regex_replace(SourceCode, std::regex("(1?-)(c[0-9]+)"), "$2 /* removed modifier $1 */"); to: SourceCode = std::regex_replace(SourceCode, std::regex("(1?)(c[0-9]+)"), "$2 /* removed modifier $1 */");

would help?

elishacloud commented 7 years ago

I don't think that will work. I just realized that c0 is a constant. Adding a modifier (such as -) to a constant is not allowed. So removing the - is needed for the shader to get reassembled.

Can you show me how to get a copy of the mod for this game and where this lighting issue happens in the game? I would like to reproduce it myself.

metallicafan212 commented 7 years ago

The mod is my own project (Tentatively called RCHD or Republic Commando HD) and I do not have it released anywhere (Nor is it anywhere close to being released, lol), it is reproducible in the base game.

Here is a screenshot from the original game:

orig game

Without d3d8to9: orig without d3d8to9

Map is geo_01b.ctm

To open it in game, press ` and type: "open geo_01b"

(I hope I don't sound mean; I'm not trying to be mean, text is just very abrupt)

The mod is quite large, quite unfinished, does not even work in some areas, and would just be a pain for me to upload at all, since my upload rate is crap (Centurylink... GRRR)

I was trying to hide it with the logs..... Oops

If anyone is wondering: size

And just to be funny: Stay on target Lets keep it on topic. (Talking about my unfinished mod makes me feel like I'm not finishing it fast enough)

metallicafan212 commented 7 years ago

I feel like the modifier is some kind of undocumented hack as it compiles correctly in game, but everything that I've seen from the documentation says it should not be possible. (You never know, these guys back then basically hacked the game together and released it, it shows in the Uscript)

elishacloud commented 7 years ago

Great. I can reproduce this now. Thanks!

elishacloud commented 7 years ago

Ok, this is a super ugly hack. But take a look at this file here and let me know if this fixes the issue.

metallicafan212 commented 7 years ago

That fixes the issue! I was trying to see if I could flip the constant in the shader using registers, but I didn't think about making it subtraction. Well, that was obvious..... (I wish the documentation was still on the internet, this makes me feel stupid...).

My mod: rchd spec

Original game: og rep spec

elishacloud commented 7 years ago

Wow. Your mod looks great!

I am glad that this hack fixed your issue. It is not actually a complete fix, which I will explain in my next post.

As far as documentation, there is still documentation on Microsoft's site for Direct3D9. Vertex shader documentation can be found here and pixel shader can be found here.

I also found a pretty good guide for Direct3D8 shaders here:

elishacloud commented 7 years ago

This issue led me down a path that showed me a whole set of issues with the current pixel shader translation regex. Below is an outline of my findings:

  1. Missed modifiers.

In this line here several modifiers are removed. However according to documentation here and here there are still several modifiers that are not removed. Modifiers are _x2, _dz, _db, _dw and _da. I know that at least some of these modifiers are used in Direct3D8 because of the documentation here. Removing these modifiers is quite easy just add _x2|_d[zbwa] to the line so that it looks like this:

SourceCode = std::regex_replace(SourceCode, std::regex("(c[0-9]+)(_bx2|_bias|_x2|_d[zbwa])"), "$1 /* removed modifier $2 */");
  1. Missed modifiers when swizzles are used.

This same line here removes modifiers but only when no swizzle is used. However I know that swizzles can be used with modifiers because Silent Hill 2 uses both swizzles and modifiers for the same constant. If this (\\.?[wxyz]?) bracked group is added to the line it will be able to remove modifiers even if swizzles exist. Here is what the updated line looks like:

SourceCode = std::regex_replace(SourceCode, std::regex("(c[0-9]+)(\\.?[wxyz]?)(_bx2|_bias|_x2|_d[zbwa])"), "$1$2 /* removed modifier $3 */");
  1. Dangling Swizzles.

The line here removes modifiers before the constant. However it adds a comment which can cause swizzles to become dangling. This actually happens in Silent Hill 2. Here is what the line looks like before the regex:

  + sub r0.w, r0, -c2.w

Here is what it looks like after regex:

  + sub r0.w, r0, c2 /* removed modifier - */.w

Notice the dangling .w swizzle. This can be fixed by adding another bracked group for the swizzle like this (\\.?[wxyz]?). Here is what the updated line looks like:

SourceCode = std::regex_replace(SourceCode, std::regex("(1?-)(c[0-9]+)(?![0-9])(\\.?[wxyz]?)"), "$2$3 /* removed modifier $1 */");
  1. Dangling Modifiers.

The same line here removes modifiers before the constant. Not only can this line cause swizzles to be dangling but in theory could also cause dangling modifiers if two modifiers are used, one before and one after the constant. Now I don't know if this is possible but the fix is quite easy and virtually no risk so I don't see any reason not to do. All you need to do is move this line here above all other lines that change or remove modifiers. So now it looks like this:

SourceCode = std::regex_replace(SourceCode, std::regex("(c[0-9]+)(\\.?[wxyz]?)(_bx2|_bias|_x2|_d[zbwa])"), "$1$2 /* removed modifier $3 */");
SourceCode = std::regex_replace(SourceCode, std::regex("(1?-)(c[0-9]+)(?![0-9])(\\.?[wxyz]?)"), "$2$3 /* removed modifier $1 */");
  1. Removing modifiers could cause graphic glitches.

Now this fifth issue is hardest to fix. Simply removing the modifiers on constants of course fixes the pixel shader so that is can be reassembled and the game will run. Leaving the modifiers will cause reassembly to fail and in many cases the game won't run. However this also can cause other issues like this lighting issue with Star Wars Republic Commando. The reason for this is that the modifiers are there to change the value of the constant. Removing them means that the end result may not be correct.

I could not find any generic fix for this. However there is a patch that will fix the issues in certain cases. If the arithmetic function is add and the modifier is - then we can change the arithmetic function to sub and remove the modifier. This is a very specific case but it happens in both Star Wars Republic Commando and in Silent Hill 2. To fix this a line can be added that looks like this:

SourceCode = std::regex_replace(SourceCode, std::regex("(add)(.*)(-)(c[0-9]+)(?![0-9])(\\.?[wxyz]?)"), "sub$2$4$5 /* changed 'add' to 'sub' to remove modifier $3 */");

The problem is that Star Wars Republic Commando also used the mad arithmetic function with the - modifier. I could not find any single arithmetic function that could easily be substituted for mad so I needed to split this into two lines with two arithmetic functions (mul and sub). However this required adding a new arithmetic functions which in some cases would not work since there is a limit of 8 arithmetic functions in ps_1_x. However changing the mad to a sub seemed to work at least for Star Wars Republic Commando. To fix this a line can be added that looks like this:

SourceCode = std::regex_replace(SourceCode, std::regex("(mad)(.*)(, )(.*)(, )(.*)(, )(.*)(-)(c[0-9]+)(?![0-9])(\\.?[wxyz]?)"), "sub$2$3$4$5$10$11 /* changed 'mad' to 'sub' to remove modifier $9 */");

But if you are taking note changing this from a mad to a sub also changes the end result. So I wanted to do better than that. I noticed there were still cases where an extra arithmetic function could be added because less than 8 arithmetic functions were being used. So to do this I needed to first get the number of arithmetic functions used and then see if I could add any more. I noticed that the dissembler would log the number of arithmetic functions used so i can get that from the string. Here is the code to get the number of arithmetic functions used:

int countArithmetic = 10;   // Default to 10
size_t ArithmeticPosition = SourceCode.find("arithmetic");
if (ArithmeticPosition > 2 && ArithmeticPosition < SourceCode.size())
{
    countArithmetic = atoi(&SourceCode[ArithmeticPosition - 2]);
    if (countArithmetic == 0)
    {
        countArithmetic = 10;   // Default to 10
    }
}

Here is the code to split the mad arithmetic function into mul and sub arithmetic functions, based on the number of arithmetic functions remaining:

for (int x = 8 - countArithmetic; x > 0; x--)
{
    // Only replace one match
    SourceCode = std::regex_replace(SourceCode, std::regex("(mad)(.*)( )(.*)(, )(.*)(, )(.*)(, )(.*)(-)(c[0-9]+)(?![0-9])(\\.?[wxyz]?)"), "mul$3$4$5$6$7$8 /* added line */\n    sub$2$3$4$5$4$7$12$13 /* changed 'mad' to 'sub' to remove modifier $11 */", std::regex_constants::format_first_only);
}

So the final code for the regex translation in pixel shader looks like this:

// Get arithmetic number
int countArithmetic = 10;   // Default to 10
size_t ArithmeticPosition = SourceCode.find("arithmetic");
if (ArithmeticPosition > 2 && ArithmeticPosition < SourceCode.size())
{
    countArithmetic = atoi(&SourceCode[ArithmeticPosition - 2]);
    if (countArithmetic == 0)
    {
        countArithmetic = 10;   // Default to 10
    }
}

// Remove line when "    // ps.1.1" sting is found and the next line does not start with a space
SourceCode = std::regex_replace(SourceCode, std::regex("    \\/\\/ ps\\.1\\.[1-4]\\n((?! ).+\\n)+"), "");

// Fix and remove modifiers for constant values
SourceCode = std::regex_replace(SourceCode, std::regex("(c[0-9]+)(\\.?[wxyz]?)(_bx2|_bias|_x2|_d[zbwa])"), "$1$2 /* removed modifier $3 */");
SourceCode = std::regex_replace(SourceCode, std::regex("(add)(.*)(-)(c[0-9]+)(?![0-9])(\\.?[wxyz]?)"), "sub$2$4$5 /* changed 'add' to 'sub' to remove modifier $3 */");
// Only replace matches upto the number of remaning arithmetic places left
for (int x = 8 - countArithmetic; x > 0; x--)
{
    // Only replace one match
    SourceCode = std::regex_replace(SourceCode, std::regex("(mad)(.*)( )(.*)(, )(.*)(, )(.*)(, )(.*)(-)(c[0-9]+)(?![0-9])(\\.?[wxyz]?)"), "mul$3$4$5$6$7$8 /* added line */\n    sub$2$3$4$5$4$7$12$13 /* changed 'mad' to 'sub' to remove modifier $11 */", std::regex_constants::format_first_only);
}
SourceCode = std::regex_replace(SourceCode, std::regex("(mad)(.*)(, )(.*)(, )(.*)(, )(.*)(-)(c[0-9]+)(?![0-9])(\\.?[wxyz]?)"), "sub$2$3$4$5$10$11 /* changed 'mad' to 'sub' to remove modifier $9 */");
SourceCode = std::regex_replace(SourceCode, std::regex("(1?-)(c[0-9]+)(?![0-9])(\\.?[wxyz]?)"), "$2$3 /* removed modifier $1 */");
metallicafan212 commented 7 years ago

Wow, you really know your stuff. Good job! I would've never figured that out on my own, since I don't know c++ or shader code that isn't hlsl. I'll go read up on the links you sent me so I can make my own shaders (I'm looking at doing parallax in the game).

I just hope now that all the problems with translation are fixed (It seems like it took this game to hammer it down since it's one of the only directx 8 games that we can access the raw shader code).

Here were some other areas that were affected, but were not as good of an example as geo_01b.ctm They do provide a good teaser on the mod though.

geo_03a.ctm

Mod swrepubliccommando_2017_07_03_12_21_10_301 Base ct_2017_07_03_12_29_36_986 Mod swrepubliccommando_2017_07_03_12_25_17_314 Base ct_2017_07_03_12_30_43_360 Mod swrepubliccommando_2017_07_03_12_25_45_059 Base ct_2017_07_03_12_30_58_573

elishacloud commented 7 years ago

I found a more generic fix for this. You can see the code updates here. I have also attached an update binary file here. The lighting in this update is noticeably better then the last one, at least I noticed an improvement.

crosire commented 6 years ago

Fixed in 8b8738335304427049121734e42997043064c27b.