Merge pull request #590 from TomHarte/Screenshots

Unifies the OpenGL screenshot code and corrects it for arbitrary alignment.

OSBindings/Mac/Clock Signal.xcodeproj/project.pbxproj

@@ -1025,6 +1025,7 @@
 		4B9252CD1E74D28200B76AF1 /* Atari ROMs */ = {isa = PBXFileReference; lastKnownFileType = folder; path = "Atari ROMs"; sourceTree = "<group>"; };
 		4B92EAC91B7C112B00246143 /* 6502TimingTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = 6502TimingTests.swift; sourceTree = "<group>"; };
 		4B95FA9C1F11893B0008E395 /* ZX8081OptionsPanel.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ZX8081OptionsPanel.swift; sourceTree = "<group>"; };
+		4B961408222760E0001A7BF2 /* Screenshot.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = Screenshot.hpp; sourceTree = "<group>"; };
 		4B98A05C1FFAD3F600ADF63B /* CSROMFetcher.hpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.h; path = CSROMFetcher.hpp; sourceTree = "<group>"; };
 		4B98A05D1FFAD3F600ADF63B /* CSROMFetcher.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = CSROMFetcher.mm; sourceTree = "<group>"; };
 		4B98A0601FFADCDE00ADF63B /* MSXStaticAnalyserTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = MSXStaticAnalyserTests.mm; sourceTree = "<group>"; };
@@ -3025,6 +3026,7 @@
 				4BD191D9219113B80042E144 /* OpenGL.hpp */,
 				4BD191F32191180E0042E144 /* ScanTarget.hpp */,
 				4BD424DC2193B5340097291A /* Primitives */,
+				4B961408222760E0001A7BF2 /* Screenshot.hpp */,
 			);
 			name = OpenGL;
 			path = ../../Outputs/OpenGL;

OSBindings/Mac/Clock Signal/Machine/CSMachine.mm

@@ -30,6 +30,7 @@
 #include <OpenGL/gl3.h>
 
 #include "../../../../Outputs/OpenGL/ScanTarget.hpp"
+#include "../../../../Outputs/OpenGL/Screenshot.hpp"
 
 @interface CSMachine() <CSFastLoading>
 - (void)speaker:(Outputs::Speaker::Speaker *)speaker didCompleteSamples:(const int16_t *)samples length:(int)length;
@@ -250,38 +251,24 @@ struct ActivityObserver: public Activity::Observer {
 }
 
 - (NSBitmapImageRep *)imageRepresentation {
-	// Get the current viewport to establish framebuffer size. Then determine how wide the
-	// centre 4/3 of that would be.
-	GLint dimensions[4];
-	glGetIntegerv(GL_VIEWPORT, dimensions);
-	GLint proportionalWidth = (dimensions[3] * 4) / 3;
+	// Grab a screenshot.
+	Outputs::Display::OpenGL::Screenshot screenshot(4, 3);
 
-	// Grab the framebuffer contents.
-	std::vector<uint8_t> temporaryData(static_cast<size_t>(proportionalWidth * dimensions[3] * 3));
-	glReadPixels((dimensions[2] - proportionalWidth) >> 1, 0, proportionalWidth, dimensions[3], GL_RGB, GL_UNSIGNED_BYTE, temporaryData.data());
-
-	// Generate an NSBitmapImageRep and populate it with a vertical flip
-	// of the original data.
+	// Generate an NSBitmapImageRep containing the screenshot's data.
 	NSBitmapImageRep *const result =
 		[[NSBitmapImageRep alloc]
 			initWithBitmapDataPlanes:NULL
-			pixelsWide:proportionalWidth
-			pixelsHigh:dimensions[3]
+			pixelsWide:screenshot.width
+			pixelsHigh:screenshot.height
 			bitsPerSample:8
-			samplesPerPixel:3
-			hasAlpha:NO
+			samplesPerPixel:4
+			hasAlpha:YES
 			isPlanar:NO
 			colorSpaceName:NSDeviceRGBColorSpace
-			bytesPerRow:3 * proportionalWidth
+			bytesPerRow:4 * screenshot.width
 			bitsPerPixel:0];
 
-	const size_t line_size = static_cast<size_t>(proportionalWidth * 3);
-	for(GLint y = 0; y < dimensions[3]; ++y) {
-		memcpy(
-			&result.bitmapData[static_cast<size_t>(y) * line_size],
-			&temporaryData[static_cast<size_t>(dimensions[3] - y - 1) * line_size],
-			line_size);
-	}
+	memcpy(result.bitmapData, screenshot.pixel_data.data(), size_t(screenshot.width*screenshot.height*4));
 
 	return result;
 }

OSBindings/SDL/main.cpp

@@ -28,6 +28,7 @@
 #include "../../Activity/Observer.hpp"
 #include "../../Outputs/OpenGL/Primitives/Rectangle.hpp"
 #include "../../Outputs/OpenGL/ScanTarget.hpp"
