Grayscale fix for Image::premultiply_alpha() and Image::convert(FORMAT_L8) while using REC.709, with added test case

This commit is contained in:
Kory Postma 2023-05-24 15:00:47 -05:00
parent 2eec9a67d5
commit c8cac39446
2 changed files with 28 additions and 6 deletions

View file

@ -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;
}
}
}

View file

@ -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