feat(screenshot): Add threaded JPEG/PNG screenshots without game stalls by bobtista · Pull Request #1785 · TheSuperHackers/GeneralsGameCode

bobtista · 2025-11-03T03:24:45Z

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 · 2025-11-03T06:48:50Z

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 · 2025-11-03T06:58:11Z

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 · 2025-11-03T07:00:32Z

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 · 2025-11-03T16:26:53Z

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/*

Generals/Code/GameEngine/Source/GameClient/MessageStream/MetaEvent.cpp

Generals/Code/GameEngineDevice/Include/W3DDevice/GameClient/W3DDisplay.h

Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DScreenshot.cpp

Generals/Code/GameEngine/Source/GameClient/MessageStream/MetaEvent.cpp

cmake/stb.cmake

xezon · 2025-11-22T17:34:07Z

Needs rebase.

bobtista · 2025-11-22T18:22:35Z

Needs rebase.

Done

Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DScreenshot.h

Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/stb_image_write_impl.cpp

Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DScreenshot.cpp

Generals/Code/GameEngine/Include/Common/MessageStream.h

Generals/Code/GameEngine/Source/GameClient/MessageStream/MetaEvent.cpp

GeneralsMD/Code/GameEngine/Include/Common/MessageStream.h

Generals/Code/GameEngine/Source/GameClient/MessageStream/CommandXlat.cpp

Skyaero42

Looks good to me

greptile-apps

_{34 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DDisplay.cpp

Generals/Code/GameEngine/Source/GameClient/MessageStream/MetaEvent.cpp

Core/GameEngineDevice/Include/W3DDevice/GameClient/W3DScreenshot.h

greptile-apps

_{34 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/OptionsMenu.cpp

greptile-apps

_{34 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-16T19:24:03Z

Generals/Code/GameEngine/Include/Common/GlobalData.h

 	Bool m_clientRetaliationModeEnabled;
 	Bool m_doubleClickAttackMove;
 	Bool m_rightMouseAlwaysScrolls;
+	Int m_jpegQuality;


m_jpegQuality is never initialized in the constructor

Every neighboring member in this struct (m_rightMouseAlwaysScrolls at GlobalData.cpp:647, m_useWaterPlane at GlobalData.cpp:648, etc.) is explicitly initialized in the GlobalData::GlobalData() constructor, but m_jpegQuality is not. It only gets set later in parseGameDataDefinition via optionPref.getJPEGQuality().

This means m_jpegQuality holds an indeterminate value between construction and the parseGameDataDefinition call. While screenshots are unlikely to be taken in that window during normal gameplay, this is still a correctness defect that should be fixed for consistency and safety. The same issue exists in the GeneralsMD mirror file.

Add initialization in both GlobalData::GlobalData() constructors, for example after m_rightMouseAlwaysScrolls = FALSE;:

m_jpegQuality = 80;

Prompt To Fix With AI

This is a comment left during a code review. Path: Generals/Code/GameEngine/Include/Common/GlobalData.h Line: 145:145 Comment: **`m_jpegQuality` is never initialized in the constructor** Every neighboring member in this struct (`m_rightMouseAlwaysScrolls` at `GlobalData.cpp:647`, `m_useWaterPlane` at `GlobalData.cpp:648`, etc.) is explicitly initialized in the `GlobalData::GlobalData()` constructor, but `m_jpegQuality` is not. It only gets set later in `parseGameDataDefinition` via `optionPref.getJPEGQuality()`. This means `m_jpegQuality` holds an indeterminate value between construction and the `parseGameDataDefinition` call. While screenshots are unlikely to be taken in that window during normal gameplay, this is still a correctness defect that should be fixed for consistency and safety. The same issue exists in the GeneralsMD mirror file. Add initialization in both `GlobalData::GlobalData()` constructors, for example after `m_rightMouseAlwaysScrolls = FALSE;`: ```cpp m_jpegQuality = 80; ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-16T19:24:04Z

Generals/Code/GameEngineDevice/CMakeLists.txt

+target_sources(g_gameenginedevice PRIVATE
+#    Source/W3DDevice/GameClient/stb_image_write_impl.cpp
+)
+
+set_source_files_properties(
+#    Source/W3DDevice/GameClient/stb_image_write_impl.cpp
+    PROPERTIES
+    SKIP_PRECOMPILE_HEADERS ON
 )


Empty CMake commands left behind after commenting out source

These target_sources and set_source_files_properties blocks are entirely commented out inside, resulting in empty CMake commands. Since stb_image_write_impl.cpp is compiled in the Core CMakeLists now, consider removing these empty shells entirely rather than leaving commented-out scaffolding. The same applies to the GeneralsMD mirror CMakeLists.

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Prompt To Fix With AI

This is a comment left during a code review. Path: Generals/Code/GameEngineDevice/CMakeLists.txt Line: 207:215 Comment: **Empty CMake commands left behind after commenting out source** These `target_sources` and `set_source_files_properties` blocks are entirely commented out inside, resulting in empty CMake commands. Since `stb_image_write_impl.cpp` is compiled in the Core CMakeLists now, consider removing these empty shells entirely rather than leaving commented-out scaffolding. The same applies to the GeneralsMD mirror CMakeLists. <sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub> How can I resolve this? If you propose a fix, please make it concise.

Add takeScreenShotCompressed() to Display interface

Implement threaded JPEG screenshot for Generals Link stb library to GameEngineDevice targets Remove excessive comments from screenshot implementation

Add shared screenshot implementation in Core

Move W3DScreenshot implementation to game-specific directories Fix include order for VC6 precompiled headers Remove default parameter from function definition Move STB implementation to separate file to avoid PCH issues Include screenshot implementation directly in W3DDisplay.cpp to avoid PCH issues Use Windows constants and switch statement in screenshot code Use vcpkg for stb dependency with FetchContent fallback

… rebase

…rmat enum

…ince it's included in W3DDisplay.cpp

Move stb_image_write_impl.cpp from game-specific directories to Core since it's identical between Generals and GeneralsMD and contains only STB library implementation code with no game-specific logic.

…o nullptr, strcpy to strlcpy, fix thread leak

…on unit instead of #including it

greptile-apps

_{34 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-16T21:45:10Z

Generals/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DScreenshot.cpp

+	UnicodeString ufileName;
+	ufileName.translate(leafname);
+	TheInGameUI->message(TheGameText->fetch("GUI:ScreenCapture"), ufileName.str());


Success message shown even when screenshot fails

If CreateThread fails (line 144), the image data and thread data are cleaned up but the function continues to display the "screen captured" message to the user at line 154. Consider moving the TheInGameUI->message(...) call into the if (hThread) block so it only displays on success, or at least log the failure more visibly. 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/W3DScreenshot.cpp Line: 152:154 Comment: **Success message shown even when screenshot fails** If `CreateThread` fails (line 144), the image data and thread data are cleaned up but the function continues to display the "screen captured" message to the user at line 154. Consider moving the `TheInGameUI->message(...)` call into the `if (hThread)` block so it only displays on success, or at least log the failure more visibly. The same applies to the GeneralsMD mirror file. How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-16T21:45:12Z

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.

bobtista force-pushed the bobtista/compressed-screenshot-f11 branch 4 times, most recently from 20a3df1 to 37bd840 Compare November 3, 2025 04:32

xezon added Enhancement Is new feature or request Major Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour labels Nov 3, 2025

L3-M added this to the GenTool features replication milestone Nov 3, 2025

L3-M added the Input label Nov 3, 2025

bobtista force-pushed the bobtista/compressed-screenshot-f11 branch 3 times, most recently from 3535e1e to efc773f Compare November 3, 2025 17:45

Skyaero42 requested changes Nov 3, 2025

View reviewed changes

bobtista force-pushed the bobtista/compressed-screenshot-f11 branch 2 times, most recently from 977a6dc to f8162f3 Compare November 3, 2025 23:07

OmniBlade reviewed Nov 5, 2025

View reviewed changes

cmake/stb.cmake Outdated Show resolved Hide resolved

bobtista force-pushed the bobtista/compressed-screenshot-f11 branch from d7e8a8d to d197bdd Compare November 5, 2025 18:04

L3-M mentioned this pull request Nov 6, 2025

Replicate relevant GenTool fixes and improvements #850

Open

34 tasks

bobtista force-pushed the bobtista/compressed-screenshot-f11 branch from d197bdd to 9669966 Compare November 11, 2025 21:27

bobtista force-pushed the bobtista/compressed-screenshot-f11 branch from 9669966 to 4897b0b Compare November 22, 2025 18:10

Skyaero42 requested changes Nov 22, 2025

View reviewed changes

Skyaero42 previously approved these changes Nov 22, 2025

View reviewed changes

bobtista force-pushed the bobtista/compressed-screenshot-f11 branch from 9c99306 to de35e57 Compare December 3, 2025 16:48

bobtista force-pushed the bobtista/compressed-screenshot-f11 branch from 93bd190 to 516acb2 Compare February 16, 2026 18:47

greptile-apps bot reviewed Feb 16, 2026

View reviewed changes

bobtista changed the title ~~feat(screenshot): Add threaded JPEG screenshots on F11 without game stalls~~ feat(screenshot): Add threaded JPEG/PNG screenshots without game stalls Feb 16, 2026

greptile-apps bot reviewed Feb 16, 2026

View reviewed changes

Generals/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/OptionsMenu.cpp Show resolved Hide resolved

greptile-apps bot reviewed Feb 16, 2026

View reviewed changes

bobtista added 21 commits February 16, 2026 14:43

Add stb library dependency via FetchContent

13430b4

Add MSG_META_TAKE_SCREENSHOT_COMPRESSED message and F11 keybinding

d53fafd

Add takeScreenShotCompressed() to Display interface

Implement threaded JPEG screenshot for GeneralsMD

50ed01e

Implement threaded JPEG screenshot for Generals Link stb library to GameEngineDevice targets Remove excessive comments from screenshot implementation

Use Win32 CreateThread for VC6 compatibility and add GUIEditDisplay stub

c8a27bc

Move screenshot logic to Core to eliminate code duplication

c346f1b

Add shared screenshot implementation in Core

Change F12 to JPEG and add CTRL+F12 for PNG screenshots

aa9dd09

Remove old BMP screenshot code

7fcbb01

Remove trailing whitespace from empty lines

1c45436

Fix copyright headers: Change Electronic Arts Inc. to TheSuperHackers

12858a9

Add file headers to W3DScreenshot.cpp files

7ac1a95

Rename MSG_META_TAKE_SCREENSHOT_JPEG to MSG_META_TAKE_SCREENSHOT_PNG

98ae29f

fix(screenshot): Fix W3DScreenshot.cpp references in CMakeLists after…

13108c2

… rebase

fix(screenshot): Add missing W3DScreenshot.h include for ScreenshotFo…

1f46d55

…rmat enum

fix(screenshot): Use full include path for W3DScreenshot.h from Core

8eb0036

fix(screenshot): Remove W3DScreenshot.cpp from separate compilation s…

f0e1a03

…ince it's included in W3DDisplay.cpp

refactor(screenshot): Consolidate stb_image_write_impl.cpp to Core

14ca7a0

Move stb_image_write_impl.cpp from game-specific directories to Core since it's identical between Generals and GeneralsMD and contains only STB library implementation code with no game-specific logic.

docs(unify): Document stb_image_write_impl.cpp unification in script

df506b5

fix(screenshot): Restore DESTROY_MESSAGE, fix quality default, NULL t…

dd65a74

…o nullptr, strcpy to strlcpy, fix thread leak

fix(screenshot): Replicate screenshot fixes to Generals

516acb2

refactor(screenshot): Compile W3DScreenshot.cpp as separate translati…

7e3c765

…on unit instead of #including it

bobtista force-pushed the bobtista/compressed-screenshot-f11 branch from c4673ea to 45c0339 Compare February 16, 2026 21:34

greptile-apps bot reviewed Feb 16, 2026

View reviewed changes

refactor(screenshot): Replicate separate compilation fix to Generals

45c0339

Conversation

bobtista commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Notes

Testing

Uh oh!

Stubbjax commented Nov 3, 2025

Uh oh!

xezon commented Nov 3, 2025

Uh oh!

xezon commented Nov 3, 2025

Uh oh!

bobtista commented Nov 3, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xezon commented Nov 22, 2025

Uh oh!

bobtista commented Nov 22, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Skyaero42 left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot commented Feb 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

bobtista commented Nov 3, 2025 •

edited

Loading