Grayscale fix for Image::premultiply_alpha() and Image::convert(FORMA…

…T_L8) while using REC.709, with added test case

core/io/image.cpp

@@ -468,7 +468,7 @@ int Image::get_mipmap_count() const {
 //using template generates perfectly optimized code due to constant expression reduction and unused variable removal present in all compilers
 template <uint32_t read_bytes, bool read_alpha, uint32_t write_bytes, bool write_alpha, bool read_gray, bool write_gray>
 static void _convert(int p_width, int p_height, const uint8_t *p_src, uint8_t *p_dst) {
-	uint32_t max_bytes = MAX(read_bytes, write_bytes);
+	constexpr uint32_t max_bytes = MAX(read_bytes, write_bytes);
 
 	for (int y = 0; y < p_height; y++) {
 		for (int x = 0; x < p_width; x++) {
@@ -492,8 +492,9 @@ static void _convert(int p_width, int p_height, const uint8_t *p_src, uint8_t *p
 			}
 
 			if constexpr (write_gray) {
-				//TODO: not correct grayscale, should use fixed point version of actual weights
-				wofs[0] = uint8_t((uint16_t(rgba[0]) + uint16_t(rgba[1]) + uint16_t(rgba[2])) / 3);
+				// REC.709
+				const uint8_t luminance = (13938U * rgba[0] + 46869U * rgba[1] + 4729U * rgba[2] + 32768U) >> 16U;
+				wofs[0] = luminance;
 			} else {
 				for (uint32_t i = 0; i < write_bytes; i++) {
 					wofs[i] = rgba[i];
@@ -3718,9 +3719,9 @@ void Image::premultiply_alpha() {
 		for (int j = 0; j < width; j++) {
 			uint8_t *ptr = &data_ptr[(i * width + j) * 4];
 
-			ptr[0] = (uint16_t(ptr[0]) * uint16_t(ptr[3])) >> 8;
-			ptr[1] = (uint16_t(ptr[1]) * uint16_t(ptr[3])) >> 8;
-			ptr[2] = (uint16_t(ptr[2]) * uint16_t(ptr[3])) >> 8;
+			ptr[0] = (uint16_t(ptr[0]) * uint16_t(ptr[3]) + 255U) >> 8;
+			ptr[1] = (uint16_t(ptr[1]) * uint16_t(ptr[3]) + 255U) >> 8;
+			ptr[2] = (uint16_t(ptr[2]) * uint16_t(ptr[3]) + 255U) >> 8;
 		}
 	}
 }

tests/core/io/test_image.h

@@ -303,6 +303,27 @@ TEST_CASE("[Image] Modifying pixels of an image") {
 	CHECK_MESSAGE(
 			image3->get_pixel(1, 0).is_equal_approx(Color(0, 0, 0, 0)),
 			"flip_y() should not leave old pixels behind.");
+
+	// Pre-multiply Alpha then Convert from RGBA to L8, checking alpha
+	{
+		Ref<Image> gray_image = memnew(Image(3, 3, false, Image::FORMAT_RGBA8));
+		CHECK_NOTHROW_MESSAGE(gray_image->fill_rect(Rect2i(0, 0, 3, 3), Color(1, 1, 1, 0)), "fill_rect() shouldn't throw for any rect.");
+		gray_image->set_pixel(1, 1, Color(1, 1, 1, 1));
+		gray_image->set_pixel(1, 2, Color(0.5, 0.5, 0.5, 0.5));
+		gray_image->set_pixel(2, 1, Color(0.25, 0.05, 0.5, 1.0));
+		gray_image->set_pixel(2, 2, Color(0.5, 0.25, 0.95, 0.75));
+		gray_image->premultiply_alpha();
+		gray_image->convert(Image::FORMAT_L8);
+		CHECK_MESSAGE(gray_image->get_pixel(0, 0).is_equal_approx(Color(0, 0, 0, 1)), "convert() RGBA to L8 should be black.");
+		CHECK_MESSAGE(gray_image->get_pixel(0, 1).is_equal_approx(Color(0, 0, 0, 1)), "convert() RGBA to L8 should be black.");
+		CHECK_MESSAGE(gray_image->get_pixel(0, 2).is_equal_approx(Color(0, 0, 0, 1)), "convert() RGBA to L8 should be black.");
+		CHECK_MESSAGE(gray_image->get_pixel(1, 0).is_equal_approx(Color(0, 0, 0, 1)), "convert() RGBA to L8 should be black.");
+		CHECK_MESSAGE(gray_image->get_pixel(1, 1).is_equal_approx(Color(1, 1, 1, 1)), "convert() RGBA to L8 should be white.");
+		CHECK_MESSAGE(gray_image->get_pixel(1, 2).is_equal_approx(Color(0.250980407, 0.250980407, 0.250980407, 1)), "convert() RGBA to L8 should be around 0.250980407 (64).");
+		CHECK_MESSAGE(gray_image->get_pixel(2, 0).is_equal_approx(Color(0, 0, 0, 1)), "convert() RGBA to L8 should be black.");
+		CHECK_MESSAGE(gray_image->get_pixel(2, 1).is_equal_approx(Color(0.121568628, 0.121568628, 0.121568628, 1)), "convert() RGBA to L8 should be around 0.121568628 (31).");
+		CHECK_MESSAGE(gray_image->get_pixel(2, 2).is_equal_approx(Color(0.266666681, 0.266666681, 0.266666681, 1)), "convert() RGBA to L8 should be around 0.266666681 (68).");
+	}
 }
 } // namespace TestImage