EditPicture: Rework the code

Author: Ghabry

src/game_interpreter.cpp

@@ -47,6 +47,7 @@
 #include "game_interpreter_control_variables.h"
 #include "game_windows.h"
 #include "json_helper.h"
+#include "lcf/rpg/savepicture.h"
 #include "maniac_patch.h"
 #include "memory_management.h"
 #include "pixel_format.h"
@@ -4722,13 +4723,11 @@ bool Game_Interpreter::CommandManiacGetPictureInfo(lcf::rpg::EventCommand const&
 	// Type 3: Pixel Data Extraction
 	if (info_type == 3) {
 		auto* sprite = pic.sprite.get();
-		if (!sprite) return true;
-
 		auto bitmap = sprite->GetBitmap();
 
 		// If this is a Window (String Picture), the visual content is generated by the Window class.
 		// We force a refresh/draw cycle here to ensure we read the actual text/window graphics.
-		if (data.easyrpg_type == lcf::rpg::SavePicture::EasyRpgType_window) {
+		if (pic.IsWindowAttached()) {
 			const auto& window = Main_Data::game_windows->GetWindow(pic_id);
 			if (window.window) {
 				bitmap->Clear();
@@ -4746,7 +4745,6 @@ bool Game_Interpreter::CommandManiacGetPictureInfo(lcf::rpg::EventCommand const&
 		// Bit 0: Ignore Alpha (return 0x00RRGGBB instead of 0xFFRRGGBB)
 		bool ignore_alpha = (com.parameters[2] & 2) != 0;
 
-		// Creates a snapshot of the current frame
 		Rect frame_rect{pic_x, pic_y, pic_w, pic_h};
 
 		if (!ManiacPatch::WritePixelsToVariable(*bitmap, frame_rect, dst_var_id, ignore_alpha, *Main_Data::game_variables)) {
@@ -5372,121 +5370,59 @@ bool Game_Interpreter::CommandManiacEditPicture(lcf::rpg::EventCommand const& co
 		return true;
 	}
 
-	auto& pic = Main_Data::game_pictures->GetPicture(pic_id);
+	auto& picture = Main_Data::game_pictures->GetPicture(pic_id);
 
-	if (pic.IsRequestPending()) {
-		pic.MakeRequestImportant();
+	if (picture.IsRequestPending()) {
+		picture.MakeRequestImportant();
 		_async_op = AsyncOp::MakeYieldRepeat();
 		return true;
 	}
 
-	auto* sprite = pic.sprite.get();
-	if (!sprite) return true;
-
+	auto* sprite = picture.sprite.get();
 	auto bitmap = sprite->GetBitmap();
-	if (!bitmap) return true;
 
-	// 1. Calculate Spritesheet Offset
+	// Calculate Spritesheet Offset
 	// Maniacs operations are relative to the currently active cell.
-	int offset_x = 0;
-	int offset_y = 0;
-
-	const auto& data = pic.data;
-	if (data.spritesheet_cols > 1 || data.spritesheet_rows > 1) {
-		int frame_width = bitmap->GetWidth() / data.spritesheet_cols;
-		int frame_height = bitmap->GetHeight() / data.spritesheet_rows;
-
-		// Map current frame index to X/Y coords
-		offset_x = (data.spritesheet_frame % data.spritesheet_cols) * frame_width;
-		offset_y = (data.spritesheet_frame / data.spritesheet_cols) * frame_height;
-	}
-
-	// 2. COW & Window Detach Logic
-	BitmapRef writeable_bitmap = bitmap;
 
-	bool is_cached = !bitmap->GetId().empty() && !StartsWith(bitmap->GetId(), "Canvas:");
-	bool wrong_format = bitmap->bpp() != 4;
-	bool is_window = data.easyrpg_type == lcf::rpg::SavePicture::EasyRpgType_window;
+	// Determine Spritesheet frame
+	Rect src_rect = sprite->GetSrcRect();
 
-	if (is_cached || wrong_format || is_window) {
-		writeable_bitmap = Bitmap::Create(bitmap->GetWidth(), bitmap->GetHeight());
-		writeable_bitmap->BlitFast(0, 0, *bitmap, bitmap->GetRect(), 255);
+	BitmapRef writable_bitmap = bitmap;
 
-		writeable_bitmap->SetId("Canvas:" + std::to_string(pic_id));
-
-		sprite->SetBitmap(writeable_bitmap);
-
-		if (is_window) {
-			pic.data.easyrpg_type = lcf::rpg::SavePicture::EasyRpgType_default;
+	if (picture.IsWindowAttached()) {
+		// If this is a Window (String Picture), the visual content is generated by the Window class.
+		// We force a refresh/draw cycle here to ensure we read the actual text/window graphics.
+		const auto& window = Main_Data::game_windows->GetWindow(pic_id);
+		if (window.window) {
+			bitmap->Clear();
+			window.window->Draw(*bitmap);
 		}
-
-		// Force sprite to recalculate its clipping rectangle immediately.
-		// Otherwise, SetBitmap resets src_rect to full size, showing the whole sheet.
-		sprite->OnPictureShow();
+	} else if (picture.IsCanvas()) {
+		// no-op
+	} else {
+		// Must be copied to avoid modifiying the original picture
+		writable_bitmap = Bitmap::Create(*bitmap, src_rect, true);
 	}
 
-	// 3. Parameters
-	int x = ValueOrVariableBitfield(com.parameters[0], 1, com.parameters[2]);
-	int y = ValueOrVariableBitfield(com.parameters[0], 2, com.parameters[3]);
-	int w = ValueOrVariableBitfield(com.parameters[0], 3, com.parameters[4]);
-	int h = ValueOrVariableBitfield(com.parameters[0], 4, com.parameters[5]);
+	picture.data.easyrpg_type = lcf::rpg::SavePicture::EasyRpgType_canvas;
 
-	int src_var_start = ValueOrVariableBitfield(com.parameters[0], 5, com.parameters[6]);
+	// Packing: x pos, y pos, width, height, var_id
+	int pic_x = ValueOrVariableBitfield(com.parameters[0], 1, com.parameters[2]);
+	int pic_y = ValueOrVariableBitfield(com.parameters[0], 2, com.parameters[3]);
+	int pic_w = ValueOrVariableBitfield(com.parameters[0], 3, com.parameters[4]);
+	int pic_h = ValueOrVariableBitfield(com.parameters[0], 4, com.parameters[5]);
+	int start_var_id = ValueOrVariableBitfield(com.parameters[0], 5, com.parameters[6]);
 
 	int flags = com.parameters[7];
-	bool flag_opaq = (flags & 1) != 0;
-	bool flag_skip_trans = (flags & 2) != 0;
-
-	// 4. Drawing Loop
-	int bmp_w = writeable_bitmap->GetWidth();
-	int bmp_h = writeable_bitmap->GetHeight();
-
-	if (w <= 0 || h <= 0) return true;
+	// When no flag is set the area is cleared and a OP_OVER blit occurs
+	bool flag_opaq = (flags & 1) != 0; // Blit with OP_SRC
+	bool flag_skip_trans = (flags & 2) != 0; // Blit with OP_OVER
 
-	uint32_t* pixels = static_cast<uint32_t*>(writeable_bitmap->pixels());
-	int pitch = writeable_bitmap->pitch() / 4;
+	bool clear_dst = !flag_opaq && !flag_skip_trans;
+	bool ignore_alpha = flag_opaq;
 
-	int current_var = src_var_start;
-	auto& vars = *Main_Data::game_variables;
-
-	for (int iy = 0; iy < h; ++iy) {
-		// Apply Y Offset here
-		int py = y + iy + offset_y;
-
-		if (py < 0 || py >= bmp_h) {
-			current_var += w;
-			continue;
-		}
-
-		for (int ix = 0; ix < w; ++ix) {
-			int32_t color_val = vars.Get(current_var++);
-
-			// Apply X Offset here
-			int px = x + ix + offset_x;
-
-			if (px < 0 || px >= bmp_w) continue;
-
-			uint8_t a = (color_val >> 24) & 0xFF;
-			uint8_t r = (color_val >> 16) & 0xFF;
-			uint8_t g = (color_val >> 8) & 0xFF;
-			uint8_t b = (color_val) & 0xFF;
-
-			if (flag_skip_trans && a == 0) continue;
-
-			if (flag_opaq) {
-				a = 0xFF;
-			}
-
-			if (a < 255) {
-				r = (r * a) / 255;
-				g = (g * a) / 255;
-				b = (b * a) / 255;
-			}
-
-			uint32_t final_pixel = Bitmap::pixel_format.rgba_to_uint32_t(r, g, b, a);
-			pixels[py * pitch + px] = final_pixel;
-		}
-	}
+	Rect frame_rect{pic_x, pic_y, pic_w, pic_h};
+	ManiacPatch::ReadPixelsFromVariable(*writable_bitmap, frame_rect, start_var_id, clear_dst, ignore_alpha, *Main_Data::game_variables);
 
 	return true;
 }
@@ -5553,9 +5489,17 @@ bool Game_Interpreter::CommandManiacWritePicture(lcf::rpg::EventCommand const& c
 		const auto sprite = picture.sprite.get();
 
 		// Retrieve bitmap
-		if (picture.IsWindowAttached() || picture.IsCanvas()) {
-			// Cannot change transparency of images that are not reloadable from a file
-			// Appears to match Maniacs behaviour
+		// Cannot change transparency of images that are not reloadable from a file (window and canvas)
+		// Appears to match Maniacs behaviour
+		if (picture.IsWindowAttached()) {
+			// If this is a Window (String Picture), the visual content is generated by the Window class.
+			// We force a refresh/draw cycle here to ensure we read the actual text/window graphics.
+			const auto& window = Main_Data::game_windows->GetWindow(pic_id);
+			if (window.window) {
+				bitmap->Clear();
+				window.window->Draw(*bitmap);
+			}
+		} else if (picture.IsCanvas()) {
 			bitmap = picture.sprite->GetBitmap();
 		} else if (picture.data.name.empty()) {
 			// Not much we can do here (also shouldn't happen normally)

src/game_pictures.cpp

@@ -503,6 +503,10 @@ void Game_Pictures::Picture::AttachWindow(const Window_Base& window) {
 	ApplyOrigin(false);
 }
 
+bool Game_Pictures::Picture::IsNormalPicture() const {
+	return data.easyrpg_type == lcf::rpg::SavePicture::EasyRpgType_default;
+}
+
 bool Game_Pictures::Picture::IsWindowAttached() const {
 	return data.easyrpg_type == lcf::rpg::SavePicture::EasyRpgType_window;
 }

src/game_pictures.h

@@ -127,8 +127,9 @@ class Game_Pictures {
 		void OnMapScrolled(int dx, int dy);
 
 		void AttachWindow(const Window_Base& window);
-		bool IsWindowAttached() const;
 
+		bool IsNormalPicture() const;
+		bool IsWindowAttached() const;
 		bool IsCanvas() const;
 	};
 

src/game_variables.cpp

@@ -210,12 +210,9 @@ void Game_Variables::WriteArray(const int first_id_a, const int last_id_a, const
 	}
 }
 
-std::vector<Var_t> Game_Variables::GetRange(int variable_id, int length) const {
-	std::vector<Var_t> vars;
-	for (int i = 0; i < length; ++i) {
-		vars.push_back(Get(variable_id + i));
-	}
-	return vars;
+lcf::Span<const Var_t> Game_Variables::GetRange(int variable_id, int length) {
+	PrepareRange(variable_id, variable_id + length, "Invalid write var[{},{}]!");
+	return lcf::Span<const Var_t>(&_variables[variable_id - 1], length);
 }
 
 lcf::Span<Var_t> Game_Variables::GetWritableRange(int variable_id, int length) {

src/game_variables.h

@@ -50,7 +50,7 @@ class Game_Variables {
 	Var_t Get(int variable_id) const;
 	Var_t GetIndirect(int variable_id) const;
 	Var_t GetWithMode(int id, int mode) const;
-	std::vector<Var_t> GetRange(int variable_id, int length) const;
+	lcf::Span<const Var_t> GetRange(int variable_id, int length);
 	lcf::Span<Var_t> GetWritableRange(int variable_id, int length);
 
 	Var_t Set(int variable_id, Var_t value);

src/maniac_patch.cpp

@@ -713,7 +713,87 @@ bool ManiacPatch::CheckString(std::string_view str_l, std::string_view str_r, in
 	return check(str_l, str_r);
 }
 
+bool ManiacPatch::ReadPixelsFromVariable(Bitmap& dst, Rect dst_rect, int start_var_id, bool clear_dst, bool ignore_alpha, Game_Variables& variables) {
+	int pic_x = dst_rect.x;
+	int pic_y = dst_rect.y;
+	int pic_w = dst_rect.width;
+	int pic_h = dst_rect.height;
+
+	if (pic_w <= 0 || pic_h <= 0) {
+		return false;
+	}
+
+	// Format expected by Maniacs
+	auto format = format_B8G8R8A8_a().format();
+	if (ignore_alpha) {
+		format = format_B8G8R8A8_n().format();
+	}
+
+	// Allocate an image as large as the requested dimensions (ignoring out of bounds)
+	Rect bmp_rect = dst.GetRect();
+	Rect frame_rect = bmp_rect.GetSubRect(dst_rect);
+
+	if (frame_rect.width <= 0 || frame_rect.height <= 0) {
+		return false;
+	}
+
+	BitmapRef frame = Bitmap::Create(nullptr, frame_rect.width, frame_rect.height, frame_rect.width * format.bytes, format);
+
+	uint32_t* pixels = static_cast<uint32_t*>(frame->pixels());
+	int px_per_row = frame->pitch() / sizeof(uint32_t);
+	uint32_t* dst_row = pixels;
+
+	// Rowwise memcpy
+	int x_l = std::min(0, pic_x);
+	int x_r = std::max(0, pic_x + pic_w - bmp_rect.width) + frame_rect.width;
+	int y_t = std::min(0, pic_y);
+	int y_b = std::max(0, pic_y + pic_h - bmp_rect.height) + frame_rect.height;
+
+	int src_var_id = start_var_id;
+	dst_row = pixels;
+	for (int y = y_t; y < y_b; ++y) {
+		// When row out of bounds skip
+		if (y < 0 || y >= frame_rect.height) {
+			src_var_id += pic_w;
+			continue;
+		}
+
+		for (int x = x_l; x < x_r;) {
+			if (x < 0) {
+				// OOB to the left (skip)
+				src_var_id += -x;
+				x = 0;
+			} else if (x >= frame_rect.width) {
+				// OOB to the right (skip)
+				int len = x_r - frame_rect.width;
+				src_var_id += len;
+				break;
+			} else {
+				auto in_range = variables.GetRange(src_var_id, frame_rect.width);
+				std::copy(in_range.begin(), in_range.end(), dst_row);
+				dst_row += px_per_row;
+				src_var_id += frame_rect.width;
+				x += frame_rect.width;
+			}
+		}
+	}
+
+	if (clear_dst) {
+		dst.ClearRect({pic_x, pic_y, frame_rect.width, frame_rect.height});
+	}
+
+	dst.Blit(pic_x, pic_y, *frame, frame->GetRect(), Opacity::Opaque(),
+		ignore_alpha ? Bitmap::BlendMode::NormalWithoutAlpha : Bitmap::BlendMode::Normal);
+
+	return true;
+}
+
 bool ManiacPatch::WritePixelsToVariable(const Bitmap& src, Rect src_rect, int start_var_id, bool ignore_alpha, Game_Variables& variables) {
+	// FIXME: Because we use premultiplied alpha the colors of transparent pixels are lost (always 0)
+	// Maniacs appears to preserve them
+	// E.g. when reading a transparent pixel from Chara1 (which was green) then Maniac will read green and we read 0
+	// This is noticable e.g. when using the EditPicture command with the opaque flag when reading from a transparent image
+
 	int pic_x = src_rect.x;
 	int pic_y = src_rect.y;
 	int pic_w = src_rect.width;
@@ -740,7 +820,8 @@ bool ManiacPatch::WritePixelsToVariable(const Bitmap& src, Rect src_rect, int st
 	BitmapRef frame = Bitmap::Create(nullptr, frame_rect.width, frame_rect.height, frame_rect.width * format.bytes, format);
 
 	// Then blit the screen (converts to the correct format)
-	frame->BlitFast(0, 0, src, frame_rect, Opacity::Opaque());
+	frame->Blit(0, 0, src, frame_rect, Opacity::Opaque(),
+		ignore_alpha ? Bitmap::BlendMode::NormalWithoutAlpha : Bitmap::BlendMode::Default);
 
 	uint32_t* pixels = static_cast<uint32_t*>(frame->pixels());
 	int px_per_row = frame->pitch() / sizeof(uint32_t);

src/maniac_patch.h

@@ -37,13 +37,26 @@ namespace ManiacPatch {
 
 	bool CheckString(std::string_view str_l, std::string_view str_r, int op, bool ignore_case);
 
+	/**
+	 * Reads pixel data in ARGB format out of variables and writes them into a bitmap
+	 *
+	 * @param dst Bitmap to write
+	 * @param dst_rect Bitmap area to write to
+	 * @param start_var_id Begin of variable range
+	 * @param clear_dst Clears the target area before blitting
+	 * @param ignore_alpha Blit as if no alpha channel exists (faster)
+	 * @param variables Variable list
+	 * @return true when any pixels were modified, false otherwise
+	 */
+	bool ReadPixelsFromVariable(Bitmap& dst, Rect dst_rect, int start_var_id, bool clear_dst, bool ignore_alpha, Game_Variables& variables);
+
 	/**
 	 * Extracts pixel data out of a bitmap writing it into a range of variables in ARGB format
 	 *
 	 * @param src Bitmap
 	 * @param src_rect Bitmap area to extract
 	 * @param start_var_id Begin of variable range
-	 * @param ignore_alpha Sets the alpha bits in all pixels to 0 (slow)
+	 * @param ignore_alpha Sets the alpha channel in all pixel to 0 (slow)
 	 * @param variables Variable list
 	 * @return true when any variables were modified, false otherwise
 	 */