Take PNG screenshots by default in SDL2 builds

PNG screenshots are also lossless and about half the size of the PCX
screenshots.

CMake/Definitions.cmake

@@ -93,6 +93,7 @@ foreach(
   DEVILUTIONX_DEFAULT_RESAMPLER
   STREAM_ALL_AUDIO_MIN_FILE_SIZE
   DEVILUTIONX_DISPLAY_TEXTURE_FORMAT
+  DEVILUTIONX_SCREENSHOT_FORMAT
 )
   if(DEFINED ${def_name} AND NOT ${def_name} STREQUAL "")
     list(APPEND DEVILUTIONX_DEFINITIONS ${def_name}=${${def_name}})

CMakeLists.txt

@@ -113,6 +113,15 @@ if(NOT USE_SDL1)
   mark_as_advanced(DEVILUTIONX_DISPLAY_TEXTURE_FORMAT)
 endif()
 
+if(USE_SDL1)
+  # SDL_image in SDL1 does not support PNG, making PCX the only option.
+  set(DEVILUTIONX_SCREENSHOT_FORMAT "DEVILUTIONX_SCREENSHOT_FORMAT_PCX")
+else()
+  set(DEVILUTIONX_SCREENSHOT_FORMAT "DEVILUTIONX_SCREENSHOT_FORMAT_PNG" CACHE STRING "Screenshot format")
+  set_property(CACHE DEVILUTIONX_SCREENSHOT_FORMAT PROPERTY STRINGS "DEVILUTIONX_SCREENSHOT_FORMAT_PNG;DEVILUTIONX_SCREENSHOT_FORMAT_PCX")
+  mark_as_advanced(DEVILUTIONX_SCREENSHOT_FORMAT)
+endif()
+
 # Sound options
 option(NOSOUND "Disable sound support" OFF)
 option(DEVILUTIONX_RESAMPLER_SPEEX "Build with Speex resampler" ON)

Source/CMakeLists.txt

@@ -270,6 +270,17 @@ if(SCREEN_READER_INTEGRATION)
   )
 endif()
 
+if(DEVILUTIONX_SCREENSHOT_FORMAT STREQUAL DEVILUTIONX_SCREENSHOT_FORMAT_PCX)
+  list(APPEND libdevilutionx_SRCS
+    utils/surface_to_pcx.cpp
+  )
+endif()
+if(DEVILUTIONX_SCREENSHOT_FORMAT STREQUAL DEVILUTIONX_SCREENSHOT_FORMAT_PNG)
+  list(APPEND libdevilutionx_SRCS
+    utils/surface_to_png.cpp
+  )
+endif()
+
 add_devilutionx_library(libdevilutionx OBJECT ${libdevilutionx_SRCS})
 target_include_directories(libdevilutionx PUBLIC ${CMAKE_CURRENT_BINARY_DIR})
 

Source/capture.cpp

@@ -3,148 +3,56 @@
  *
  * Implementation of the screenshot function.
  */
+#include <cerrno>
 #include <cstdint>
 #include <cstdio>
+#include <cstring>
 #include <ctime>
 
+#include <SDL.h>
+#include <expected.hpp>
 #include <fmt/format.h>
 
-#include "DiabloUI/diabloui.h"
+#define DEVILUTIONX_SCREENSHOT_FORMAT_PCX 0
+#define DEVILUTIONX_SCREENSHOT_FORMAT_PNG 1
+
+#if DEVILUTIONX_SCREENSHOT_FORMAT == DEVILUTIONX_SCREENSHOT_FORMAT_PCX
+#include "utils/surface_to_pcx.hpp"
+#endif
+#if DEVILUTIONX_SCREENSHOT_FORMAT == DEVILUTIONX_SCREENSHOT_FORMAT_PNG
+#include "utils/surface_to_png.hpp"
+#endif
+
 #include "engine/backbuffer_state.hpp"
 #include "engine/dx.h"
 #include "engine/palette.h"
 #include "utils/file_util.h"
 #include "utils/log.hpp"
 #include "utils/paths.h"
-#include "utils/pcx.hpp"
 #include "utils/str_cat.hpp"
