From 1f0ade29f05c9a1413532a894810b7550deb9392 Mon Sep 17 00:00:00 2001 From: Eric Raijmakers Date: Wed, 30 Sep 2020 12:09:23 +0200 Subject: [PATCH] Fix using mask colour even if there is no mask in wxImage::Paste In case an image without alpha is pasted on top of an image with alpha, the alpha blending gives wrong results. This is caused by the fact that the final (if nothing has been pasted yet) pixel copying algorithm in Paste() does not take into account whether the pasted image actually uses a mask. To fix this: - Add the check for image.HasMask(). - In case there is no mask, simply copy the image via memcpy. - Finally, update the alpha channel of the changed image (if present): whenever a pixel is copied, the alpha is set to fully opaque. Closes https://github.com/wxWidgets/wxWidgets/pull/2065 --- src/common/image.cpp | 64 ++++++++++++++++------ tests/image/image.cpp | 87 +++++++++++++++++++++++++++--- tests/image/paste_input_black.png | Bin 0 -> 150 bytes 3 files changed, 127 insertions(+), 24 deletions(-) create mode 100644 tests/image/paste_input_black.png diff --git a/src/common/image.cpp b/src/common/image.cpp index 8d935a1cfb..8515cf4c3a 100644 --- a/src/common/image.cpp +++ b/src/common/image.cpp @@ -1739,29 +1739,61 @@ wxImage::Paste(const wxImage & image, int x, int y, // being pasted into account. if (!copiedPixels) { - unsigned char r = image.GetMaskRed(); - unsigned char g = image.GetMaskGreen(); - unsigned char b = image.GetMaskBlue(); + const unsigned char* source_data = image.GetData() + 3 * (xx + yy * image.GetWidth()); + int source_step = image.GetWidth() * 3; - const unsigned char* source_data = image.GetData() + 3*(xx + yy*image.GetWidth()); - int source_step = image.GetWidth()*3; + unsigned char* target_data = GetData() + 3 * ((x + xx) + (y + yy) * M_IMGDATA->m_width); + int target_step = M_IMGDATA->m_width * 3; - unsigned char* target_data = GetData() + 3*((x+xx) + (y+yy)*M_IMGDATA->m_width); - int target_step = M_IMGDATA->m_width*3; - - for (int j = 0; j < height; j++) + unsigned char* alpha_target_data = NULL; + const int target_alpha_step = M_IMGDATA->m_width; + if (HasAlpha()) { - for (int i = 0; i < width*3; i+=3) + alpha_target_data = GetAlpha() + (x + xx) + (y + yy) * M_IMGDATA->m_width; + } + + // The mask colours should only be taken into account if the mask is actually enabled + if (!image.HasMask()) + { + // Copy all pixels + for (int j = 0; j < height; j++) { - if ((source_data[i] != r) || - (source_data[i+1] != g) || - (source_data[i+2] != b)) + memcpy(target_data, source_data, width * 3); + source_data += source_step; + target_data += target_step; + // Make all the copied pixels fully opaque + if (alpha_target_data != NULL) { - memcpy( target_data+i, source_data+i, 3 ); + memset(alpha_target_data, wxALPHA_OPAQUE, target_alpha_step); + alpha_target_data += target_alpha_step; } } - source_data += source_step; - target_data += target_step; + } + else + { + // Copy all 'non masked' pixels + unsigned char r = image.GetMaskRed(); + unsigned char g = image.GetMaskGreen(); + unsigned char b = image.GetMaskBlue(); + + for (int j = 0; j < height; j++) + { + for (int i = 0; i < width * 3; i += 3) + { + if ((source_data[i] != r) || + (source_data[i + 1] != g) || + (source_data[i + 2] != b)) + { + // Copy the non masked pixel + memcpy(target_data + i, source_data + i, 3); + // Make the copied pixel fully opaque + alpha_target_data[i / 3] = wxALPHA_OPAQUE; + } + } + source_data += source_step; + target_data += target_step; + alpha_target_data += target_alpha_step; + } } } } diff --git a/tests/image/image.cpp b/tests/image/image.cpp index aa67d7678f..3672a31ff5 100644 --- a/tests/image/image.cpp +++ b/tests/image/image.cpp @@ -1460,7 +1460,7 @@ TEST_CASE("wxImage::Paste", "[image][paste]") { "9 9 7 1", " c None", - "y c #FF0000", + "y c #FFFF00", "r c #FF0000", "g c #00FF00", "b c #0000FF", @@ -1481,7 +1481,7 @@ TEST_CASE("wxImage::Paste", "[image][paste]") { "9 9 2 1", " c None", - "y c #FF0000", + "y c #FFFF00", "y y y y y", " y y y y ", "y y y y y", @@ -1508,7 +1508,7 @@ TEST_CASE("wxImage::Paste", "[image][paste]") { "13 13 2 1", " c None", - "y c #FF0000", + "y c #FFFF00", "y y y y y y y", " y y y y y y ", "y y y y y y y", @@ -1537,7 +1537,7 @@ TEST_CASE("wxImage::Paste", "[image][paste]") { "5 5 2 1", " c None", - "y c #FF0000", + "y c #FFFF00", "y y y", " y y ", "y y y", @@ -1549,7 +1549,7 @@ TEST_CASE("wxImage::Paste", "[image][paste]") { "9 9 7 1", " c None", - "y c #FF0000", + "y c #FFFF00", "r c #FF0000", "g c #00FF00", "b c #0000FF", @@ -1579,7 +1579,7 @@ TEST_CASE("wxImage::Paste", "[image][paste]") { "9 9 7 1", " c None", - "y c #FF0000", + "y c #FFFF00", "r c #FF0000", "g c #00FF00", "b c #0000FF", @@ -1609,7 +1609,7 @@ TEST_CASE("wxImage::Paste", "[image][paste]") { "9 9 7 1", " c None", - "y c #FF0000", + "y c #FFFF00", "r c #FF0000", "g c #00FF00", "b c #0000FF", @@ -1667,7 +1667,7 @@ TEST_CASE("wxImage::Paste", "[image][paste]") { "9 9 7 1", " c None", - "y c #FF0000", + "y c #FFFF00", "r c #FF0000", "g c #00FF00", "b c #0000FF", @@ -1768,6 +1768,77 @@ TEST_CASE("wxImage::Paste", "[image][paste]") CHECK_THAT(actual, RGBSimilarTo(wxImage("image/paste_result_no_background_square_over_circle.png"), 1)); CHECK_THAT(actual, CenterAlphaPixelEquals(224)); } + SECTION("Paste fully transparent (masked) image over light image") // todo make test case for 'blend with mask' + { + const static char* transparent_image_xpm[] = + { + "5 5 2 1", + " c None", // Mask + "y c #FFFF00", + " ", + " ", + " ", + " ", + " ", + }; + const static char* light_image_xpm[] = + { + "5 5 2 1", + " c None", + "y c #FFFF00", + "yyyyy", + "yyyyy", + "yyyyy", + "yyyyy", + "yyyyy", + }; + wxImage actual(light_image_xpm); + actual.InitAlpha(); + wxImage paste(transparent_image_xpm); + wxImage expected(light_image_xpm); + actual.Paste(paste, 0, 0, wxIMAGE_ALPHA_BLEND_COMPOSE); + CHECK_THAT(actual, RGBSameAs(expected)); + } + SECTION("Paste fully black (masked) image over light image") // todo make test case for 'blend with mask' + { + const static char* black_image_xpm[] = + { + "5 5 2 1", + " c #000000", + "y c None", // Mask + " ", + " ", + " ", + " ", + " ", + }; + const static char* light_image_xpm[] = + { + "5 5 2 1", + " c None", + "y c #FFFF00", + "yyyyy", + "yyyyy", + "yyyyy", + "yyyyy", + "yyyyy", + }; + wxImage actual(light_image_xpm); + actual.InitAlpha(); + wxImage paste(black_image_xpm); + wxImage expected(black_image_xpm); + actual.Paste(paste, 0, 0, wxIMAGE_ALPHA_BLEND_COMPOSE); + CHECK_THAT(actual, RGBSameAs(expected)); + } + SECTION("Paste dark image over light image") + { + wxImage black("image/paste_input_black.png"); + wxImage actual("image/paste_input_background.png"); + actual.InitAlpha(); + actual.Paste(black, 0, 0, wxIMAGE_ALPHA_BLEND_COMPOSE); + CHECK_THAT(actual, CenterAlphaPixelEquals(255)); + CHECK_THAT(actual, RGBSameAs(black)); + } } /* diff --git a/tests/image/paste_input_black.png b/tests/image/paste_input_black.png new file mode 100644 index 0000000000000000000000000000000000000000..43f4668bba84b12d117391106779f52314d5d9db GIT binary patch literal 150 zcmeAS@N?(olHy`uVBq!ia0vp^DIm!lvI6O>_%)r2R5QG_bOw4`@6qNCFaSVw#{PrLtFOYX= j!FT^$es&fhlR;($qwpIB#salh8X&HxtDnm{r-UW|