narzoul / DDrawCompat

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

Wild Wild West: The Steel Assassin pink outline glitch #330

Closed automatedbugreportingfacility closed 4 months ago

automatedbugreportingfacility commented 5 months ago

I noticed that a pink outline is drawn around some objects in the journal:

ppp2-ddrawcompat

It seemed a bit off and I was curious to know if that's supposed to happen, so I checked on Windows 98 SE (in 86Box/PCEm, using Voodoo 3 2000). It turns out the pink outline is not present there, although there is a single pink point near the upper right corner:

ppp2-86box

Also, letter spacing looks a bit better on Windows 98 SE, although I'm not sure if that's related to ddraw.dll.

Steps to reproduce:

  1. Extract pink.zip into C:\Program Files (x86)\Southpeak Interactive\west\SavedGames\.
  2. Restore the saved game, click the saddlebag, click the journal.

GPU: GeForce GTX 970 (driver: 555.99) OS: Windows 10 22H2

Btw, as a user going by the name of Winterfury discovered many years ago (while investigating another SouthPeak game: Dark Side of the Moon -- but his findings are also valid for this game), Wild Wild West: The Steel Assassin depends on the original Rich Edit 1.0 implementation, and using the Rich Edit 1.0 emulator provided in the modern systems will cause some problems, such as missing dialog lines or incorrect scrolling/selection in the save menu. Therefore, one needs to copy RICHED32.DLL from one of the legacy operating systems into the game /bin/ folder in order to fix those issues. I'm using RICHED32.DLL 4.0.834.839 copied from Windows 98 SE (SHA1: 2aed136ab27d6e528877a5edd6cc49e599594a9f), which seems to work well. That's probably not related to this issue, but since reproducing it requires using the save menu, I thought I'd mention this detail.

narzoul commented 4 months ago

It seemed a bit off and I was curious to know if that's supposed to happen, so I checked on Windows 98 SE (in 86Box/PCEm, using Voodoo 3 2000). It turns out the pink outline is not present there, although there is a single pink point near the upper right corner

I've checked native NVIDIA drivers on Windows 11 with and without DDrawCompat, D3D9On12 drivers, the "Forced software 3D renderer" setting in the launcher's Hardware Configuration, Windows XP and Windows 2000 VMs, and the pink outline was present with all of them. I also made sure it's not a color key bleeding issue by disabling all colorkeying in DDrawCompat, but this part of the image was not affected at all. At this point, I think it's more likely to be a bug in PCEm, or some other Windows API difference between 98 (which I couldn't test) and 2000.

Also, letter spacing looks a bit better on Windows 98 SE, although I'm not sure if that's related to ddraw.dll.

I think the spacing looked "worse" in every test I mentioned above, but I'm pretty sure this is handled by GDI and not DirectDraw anyway. I didn't bother with any of the Rich Edit stuff though,

narzoul commented 4 months ago

After some further digging, I found that the image with the pink outline is some embedded image in the RichEdit's contents, which is rendered during the RICHEDIT class' WM_PAINT handler via StretchDIBits from some memory pointer containing the image bits:

StretchDIBits(DC{&06011690,null}, 0, 0, 144, 183, 0, 0, 144, 183, &14ACC1C4, {{40,144,183,1,24,0,79056,2834,2834,0,0},[&14ACC1C4]}, 0, 13369376)

But I don't see anything unusual about the call. Either your Win98 machine handles this differently somehow (might even be related to different RichEdit versions or one of its dependencies), or the image is already different in the memory buffer, but I don't know how to dump that on Win98 without writing a Win98-compatible wrapper.

narzoul commented 4 months ago

Well, I just dumped the source image from the memory buffer on Windows 11, and the result is certainly surprising.

journal

Never mind that it's upside down, since the height is set as positive in BITMAPINFOHEADER, it's supposed to be a "bottom-up DIB" as per the docs, so that part checks out.

What doesn't check out is how StretchDIBits achieves any sort of transparency when drawing this on a DC, since the docs don't mention any color key capabilities for this function, and from a quick search, several forum discussions agree that this shouldn't be possible.

Also, while the docs for SetStretchBltMode mention that it should only affect StretchBlt, this is clearly not the case, and StretchDIBits is also affected. There isn't even any stretching involved, since the source and destination size is the same. Yet SetStretchBltMode is called right before drawing this, setting the stretching mode to HALFTONE, and replacing this with COLORONCOLOR achieves the effect you wanted. Here is a test version that does this (though I don't intend to add this to DDrawCompat): ddraw.zip (diff.txt compared to v0.5.2)

