Draft: hooklib: Add D3D11 and DXGI graphics hooks - [opened] #70

Open icex2 opened 2 years ago

In GitLab by @Felix on Oct 25, 2021, 22:19

Merges gfx -> master

New Feature

Summary

Add D3D11 and DXGI graphics hooks and a fullscreen fix for Initial D Zero.

Description

IDZ only supports D3D11 and Ongeki/Unity uses D3D11 by default so the existing D3D9 graphics hook would not apply to them. I also wanted to play Initial D Zero in full fullscreen or in borderless windowed.

Related Issue

The Initial D Zero fullscreen issue was documented and workaround binary patches were made and I like to run the game executables as close to stock as possible so I wanted to fix this at the API level.

How Has This Been Tested?

I tested Initial D Zero running in both windowed toggled via segatools.ini and seeing if it would run and stay maximized. I also tested to make sure Ongeki would startup and not crash in DXGI or D3D11 initialization (now I see why some people say that older Unity does not use D3D11 properly, they use both DXGI 1.0 and 1.1!).

Checklist

Tested with the following games:
- [x] Initial D Zero 1.31
- [x] Initial D Zero 2.11
- [x] Initial D Zero 2.30
- [x] Ongeki 1.20
[x] Followed the developer (style) guidelines
[ ] Updated existing doc of or add new doc to README file(s)
[ ] Updated development documentation

In GitLab by @Felix on Oct 25, 2021, 22:21

I marked this as a draft because there is some code duplication between IDXGIFactory::CreateSwapChain and D3D11CreateDeviceAndSwapChain. Should I factor out the code into a separate helper function?

In GitLab by @Felix on Oct 25, 2021, 22:25

I also realized that CreateDXGIFactory2 is a thing so I will add a hook for that in case any game dynamically checks for it and also MSDN indicates it may be used internally by D3D.

In GitLab by @Felix on Oct 25, 2021, 23:57

added 3 commits

690fd412 - fixup
5858caea - Draft: hooklib/gfx/dxgi.c: Add CreateDXGIFactory2 hook
14660f3f - Draft: hooklib/gfx/dxgi.c: Delegate IDXGIFactory hook selection to helper method

Compare with previous version

In GitLab by @Felix on Oct 26, 2021, 24:00

added 4 commits

f251283e - hooklib: Add D3D11 and DXGI graphics hooks
b072f2d8 - idzhook: Force framed window for ServerBox.exe in graphics hook
636ed0f4 - Draft: hooklib/gfx/dxgi.c: Add CreateDXGIFactory2 hook
a24cd0a1 - Draft: hooklib/gfx/dxgi.c: Delegate IDXGIFactory hook selection to helper method

Compare with previous version

Even this is likely never going to happen, I would check the return value for NULL and run into a controlled death rather than this just crashing without any clue.

Maybe we want to wrap malloc and realloc like bemanitools and have the guarding abstracted, so we don't have to do this everywhere all the time.

I might simplify this by just going with a static buffer size of MAX_PATH * 2. Or does the API imply some sort of edge cases?

Just call GetModuleFileNameW and if it returns an error, I would log it as an error with some info and just die.

Why do we need a a hook for d3d9 and d3d11 here? I would expect only one API to be active. Is there some kind of switch? If yes, how does the game (?) decide which one to use?

I would suggest moving this new block to a separate function to increase readability, e.g.

static void idz_force_fullscreen_serverbox(struct gfx_config* gfx_config)

I think this is fine for now, but might be worth moving the gfx related config stuff to it's own module under hooklib/gfx/config.h/.c

Why is this exposed to other modules now?

Regarding the general architecture, I would suggest to stick to a mostly copy-paste focused approach. We had a similar use-case in bemanitools with d3d8, d3d9 and d3d9ex hooks. Though most things are compatible, not all of them are.

IIRC I tried to re-use as much code as possible, but this created coupling that increased complexity and made it difficult to maintain: Say you fix something on a shared piece of code due to encountering an issue with d3d8. How do you know this is fine on the d3d9 and d3d9ex implementation? You would at least have to test one game using each API and trigger the specific code path.

Even at first glance it looks like copy-pasting increases maintenance, the experience from bemanitools shows the opposite.

Also, I would suggest to split those gfx configs and not use a shared one. Same reasoning.

Hm, that's a fair question. I think you are talking about the part "ensure window is maximized"? It's not very complex and cannot even be tested in isolation. I would stick to copy-pasting here for reasons given in my comment here.

That naming <_>

In GitLab by @tau on Oct 27, 2021, 24:42

I think there should be a separate "gfxhook" static library instead of making a nested directory inside hooklib.

In GitLab by @Felix on Oct 27, 2021, 08:52

Luckily Microsoft has been mostly consistent with naming in this regard.

IDXGIAdapter
IDXGIAdapter1
IDXGIAdapter2
IDXGIAdapter3
IDXGIAdapter4
IDXGIFactory
IDXGIFactory1
IDXGIFactory2
IDXGIFactory3
IDXGIFactory4
IDXGIFactory5
IDXGIFactory6
IDXGIFactory7
etc

In GitLab by @Felix on Oct 27, 2021, 08:52

Commented on util/lib.c line 14

Based on Microsoft's API list at [https://docs.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation]() that lists the functions that are long path aware, GetModuleFileNameW is not a function that is aware of long paths.

It would not surprise me if every module path function in Win32 is still bound to MAX_PATH.

If the function succeeds, the return value is the length of the string that is copied to the buffer, in characters, not including the terminating null character. If the buffer is too small to hold the module name, the string is truncated to nSize characters including the terminating null character, the function returns nSize, and the function sets the last error to ERROR_INSUFFICIENT_BUFFER.

Windows XP: If the buffer is too small to hold the module name, the function returns nSize. The last error code remains ERROR_SUCCESS. If nSize is zero, the return value is zero and the last error code is ERROR_SUCCESS.

If the function fails, the return value is 0 (zero). To get extended error information, call GetLastError.

In GitLab by @Felix on Oct 27, 2021, 08:52

Commented on mu3hook/dllmain.c line 47

It tries D3D11 first then D3D9 then OpenGL. The graphics API used can also be forced via the command line.

[https://docs.unity3d.com/560/Documentation/Manual/CommandLineArguments.html]()

In GitLab by @Felix on Oct 27, 2021, 08:52

Commented on hooklib/gfx/gfx.h line 16

It's exposed to the other graphics submodules. Should I make something like gfx-private.h that the other submodules (D3D9, D3D11, DXGI) can use?

In GitLab by @Felix on Oct 27, 2021, 09:08

I like that idea too. I will do that.

Separate module, agreed. Naming suggestion: gfx-util.c/.h? -> HRESULT gfx_util_frame_window(HWND hwnd)

In GitLab by @Felix on Oct 28, 2021, 01:20

I am not a fan of splitting out the config for shared options between them like how bemanitools has most of the config options under the gfx shared between them. Unless you are referring to have separate config structs that share the [gfx] namespace.

My comment didn't consider that the game isn't using the backends at the same time but only the one that the selection logic decides to activate. Therefore, I think it's ok for now to share the gfx config due to the given use-case. Some sort of split/separation of concerns can be addressed later as well on that level.

In GitLab by @Felix on Dec 22, 2021, 04:15

Commented on hooklib/gfx/gfx.h line 16

changed this line in version 4 of the diff

In GitLab by @Felix on Dec 22, 2021, 04:15

added 3 commits