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

SCUMM: HE: Rewrite the majority of the graphics system #5752

Merged
merged 71 commits into from Apr 26, 2024

Conversation

AndywinXp
Copy link
Contributor

@AndywinXp AndywinXp commented Apr 20, 2024

This took way too long and produced way less results than I expected. I need a break. πŸ™‚
Also, as usual this writing is way too long, kudos to whoever gets to the end πŸ™

Changes in the PR

This PR brings you an almost full rewrite of the graphics subsystem for SCUMM Humongous Entertainment games. The details are as follows:

  1. Rewrote (and completed) the entirety of the WIZ system (except the obtuse font system used in the "One Stop Fun Shop" series, which calls Windows font rendering primitives and I'll let the pleasure of handling it to someone else...);
  2. Rewrote the entirety of the Moonbase Commander bite-sized graphics system to match the original (uses WIZ only on a high level of abstraction, the insides are almost totally different);
  3. Rewrote the WARP font system rendering part (which is the font format used by late HE games);
  4. Rewrote the entire Sprites subsystem;
  5. Rewrote the "HEAux" animation system;
  6. Rewrote the HE-specific TRLE cel decompression routine inside the drawLimb() function in the AKOS system;
  7. Implement the custom SCUMM Math functions (they are table-based and they produce different results with respect to our general math primitives).

Just as a word of warnings: some of these changes do some unorthodox things, e.g. they need their own rectangle primitives, which differ from ours. Please do not attempt to change these aspects, the original games worked with those in mind, so we better get used to it. πŸ˜›

Tested with?

The changes have been tested with basically every HE game I could get my hands on (every Steam release + other older spare releases which used alternate versions of the SCUMM HE engine).
I went out of my way to test as many games as possible (without completing them, though), but needless to say:

  • There might be bugs;
  • There might be crashes;
  • There will surely be some warnings I missed;
  • The buildbot is probably going to die building this PR;
  • Don't get me started on the various static analysis tools...

Do these changes fix anything?

Unfortunately these changes do not fix everything I hoped they would fix. I checked every tiny function to ensure that we are replicating what the original code is doing, but it seems that some of the issues we had in the tracker have roots in something else.

The good news though is that every single wonky WIZ flag is implemented so things like the Backyard games and Moonbase Commander now render properly.

Let's get to some fun details...

The "One Stop Fun Shop" series now doesn't crash and renders stuff correctly

These games needed some features which we didn't have before and now works as intended except for the obvious absence of the printing feature, and the absence of fonts in the "creations" of the user. These fonts are rendered Windows primitives. I believe we have a way to emulate that in ScummVM (like some of our devs are doing with a new in-development supported game which I don't know if I can name yet).


Moonbase Commander

Screen transitions are now working great. Transparency issues are gone as well. I rewrote the distortion blitter from scratch from the code and it solved some issues, but the distortion effect still looks different from whichever thing is compiled in the executable. I would go as far as to theorize that whichever codebase we have access to, it doesn't completely match the final exe? I don't know, I disassembled the exe, but the code was written with way too many templates, and I'm just too tired to figure out if something is different.

Also, we're not rendering this thing correctly (it should be some kind of radar pointing to the enemy), and I can't figure out why... EDIT: The good LittleToonCat just confirmed that I had the "Wind" setting off, so I can now confirm that this works as intended πŸ˜„
image


SCUMM/HE: Spyfox 3 - Sprite problem with Spyfox's head

I'm sorry, I couldn't figure this out for the life of me. I don't know where the root of the issue lies with this one.


SCUMM/HE: SPYFOX3 - No transparency in "Radioactive Trash Collector"

Fixed!


SCUMM/HE: SPYFOX3 - Text drawing glitch in "Radioactive Trash Collector"

We are now drawing the text at the correct color, but it's not being erased when new things are being written. I'm very very very sure that the code is correct, the Moonbase code even says something like "these are WARP fonts, they are faster but they can't be erased". These fonts are used elsewhere in the various games and work properly there, so it might be that this particular game is doing some background restoring magic to erase the numbers that we are not currently doing.


Two sides of the same bug: Putt-Putt Goes to the Zoo

The following tickets appear to be caused by the same cause:

Both these issues are caused by some actor ordering/drawing/erasing peculiarities. Both of these bugs are present in the demo/HE70 releases, while they are solved in the HE99+ releases, so there must be something somewhere we're not doing correctly for older versions. As with other things above, I couldn't figure this one out.