+#include "../../Outputs/OpenGL/Screenshot.hpp"
 
 namespace {
 
@@ -622,21 +623,8 @@ int main(int argc, char *argv[]) {
 
 					// Capture ctrl+shift+d as a take-a-screenshot command.
 					if(event.key.keysym.sym == SDLK_d && (SDL_GetModState()&KMOD_CTRL) && (SDL_GetModState()&KMOD_SHIFT)) {
-						// Pick a width to capture that will preserve a 4:3 output aspect ratio.
-						const int proportional_width = (window_height * 4) / 3;
-
 						// Grab the screen buffer.
-						std::vector<uint8_t> pixels(proportional_width * window_height * 4);
-						glReadPixels((window_width - proportional_width) >> 1, 0, proportional_width, window_height, GL_RGBA, GL_UNSIGNED_BYTE, pixels.data());
-
-						// Flip the buffer vertically, because SDL and OpenGL do not agree about
-						// the basis axes.
-						std::vector<uint8_t> swap_buffer(proportional_width*4);
-						for(int y = 0; y < window_height >> 1; ++y) {
-							memcpy(swap_buffer.data(), &pixels[y*proportional_width*4], swap_buffer.size());
-							memcpy(&pixels[y*proportional_width*4], &pixels[(window_height - 1 - y)*proportional_width*4], swap_buffer.size());
-							memcpy(&pixels[(window_height - 1 - y)*proportional_width*4], swap_buffer.data(), swap_buffer.size());
-						}
+						Outputs::Display::OpenGL::Screenshot screenshot(4, 3);
 
 						// Pick the directory for images. Try `xdg-user-dir PICTURES` first.
 						std::string target_directory = system_get("xdg-user-dir PICTURES");
@@ -664,10 +652,10 @@ int main(int argc, char *argv[]) {
 						// Create a suitable SDL surface and save the thing.
 						const bool is_big_endian = SDL_BYTEORDER == SDL_BIG_ENDIAN;
 						SDL_Surface *const surface = SDL_CreateRGBSurfaceFrom(
-							pixels.data(),
-							proportional_width, window_height,
+							screenshot.pixel_data.data(),
+							screenshot.width, screenshot.height,
 							8*4,
-							proportional_width*4,
+							screenshot.width*4,
 							is_big_endian ? 0xff000000 : 0x000000ff,
 							is_big_endian ? 0x00ff0000 : 0x0000ff00,
 							is_big_endian ? 0x0000ff00 : 0x00ff0000,

Outputs/OpenGL/Screenshot.hpp

@@ -0,0 +1,61 @@
+//
+//  Screenshot.hpp
+//  Clock Signal
+//
+//  Created by Thomas Harte on 27/02/2019.
+//  Copyright © 2019 Thomas Harte. All rights reserved.
+//
+
+#ifndef Screenshot_h
+#define Screenshot_h
+
+#include "OpenGL.hpp"
+
+namespace Outputs {
+namespace Display {
+namespace OpenGL {
+
+/*!
+	Upon construction, Screenshot will capture the centre portion of the currently-bound framebuffer,
+	cropping to an image that matches the requested aspect ratio.
+
+	The image will then be available as RGBA data, in raster order via the struct members.
+*/
+struct Screenshot {
+	Screenshot(int aspect_width, int aspect_height) {
+		// Get the current viewport to establish framebuffer size. Then determine how wide the
+		// centre portion of that would be, allowing for the requested aspect ratio.
+		GLint dimensions[4];
+		glGetIntegerv(GL_VIEWPORT, dimensions);
+
+		height = int(dimensions[3]);
+		width = (height * aspect_width) / aspect_height;
+		pixel_data.resize(size_t(width * height * 4));
+
+		// Grab the framebuffer contents, temporarily setting single-byte alignment.
+		int prior_alignment;
+		glGetIntegerv(GL_PACK_ALIGNMENT, &prior_alignment);
+		glReadPixels((dimensions[2] - GLint(width)) >> 1, 0, GLint(width), GLint(height), GL_RGBA, GL_UNSIGNED_BYTE, pixel_data.data());
+		glPixelStorei(GL_PACK_ALIGNMENT, prior_alignment);
+
+		// Flip the contents into raster order.
+		const size_t line_size = size_t(width * 4);
+		for(size_t y = 0; y < size_t(height) / 2; ++y) {
+			const size_t flipped_y = size_t(height - 1) - y;
+
+			uint8_t temp[line_size];
+			memcpy(temp, &pixel_data[flipped_y * line_size], line_size);
+			memcpy(&pixel_data[flipped_y * line_size], &pixel_data[y * line_size], line_size);
+			memcpy(&pixel_data[y * line_size], temp, line_size);
+		}
+	}
+
+	std::vector<uint8_t> pixel_data;
+	int width, height;
+};
+
+}
+}
+}
+
+#endif /* Screenshot_h */