narzoul / DDrawCompat

DirectDraw and Direct3D 1-7 compatibility, performance and visual enhancements for Windows Vista, 7, 8, 10 and 11
BSD Zero Clause License
878 stars 67 forks source link

Wild Wild West: The Steel Assassin DPI scaling issues #319

Closed automatedbugreportingfacility closed 3 weeks ago

automatedbugreportingfacility commented 1 month ago

I'm splitting this off of #306 as it's a complicated problem (or a set of problems) that I believe warrants a separate ticket.

There are 2 unrelated problems with DPI scaling (tested at the size of 150%):

1) DDrawCompat defaults to the PerMonitor mode, and it switches the game to use it from the default "Unaware" mode. Except for some minor issues with the initial configuration window, there's a major problem with the in-game cursor being corrupt in this mode:

dpi2

The cursor is off both with and without DDrawCompat, so clearly Microsoft is at fault, but I'm letting you know in case you can think of some GDI::Cursor magic to improve the situation.

2) If DDrawCompat is configured to use DpiAwareness = app, then the game runs in its default "Unaware" mode. It's mostly good (except for some expected blurriness of the scaled elements), but DDrawCompat causes a crash if the cursor is positioned on the top-left edge of a 3D screen (such as the shooting range). It may take a couple of tries -- position the cursor on the left edge of the screen and try touching the top a few times.

