bo3b / 3Dmigoto

Chiri's DX11 wrapper to enable fixing broken stereoscopic effects.
Other
688 stars 109 forks source link

DX9 shader decompile #156

Open Nintynuts opened 2 years ago

Nintynuts commented 2 years ago

What I've done so far:

Issues:

Remaining work?

I think I know what I need to do next, but it seems like a big task...

Rather than building the shader output as the source is parsed, I could save the components in vectors and construct it at the end. This would allow me to collect the input, output and temporary registers as the individual commands are processed, and output them in a sensible order at the end.

I could also detect any undeclared constant/uniform registers and infer their type from usage and use the register name as the variable name, which isn't particularly helpful but would make it valid.

I then need to figure out is whether it can be compiled back to bytecode that's equivalent to what was disassembled.

Also, I've had to change MIGOTO_DX to be 9 so that it uses DX9. Not sure if we can have both DX versions included?

Nintynuts commented 2 years ago

Thanks for the feedback. Response inline.

  1. We aren't certain that we would want to update to v142 toolset. We want to still be compatible with the handful of people that still use 3Dmigoto on Win7. There is rarely any value whatsoever in updating toolsets, and we discourage it unless you know of something specific you want or need.
  2. It looks like you just want to use the cmd_Decompiler project? No need to update the other tools if that is the case. Each subproject is OK to have its own toolset.

This is a hangover from when I was working on something else (which went nowhere since I got stuck with no help). I just happen to have 2019 installed and don't like having multiple versions take up space. It would be nice if the compiler could be installed without the rest of the IDE! They're separate commits, so can be cherry picked separately if eventually merged.

  1. I'm not sure I understand the need for DX9 changes here? The Decompiler should be able to handle any DX9 shaders, because it already handles SM1. Maybe there is something special about them I'm missing. Noting that only the DX9 variant of 3Dmigoto will actually dump those shaders, which can be then be Decompiled or disassembled with the stand alone cmd_decompiler tool.
  2. I don't understand the need to add -T translate option? Isn't -d already doing the job you want?