SCUMM/HE: Missing visual effects on Spy Fox in Cheese Chase

This is a fun one: the reason why the visual effects are missing is because... the WIZ renderer never receives them.
For some reason these effects (e.g. the motor exhaust) are working fine for the first second or so of any level, and then they vanish forever. Couldn't figure this one out, I decompiled and read script after script, but this game uses arrays way too extensively and now I'm worried that there might be something wrong in our arrays handling. Couldn't figure this one out, sorry.


SCUMM: Backyard Baseball (all three SCUMM games) - Hitting Not Being Interpreted Properly

Apparently I might have fixed this by accident when rewriting the SCUMM Math functions? This is yet to be confirmed by experts of these games. EDIT: Fixed by sev a while back (c73af63)!


SCUMM/HE: FREDDI5: Outline of Freddi appears in the leftmost tide pool

Guess what? This outline is an actual sprite image which is being drawn on purpose by the scripts at that spot. I even debugged the disassembly: the executable receives that command and sends it to the WIZ renderer, all the way through the end. Why is the actual executable not displaying it on the screen while we are doing that? I dunno. πŸ€·β€β™‚οΈ


SCUMM/HE: Tactics not flipping in Backyard Football

Solved!


Can someone confirm if the following tickets are now solved?

Overall?

Overall, I'm not happy about the compatibility boost we gained with this, I frankly expected more. But at least there's something good about this: we are now 99% sure that whatever thing is going through the WIZ renderer is being rendered exactly as it should, and other remaining issues are apparently to be found in other parts of the code. Also, if someone now wants to find something in the disassembly, we now have a code which matches and makes side-by-side debugging easier by an humongous amount.

What's next?

I'm gonna take a break, I've worked on this thing for no less than 6-7 months, including the preparatory work and disassembling. πŸ’€

@sluicebox
Copy link
Member

sluicebox commented Apr 20, 2024

JAW DROP

Amazing. Inspiring. I could read this write-up all day! [ EDIT: so I just re-read it! ]

Upending long-standing core engine code like this to replace with a more accurate and maintainable system is hero's work. I think it's the hardest thing you can do here. The effort-to-reward ratio is absolutely awful. The regression risk is huge. Cross referencing disassembly with original source and our own mostly-working source compounds the difficulty at every step. But it's the only way forward! I love this project because it's fundamentally impractical, and this PR is exactly that, so I love it too.

We have a SCI subsystem with the same dynamics that can only be dealt with like this. It's completely impractical because what we have really is good enough, but people notice, and I can't live with the imperfections. I've mapped out exactly what to do, and it has all of the pain points you've had, so it's on the back burner until I run out of fun stuff. This PR really is inspiring, it makes me yearn for a future where I'm writing up the SCI one. (Or, a future where I can trick you into SCI!)

Astounding work. Only 6-7 months?? You've earned a Take A Break Dot Exe!

@AndywinXp
Copy link
Contributor Author

JAW DROP

Amazing. Inspiring. I could read this write-up all day! [ EDIT: so I just re-read it! ]

Upending long-standing core engine code like this to replace with a more accurate and maintainable system is hero's work. I think it's the hardest thing you can do here. The effort-to-reward ratio is absolutely awful. The regression risk is huge. Cross referencing disassembly with original source and our own mostly-working source compounds the difficulty at every step. But it's the only way forward! I love this project because it's fundamentally impractical, and this PR is exactly that, so I love it too.

We have a SCI subsystem with the same dynamics that can only be dealt with like this. It's completely impractical because what we have really is good enough, but people notice, and I can't live with the imperfections. I've mapped out exactly what to do, and it has all of the pain points you've had, so it's on the back burner until I run out of fun stuff. This PR really is inspiring, it makes me yearn for a future where I'm writing up the SCI one. (Or, a future where I can trick you into SCI!)

Astounding work. Only 6-7 months?? You've earned a Take A Break Dot Exe!

This comment warms my heart πŸ™‚ And it's by sluicebox! What more is there to ask?! Thank you, I needed it.