The crash occurs at 0x28524795 in vrd3d.dll:

        28524768 8b  8d  48       MOV        param_1, dword ptr [EBP + 0x80348]  ; [EBP + 0x80348] is the Y position of the cursor
                 03  08  00
        2852476e 8b  84  24       MOV        EAX, dword ptr [ESP + 0x174]
                 74  01  00 
                 00
        28524775 8b  f1           MOV        ESI, param_1
        28524777 0f  af  c1       IMUL       EAX, param_1
        2852477a 0f  af  35       IMUL       ESI, dword ptr [DAT_285e0828]   ; [0x285e0828] == 800 (X of the game's resolution)
                 28  08  5e 
                 28
        28524781 8b  95  44       MOV        EDX, dword ptr [EBP + 0x80344]  ; [EBP + 0x80344] is the X position of the cursor
                 03  08  00
        28524787 8b  bd  c4       MOV        EDI, dword ptr [EBP + 0x400c4]  ; a stable pointer to some table
                 00  04  00
        2852478d 8b  6c  24       MOV        EBP, dword ptr [ESP + 0x30]
                 30
        28524791 03  f2           ADD        ESI, EDX
        28524793 d1  e8           SHR        EAX, 0x1
        28524795 66  8b  0c       MOV        param_1, word ptr [EDI + ESI * 0x2] ; crash due to ESI being negative
                 77

The game tries to read a table with the index being CURSOR_Y * 800 + CURSOR_X. With DDrawCompat, positioning the cursor in the top-left corner may cause the game to read -1 as the cursor Y coordinate, which leads to an out-of-bounds access. Without DDrawCompat, or with DisplayResolution = app, the cursor coordinates in the top-left corner of the screen are always (0, 0). The crash emerged after d5a44eb -- before that revision, there was a problem with the minimum cursor position being (1, 1), i.e. it was unable to actually reach the edge.

Unrelated minor nit: at https://github.com/narzoul/DDrawCompat/wiki/Configuration, ColorKeyMethod should default to auto.

narzoul commented 3 weeks ago

The cursor is off both with and without DDrawCompat, so clearly Microsoft is at fault, but I'm letting you know in case you can think of some GDI::Cursor magic to improve the situation.

Microsoft is not at fault either. The game uses DrawIcon to place the cursor image in DirectDraw surfaces and always expects the cursors to be 32x32, but at 150% they're scaled to 48x48. DrawIcon uses the correct cursor metrics, which is documented.

It's technically lying that the application is "high DPI aware" in the first place, as obviously none of these old applications would be. But there are drawbacks to both DPI awareness modes, so it depends on the game which one happens to work better. For fullscreen games, native DirectDraw itself enables the "high DPI aware" flag in the registry for the process, so Microsoft probably thought it works best for most games.

DDrawCompat causes a crash if the cursor is positioned on the top-left edge of a 3D screen (such as the shooting range).

To be fair, DDrawCompat doesn't "cause" a crash, but admittedly it seems I haven't successfully fixed the DPI unaware cursor position issues that are present without DDrawCompat also. The reason it doesn't crash without DDrawCompat is the same reason it doesn't crash when DisplayResolution=app is used: at low resolutions like 800x600, Windows automatically sets the DPI scale factor back to 100%.

The cursor position is quite inaccurate (even natively) while the cursor is above a DPI unaware window and the DPI scale factor is greater than 100%. It's not a straight mapping between the real (DPI aware) coordinates and the DPI unaware ones.

Originally, I thought the issue was related only to the ClipCursor API, as I observed that the cursor position can escape the bounds by 1 pixel in DPI unaware mode. But it turns out that ClipCursor is not needed, it's enough to have the cursor above a DPI unaware window. This can even result in negative cursor positions, both from GetCursorPos and from various other means of getting a cursor position, such as from window messages. In the game, the problematic source seems to be the lParam of the WM_MOUSEMOVE message.

I revisited my previous fixes with this in mind, and while this is probably still far from covering all possible error cases, it should cover the most frequent ones: ddraw.zip (diff.txt compared to v0.5.2)

The crash emerged after https://github.com/narzoul/DDrawCompat/commit/d5a44eb40c05a8047dd26a2d3a10aa86f879d30b -- before that revision, there was a problem with the minimum cursor position being (1, 1), i.e. it was unable to actually reach the edge.

This was the previous workaround for wrong cursor coordinates, which also used to be documented in the wiki under the DpiAwareness setting, until I thought I fixed the issue by the mentioned commit. The issue was actually worse, because it was possible to click outside of the fullscreen application window, which caused the app to lose focus and minimize itself. But it seems I have somehow accidentally fixed this in some later version, and now only the coordinates remain inaccurate, but clicking at coordinates that are supposed to be 1 pixel outside the window somehow still end up clicking on the window itself. I don't fully understand what Microsoft did here, and I probably never will...

Unrelated minor nit: at https://github.com/narzoul/DDrawCompat/wiki/Configuration, ColorKeyMethod should default to auto.

Thanks! I fixed that now.

automatedbugreportingfacility commented 3 weeks ago

I can confirm that moving the cursor to the top edge doesn't crash the game anymore, thanks!

The reason it doesn't crash without DDrawCompat is the same reason it doesn't crash when DisplayResolution=app is used: at low resolutions like 800x600, Windows automatically sets the DPI scale factor back to 100%.

This is something that I noticed while quitting the game: items on the taskbar will be smaller for a short while until the DPI scale factor is set back to 150%. However, I have difficulty reconciling that with your explanation of the cursor problem with DpiAwareness=permonitor, specifically this part:

The game uses DrawIcon to place the cursor image in DirectDraw surfaces and always expects the cursors to be 32x32, but at 150% they're scaled to 48x48.

The way I understand it, using DisplayResolution=app, which results in changing the scaling factor to 100%, should fix the corrupt cursor, but it does not.

narzoul commented 3 weeks ago

The way I understand it, using DisplayResolution=app, which results in changing the scaling factor to 100%, should fix the corrupt cursor, but it does not.

I'm not sure how it's supposed to work either, but changing the scaling factor after the process has started does not change the cursor size, as reported by GetSystemMetrics, for example. A process restart is needed for that. Probably DrawIcon suffers from the same bug/feature.

automatedbugreportingfacility commented 3 weeks ago

Okay, thanks, I guess that they just send WM_DPICHANGED and don't care.

Too bad Windows doesn't allow per-app scale factors -- at 200%, the Unaware mode looks fantastic with zero blur whatsoever due to pure integer scaling (I imagine that the cursor position accuracy could potentially be improved too due to a very straightforward mapping), but one needs to change the scale factor for the whole OS instead of just one process tree. I guess I'll just wrap the main .exe with a script to change the OS DPI upon entry/exit to save some manual work.