I wanted to convert ASM disassemblies from HelixMod into HLSL since I really hate reading ASM. I tried running it through the standalone cmd_Decompiler exe but it requires a binary (and I don't know how to export Binary shaders from HelixMod) so I added the translate option to convert disassembled ASM to HLSL.

The code gave me the impression that it should work with a DX9 (SM1-3) shader due to various code blocks marked DX9, but once I got the code compiling for DX9 (see below) I found it left out significant chunks of the shader which meant it couldn't be recompiled. This is what I've tried to add, which I've done by studying the MS documentation for SM1-3 and trying to find appropriate places to integrate the alternative logic.

  1. Adding the MIGOTO_DX=9 is right for dx9 source base, but typically we'll define that in the solution options instead of files, so that it can change for any given build type. We could add a specific DX9 Debug/Release build for example.

I guessed making a DX9 specific configuration might be the solution, I just dislike configurations beyond (Debug|Release)(x86|x64) since it makes everything more complicated. Is it possible to hook into DX9 and DX11 from the same process?

  1. Just FYI, we are possibly moving to a new code format which should dramatically improve our code quality, at the expense of breaking everything prior and making commit comparisons impossible because of so many changes. Look at Rename_Reformat branch if interested.
  2. Minor white space changes that are undesirable. As a general rule, we avoid white space changes on commits because it adds noise to the history, and makes comparisons with older versions much harder. You should set your editor to avoid adding white noise.

Thanks, I will have a look. In my day-to-day development I rely on Autoformatting for consistency. This project is not at all consistent, so hopefully your new branch will sort that out so autoformatting can be used from now on? I will make a separate commit with the autoformatting so that can be ignored with cherry picking too.

bo3b commented 2 years ago
  1. We aren't certain that we would want to update to v142 toolset. We want to still be compatible with the handful of people that still use 3Dmigoto on Win7. There is rarely any value whatsoever in updating toolsets, and we discourage it unless you know of something specific you want or need.
  2. It looks like you just want to use the cmd_Decompiler project? No need to update the other tools if that is the case. Each subproject is OK to have its own toolset.

This is a hangover from when I was working on something else (which went nowhere since I got stuck with no help). I just happen to have 2019 installed and don't like having multiple versions take up space. It would be nice if the compiler could be installed without the rest of the IDE! They're separate commits, so can be cherry picked separately if eventually merged.

Should be no problem. I'm currently using VS2019, and have not seen any drawbacks. We can probably update the sln files for this version, as pretty much everyone will likely be using VS2019 now. This is already updated this way on the Rename_Refactor branch for this reason.

For master I'd say it's probably best to stay with toolset v141 though, as I'm not sure what the impact would be on Win7 support, and it's really easy to have two toolsets installed without any problems.

I have fairly limited time, so rather than doing cherry-pick or other more complicated pulls, I think it would best to either create a new branch with simpler and cleaner setup, or I can add you as a direct contributor to the project. In general we don't really use pull requests because pretty much anyone who wants to contribute is OK with us.

  1. I'm not sure I understand the need for DX9 changes here? The Decompiler should be able to handle any DX9 shaders, because it already handles SM1. Maybe there is something special about them I'm missing. Noting that only the DX9 variant of 3Dmigoto will actually dump those shaders, which can be then be Decompiled or disassembled with the stand alone cmd_decompiler tool.
  2. I don't understand the need to add -T translate option? Isn't -d already doing the job you want?

I wanted to convert ASM disassemblies from HelixMod into HLSL since I really hate reading ASM. I tried running it through the standalone cmd_Decompiler exe but it requires a binary (and I don't know how to export Binary shaders from HelixMod) so I added the translate option to convert disassembled ASM to HLSL.

The code gave me the impression that it should work with a DX9 (SM1-3) shader due to various code blocks marked DX9, but once I got the code compiling for DX9 (see below) I found it left out significant chunks of the shader which meant it couldn't be recompiled. This is what I've tried to add, which I've done by studying the MS documentation for SM1-3 and trying to find appropriate places to integrate the alternative logic.

The code blocks for DX9 were added by DarkStarSword when he integrated in DaveGL1234's code for full DX9 support. Dave's code is setup to handle running directly out of DX9 games, and I guess there was too much conflict in the code, so that when DarkStarSword integrated it, he added #ifdef for the variants. I wasn't available during this time, so I don't really know this part. I'd really like to avoid this DX9/DX11 schism via #ifdef, that doesn't seem like the right tool, but I don't know how much work it would be to get them both live at once. At present it's one or the other, not both at the same time, and are separate output builds.

It certainly does not hurt to grab the shaders from HelixMod for conversion. Those will have been exported via the Microsoft disassembler and should be fine, except for some floating point conversions that can be bad because they don't output enough significant digits. However, the DX9 variant of 3Dmigoto should in theory give you direct access to those DX9 shaders and when marking shaders it should generate an HLSL directly. Not sure that all works, have not tried, and some notes suggest maybe it's not quite there.

If you want to do the Helixmod->cmd_decompiler path, that's perfectly OK and probably less work. Sounds like the HLSL decompiler needs some missing opcodes or setup, but that would be true in either case.

BTW- I did almost all the work on the Decompiler after Chiri, and I am keenly interested in having it generate HLSL for DX9 shaders as well, so please ping me for anything you run into here.

Since the cmd_decompiler already has the reassemble operation, another possibility is to take the HelixMod ASM.txt file and assemble it to binary, then use the -d flag to convert back to HLSL.

It's fine to add the -t if you feel that is a superior path, but it will need to do the assemble regardless, because we need the binary for the James-Jones cross compiler to inspect to give us some details needed to properly decompile. It's also worth noting that you don't have to use cmd_decompiler for that, you can also use the Microsoft assembler, which worked up until SM4.

Another option might be to use the -d operation, but have it recognize when the input file is .bin vs .txt and go ahead with the assemble operation first. No strong preference from me, but think about your workflow and how you'd prefer it works.

  1. Adding the MIGOTO_DX=9 is right for dx9 source base, but typically we'll define that in the solution options instead of files, so that it can change for any given build type. We could add a specific DX9 Debug/Release build for example.

I guessed making a DX9 specific configuration might be the solution, I just dislike configurations beyond (Debug|Release)(x86|x64) since it makes everything more complicated. Is it possible to hook into DX9 and DX11 from the same process?

Not currently possible to hook into DX9 and DX11 at the same time because the code is setup via #ifdef everywhere, so they are independent outputs.

In principle there is no actual conflict, and I'd much rather see an object oriented approach here where both can be live at the same time, but that would take a fair amount of work to revise. The DX9 objects are distinct from the DX11 objects, and can have similar wrapper objects. If our Hacker* objects descended from a common root object, we'd be able to do this without having to #ifdef or have duplicated code.

But as noted above, I'm not sure about the DX9 state. As far as I know no one has used it yet, so it's up to you whether you'd like to see if 3Dmigoto can do this directly, or whether it's better to just focus on the Decompiler part.

  1. Just FYI, we are possibly moving to a new code format which should dramatically improve our code quality, at the expense of breaking everything prior and making commit comparisons impossible because of so many changes. Look at Rename_Reformat branch if interested.
  2. Minor white space changes that are undesirable. As a general rule, we avoid white space changes on commits because it adds noise to the history, and makes comparisons with older versions much harder. You should set your editor to avoid adding white noise.

Thanks, I will have a look. In my day-to-day development I rely on Autoformatting for consistency. This project is not at all consistent, so hopefully your new branch will sort that out so autoformatting can be used from now on? I will make a separate commit with the autoformatting so that can be ignored with cherry picking too.

Yeah, no argument there, all of us have not been too happy with the code consistency here, it's really degraded over time. I've currently spent 4 months working on renaming and reformatting, with a smidge of refactoring. This new version should be a lot better. In particular, I've added a clang-format and clang-tidy files to do all this automatically and to keep things consistent using those as a style guide for us going forward.

For master we won't apply this to all the code because it's a massive amount of white space changes and will make any historical comparisons impossible. We'll have to decide how to manage this juncture assuming I ever finish this cleanup pass. My best guess is that we will archive master as the 1.3.16, and switch to the new code base as maybe a 2.0, especially if we add DX9 support.

Nintynuts commented 2 years ago

I understand that you don't have lots of time, but maybe not doing PRs is the reason the code got a bit messy? It's a really useful process, and it's non-optional in most professional settings.

Regarding everything you explained regarding the DX9 shader decompile process, I think I pretty much figured most of that out and you should see that in the code when you get to have a look. The reason I can't compile the ASM then decompile the binary is it loses the comment header (which is why I added the translate option).

I would like to head down the route of combined DX9 and DX11 code if possible. My previous attempt to make changes to this project was trying to properly abstract the common 3DMigoto code from the DX wrapper classes (in the view to add some useful new features), but it was way too complicated without more intimate knowledge of how everything works.

This isn't done yet, but if you're interested in the decompiler in particular then hopefully we can make some solid improvements and merge it into your tidied up branch when it's ready.

I'm on discord (also in the 3D vision group) if you want to talk there, although I have the impression you don't use it.

bo3b commented 2 years ago

I understand that you don't have lots of time, but maybe not doing PRs is the reason the code got a bit messy? It's a really useful process, and it's non-optional in most professional settings.

I've used PR plenty of times, but it's not always the right tool for the job. We used it early on in 3Dmigoto, but decided it added too much friction for more novice devs, and our reasoning was that we preferred having more people work on the code to having a strict process.

PR are the right tool if you can't trust your devs to do right, and need a filter. And it definitely makes the code worse without it, but it's a judgment call as to which is most important. Maybe we were wrong, but that's what we decided.

Also, even though me and DarkStarSword are professional devs, this code does not get the full treatment because we can't afford the time that takes. So it also has the 'just-get-it-working' syndrome pretty much all throughout. It's the nature of free software. At the end of the day, working code is always going to be more valuable than anything else, and we cared much more about getting stuff working.

Regarding everything you explained regarding the DX9 shader decompile process, I think I pretty much figured most of that out and you should see that in the code when you get to have a look. The reason I can't compile the ASM then decompile the binary is it loses the comment header (which is why I added the translate option).

OK with me. If you think that's the best approach we can do that.

There was a prior attempt at adding this to 3Dmigoto as found in old pull requests. https://github.com/bo3b/3Dmigoto/pull/39

DarkStarSword notes there that it is no longer necessary because Dave's DX9 port already does everything it needs. Probably that is ASM only, I'm not sure Dave did any work on the Decompiler to get DX9 shaders working. In any case, based on that comment, it is probably worth your time to look at the DirectX9 subproject and see if it handles the dumping of shaders for you. That will build a d3d9.dll that you would use, so maybe that's not exactly what you want.

I would like to head down the route of combined DX9 and DX11 code if possible. My previous attempt to make changes to this project was trying to properly abstract the common 3DMigoto code from the DX wrapper classes (in the view to add some useful new features), but it was way too complicated without more intimate knowledge of how everything works.

Yeah this is pretty much really complicated now. DarkStarSword has made some changes that will indicate differences between the DX9 and DX11 paths, as seen by the #ifdef variants for stuff like Overlay, and Hunting.

We don't have any sort of design docs because of the prior mentioned best effort to get it working, but I can help explain structure. The DirectX9 subproject builds a d3d9.dll, the DirectX11 builds the original d3d11.dll. These are wrapper objects of the dlls, not hooking, it most cases. So it's not really possible to get both d3d9.dll and d3d11.dll code working at the same time with the current structure, because a given game will only use one API. And sharing between the two dlls is not done at all at present, the idea is that you'd builld one or the other, not both.

The DirectX9 does use some common code like for the hunting, overlay, and regex functionality. I'm not sure about the frameanalysis part. I expect it also has connections to DecompilerHLSL but have not looked. If the DirectX9 doesn't connect the Decompiler, it definitely could.

I've spent some time thinking about a combined tool here, as we've always liked the ability that Reshade has of being able to rename the dll to d3d9 or d3d11 or dxgi as desired, and have it load. (The DirectXGI subproject is our loader dll that just LoadLibrary(d3d11).) That part is easy enough, we can export the required functions for the dll and make a single build that combines both tools into one. However, I don't think it's possible to have both variants run at the same time, because we are using wrapping as the primary approach, which makes a schism in the two worlds. It's not impossible that we could use hooking to do all this, there is already a full hooking chunk for DX11 in the Hooked* objects, which pass through to the wrapped Hacker* objects. Still, not sure it's worth the trouble to get both working at the same time. The only time I'd expect that to be useful is for injected overlays or other non-game code. It's certainly technically possible to get all that working, I just don't see it as worth the effort and time. Please let me know your use case here, and why you feel having them combined would be best. > This isn't done yet, but if you're interested in the decompiler in particular then hopefully we can make some solid improvements and merge it into your tidied up branch when it's ready. > > I'm on discord (also in the 3D vision group) if you want to talk there, although I have the impression you don't use it. Sounds good. I'm definitely interested in bringing Decompiler up to speed for DX9 for anything it cannot currently handle. I also vastly prefer HLSL to the ASM, even as we have good tools for handling ASM via regex now. One reason DarkStarSword wanted ASM instead was because it's not possbile to get 100% accurate Decompilation. There are code glitches that happen because the semantics don't match. As an example, the Decompiler currently will use a float4 for an output that should be a bool, but there is no way for the Decompiler to know it should be bool. So some bool ops turn into 0x0/0xffffffff tests instead, which generates subtly different code. In general these have no actual impact on the output, but there are cases where it breaks. I've studied this fairly deeply long back, and there is quite a lot we could improve there, because some of the opcodes we can know have bool outputs, and we could manage those more correctly. We can't get it to be 100% because some information is lost in the process, but it could definitely be better if we deemed it worth the effort. I didn't do this effort because we have a lot of Compute Shader glitches and SM5 mistakes that we bypassed by switching to ASM. For SM1 shaders, we'd be able to make it better, but again it depends on whether it's worth the time spent. The Decompiler also only partly uses the James-Jones cross compiler, and use text matching for other parts that should come from his binary inspection instead. Another piece in the 'nice-to-have' category, not sure it's worth the time. I think it's clear that handling all SM1 opcodes and shaders for DX9 is clearly worth the effort, less sure about fixing stuff like bool/float mismatches, as it was not a problem when we used it for DX11 fixes, even as the code we generated was not identical. It was good enough to not generate graphic glitches or crashes, except for rare cases that we could fix manually. Anyway, hope that fills in some gaps. Let me know what you feel you need, and I'll help as much as I can.
Nintynuts commented 2 years ago

@bo3b That does fill in some gaps, although some of it is still a bit outside my understanding, thanks. I don't know which time zone you're in and what platform(s) you prefer to communicate on, but if you're available to discuss this a little more real-time, that could be really helpful.

This PR

What I've done so far and the previous DX9 PR

I had a look at the changes in that PR, and it seems identical to the changes that ultimately got merged. I've not checked that every line is there, but large chunks look familiar. The main difference I can see is it importing a different compiler called mojoshader, rather than using DX9's built in (dis)assembler.

If someone could verify that I'm not being really dumb and it's not capable of generating a compilable HLSL file from SM1-3 ASM I'd appreciate it. It seems odd that 2 separate people have tried this, it got merged, and it doesn't work; so I feel like I might be missing something.

The main thing which supports my conclusion is that the translator replaced the constant registers with variable names from the comment header, but it never added declarations to link those variable names back to the correct constant register (so I don't know how it would compile). Plus, all the input/output parameters are undeclared and there was no closing bracket on the main method parameters. This is what I was fixing/adding.

What could be done to solve remaining issues

To properly decompile SM1-3 I think I will need to parse the entire main body before outputting the HLSL file (to capture used registers and IO variables), so in principle there might be an opportunity to make notes about register usage and their likely type, which could solve some of those errors.

This project

Review process

I would argue that novice devs wouldn't touch this with a barge pole 😂. But seriously, anyone competent enough to be working on this code is probably aware of the PR process to some extent so it shouldn't be a barrier to entry. Also, even with devs you trust to do a good job there's always aspects where something might not be clear, might be a mistake, or could be done better that the author might not see. You can't go back and change things now, but maybe putting small amounts of effort into PRs in the short term can help avoid large amounts of effort to straighten things long term (like you're doing now). It's your repo so ultimately up to you, but that's just my perspective. Personally, I can't help but do something properly or not at all, and although I think I can write good code I know I can miss things too, so a second set of (more experienced) eyes is invaluable.

Multiple DX versions:

Although having support for multiple DXs in the same dll would be neat, if it's a lot of effort I can understand the reluctance. I was thinking that there would still be separate DLLs built for each DX wrapper, but they could make use of shared library projects that depended on the real DX9 (10?) and 11 libraries. So a thin project for each DX version supported, which just contains the static exported DX API which redirects certain calls to a separate project with the shader hacking engine (which can be broken down into frame analysis, de/compiler, ini parsing/overrides/commands, etc as appropriate). I know it's not quite that simple as different DX versions support different features, but maybe a long term goal to aim for? And we can start with just the Decompiler.

How I imagine it could be structured (It's a class diagram, but think of the classes as libraries/projects, except for the DX devices and interfaces). Methods are just examples. There would need to be a common project with IHackingContext and any types and enums that would be a standardised interpretation of the types and enums passed around by each DX version (probably using generic templates to simplify things), and the wrapper would have to translate those enums and types back and forth when intercepting them.

Nintynuts commented 2 years ago

It turns out newer versions of VS do allow you to install older compilers independently, so I've done that and restored all the projects to v141. I've also moved the DX9 specific code out of the DX11 project so everything builds. Only configuration change I've made is that Injector is now using the same Win SDK version as everything else.

bo3b commented 2 years ago

I got a chance to test the DX9 project, and it seems to mostly work. When I do a full Build of the DirectX9 project on master branch, it compiles and builds without errors. The generated d3d9.dll loads and seems to work fine in a couple of games, but not all. It works best with a 3dmigoto9.ini file. The overlay and hunting UI was working, and it would successfully dump shaders upon choosing a Mark command.

The HLSL is not generated correctly, but the user interface, hunting flow, and ASM is there. Here is an example of the .txt shader generated:

//
// Generated by Microsoft (R) HLSL Shader Compiler 9.29.952.3111
//   using 3Dmigoto v1.3.16 on Sun May 15 12:17:02 2022
//
//
// Parameters:
//
//   float4 SampleOffsets[2];
//
//
// Registers:
//
//   Name          Reg   Size
//   ------------- ----- ----
//   SampleOffsets c0       2
//

vs_3_0
dcl_position v0
dcl_texcoord v1
dcl_texcoord o0
dcl_texcoord1 o1
dcl_position o2
add o0, c0, v1.xyyx
add o1, c1, v1.xyyx
mov o2, v0

// approximately 3 instruction slots used

///////////////////////////////// HLSL Code /////////////////////////////////
// // ---- Created with 3Dmigoto v1.3.16 on Sun May 15 12:17:02 2022
//
//
//
// // 3Dmigoto declarations
// #define cmp -
//
//
// void main(
// // Needs manual fix for instruction:
// // unknown dcl_: dcl_position v0
// // Needs manual fix for instruction:
// // unknown dcl_: dcl_texcoord v1
// // Needs manual fix for instruction:
// // unknown dcl_: dcl_texcoord o0
// // Needs manual fix for instruction:
// // unknown dcl_: dcl_texcoord1 o1
// // Needs manual fix for instruction:
// // unknown dcl_: dcl_position o2
//   o0 = v1.x + SampleOffsets.xyzw;
//   o1 = v1.x + c1;
//   o2 = v0.x;
// }
//////////////////////////////// HLSL Errors ////////////////////////////////
// W:\SteamLibrary\steamapps\common\Contrast\Binaries\Win32\ShaderFixes\685dc6fa7119e834-vs_replace.txt(20,3-4): error X3000: unrecognized identifier 'o0'
/////////////////////////////////////////////////////////////////////////////

So, unless I'm missing something fundamental, it seems to me that the DirectX9 project is mostly what you need. Although you have yet to respond to my question as to what is your motivation? We all have limited time to work on things, and it is important to keep in mind the opportunity cost of working on stuff that seems to have no other users. Always OK if it's just hobby programming or something just for you, but for me I want my stuff to be used in the real world and so I care more about shipping than purity.

The Decompiler can be extended to work here, and currently does not work on Directx9 shaders. I looked this up, and the reason is because Microsoft changed the input and output semantics as part of DX11. So even though it's a SM3 vertex shader, it's using DX9 semantics, not DX11 semantics.

Best example is the missing handler for _dclposition v0 which is done via VS_Position for DX11. The docs say these are the same. So a prior SV_Position is the same as a DX9 POSITION used in HLSL, which turns into the dcl_position at asm time. This one can map directly to the SV_Position handler in the code already.

The Decompiler works on a line by line basis, and has very limited look ahead. Unless you want to spend time rearchitecting the entire thing, you are better off to just follow the model there, and do line by line output. The only things that are saved over multiple lines are a handful of variables and their uses. The Decompiler is a crazy thing that Chiri originally wrote, and not something I would have even attempted, because from a professional dev perspective it's just madness. However, it works surprisingly well, even in it's crufty state. A good example of wildly imperfect code still being amazingly useful. We used it for years before SM5 and CS broke us down.

In any case, based on these limited tests, I think you are pretty much on the right track here with what the Decompiler needs to be able to support DX9 shaders. However, I would say that doing other work outside of getting the Decompiler up to speed is probably not time well spent, since the d3d9.dll is already running and 90% of the way there. Your call though.

Nintynuts commented 2 years ago

I got a chance to test the DX9 project, and it seems to mostly work. When I do a full Build of the DirectX9 project on master branch, it compiles and builds without errors. The generated d3d9.dll loads and seems to work fine in a couple of games, but not all. It works best with a 3dmigoto9.ini file. The overlay and hunting UI was working, and it would successfully dump shaders upon choosing a Mark command.

I guessed it would be mostly functional, so it's good that's the case. When I'm done, that should mean I can use 3DM rather than Helixmod 👍.

The HLSL is not generated correctly, but the user interface, hunting flow, and ASM is there.

Yep, that's what I'm trying to fix.

So, unless I'm missing something fundamental, it seems to me that the DirectX9 project is mostly what you need. Although you have yet to respond to my question as to what is your motivation? We all have limited time to work on things, and it is important to keep in mind the opportunity cost of working on stuff that seems to have no other users. Always OK if it's just hobby programming or something just for you, but for me I want my stuff to be used in the real world and so I care more about shipping than purity.

From the point of view of this PR, the motivation is to add HLSL decompile for DX9 shaders so DX9 fixes are easier for people to do.

The Decompiler can be extended to work here, and currently does not work on Directx9 shaders. I looked this up, and the reason is because Microsoft changed the input and output semantics as part of DX11. So even though it's a SM3 vertex shader, it's using DX9 semantics, not DX11 semantics.

Yep, this is what I worked out too. I assume you're using DX9 as short hand for SM1-3 and DX11 for SM4-5, although there are more differences between each shader model version that affect IO variables. I'm not as confident about how those SM versions differ in HLSL compared to ASM, but I've found some slide packs and documentation which gives some examples that have guided me so far.

Best example is the missing handler for _dclposition v0 which is done via VS_Position for DX11. The docs say these are the same. So a prior SV_Position is the same as a DX9 POSITION used in HLSL, which turns into the dcl_position at asm time. This one can map directly to the SV_Position handler in the code already.

I think I got that one already, but I will check the link, thanks.

The Decompiler works on a line by line basis, and has very limited look ahead. Unless you want to spend time rearchitecting the entire thing, you are better off to just follow the model there, and do line by line output. The only things that are saved over multiple lines are a handful of variables and their uses. The Decompiler is a crazy thing that Chiri originally wrote, and not something I would have even attempted, because from a professional dev perspective it's just madness. However, it works surprisingly well, even in it's crufty state. A good example of wildly imperfect code still being amazingly useful. We used it for years before SM5 and CS broke us down.

In any case, based on these limited tests, I think you are pretty much on the right track here with what the Decompiler needs to be able to support DX9 shaders. However, I would say that doing other work outside of getting the Decompiler up to speed is probably not time well spent, since the d3d9.dll is already running and 90% of the way there. Your call though.

In this PR at least, that's my plan. I think storing the main body in a separate variable rather than writing it out immediately would be a sensible compromise, then I can build up the IO and temp registers as it translates, write those out first rather than jumping backwards in the file, and that should allow me to fix those issues with minimal changes.

Last night I spent some time moving files into projects where it makes sense for then to live without having files added to projects from others. It's nearly there, I was just being thrown by linker errors caused by referring to projects that are exes not libs, but I think I know how to fix that. There may be some crossover with your branch (I've not checked yet), but I'm only touching files relevant to the decompiler. If you think it would be sensible to rebase onto your branch, I can do that instead.

DarkStarSword commented 2 years ago

I got a chance to test the DX9 project, and it seems to mostly work. When I do a full Build of the DirectX9 project on master branch, it compiles and builds without errors. The generated d3d9.dll loads and seems to work fine in a couple of games, but not all. It works best with a 3dmigoto9.ini file.

It has been a long time since I looked at the DX9 port, which I had to drop pretty abruptly due to some personal/family issues at the time. I recall that there were some fairly serious crash issues that were looking like they may have potentially required a bit of an architectural overhaul to properly address (I think I was trying to work out whether there was a quick fix when I dropped it), and some potential licensing concerns over the use of 3rd party font rendering code that I was trying to determine if it could be a problem / potentially incompatible with GPL (for our community it is unlikely to be a problem as IIRC the code seems to have been released openly with intent for anyone to use, however if memory serves it was released without a defined license on some random internet forum (IIRC it was a cheater community, odd place to release it) and was vaguely derived from Microsoft example code found in an ancient DX SDK... the lack of clarity on the licensing situation on that code would be of concern to any companies planning to release a version of 3DMigoto and would probably need to run it by their IP lawyers first), and I think there was also 3rd party code included that I traced back to GeDoSaTo that effectively removed our compatibility with MIT (pretty sure I already committed the removal of MIT to the license file), making 3DMigoto GPL only (unless we decide to drop/replace that code before next release).

I had a bit of a todo list for the port here: https://trello.com/c/cucbVAKg/316-dx9-support

DarkStarSword commented 2 years ago

My review comments for the DX9 port can be found on these two commits, which might give some more insight as to what I was thinking at the time (@davegl1234 never responded to these): https://github.com/bo3b/3Dmigoto/commit/dae919eb51a761cd9daab6deaff8021a1cd27a97 https://github.com/bo3b/3Dmigoto/commit/915a0d7124570fa1700656d3e180ef197bb1e30f

Nintynuts commented 2 years ago

Thanks for the insight @DarkStarSword. It sounds like after this hole in HLSL decompilation is fixed there will be quite of bit of work before DX9 3DM is reliably usable. I'm happy to help with that, but I wouldn't be confident going about it alone.

Last night I restructured the decompiler code and got all the projects building locally except DX10 (missing IUnknown Interface header file) and DXGI where lots of these are undefined, but there are quite a few restructuring changes which may not be appropriate for this PR or may conflict with @bo3b's branch. I could either commit it once I'm happy and you can tell me if you want any of it reverted, or we could discuss it first on discord or something.

Brief summary:

The HLSLDecompiler project references d3dx9shader.h for D3DXAssembleShader, so d3dx9.lib is now a dependency of all those projects, which isn't ideal but I couldn't see a clean way around it. My understanding is that there is no assemble method in D3DCompiler (just compile directly from HLSL->binary), and the DX10/11 implementation is all local code (presumably not compatible with SM1-3)?

Moving things around also threw up some errors which I've fixed where I guess the compiler is being stricter than it was before (using for each (x in y) which is a non-standard extension, and assigning const char* to char* in multiple places) and added some missing includes that I guess were being obtained somewhere in one of the previously included headers that I removed.

bo3b commented 2 years ago

the lack of clarity on the licensing situation on that code would be of concern to any companies planning to release a version of 3DMigoto and would probably need to run it by their IP lawyers first), and I think there was also 3rd party code included that I traced back to GeDoSaTo that effectively removed our compatibility with MIT (pretty sure I already committed the removal of MIT to the license file), making 3DMigoto GPL only (unless we decide to drop/replace that code before next release).

@Nintynuts I saw you asking a question of DirectXTK, did you happen to get fonts working from there in DX9? That would alleviate needing the font library here. The goal of adding DirectXTK was as a set of libraries to provide some basic functions like font rendering and screen shots that are boilerplate stuff. I have not looked closely at what Dave brought in for DX9 subproject.

Pretty much agree this licensing won't fly for our upcoming stuff, but is probably fine for HelixMod use.

On the crashes for d3d9.dll- just noting that the first place to start here will be to use the debug layer for DX9, which only works on Windows 7. Anything else is a waste of time, because until it can pass the debug layer, it's not worth looking at. I think, but am not sure, that this can run inside a VM running Win7.

Last night I restructured the decompiler code and got all the projects building locally except DX10 (missing IUnknown Interface header file) and DXGI where lots of these are undefined, but there are quite a few restructuring changes which may not be appropriate for this PR or may conflict with @bo3b's branch. I could either commit it once I'm happy and you can tell me if you want any of it reverted, or we could discuss it first on discord or something.

I don't think these changes should be necessary. I can't quite pin down what is happening there, but when I compile and build the DirectX9 subproject directly it works in all cases without errors. I don't think we should move the folders around, because that will cause grief with the other subprojects. The basic structure is to have folders for the subprojects, and then use the solution root for shared files like util.h, so that any subproject can find them.

The D3D_Shader subproject is actually Flugan's Assembler. I've renamed all the components Assembler in my upcoming branch, but it's not the branch to use for these changes. Probably a mistake on my part, but I'm only renaming and reformatting the DirectX11 subproject because of how long all this takes. I didn't feel I could take on everything and be successful. I've already spent 3 months on it, which is over the top, and I really, really need to wind this up. I've kept good notes, so we can always come back and do the other subprojects as needed.

The DirectX10 subproject should be skipped because it's horribly out of date and is only for historical reference, because there are only 1 or two games that ever could use it. The D3DCompiler_ projects are just loaders that pre-load d3d11.dll for strange games where we needed it. Only the _46 is probably useful nowadays. DirectXGI subproject is still used, and is also just a wrapper for dxgi.dll to pre-load d3d11.dll. Injector subproject uses the hooking mechanism to hook into a game, instead of wrap the d3d11.dll. Useful in several odd games. BTW- a lot of our complexity stems from having to work around various freakish behavior of games that we obviously have no control over.

New HLSLDecompiler .lib project

This is probably OK, but seems kind of arbitrary to me? The project structure as it stands is perfectly OK with subprojects in folders, and top level objects used by anything that needs it. I don't see this change as being particular better, just different.

BinaryDecompiler

For this project, changing this is wrong. We are very strict about not ever modifying the included library functions, because we don't want to be in a position where we've drifted off the master code. We could use submodules for these, but I'm not a huge fan of those, because I always end up having to answer questions about them. We use the source code instead of binaries or libs so that we can source level debug step through all code. And we update the code from the master as needed if we hit bugs that are fixed, but otherwise make no attempt to keep up to date. The BinaryDecompiler, crc32c_hw, DirectXTK are all in this category.

DirectX11 and DirectX9

Something is definitely wrong with your build somehow, DirectX11 has compiled and built properly for years and years, and already includes the HLSLDecompiler components. In addition, it absolutely requires the use of D3D_Shader/Flugan's Assembler, because that is the primary way that 3Dmigoto is used today- to regex Assembly code, and then reassemble the modified code using Flugan's Assembler. You seem to think that this project is somehow horribly broken and terrible, but I assure you that is not the case. I've already refactored the code completely twice, and successfully setup a nice framework that DarkStarSword was able to use very effectively to add an incredible array of features like the FrameAnalysis, CommandLists for arbitrary shaders and resource manipulation, and regex automatic shader fixing.

I would recommend doing a 100% clean clone of top of tree master, and try just building that. Don't change toolsets, don't change Windows SDK version. If you don't have 10.0.17763.0 loaded, you can get it easily via the Tools->GetToolsAndFeatures. I can't explain why you would see any errors here, as it's the code base that builds 1.3.16, our latest release.

It's worth noting that for DirectX9, you need to have the DXSDK_DIR defined, and it needs the last June DirectX9 SDK. Given how broken that SDK is setup with installers, and it being just a tad hard to find, it probably makes sense to bring that in as a set of files like the DirectXTK. However, when I build DirectX9 subproject with the SDK properly setup, I get proper d3d9.dll output, and no errors. Lots of warnings though.


This is not an error from using VS2019. I just tested this using a clean clone of master, and both DirectX11 and DirectX9 compile and build without errors. One error on DirectX9, which is that the copy from Dependencies for the d3dx.ini and Uninstall.bat are the wrong paths, but also DirectX9 needs a different file name. But the build itself works on VS2019.

Especially for this variant, I don't see any advantage to changing toolsets. For some background, as a general rule I strongly prefer to be a lagging consumer, because backwards compatibility is far more valuable to us than anything they've added recently. If we change toolsets and then break one of our fixes because of that, we don't do our end users any favors. And no one is going to test over 1000 fixes for compatibility, so it's important to be judicious when making changes.

In my book there are only ever two reasons to update- 1) Are their bugs or problems we cannot tolerate? 2) Is there a new feature we simply must have? Being on the update treadmill is what the corporations want us to do, but it's not necessarily in our best interest. I use update, not upgrade. In today's world, this is all just change for the sake of change, and typically introduces as many problems as it solves, and I prefer to use my time for stuff that makes a difference for our users. Microsoft is fairly good about keeping their build tools running long term and not requiring updates.

DarkStarSword commented 2 years ago

Pretty much agree this licensing won't fly for our upcoming stuff, but is probably fine for HelixMod use.

FWIW since the 3rd party code is only used in the DX9 project we do still have the option of releasing the DX11 project under MIT, though this could be confusing if we have different releases of 3DMigoto under different licenses. I do have some preference to replacing the DX9 font rendering library and dropping GeDoSaTo code so we can revert to the previous dual MIT+GPL license for all projects.

Nintynuts commented 2 years ago

@bo3b

Regarding the folder structure, I can understand not wanting to touch projects from external sources, although if they're on GH, then submodules would be better (a readme might help avoid questions). It wasn't failing to build before, I was just saying it's building with these changes. I'm just trying to make a clearer structure with better encapsulation.

The reason for the new project is that it's a library, so using more default project settings and the project reference system works properly. Now there are 2 exe and 2 dll projects referencing a static library rather than exe projects, and the exe projects only contain code exclusive to them.

I come from working mostly on C#, and all this stuff is non-optional there. So, although almost anything is possible in C++, I think these changes help make it more accessible and easier to maintain.

FWIW, I really hate having 'shared code' in the solution directory which added to multiple projects and implementations in header files. I would prefer this to be in another shared library, but I'm not trying to do that here.

DarkStarSword commented 2 years ago

Regarding the folder structure, I can understand not wanting to touch projects from external sources, although if they're on GH, then submodules would be better (a readme might help avoid questions). It wasn't failing to build before, I was just saying it's building with these changes. I'm just trying to make a clearer structure with better encapsulation.

Notably we do have some important bug fixes for James-Jones CrossCompiler AKA BinaryDecompiler in our tree so we can't just submodule the "official" repository without those being merged upstream first... however no one is maintaining the upstream repository, so that isn't likely to accept them any time soon. We could submodule my fork that I used to send those pull requests upstream (I think there is at least one additional bug fix I'd have to add to that fork first), but it's pretty questionable as to what advantage that would have. Regardless of whether our version is in the 3DMigoto tree or our own fork we maintain, the diff to the upstream CrossCompiler should be kept as small as possible.

Nintynuts commented 2 years ago

@DarkStarSword I can restore the includes and internal_includes folders, although it should be noted we are including from both. Just makes consuming it messier. I need to get a better understanding of what project references actually do, since in most cases I can just do #include "blah.h" from another project and it works, whereas in one place it can't find it without the full path (in both cases this is with the custom include paths removed). Maybe if I add BinaryDecompiler's include folder to its own include directories it will be inherited by things referencing it?

davegl1234 commented 2 years ago

Regarding the gedosato code, the functionality that was actually needed for DX9 I've since reimplemented, but functionally different (so no concerns there). E.g Rather than hooking GetMessage, PeakMessage, and the SetWindowLong wndproc stuff, I am replacing the wndproc of the window and forwarding (manipulated) messages to the real wndproc. So I can probably replace the gedosato stuff with this, to avoid the licensing issues.

I guess we should have DX9/DX11 using the same cursor/upscaling hooking code anyway, right?

Nintynuts commented 2 years ago

@davegl1234 Sounds worth discussing/looking at, but maybe not in this PR? #158

@DarkStarSword @bo3b Also made this for Fonts #157

Nintynuts commented 2 years ago

Quick question: As best I can understand, all the cmp operations do addBoolean(op1) and most of the other operations have a call to removeBoolean(op1), presumably because the outcome of those operations cannot be a boolean, all of this for the benefit of a few operations where it needs to know if a register contains a bool. Some of the non-cmp operations are missing the removeBoolean call, is that right?

Ones I have noticed so far:

I also wonder if:

should have addBoolean (at least if the inputs are booleans)?

bo3b commented 2 years ago

It's been too long, so I cannot be positive, but I expect you are right. The addBoolean and removeBoolean were original code from Chiri that I extended to use the cmp macro to improve the code generation. Prior to that, And and Or operations would get converted into math functions instead. Mostly worked, but I was trying to get closer to original ASM generation.

As another aside that I mentioned above, I was in the process of moving toward making the operand specifications boolean also, instead of the default float4 for everything. After some experiments it was clear that the code generation could be pretty dramatically improved by correcting the variable types. That will be relatively hard to do though, and require some new mechanism to manage the different types. Just mentioning this for background, not expecting you to implement this. I think this is not worthy of effort for DX11 HLSL because as far as I know no one uses it anymore for those fixes, because the regex asm is far superior. But assuming you get dx9 working, it would raise the value quite a lot and I'd probably come back to it.

Having a few opcodes missed would not be a surprise, although it would be worth checking the git commit notes to get more background on the changes. I might have specifically avoided some opcodes if they were generating properly for example, as my focus was the output ASM, not a purity of the C++. Would possibly be worth doing a git blame to see commits and the motivations at the time. If I don't mention explicitly skipping some, it's most likely an oversight. As a general idea, both me and DarkStarSword try to make informative, high quality, commit notes. And this is also why we try pretty hard to keep past history usable.

BTW there is a repo that included a lot of our original test files, and I used these as a sort of test matrix for HLSL generation and unit tests to avoid breaking stuff that previously worked. https://github.com/bo3b/3d-fixes Not sure that is worth your time, it'll be a mess, but it's a resource anyway. In any case, I would say it would be worthwhile to go ahead and make sure those opcodes are also handled, and maybe double check by doing a spot check on a handful of shaders from this repo to ensure it doesn't make them worse.

bo3b commented 2 years ago

@DarkStarSword I can restore the includes and internal_includes folders, although it should be noted we are including from both. Just makes consuming it messier. I need to get a better understanding of what project references actually do, since in most cases I can just do #include "blah.h" from another project and it works, whereas in one place it can't find it without the full path (in both cases this is with the custom include paths removed). Maybe if I add BinaryDecompiler's include folder to its own include directories it will be inherited by things referencing it?

For these, we want to add specific include directories to the solution properties under C++ AdditionalIncludeDirectories. Anything that is 'our' code is intended to be part of the C++ Additionals, and anything that is system-wide/Win10 is part of the VC++Directories includes. And 'our' stuff will be included with "" whereas system stuff will use <>. It's not currently consistent and setup this way, but that is where we are headed with renaming to put some consistency behind it.

As part of that, we have $(SolutionDir) as a root include, and that means that it is always OK to add a path for something owned by a subproject. If you are making HLSLDecompiler a lib subproject, that would then be OK to use its headers in other places, specifying the subpath from the root down. For some of these that are genuinely shared headers between subprojects, it might make sense to move them to the solution root to make their shared nature explicit. Some of these are a judgment call, like DecompileHLSL.h is clearly mostly involved with the Decompiler chunk, but also used by cmd_decompiler. Util.h has no ambiguity and is clearly better shared.

We specifically want to avoid any use of ../../stuff.h because of the hidden nature of .. and lack of clarity on where that exactly is. It's better practice to use the paths going down because they document the exact location this way. Again, not fully consistent in current code base, but that is the goal.

I understand that you object to having common files like util.h, log.h, version.h, shader.h, et. al. in the root, but this is how the project is structured to share those common files with all the subprojects. Because we add $(SolutionDir) as an include dir, that means any use merely has to be #include "util.h" to be found.

I don't see anything wrong with this setup, but if you can explain why this is bad, I'm always willing to entertain improvements. My fundamental goal is to always start with best practices, and only deviate in rare special circumstances.

Nintynuts commented 2 years ago

Would possibly be worth doing a git blame to see commits and the motivations at the time. If I don't mention explicitly skipping some, it's most likely an oversight.

Will do.

BTW there is a repo that included a lot of our original test files, and I used these as a sort of test matrix for HLSL generation and unit tests to avoid breaking stuff that previously worked. https://github.com/bo3b/3d-fixes Not sure that is worth your time, it'll be a mess, but it's a resource anyway. In any case, I would say it would be worthwhile to go ahead and make sure those opcodes are also handled, and maybe double check by doing a spot check on a handful of shaders from this repo to ensure it doesn't make them worse.

I was actually going to ask if there were any files I could use to ensure the code behaves the same or better, so that's perfect, thanks.

We specifically want to avoid any use of ../../stuff.h because of the hidden nature of .. and lack of clarity on where that exactly is.

Generally I agree if it goes outside the project, and I've tried to avoid that. Things in $(SolutionDir) do have this issue (see below), which for the sake of this PR I will probably try to revert. There's one place where for some reason it couldn't find the file unless I added an AdditionalIncludeDirectories (which I'm trying to avoid) or a ../ path.

I understand that you object to having common files like util.h, log.h, version.h, shader.h, et. al. in the root, but this is how the project is structured to share those common files with all the subprojects. Because we add $(SolutionDir) as an include dir, that means any use merely has to be #include "util.h" to be found.

I don't see anything wrong with this setup, but if you can explain why this is bad, I'm always willing to entertain improvements. My fundamental goal is to always start with best practices, and only deviate in rare special circumstances.

In all code I've ever worked on (albeit mostly C#), each file belongs to a project and anything wanting to use that code includes the project. I've never seen shared code in the root directory before, and it means that the associated CPP files have to be compiled by each project wanting to use that shared code. I appreciate that in some cases this solution takes advantage of that by using different preprocessor definitions for each project, which means the shared files produce different code depending on who compiles them and how. In my view all of that is bad, and generally preprocessor statements shouldn't be used like that if it can be avoided, but this is an unusual case. It's beyond the scope of this PR, so no need to keep discussing here.

Most of what you're mentioning is general information, which is useful, but it's not clear if you've looked specifically at the changes I've made. Usually in PRs I see comments or suggestions against my changes, but I understand if you haven't had time to do that yet. I'm trying to make things better in a way that feels right to me, and that might be easier to see by trying it out.