The amount of versions and subversions to crossreference for the SCUMM HE engine is frankly insane. On paper there are only a few major versions (HE70, HE80, HE90, HE95-98-99, HE100), but in our plane of existence there are at least 3, three, THREE!!! subversions of HE99, and it's up to chance which subversion any of those involved games adheres to. One would think that release date would be a valid way to distinguish them, but no. It's just chance.

Thanks again, I look forward to more and more of your great write-ups/rants, they make me (and I believe all of us) feel like we're not alone πŸ˜›

@LittleToonCat
Copy link
Contributor

The dedication towards improving the engine is insane. First the sound rewrite and now this? You might have single-handedly improved the HE SCUMM engine by a wide-margin, and I must bow down to you. I can't wait to see this gets released!

Moonbase Commander
Screen transitions are now working great. Transparency issues are gone as well. I rewrote the distortion blitter from scratch from the code and it solved some issues, but the distortion effect still looks different from whichever thing is compiled in the executable. I would go as far as to theorize that whichever codebase we have access to, it doesn't completely match the final exe? I don't know, I disassembled the exe, but the code was written with way too many templates, and I'm just too tired to figure out if something is different.
Also, we're not rendering this thing correctly (it should be some kind of radar pointing to the enemy), and I can't figure out why...

I'm pretty sure that's supposed to be the wind direction (setting the wind speed during the game setup changes this), but the people in the Moonbase Commander Discord can confirm this for you (along with any other questions you may have about this game πŸ‘ )
image

@AndywinXp
Copy link
Contributor Author

I'm pretty sure that's supposed to be the wind direction (setting the wind speed during the game setup changes this), but the people in the Moonbase Commander Discord can confirm this for you (along with any other questions you may have about this game πŸ‘ )

No way! It's the wind! Thank you so much, I can now confirm that this gets rendered correctly in this PR's codebase πŸ˜„

@LittleToonCat
Copy link
Contributor

SCUMM: Backyard Baseball (all three SCUMM games) - Hitting Not Being Interpreted Properly
Apparently I might have fixed this by accident when rewriting the SCUMM Math functions? This is yet to be confirmed by experts of these games.

I just got word from Vissery (the person who wrote the bug ticket) that this has been long fixed since the RNG patch (c73af63), so that's already safe to close.

@raziel-
Copy link
Contributor

raziel- commented Apr 21, 2024

@AndywinXp

thank you πŸ™‡πŸΌβ€β™‚οΈ

awesome effort πŸ‘πŸΌ

@bluegr
Copy link
Member

bluegr commented Apr 21, 2024

Awesome work! Well done!
The SCUMM engine has been revived by your effort :)

@sev-
Copy link
Member

sev- commented Apr 22, 2024

Wow, so you finally baked this into a releasable state. Hat off! Kudos! It will take some time to review :D

@ccawley2011
Copy link
Member

Impressive work!

Since you're working on the graphics code, would it be possible to support screen pixel formats other than RGB555 for 16-bit games? It would be useful for performance on older platforms where supporting specific pixel formats requires extra conversion by the backend.

@AndywinXp
Copy link
Contributor Author

AndywinXp commented Apr 23, 2024

Impressive work!

Since you're working on the graphics code, would it be possible to support screen pixel formats other than RGB555 for 16-bit games? It would be useful for performance on older platforms where supporting specific pixel formats requires extra conversion by the backend.

Thanks! I'm afraid I have no plans for that as this is purely a disasm/porting effort and while I'm knowledgeable on audio stuff, graphics is where I come up short; so even if I had plans and the time to dedicate to that effort, I wouldn't even know where to begin with that.

Copy link
Member

@sev- sev- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Outstanding work, incredible code quality.

I found only couple of TODOs and a typo that leads on an endless loop in wizwarp_he.cpp.

I prefer to merge it and fix in-tree

engines/scumm/he/moonbase/moonbase_fow.cpp Show resolved Hide resolved
engines/scumm/he/moonbase/moonbase_fow.cpp Show resolved Hide resolved
engines/scumm/he/script_v80he.cpp Show resolved Hide resolved
engines/scumm/he/wizwarp_he.cpp Show resolved Hide resolved
@sev-
Copy link
Member

sev- commented Apr 26, 2024

Merging, so the work continues in-tree.

@sev- sev- merged commit f48a0ac into scummvm:master Apr 26, 2024
8 checks passed
@AndywinXp
Copy link
Contributor Author

Thank you for the review and for merging! I will address every single point tomorrow!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants