Thatcher's Techbase 4x/8x resolution crash

[Sohl42]

While playing Thatcher's Techbase(original or arcade), a boom compatible wad, changing the resolution to 4x or 8x consistently produces a crash right when selecting the option. All other settings are default. I've tried a couple other boom compatible wads and all seem to work fine with no crashes.

I'm not sure if this is just the wad being a problem or it's on Nugget's end.

[MrAlaux]

From some quick testing, I've managed to provoke crashes by setting the resolution to 1X and 2X too.

I immediately suspected that the problem had to do with some HUD graphic(s), and I was right: the Status Bar is 33px tall, when it should be 32px. Cropping it to 32px stops the crashes.

Woof seems to work fine whether the Status Bar is 32 or 33px tall.

@ceski-1, what should we do?

[MrAlaux]

I compared Woof and Nugget's code, and I've noticed something:

Woof allocates memory for the largest possible Status Bar screen area (note: not the Status Bar graphic itself, although they'd match anyways), taking widescreen and high resolution into account.

Nugget, on the other hand, allocates only as much memory as necessary, which depends on the current (not maximum) widescreen and resolution settings. If I tweak the code to allocate memory like Woof does, the crash disappears. EDIT: Actually, I think it crashes at 21:9 1600p. I'll have to test again later.

@fabiangreffrath, I hope you don't mind if I ask for your opinion on this too. Should we just let WADs get away with it?

[ceski-1]

Here's an ASAN log: asan.txt That's with a clean debug build with no config, crash occurs when starting a new game. I haven't had a chance to look any closer yet.

[fabiangreffrath]

Seems like we accidentally tolerate bigger patches in Woof by always allocating a bit more memory than strictly necessary. I understand you shouldn't do the same in Nugget, else you would always allocate for 8xhires+widescreen. I think we should check the actual height of the status bar background patch and use this (but at least ST_HEIGHT) to calculate the memory to allocate.

[ceski-1]

@fabiangreffrath It may be worth fixing this for Woof too.

[fabiangreffrath]

Yep, but I'm currently away from my computer.

[MrAlaux]

To be honest, I'm not too fond of the idea of supporting something that would probably crash vanilla/Boom/MBF and has seemingly no real utility (unlike wide Status Bars, which are useful). I should test those engines to confirm if they support it or not.

If they don't, maybe it'd be worth to print a warning when the Status Bar graphic doesn't have the expected height?

[fabiangreffrath]

Sure, we could print a warning. That's a good idea.

However, we already e.g. support maps with non-Vanilla BSP nodes or empty REJECT lumps for no other reason than to enable the user to play them. 😉

[MrAlaux]

However, we already e.g. support maps with non-Vanilla BSP nodes or empty REJECT lumps for no other reason than to enable the user to play them. 😉

Do those affect the game in any way? It sounds like they might, to some degree, while having a taller Status Bar seems completely pointless since the extra rows go off-screen unless you were to give it an upwards offset, but I believe it'd still be limited to the 32px-tall screen buffer. I'll have to test that too.

[ceski-1]

unless you were to give it an upwards offset

You're right, it's pointless since it's cropped to a height of 32. Example of arbitrarily setting the vertical offset to 20:

[fabiangreffrath]

It's pointless, but it shouldn't crash the engine.

[ceski-1]

STARMS and STFB# (face background for multiplayer) have the same issue. Should we cover those cases as well?

[fabiangreffrath]

STARMS and STFB# (face background for multiplayer) have the same issue. Should we cover those cases as well?

Well, I'm afraid we need to catch the tallest patch that's drawn to this frame buffer.

[MrAlaux]

Thanks everyone!