Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update pixel shader translations to fix lighting issues #32

Merged
merged 1 commit into from
Jul 14, 2017

Conversation

elishacloud
Copy link
Contributor

  • Minor updates in vertex and pixel shaders regex to merge two negative lookbacks into one and slightly reduce the complexity of the regex
  • Added code to get the arithmetic count in use
  • Updated regex code that changes 'add' with a '-' modifier to 'sub' to make it more generic and work with swizzles and modifiers
  • Added code to generically update any modifier on a constant by using the extra arithmetic functions available, this fixes one of the lighting issues with Star Wars Republic Commando
  • Added code that changes 'mad' with a '-' modifier to 'sub', this fixes one of the lighting issues with Star Wars Republic Commando
  • Added code that changes '_bx2' source modifier for constant to use _x2 destination modifier, this fixes a minor lighting issue with Silent Hill 2

This update completely resolves the lighting issues mentioned in #24 and #31.

- Minor updates in vertex and pixel shaders regex to merge two negative
lookbacks into one
- Added code to get the arithmetic count in use
- Updated regex code that changes 'add' with a '-' modifier to 'sub' to
make it more generic
- Added code to generically update any modifier on a constant by using
the extra arithmetic functions available, this fixes the lighting issues
with Star Wars Republic Commando
- Added code that changes 'mad' with a '-' modifier to 'sub', this fixes
the lighting issues with Star Wars Republic Commando
- Added code that changes '_bx2' source modifier for constant to use _x2
destination modifier, this fixes a minor lighting issue with Silent Hill
2
@elishacloud
Copy link
Contributor Author

Let me explain this update in a bit more detail. The regex expressions are quite detailed and not easy to follow.

CreateVertexShader

SourceCode = std::regex_replace(SourceCode, std::regex("mov (oFog|oPts)(.*), (-?)([crv][0-9]+(?![\\.0-9]))"), "mov $1$2, $3$4.x /* select single component */");

For this function I changed one line. Basically I merged these bracketed groups (?![0-9])(?!\\.) into one group here (?![\\.0-9]). These are negative lookbacks and this just simplified the two into one. It ensures that there is no dangling swizzle (or swizzle part).

CreatePixelShader

There were a few more updates I did here.

Get Arithmetic Count

int countArithmetic = 10;	// Default to 10
const 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
	}
}

This code simply finds the number of arithmetic operations done by looking at the decompiled SourceCode string and stores this number in the countArithmetic variable because we don't want to go over 8 operations. Note: I default to 10 operations because if something goes wrong I don't want to add additional operations when it may already be at max.

Update 'sub' operation when using '-' constant

SourceCode = std::regex_replace(SourceCode, std::regex("(add)([_satxd248]*) (r[0-9][\\.wxyz]*), ((1-|)[crtv][0-9][\\.wxyz_abdis2]*), (-)(c[0-9][\\.wxyz]*)(_bx2|_bias|_x2|_d[zbwa]|)(?![_\\.wxyz])"),
	"sub$2 $3, $4, $7$8 /* changed 'add' to 'sub' removed modifier $6 */");

The next line will change the add to a sub when a constant is being used. This line replaces the line from my last update since the last one could not handle swizzles or modifiers. Let me break down the regex. There are basically 8 bracketed groups with one negative lookback.

  1. (add) - This is ensuring that the operator is an add operator.
  2. ([_satxd248]*) - This will handle any modifier on the add operator, such as _sat or _x2
  3. (r[0-9][\\.wxyz]*) - This will handle any register r with any swizzle. Since pixel shaders can only handle up to 6 registers with ps_1_4 and 2 with ps_1_1 through ps_1_3 we only need to handle from 0 to 9, single digit.
  4. ((1-|)[crtv][0-9][\\.wxyz_abdis2]*) - This will handle any register of any kind [crtv] with any swizzle or trailing modifier [\\.wxyz_abdis2] or leading modifier (1-|) except the - modifier. All of this will be stored in one group.
  5. (1-|) - A group inside $4. This will optionally get the modifier 1-, but we don't need to use this since this is already included in $4 above.
  6. (-) - This is the negative modifier that is being removed from the constant.
  7. (c[0-9][\\.wxyz]*) - This is the constant c[0-9] plus any possible swizzle [\\.wxyz]*
  8. (_bx2|_bias|_x2|_d[zbwa]|) - This is to catch any trailing modifier.
  9. (?![_\\.wxyz]) - This is a negative lookback and ensures that there is no dangling swizzle (or swizzle part).