So I'm not sure why it appears different on your Win 98 setup, either COLORONCOLOR is requested instead of HALFTONE, or maybe some hardware acceleration is used which doesn't support HALFTONE or something, I'm not really familiar with GDI architecture, especially on such ancient systems. I also don't know of any API tracing software that works on Win 98, which could be used to check what happens.

Well, if anyone knows why StretchDIBits handles any transparency in the first place, do let me know.

automatedbugreportingfacility commented 4 months ago

Whoa, fantastic -- even the odd single pink dot is drawn as on Windows 98 with your hack, which proves that this your analysis is spot-on. I'll be trying to figure out what happens on Windows 98. The stack trace when the SetStretchBltMode(hdc, HALFTONE) call is made is as follows:

 # ChildEBP RetAddr      
00 0019d9c8 764b49f6     GDI32!SetStretchBltMode
01 0019daf8 754e54fd     gdi32full!PlayMetaFileRecord+0x1336
02 0019db20 754e5fa9     ole32!CMfObject::CallbackFuncForDraw+0xfd [com\ole32\ole232\stdimpl\mf.cpp @ 1372] 
03 0019db38 764b32bd     ole32!MfCallbackFuncForDraw+0x19 [com\ole32\ole232\stdimpl\mf.cpp @ 970] 
04 0019dbec 764ae309     gdi32full!CommonEnumMetaFile+0x14a
05 0019dbfc 75774b58     gdi32full!EnumMetaFile+0x29
06 0019dc18 754e5b99     GDI32!EnumMetaFileStub+0x28
07 0019dc64 754cac89     ole32!CMfObject::Draw+0x239 [com\ole32\ole232\stdimpl\mf.cpp @ 804] 
08 0019dc90 754c8177     ole32!CCacheNode::Draw+0x4c [com\ole32\ole232\cache\cachenod.cpp @ 1214] 
09 0019dcb8 754c6602     ole32!COleCache::CCacheViewImpl::Draw+0x87 [com\ole32\ole232\cache\olecache.cpp @ 2606] 
0a 0019dcf8 7f9ebd95     ole32!OleDraw+0x62 [com\ole32\ole232\base\api.cpp @ 1754] 
WARNING: Stack unwind information not available. Following frames may be wrong.
0b 0019dd44 7f9ebc0e     RICHED32+0x1bd95
0c 0019dd74 7f9d7cac     RICHED32+0x1bc0e
0d 0019de14 7f9e63d6     RICHED32+0x7cac
0e 0019de7c 7f9e62b7     RICHED32+0x163d6
0f 0019dea4 7f9d4734     RICHED32+0x162b7
10 0019df1c 75d2199b     RICHED32+0x4734
11 0019df48 75d17dba     USER32!_InternalCallWinProc+0x2b
12 0019e030 75d17576     USER32!UserCallWinProcCheckWow+0x33a
13 0019e068 75d0465b     USER32!CallWindowProcAorW+0x7f
14 0019e080 5f401ee9     USER32!CallWindowProcA+0x1b
15 0019e0a0 2c9015e9     MFC42!Ordinal2385+0x24
16 0019e17c 5f401b63     gametext!DllUnregisterServer+0x4b9
17 0019e19c 5f401aec     MFC42!Ordinal6374+0x22
18 0019e1fc 5f401a74     MFC42!Ordinal1109+0x74
19 0019e218 2c904e1c     MFC42!Ordinal1578+0x2a
1a 0019e244 75d2199b     gametext!DllUnregisterServer+0x3cec
1b 0019e270 75d17dba     USER32!_InternalCallWinProc+0x2b
1c 0019e358 75d17576     USER32!UserCallWinProcCheckWow+0x33a
1d 0019e390 75d0465b     USER32!CallWindowProcAorW+0x7f
1e 0019e3a8 62e188a1     USER32!CallWindowProcA+0x1b
1f 0019e588 62e18e88     DDRAW!`anonymous namespace'::ddcWindowProc+0x641 [D:\Development\DDrawCompat\DDrawCompat\Gdi\WinProc.cpp @ 164] 
20 0019e5c0 75d2199b     DDRAW!`anonymous namespace'::ddcWindowProcA+0x38 [D:\Development\DDrawCompat\DDrawCompat\Gdi\WinProc.cpp @ 249] 