-#include "utils/ui_fwd.h"
 
 namespace devilution {
 namespace {
 
-/**
- * @brief Write the PCX-file header
- * @param width Image width
- * @param height Image height
- * @param out File stream to write to
- * @return True on success
- */
-bool CaptureHdr(int16_t width, int16_t height, FILE *out)
-{
-	PCXHeader buffer;
-
-	memset(&buffer, 0, sizeof(buffer));
-	buffer.Manufacturer = 10;
-	buffer.Version = 5;
-	buffer.Encoding = 1;
-	buffer.BitsPerPixel = 8;
-	buffer.Xmax = SDL_SwapLE16(width - 1);
-	buffer.Ymax = SDL_SwapLE16(height - 1);
-	buffer.HDpi = SDL_SwapLE16(width);
-	buffer.VDpi = SDL_SwapLE16(height);
-	buffer.NPlanes = 1;
-	buffer.BytesPerLine = SDL_SwapLE16(width);
-
-	return std::fwrite(&buffer, sizeof(buffer), 1, out) == 1;
-}
-
-/**
- * @brief Write the current in-game palette to the PCX file
- * @param palette Current palette
- * @param out File stream for the PCX file.
- * @return True if successful, else false
- */
-bool CapturePal(SDL_Color *palette, FILE *out)
-{
-	uint8_t pcxPalette[1 + 256 * 3];
-
-	pcxPalette[0] = 12;
-	for (int i = 0; i < 256; i++) {
-		pcxPalette[1 + 3 * i + 0] = palette[i].r;
-		pcxPalette[1 + 3 * i + 1] = palette[i].g;
-		pcxPalette[1 + 3 * i + 2] = palette[i].b;
-	}
-
-	return std::fwrite(pcxPalette, sizeof(pcxPalette), 1, out) == 1;
-}
-
-/**
- * @brief RLE compress the pixel data
- * @param src Raw pixel buffer
- * @param dst Output buffer
- * @param width Width of pixel buffer
-
- * @return Output buffer
- */
-uint8_t *CaptureEnc(uint8_t *src, uint8_t *dst, int width)
-{
-	int rleLength;
-
-	do {
-		uint8_t rlePixel = *src;
-		src++;
-		rleLength = 1;
-
-		width--;
-
-		while (rlePixel == *src) {
-			if (rleLength >= 63)
-				break;
-			if (width == 0)
-				break;
-			rleLength++;
-
-			width--;
-			src++;
-		}
-
-		if (rleLength > 1 || rlePixel > 0xBF) {
-			*dst = rleLength | 0xC0;
-			dst++;
-		}
-
-		*dst = rlePixel;
-		dst++;
-	} while (width > 0);
-
-	return dst;
-}
-
-/**
- * @brief Write the pixel data to the PCX file
- *
- * @param buf Pixel data
- * @param out File stream for the PCX file.
- * @return True if successful, else false
- */
-bool CapturePix(const Surface &buf, FILE *out)
-{
-	int width = buf.w();
-	std::unique_ptr<uint8_t[]> pBuffer { new uint8_t[2 * width] };
-	uint8_t *pixels = buf.begin();
-	for (int height = buf.h(); height > 0; height--) {
-		const uint8_t *pBufferEnd = CaptureEnc(pixels, pBuffer.get(), width);
-		pixels += buf.pitch();
-		if (std::fwrite(pBuffer.get(), pBufferEnd - pBuffer.get(), 1, out) != 1)
-			return false;
-	}
-	return true;
-}
-
 FILE *CaptureFile(std::string *dstPath)
 {
+	const char *ext =
+#if DEVILUTIONX_SCREENSHOT_FORMAT == DEVILUTIONX_SCREENSHOT_FORMAT_PCX
+	    ".pcx";
+#elif DEVILUTIONX_SCREENSHOT_FORMAT == DEVILUTIONX_SCREENSHOT_FORMAT_PNG
+	    ".png";
+#endif
 	const std::time_t tt = std::time(nullptr);
 	const std::tm *tm = std::localtime(&tt);
 	const std::string filename = tm != nullptr
 	    ? fmt::format("Screenshot from {:04}-{:02}-{:02} {:02}-{:02}-{:02}",
 	        tm->tm_year + 1900, tm->tm_mon + 1, tm->tm_mday, tm->tm_hour, tm->tm_min, tm->tm_sec)
 	    : "Screenshot";
-	*dstPath = StrCat(paths::PrefPath(), filename, ".pcx");
+	*dstPath = StrCat(paths::PrefPath(), filename, ext);
 	int i = 0;
 	while (FileExists(dstPath->c_str())) {
 		i++;
-		*dstPath = StrCat(paths::PrefPath(), filename, "-", i, ".pcx");
+		*dstPath = StrCat(paths::PrefPath(), filename, "-", i, ext);
 	}
 	return OpenFile(dstPath->c_str(), "wb");
 }
@@ -162,42 +70,45 @@ void RedPalette()
 	BltFast(nullptr, nullptr);
 	RenderPresent();
 }
+
 } // namespace
 
 void CaptureScreen()
 {
 	SDL_Color palette[256];
 	std::string fileName;
-	bool success;
+	const uint32_t startTime = SDL_GetTicks();
 
 	FILE *outStream = CaptureFile(&fileName);
-	if (outStream == nullptr)
+	if (outStream == nullptr) {
+		LogError("Failed to open {} for writing: {}", fileName, std::strerror(errno));
 		return;
+	}
 	DrawAndBlit();
 	PaletteGetEntries(256, palette);
 	RedPalette();
-
+	for (int i = 0; i < 256; i++) {
-	const Surface &buf = GlobalBackBuffer();
+		system_palette[i] = palette[i];
-	success = CaptureHdr(buf.w(), buf.h(), outStream);
-	if (success) {
-		success = CapturePix(buf, outStream);
-	}
-	if (success) {
-		success = CapturePal(palette, outStream);
 	}
-	std::fclose(outStream);
+	palette_update();
+
+	const tl::expected<void, std::string> result =
+#if DEVILUTIONX_SCREENSHOT_FORMAT == DEVILUTIONX_SCREENSHOT_FORMAT_PCX
+	    WriteSurfaceToFilePcx(GlobalBackBuffer(), outStream);
+#elif DEVILUTIONX_SCREENSHOT_FORMAT == DEVILUTIONX_SCREENSHOT_FORMAT_PNG
+	    WriteSurfaceToFilePng(GlobalBackBuffer(), outStream);
+#endif
 
-	if (!success) {
+	if (!result.has_value()) {
-		Log("Failed to save screenshot at {}", fileName);
+		LogError("Failed to save screenshot at {}: ", fileName, result.error());
 		RemoveFile(fileName.c_str());
 	} else {
 		Log("Screenshot saved at {}", fileName);
 	}
-	SDL_Delay(300);
+	const uint32_t timePassed = SDL_GetTicks() - startTime;
-	for (int i = 0; i < 256; i++) {
+	if (timePassed < 300) {
-		system_palette[i] = palette[i];
+		SDL_Delay(300 - timePassed);
 	}
-	palette_update();
 	RedrawEverything();
 }
 

Source/engine/palette.cpp

@@ -283,6 +283,9 @@ void SetFadeLevel(int fadeval, bool updateHardwareCursor)
 		system_palette[i].r = (fadeval * logical_palette[i].r) / 256;
 		system_palette[i].g = (fadeval * logical_palette[i].g) / 256;
 		system_palette[i].b = (fadeval * logical_palette[i].b) / 256;
+#if SDL_VERSION_ATLEAST(2, 0, 0)
+		system_palette[i].a = SDL_ALPHA_OPAQUE;
+#endif
 	}
 	palette_update();
 	if (updateHardwareCursor && IsHardwareCursor()) {

Source/utils/surface_to_pcx.cpp

@@ -0,0 +1,153 @@
+#include "utils/surface_to_pcx.hpp"
+
+#include <cerrno>
+#include <cstdint>
+#include <cstdio>
+#include <cstring>
+#include <string>
+
+#include <SDL.h>
+#include <expected.hpp>
+
+#include "engine/surface.hpp"
+#include "utils/pcx.hpp"
+
+namespace devilution {
+namespace {
+
+tl::expected<void, std::string> CheckedFWrite(const void *ptr, size_t size, FILE *out)
+{
+	if (std::fwrite(ptr, size, 1, out) != 1) {
+		const char *errorMessage = std::strerror(errno);
+		if (errorMessage == nullptr)
+			errorMessage = "";
+		return tl::make_unexpected(std::string("fwrite failed with: ").append(errorMessage));
+	}
+	return {};
+}
+
+/**
+ * @brief Write the PCX-file header
+ * @param width Image width
+ * @param height Image height
+ * @param out File stream to write to
+ * @return True on success
+ */
+tl::expected<void, std::string> WritePcxHeader(int16_t width, int16_t height, FILE *out)
+{
+	PCXHeader buffer;
+
+	memset(&buffer, 0, sizeof(buffer));
+	buffer.Manufacturer = 10;
+	buffer.Version = 5;
+	buffer.Encoding = 1;
+	buffer.BitsPerPixel = 8;
+	buffer.Xmax = SDL_SwapLE16(width - 1);
+	buffer.Ymax = SDL_SwapLE16(height - 1);
+	buffer.HDpi = SDL_SwapLE16(width);
+	buffer.VDpi = SDL_SwapLE16(height);
+	buffer.NPlanes = 1;
+	buffer.BytesPerLine = SDL_SwapLE16(width);
+
+	return CheckedFWrite(&buffer, sizeof(buffer), out);
+}
+
+/**
+ * @brief Write the current in-game palette to the PCX file
+ * @param palette Current palette
+ * @param out File stream for the PCX file.
+ * @return True if successful, else false
+ */
+tl::expected<void, std::string> WritePcxPalette(SDL_Color *palette, FILE *out)
+{
+	uint8_t pcxPalette[1 + 256 * 3];
+
+	pcxPalette[0] = 12;
+	for (int i = 0; i < 256; i++) {
+		pcxPalette[1 + 3 * i + 0] = palette[i].r;
+		pcxPalette[1 + 3 * i + 1] = palette[i].g;
+		pcxPalette[1 + 3 * i + 2] = palette[i].b;
+	}
+
+	return CheckedFWrite(pcxPalette, sizeof(pcxPalette), out);
+}
+
+/**
+ * @brief RLE compress the pixel data
+ * @param src Raw pixel buffer
+ * @param dst Output buffer
+ * @param width Width of pixel buffer
+
+ * @return Output buffer
+ */
+uint8_t *WritePcxLine(uint8_t *src, uint8_t *dst, int width)
+{
+	int rleLength;
+
+	do {
+		const uint8_t rlePixel = *src;
+		src++;
+		rleLength = 1;
+
+		width--;
+
+		while (rlePixel == *src) {
+			if (rleLength >= 63)
+				break;
+			if (width == 0)
+				break;
+			rleLength++;
+
+			width--;
+			src++;
+		}
+
+		if (rleLength > 1 || rlePixel > 0xBF) {
+			*dst = rleLength | 0xC0;
+			dst++;
+		}
+
+		*dst = rlePixel;
+		dst++;
+	} while (width > 0);
+
+	return dst;
+}
+
+/**
+ * @brief Write the pixel data to the PCX file
+ *
+ * @param buf Pixel data
+ * @param out File stream for the PCX file.
+ * @return True if successful, else false
+ */
+tl::expected<void, std::string> WritePcxPixels(const Surface &buf, FILE *out)
+{
+	const int width = buf.w();
+	const std::unique_ptr<uint8_t[]> pBuffer { new uint8_t[static_cast<size_t>(2 * width)] };
+	uint8_t *pixels = buf.begin();
+	for (int height = buf.h(); height > 0; height--) {
+		const uint8_t *pBufferEnd = WritePcxLine(pixels, pBuffer.get(), width);
+		pixels += buf.pitch();
+		tl::expected<void, std::string> result = CheckedFWrite(pBuffer.get(), pBufferEnd - pBuffer.get(), out);
+		if (!result.has_value()) return result;
+	}
+	return {};
+}
+
+} // namespace
+
+tl::expected<void, std::string>
+WriteSurfaceToFilePcx(const Surface &buf, FILE *outStream)
+{
+	tl::expected<void, std::string> result = WritePcxHeader(buf.w(), buf.h(), outStream);
+	if (!result.has_value()) return result;
+	result = WritePcxPixels(buf, outStream);
+	if (!result.has_value()) return result;
+	result = WritePcxPalette(buf.surface->format->palette->colors, outStream);
+	if (!result.has_value()) return result;
+	std::fclose(outStream);
+	return {};
+}
+
+} // namespace devilution

Source/utils/surface_to_pcx.hpp

@@ -0,0 +1,13 @@
+#include <cstdio>
+#include <string>
+
+#include <expected.hpp>
+
+#include "engine/surface.hpp"
+
+namespace devilution {
+
+tl::expected<void, std::string>
+WriteSurfaceToFilePcx(const Surface &buf, FILE *outStream);
+
+} // namespace devilution

Source/utils/surface_to_png.cpp

@@ -0,0 +1,27 @@
+#include "utils/surface_to_png.hpp"
+
+#include <cstdio>
+#include <string>
+
+#include <SDL.h>
+#include <expected.hpp>
+
+#include "engine/surface.hpp"
+
+namespace devilution {
+
+extern "C" int IMG_SavePNG_RW(SDL_Surface *surface, SDL_RWops *dst, int freedst);
+
+tl::expected<void, std::string>
+WriteSurfaceToFilePng(const Surface &buf, FILE *outStream)
+{
+	SDL_RWops *rwops = SDL_RWFromFP(outStream, /*autoclose=*/SDL_TRUE);
+	if (rwops == nullptr || IMG_SavePNG_RW(buf.surface, rwops, /*freedst=*/1) != 0) {
+		tl::expected<void, std::string> result = tl::make_unexpected(std::string(SDL_GetError()));
+		SDL_ClearError();
+		return result;
+	}
+	return {};
+}
+
+} // namespace devilution

Source/utils/surface_to_png.hpp

@@ -0,0 +1,13 @@
+#include <cstdio>
+#include <string>
+
+#include <expected.hpp>
+
+#include "engine/surface.hpp"
+
+namespace devilution {
+
+tl::expected<void, std::string>
+WriteSurfaceToFilePng(const Surface &buf, FILE *outStream);
+
+} // namespace devilution