Changing a - modifier with a add operation to a sub should be an exact translation, meaning the output is exactly the same in Direct3D9 as it was in Direct3D8.

This fixes part of the lighting issue in Star Wars Republic Commando. The next two updates are also required for the full fix. Each of these affect a different set of shaders where the lighting is way too bright.

Replacing any modifier on a constant by using extra arithmetic operators

for (int x = 8 - countArithmetic; x > 0; x--)
{
	const size_t beforeReplace = SourceCode.size();
	// Only replace one match
	SourceCode = std::regex_replace(SourceCode, std::regex("(...)(_[_satxd248]*|) (r[0-9][\\.wxyz]*), (1?-?[crtv][0-9][\\.wxyz_abdis2]*, )?(1?-?[crtv][0-9][\\.wxyz_abdis2]*, )?(1?-?[crtv][0-9][\\.wxyz_abdis2]*, )?((1?-)(c[0-9])([\\.wxyz]*)(_bx2|_bias|_x2|_d[zbwa]|)|(1?-?)(c[0-9])([\\.wxyz]*)(_bx2|_bias|_x2|_d[zbwa]))(?![_\\.wxyz])"),
		"mov $3, $9$10$13$14 /* added line */\n    $1$2 $3, $4$5$8$12$3$10$11$14$15 /* changed $9$13 to $3 */", std::regex_constants::format_first_only);
	// Check if string was replaced
	if (SourceCode.size() - beforeReplace == 0)
	{
		break;
	}
}

The next part of the code will take the modifier on the constant and mov it into a register. This code will run in a loop, each loop replacing one modifier and adding one new arithmetic operator until all remaing operators are used or no more modifiers exist on constants. This is the most complicated one, but I will break it down so that it is more understandable. There are 15 bracketed groups and one negative lookback.

  1. (...) - This will match on the operator
  2. (_[_satxd248]*|) - This will handle any modifier on the operator, such as _sat or _x2
  3. (r[0-9][\\.wxyz]*) - This will handle any register r with any swizzle. Since pixel shaders can only handle up to 6 registers we only need to handle from 0 to 9, single digit.
  4. (1?-?[crtv][0-9][\\.wxyz_abdis2]*, )? - This line and $5 and $6 are the same. This matches on any register [crtv] with any swizzle or trailing modifier [\\.wxyz_abdis2]* and any leading modifier 1?-?. Also there is a ? at the end so this whole group is optional. The reason for that is because the constant could be anywhere in the line so making this optional allows any number of leading registers, up to 3 registers.
  5. (1?-?[crtv][0-9][\\.wxyz_abdis2]*, )? - Same as $4.
  6. (1?-?[crtv][0-9][\\.wxyz_abdis2]*, )? - Same as $4.
  7. ((1?-)(c[0-9])([\\.wxyz]*)(_bx2|_bias|_x2|_d[zbwa]|)|(1?-?)(c[0-9])([\\.wxyz]*)(_bx2|_bias|_x2|_d[zbwa])) - This is basically an or | operation to verify there is a modifier on the constant either before (1?-)(c[0-9])([\\.wxyz]*)(_bx2|_bias|_x2|_d[zbwa]|) or after 1?-?)(c[0-9])([\\.wxyz]*)(_bx2|_bias|_x2|_d[zbwa]) the constant. This group ensures that we only update lines that have a modifier on a constant.
  8. (1?-) - A group inside $7 on the left part of the or |. This covers the part that ensures there is a leading modifier before the constant.
  9. (c[0-9]) - A group inside $7 on the left part of the or |. This matches on the constant c[0-9].
  10. ([\\.wxyz]*) - A group inside $7 on the left part of the or |. This catches any possible swizzle.
  11. (_bx2|_bias|_x2|_d[zbwa]|) - A group inside $7 on the left part of the or |. This catches any possible trailing modifier.
  12. (1?-?) - A group inside $7 on the right part of the or |. This catches any possible leading modifier.
  13. (c[0-9]) - A group inside $7 on the right part of the or |. This matches on the constant c[0-9].
  14. ([\\.wxyz]*) - A group inside $7 on the right part of the or |. This catches any possible swizzle.
  15. (_bx2|_bias|_x2|_d[zbwa]) - A group inside $7 on the right part of the or |. This covers the part that ensures there is a trailing modifier on the constant.
  16. (?![_\\.wxyz]) - This is a negative lookback and ensures that there is no dangling swizzle or modifier.