RICHED32.DLL is taken straight from Windows 98, so it's unlikely to be the problem. As for ole32 and gdi32/gdi32full libraries, I'd love to try to see if side-loading the Windows 98 equivalents would be possible (that'd make debugging a lot easier), but I can't even try -- both ole32 and gdi32 are present in KnownDLLs, which can't be modified even with the administrator rights. I tried using psexec to run regedit as SYSTEM to temporarily remove these entries, but I hit https://github.com/MicrosoftDocs/sysinternals/issues/442 and got stuck.

narzoul commented 4 months ago

I found multiple sources that claim HALFTONE is not supported on Windows 95/98/Me.

https://www.sinis.ro/static/ch15c.htm

Refer to the documentation on SetStretchBltMode for further details, but be advised that the most potentially useful alternative stretching mode—HALFTONE, which uses dithering to simulate colors that can't be displayed directly—works in Windows NT and Windows 2000 but not in Windows 95 and Windows 98.

https://forums.codeguru.com/showthread.php?293866-StretchBlt-Halftone-mode

I have no idea what is happening, but I just have to warn you that HALFTONE is not available on Windows 95/98/Me according to the MSDN.

automatedbugreportingfacility commented 4 months ago

Thanks! This proves that the patch does the right thing. Given that the game was made for Windows 95/98, would you consider adding your patch to DDrawCompat under a setting that is disabled by default? It's not a lot of code, and would be actually beneficial for people aiming to accurately emulate the original look of Windows 95/98 games and avoid potential unintended glitches.

narzoul commented 4 months ago

I'll think about it. I don't really want a separate setting for every little difference like this. Maybe I'll just tie it to the existing WinVersionLie setting, if that doesn't cause problems with this game.

automatedbugreportingfacility commented 4 months ago

Sure thing. Another idea would be grouping the little things under a single multi-value setting (StatsRows works like that, I think) where one could include/exclude minor compatibility features in a granular way as they see fit. As long as they're properly documented, that'd be pretty clean. Also, that'd prevent any "I love feature A, but hate feature B" or "that'd fix problem X, but introduces problem Y" dillemas common to many all-in-one settings.

narzoul commented 4 months ago

Yes, I've thought about such an option too for some time now, something like a "CompatibilityFixes" setting. In retrospect, maybe some existing options would have been better done that way too.

I feel like there are no right options. More granularity gives more flexibility, but also makes the configuration more overwhelming. I think the current amount is already a bit too much, though not DxWnd levels of complex yet.

No matter which path is taken, I'm sure there will be people who won't like it. Can't please everybody, I guess.

automatedbugreportingfacility commented 4 months ago

More granularity gives more flexibility, but also makes the configuration more overwhelming.

This is true. One way to mitigate is grouping options by their importance: basic and advanced, so that users are initially exposed to a limited set of preferences. Sorting them only in the alphabetical order has the downside of common/important options being intertwined with very specific stuff. Also maybe abstracting the granularity away through more general aliases when possible. For example, a simple alias "Win98" would enable all features you deem the most fitting for that particular usage of an average Joe. More advanced users could still enable individual things to match their needs or test things. Then again, aggregating options into groups results in even more options to choose from :)

I think the current amount is already a bit too much, though not DxWnd levels of complex yet.

Well, just a reflection of the fact that you're dealing with a complex problem with no universal, one-size-fits-all solutions. And as modern systems diverge further from their predecessors, things won't get better I'm afraid.

narzoul commented 4 months ago

Use the GdiStretchBltMode=coloroncolor setting in v0.5.3 to fix this issue. I went with a separate option to allow toggling it via the config overlay, but in this particular game it still requires reopening the journal for the change to take effect, despite forcing a full GDI repaint when the setting is changed.