From 99aab0a12be3d83bede45ad8cd5e04ae9c1da1f2 Mon Sep 17 00:00:00 2001
From: Jonathan Thomas <jonathan@openshot.org>
Date: Wed, 21 Dec 2022 22:18:42 -0600
Subject: [PATCH] Fix Brightness, Hue, and Saturation effects (#889)

* Fix Brightness, Hue, and Saturation effects to fully support pre-multiplied colors (when alpha channel found)

* Fixing whitespace on effects
---
 src/effects/Brightness.cpp | 27 ++++++++++++++++++++-------
 src/effects/Hue.cpp        | 17 +++++++++++++----
 src/effects/Saturation.cpp | 30 +++++++++++++++++-------------
 3 files changed, 50 insertions(+), 24 deletions(-)

diff --git a/src/effects/Brightness.cpp b/src/effects/Brightness.cpp
index 8b2ddc87..58340fff 100644
--- a/src/effects/Brightness.cpp
+++ b/src/effects/Brightness.cpp
@@ -63,16 +63,29 @@ std::shared_ptr<openshot::Frame> Brightness::GetFrame(std::shared_ptr<openshot::
 		// Compute contrast adjustment factor
 		float factor = (259 * (contrast_value + 255)) / (255 * (259 - contrast_value));
 
-		// Get RGB pixels from image and apply constrained contrast adjustment
-		int R = constrain((factor * (pixels[pixel * 4] - 128)) + 128);
-		int G = constrain((factor * (pixels[pixel * 4 + 1] - 128)) + 128);
-		int B = constrain((factor * (pixels[pixel * 4 + 2] - 128)) + 128);
-		// (Don't modify Alpha value)
+		// Calculate alpha % (to be used for removing pre-multiplied alpha value)
+		int A = pixels[pixel * 4 + 3];
+		float alpha_percent = A / 255.0;
+
+		// Get RGB values, and remove pre-multiplied alpha
+		unsigned char R = pixels[pixel * 4 + 0] / alpha_percent;
+		unsigned char G = pixels[pixel * 4 + 1] / alpha_percent;
+		unsigned char B = pixels[pixel * 4 + 2] / alpha_percent;
+
+		// Apply constrained contrast adjustment
+		R = constrain((factor * (R - 128)) + 128);
+		G = constrain((factor * (G - 128)) + 128);
+		B = constrain((factor * (B - 128)) + 128);
 
 		// Adjust brightness and write constrained values back to image
-		pixels[pixel * 4] = constrain(R + (255 * brightness_value));
+		pixels[pixel * 4 + 0] = constrain(R + (255 * brightness_value));
 		pixels[pixel * 4 + 1] = constrain(G + (255 * brightness_value));
 		pixels[pixel * 4 + 2] = constrain(B + (255 * brightness_value));
+
+		// Pre-multiply the alpha back into the color channels
+		pixels[pixel * 4 + 0] *= alpha_percent;
+		pixels[pixel * 4 + 1] *= alpha_percent;
+		pixels[pixel * 4 + 2] *= alpha_percent;
 	}
 
 	// return the modified frame
@@ -143,7 +156,7 @@ std::string Brightness::PropertiesJSON(int64_t requested_frame) const {
 
 	// Keyframes
 	root["brightness"] = add_property_json("Brightness", brightness.GetValue(requested_frame), "float", "", &brightness, -1.0, 1.0, false, requested_frame);
-	root["contrast"] = add_property_json("Contrast", contrast.GetValue(requested_frame), "float", "", &contrast, 0.0, 100.0, false, requested_frame);
+	root["contrast"] = add_property_json("Contrast", contrast.GetValue(requested_frame), "float", "", &contrast, -128, 128.0, false, requested_frame);
 
 	// Set the parent effect which properties this effect will inherit
 	root["parent_effect_id"] = add_property_json("Parent", 0.0, "string", info.parent_effect_id, NULL, -1, -1, false, requested_frame);
diff --git a/src/effects/Hue.cpp b/src/effects/Hue.cpp
index 9bc84b5c..750992b4 100644
--- a/src/effects/Hue.cpp
+++ b/src/effects/Hue.cpp
@@ -69,15 +69,24 @@ std::shared_ptr<openshot::Frame> Hue::GetFrame(std::shared_ptr<openshot::Frame>
 	#pragma omp parallel for shared (pixels)
 	for (int pixel = 0; pixel < pixel_count; ++pixel)
 	{
-		// Get the RGB values from the pixel (ignore the alpha channel)
-		int R = pixels[pixel * 4];
-		int G = pixels[pixel * 4 + 1];
-		int B = pixels[pixel * 4 + 2];
+		// Calculate alpha % (to be used for removing pre-multiplied alpha value)
+		int A = pixels[pixel * 4 + 3];
+		float alpha_percent = A / 255.0;
+
+		// Get RGB values, and remove pre-multiplied alpha
+		int R = pixels[pixel * 4 + 0] / alpha_percent;
+		int G = pixels[pixel * 4 + 1] / alpha_percent;
+		int B = pixels[pixel * 4 + 2] / alpha_percent;
 
 		// Multiply each color by the hue rotation matrix
 		pixels[pixel * 4] = constrain(R * matrix[0] + G * matrix[1] + B * matrix[2]);
 		pixels[pixel * 4 + 1] = constrain(R * matrix[2] + G * matrix[0] + B * matrix[1]);
 		pixels[pixel * 4 + 2] = constrain(R * matrix[1] + G * matrix[2] + B * matrix[0]);
+
+		// Pre-multiply the alpha back into the color channels
+		pixels[pixel * 4 + 0] *= alpha_percent;
+		pixels[pixel * 4 + 1] *= alpha_percent;
+		pixels[pixel * 4 + 2] *= alpha_percent;
 	}
 
 	// return the modified frame
diff --git a/src/effects/Saturation.cpp b/src/effects/Saturation.cpp
index f242ee14..6733deec 100644
--- a/src/effects/Saturation.cpp
+++ b/src/effects/Saturation.cpp
@@ -72,10 +72,14 @@ std::shared_ptr<openshot::Frame> Saturation::GetFrame(std::shared_ptr<openshot::
 	#pragma omp parallel for shared (pixels)
 	for (int pixel = 0; pixel < pixel_count; ++pixel)
 	{
-		// Get the RGB values from the pixel
-		int R = pixels[pixel * 4];
-		int G = pixels[pixel * 4 + 1];
-		int B = pixels[pixel * 4 + 2];
+		// Calculate alpha % (to be used for removing pre-multiplied alpha value)
+		int A = pixels[pixel * 4 + 3];
+		float alpha_percent = A / 255.0;
+
+		// Get RGB values, and remove pre-multiplied alpha
+		int R = pixels[pixel * 4 + 0] / alpha_percent;
+		int G = pixels[pixel * 4 + 1] / alpha_percent;
+		int B = pixels[pixel * 4 + 2] / alpha_percent;
 
 		/*
 		 * Common saturation adjustment
@@ -87,14 +91,9 @@ std::shared_ptr<openshot::Frame> Saturation::GetFrame(std::shared_ptr<openshot::
 						 (B * B * pB) );
 
 		// Adjust the saturation
-		R = p + (R - p) * saturation_value;
-		G = p + (G - p) * saturation_value;
-		B = p + (B - p) * saturation_value;
-
-		// Constrain the value from 0 to 255
-		R = constrain(R);
-		G = constrain(G);
-		B = constrain(B);
+		R = constrain(p + (R - p) * saturation_value);
+		G = constrain(p + (G - p) * saturation_value);
+		B = constrain(p + (B - p) * saturation_value);
 
 		/*
 		 * Color-separated saturation adjustment
@@ -134,9 +133,14 @@ std::shared_ptr<openshot::Frame> Saturation::GetFrame(std::shared_ptr<openshot::
 		B = constrain(B);
 
 		// Set all pixels to new value
-		pixels[pixel * 4]     = R;
+		pixels[pixel * 4 + 0] = R;
 		pixels[pixel * 4 + 1] = G;
 		pixels[pixel * 4 + 2] = B;
+
+		// Pre-multiply the alpha back into the color channels
+		pixels[pixel * 4 + 0] *= alpha_percent;
+		pixels[pixel * 4 + 1] *= alpha_percent;
+		pixels[pixel * 4 + 2] *= alpha_percent;
 	}
 
 	// return the modified frame