This whole section should also result in an exact match because rather than just removing the modifier we can use the destination register to replace the source constant.

This fixes another part of the lighting issue in Star Wars Republic Commando.

Update 'mad' operation when using '-' constant

SourceCode = std::regex_replace(SourceCode, std::regex("(mad)([_satxd248]*) (r[0-9][\\.wxyz]*), (1?-?[crtv][0-9][\\.wxyz_abdis2]*), (1?-?[crtv][0-9][\\.wxyz_abdis2]*), (-)(c[0-9][\\.wxyz]*)(_bx2|_bias|_x2|_d[zbwa]|)(?![_\\.wxyz])"),
	"sub$2 $3, $4, $7$8 /* changed 'mad' to 'sub' removed $5 removed modifier $6 */");

This is similar to the Update 'sub' operation above so I won't spell out all the regex groups here. But there are two main differences:

  1. This one changes a mad (rather than a add) into a sub.
  2. This will remove not only the modifier on the constant but it will also remove one of the registers (the one in group $5).

This is not an exact match, meaning it will not do exactly what the shader does in Direct3D8. However the code is already implemented to remove all modifiers on constants which means it already does not do exactly what Direct3D8 does. At least this change gets the behavior closer to what the Direct3D8 shader does.

This fixes the final part of the lighting issue in Star Wars Republic Commando.

Update '_bx2' modifier to use '_x2'

SourceCode = std::regex_replace(SourceCode, std::regex("(...)(_sat|) (r[0-9][\\.wxyz]*), ([crtv][0-9][\\.wxyz]*)_bx2, (c[0-9][\\.wxyz]*)_bx2(?!,)"),
	"$1_x2$2 $3, $4, $5 /* removed modifiers _bx2 added modifier _x2 */");

The regex on this is quite a bit simpler, so I won't detail it out unless needed. Basically if both source registers use the _bx2 modifier and the second source register is a constant it will replace both source modifiers with a destination _x2 modifier. This is also not an exact match. But as stated above it is a closer match to what Direct3D8 does, so I think it is an improvement.

This issue was found in Silent Hill 2.

Remove all trailing modifiers on constants

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

This line has not changed much. However I did move it down some. It no longer needs to be at the top because all the regex expressions above it already ensure that no trailing modifier becomes dangling.

I did change group $1 from (c[0-9]+\\.?[wxyz]*) to (c[0-9][\\.wxyz]*) to simplify the regex. Outcome is exactly the same for this shader use case.

Remove all leading modifiers on constants

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

Again this line has not changed much either. I did change group $2 from (c[0-9]+(?![0-9])\\.?[wxyz]*(?![wxyz])) to (c[0-9][\\.wxyz]*(?![\\.wxyz])) which is a minor simplification, merging two negative lookbacks into one.

Testing

For all of these changes I have tested them extensively with the regex debugger here, trying all sorts of combinations to ensure the regex would do what is expected in all cases with no side effects.

I also tested this with the following Direct3D8 games to ensure that there was no ill side-effect:

  • Haegemonia The Solon Heritage
  • Hitman 2 Silent Assassin
  • Indiana Jones and the Emperor's Tomb
  • Max Payne 2
  • Moto Racer 3
  • Need for Speed III
  • Raymond 3
  • Silent Hill 2
  • Star Wars Republic Commando
  • Star Wars Starfighter

@elishacloud
Copy link
Contributor Author

Please let me know if you have any concerns with this code or any modifications you would like me to make before merging it.

@crosire
Copy link
Owner

crosire commented Jul 10, 2017

Awesome summary. I haven't merged it yet because I'm currently limited to my cell phone which makes viewing bigger changes rather complicated. As such I prefer to wait until I'm back at a desktop computer in a few days.

@crosire crosire merged commit 8b87383 into crosire:master Jul 14, 2017
@elishacloud elishacloud deleted the ShaderTranslation branch July 14, 2017 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants