feat(screenshot): Add threaded JPEG/PNG screenshots without game stalls

[bobtista]

Summary

Replaces the old BMP screenshot with compressed JPEG screenshots that don't stall the game, and adds PNG support.

Closes #1555
Closes #106 ... sort of

Adds a new screenshot function using the stb_image_write library with background threading:

• F12 - Compressed JPEG screenshot (no stalling, ~600KB files)
• Ctrl+F12 - Lossless PNG screenshot

Notes

• stb_image-write is public domain licensed, single-header, zero compilation needed
• Captures frame buffer on main thread (~1-2ms)
• Spawns detached thread for JPEG/PNG compression and disk I/O
• JPEG quality configurable via Options.ini (default 80)
• Works for both Generals and Zero Hour

Testing

• Screenshots save correctly with F12 (jpg) and Ctrl+F12 (png)
• No game stalling during screenshot
• Files appear in correct directory with sequential numbering
  

[Stubbjax]

Some initial thoughts:

• Keep variants of the same function to a single hotkey; instead toggle image format via a new setting in Options.ini
• Apply image compression / quality via a new setting in Options.ini
• Can we consolidate this logic in core instead of repeating the implementation twice?

[xezon]

Agree with Stubbjax.

JPG 90 is big file. Better make it default 80.

Replace BMP screenshot with PNG screenshot. PNG is lossless compressed and always better than BMP.

Make F12 take JPG 80 screenshot. Make CTRL+F12 take PNG screenshot. Make JPG Quality adjustable.

Remove the old BMP code(s) and only use the new code for screenshot.

[xezon]

Regarding Github formatting:

When you write

Addresses #1555

Then it will not close this report when this is merged.

Please read up on it here:
https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/linking-a-pull-request-to-an-issue#closing-multiple-issues

[bobtista]

Some initial thoughts:

• Keep variants of the same function to a single hotkey; instead toggle image format via a new setting in Options.ini
• Apply image compression / quality via a new setting in Options.ini
• Can we consolidate this logic in core instead of repeating the implementation twice?

RE moving logic to core, I moved what I could to core - but there are a lot more files that need to be moved to core before this can be moved there eg WWVegas/WW3D2/*

[xezon]

Needs rebase.

[bobtista]

Needs rebase.

Done

[Skyaero42]

Looks good to me

[greptile-apps]

_{34 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

_{34 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

_{34 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

_{34 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Additional Comments (1)

Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp
Dead CreateBMPFile function left behind

The old takeScreenShot method (which was the only caller of CreateBMPFile) has been removed, leaving this ~75-line static function as dead code. It will produce a compiler warning for unused static function. Consider removing it since BMP screenshot functionality is fully replaced by the new JPEG/PNG implementation. The same applies to the GeneralsMD mirror file.

Prompt To Fix With AI

This is a comment left during a code review.
Path: Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp
Line: 2807:2880

Comment:
**Dead `CreateBMPFile` function left behind**

The old `takeScreenShot` method (which was the only caller of `CreateBMPFile`) has been removed, leaving this ~75-line static function as dead code. It will produce a compiler warning for unused static function. Consider removing it since BMP screenshot functionality is fully replaced by the new JPEG/PNG implementation. The same applies to the GeneralsMD mirror file.

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

_{33 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

_{33 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[bobtista]

Any more changes requested? Or is this good to merge?

[githubawn]

LGTM, frametime analysis shows zero stutter.

Nitpick 1: The screenshot naming logic seems to be incrementing per file extension (or filling the lowest available index) rather than using a global counter. This could break 'Sort by Name' in the destination folder because a newer PNG might get a lower index than an older JPG.

Nitpick 2: The shortcut for uncompressed images has been changed from F12 to Ctrl+F12. This took me a whole few seconds to get used to. But for users this might be preferred.

Neither of these are blockers for me personally.

[Skyaero42]

Nitpick 1: The screenshot naming logic seems to be incrementing per file extension (or filling the lowest available index) rather than using a global counter. This could break 'Sort by Name' in the destination folder because a newer PNG might get a lower index than an older JPG.

Maybe the filename should contain the date and time (incl milliseconds) instead as a sequencer

[bobtista]

JPEG

I'm happy to add this now or as a follow on PR, would be good to get this merged though. Edit: Done, went with Skyaero's suggestion.

[bobtista]

Any other change requests? Good to merge?

[xezon]

I have not yet looked over the code. Is the code AI generated?

[bobtista]

I have not yet looked over the code. Is the code AI generated?

Yes, and it was last November too, hence using OutputDebugStringA instead of DEBUG_LOG - fixing that now, and I noticed some brace formatting I can add. It's in good shape and works. I just pushed the changes