From 4f5af8888e5a29fd08ccc92e3337b6170052c272 Mon Sep 17 00:00:00 2001 From: Matthew Kilgore Date: Mon, 7 Nov 2022 01:14:49 -0500 Subject: [PATCH 1/3] Fix PAINT when border color is not supplied The border color parameter to PAINT is optional, but sub_paint was not handling that case. What it should do in that situation is keep painting until it finds pixels that are not the same color as the original starting pixel was. Instead it would simply assume border color was valid and paint until it finds color zero (the default parameter value when it's not provided). This was originally reported in QB64Official/qb64#2, and Walt (@TheJoyfulProgrammer) fixed it in QB64Official/qb64#38. Functionally this is the same change, however I'm checking `passed & 4` to see whether a border color was provided rather than `bordercol == NULL`. The later has problems if the provided bordercolor is zero, which is allowed since zero is a valid color. The `passed` argument indicates which of the function arguments were actually provided in the QB64 source that called PAINT. Additionally, along with the `while (true)` loop that Walt changed I went ahead and removed the duplication of the sections for each direction. We now just use a couple of arrays to determine which direction we're checking and loop over all 4. --- internal/c/libqb.cpp | 168 ++++++++++++++++++------------------------- 1 file changed, 69 insertions(+), 99 deletions(-) diff --git a/internal/c/libqb.cpp b/internal/c/libqb.cpp index d3d4e78e6..b37c4fa5c 100644 --- a/internal/c/libqb.cpp +++ b/internal/c/libqb.cpp @@ -13503,128 +13503,98 @@ void sub_paint(float x, float y, qbs *fillstr, uint32 bordercol, qbs *background done = (uint8 *)calloc(i, 1); } - // return if first point is the bordercolor - if (qbg_active_page_offset[iy * qbg_width + ix] == bordercol) + // The original color of the starting location + uint32_t startingColor = qbg_active_page_offset[iy * qbg_width + ix]; + + bool borderColorProvided = passed & 4; + + // Exit early if we're already at the border color + if (borderColorProvided && qbg_active_page_offset[iy * qbg_width + ix] == bordercol) return; // create first node a_x[0] = ix; a_y[0] = iy; a_t[0] = 15; - // types: - //&1=check left - //&2=check right - //&4=check above - //&8=check below a_n = 1; qbg_active_page_offset[iy * qbg_width + ix] = tile[ix % sx][iy % sy]; done[iy * qbg_width + ix] = 1; -nextpass: - b_n = 0; - for (i = 0; i < a_n; i++) { - t = a_t[i]; - ix = a_x[i]; - iy = a_y[i]; + // Each index maps to a direction, in the order: + // Left, Right, Up, Down + uint32_t xdelta[4] = { -1, 1, 0, 0 }; + uint32_t ydelta[4] = { 0, 0, -1, 1 }; - // left - if (t & 1) { - x2 = ix - 1; - y2 = iy; - if (x2 >= qbg_view_x1) { - offset = y2 * qbg_width + x2; - if (!done[offset]) { - done[offset] = 1; - if (qbg_active_page_offset[offset] != bordercol) { - qbg_active_page_offset[offset] = tile[x2 % sx][y2 % sy]; - b_t[b_n] = 13; - b_x[b_n] = x2; - b_y[b_n] = y2; - b_n++; // add new node + // The bits indicate the directions that should be checked for the next + // pixel, we ignore the direction we came from. + uint32_t dirCheckMap[4] = { + 1 | 4 | 8, // Left, Up, Down + 2 | 4 | 8, // Right, Up, Down + 1 | 2 | 4, // Left, Right, Up + 1 | 2 | 8, // Left, Right, Down + }; + + while (true) { + b_n = 0; + for (i = 0; i < a_n; i++) { + t = a_t[i]; + ix = a_x[i]; + iy = a_y[i]; + + for (int k = 0; k < 4; k++) { + if ((t & (1 << k)) == 0) + continue; + + x2 = ix + xdelta[k]; + y2 = iy + ydelta[k]; + + // Verify dimensions are within bounds + if (x2 >= qbg_view_x1 && x2 <= qbg_view_x2 && y2 >= qbg_view_y1 && y2 <= qbg_view_y2) { + offset = y2 * qbg_width + x2; + + // Check that we haven't done this pixel yet + if (!done[offset]) { + done[offset] = 1; + + // We either check that we didn't hit the border color + // (if provided), or that we're still the starting + // color. + if ((borderColorProvided && qbg_active_page_offset[offset] != bordercol) + || (!borderColorProvided && qbg_active_page_offset[offset] == startingColor)) { + + qbg_active_page_offset[offset] = tile[x2 % sx][y2 % sy]; + b_t[b_n] = dirCheckMap[k]; + b_x[b_n] = x2; + b_y[b_n] = y2; + b_n++; // add new node + } } } } } - // right - if (t & 2) { - x2 = ix + 1; - y2 = iy; - if (x2 <= qbg_view_x2) { - offset = y2 * qbg_width + x2; - if (!done[offset]) { - done[offset] = 1; - if (qbg_active_page_offset[offset] != bordercol) { - qbg_active_page_offset[offset] = tile[x2 % sx][y2 % sy]; - b_t[b_n] = 14; - b_x[b_n] = x2; - b_y[b_n] = y2; - b_n++; // add new node - } - } - } + // no new nodes? + if (b_n == 0) { + memset(done, 0, write_page->width * write_page->height); // cleanup + return; // finished! } - // above - if (t & 4) { - x2 = ix; - y2 = iy - 1; - if (y2 >= qbg_view_y1) { - offset = y2 * qbg_width + x2; - if (!done[offset]) { - done[offset] = 1; - if (qbg_active_page_offset[offset] != bordercol) { - qbg_active_page_offset[offset] = tile[x2 % sx][y2 % sy]; - b_t[b_n] = 7; - b_x[b_n] = x2; - b_y[b_n] = y2; - b_n++; // add new node - } - } - } - } + // swap a & b arrays + sp = a_x; + a_x = b_x; + b_x = sp; - // below - if (t & 8) { - x2 = ix; - y2 = iy + 1; - if (y2 <= qbg_view_y2) { - offset = y2 * qbg_width + x2; - if (!done[offset]) { - done[offset] = 1; - if (qbg_active_page_offset[offset] != bordercol) { - qbg_active_page_offset[offset] = tile[x2 % sx][y2 % sy]; - b_t[b_n] = 11; - b_x[b_n] = x2; - b_y[b_n] = y2; - b_n++; // add new node - } - } - } - } + sp = a_y; + a_y = b_y; + b_y = sp; - } // i + cp = a_t; + a_t = b_t; + b_t = cp; - // no new nodes? - if (b_n == 0) { - memset(done, 0, write_page->width * write_page->height); // cleanup - return; // finished! + a_n = b_n; } - - // swap a & b arrays - sp = a_x; - a_x = b_x; - b_x = sp; - sp = a_y; - a_y = b_y; - b_y = sp; - cp = a_t; - a_t = b_t; - b_t = cp; - a_n = b_n; - - goto nextpass; } void sub_circle(double x, double y, double r, uint32 col, double start, double end, double aspect, int32 passed) { From 8005e62402813f0f9531c8af60401823be1b41c4 Mon Sep 17 00:00:00 2001 From: Matthew Kilgore Date: Tue, 8 Nov 2022 00:47:06 -0500 Subject: [PATCH 2/3] Add PAINT tile image-based tests --- tests/compile_tests.sh | 2 +- .../paint/tile_border_black_bg.bas | 19 +++ .../paint/tile_border_black_bg.bmp | Bin 0 -> 19254 bytes .../paint/tile_border_black_bg.output | 1 + .../paint/tile_border_black_nocolor.bas | 23 ++++ .../paint/tile_border_black_nocolor.bmp | Bin 0 -> 19254 bytes .../paint/tile_border_black_nocolor.output | 1 + .../paint/tile_border_white_bg.bas | 28 +++++ .../paint/tile_border_white_bg.bmp | Bin 0 -> 19254 bytes .../paint/tile_border_white_bg.output | 1 + .../paint/tile_border_white_nocolor.bas | 23 ++++ .../paint/tile_border_white_nocolor.bmp | Bin 0 -> 19254 bytes .../paint/tile_border_white_nocolor.output | 1 + .../paint/tile_noborder_black_bg.bas | 19 +++ .../paint/tile_noborder_black_bg.bmp | Bin 0 -> 19254 bytes .../paint/tile_noborder_black_bg.output | 1 + .../paint/tile_noborder_white_bg.bas | 22 ++++ .../paint/tile_noborder_white_bg.bmp | Bin 0 -> 19254 bytes .../paint/tile_noborder_white_bg.output | 1 + tests/compile_tests/utilities/imageassert.bm | 111 ++++++++++++++++++ 20 files changed, 252 insertions(+), 1 deletion(-) create mode 100644 tests/compile_tests/paint/tile_border_black_bg.bas create mode 100644 tests/compile_tests/paint/tile_border_black_bg.bmp create mode 100644 tests/compile_tests/paint/tile_border_black_bg.output create mode 100644 tests/compile_tests/paint/tile_border_black_nocolor.bas create mode 100644 tests/compile_tests/paint/tile_border_black_nocolor.bmp create mode 100644 tests/compile_tests/paint/tile_border_black_nocolor.output create mode 100644 tests/compile_tests/paint/tile_border_white_bg.bas create mode 100644 tests/compile_tests/paint/tile_border_white_bg.bmp create mode 100644 tests/compile_tests/paint/tile_border_white_bg.output create mode 100644 tests/compile_tests/paint/tile_border_white_nocolor.bas create mode 100644 tests/compile_tests/paint/tile_border_white_nocolor.bmp create mode 100644 tests/compile_tests/paint/tile_border_white_nocolor.output create mode 100644 tests/compile_tests/paint/tile_noborder_black_bg.bas create mode 100644 tests/compile_tests/paint/tile_noborder_black_bg.bmp create mode 100644 tests/compile_tests/paint/tile_noborder_black_bg.output create mode 100644 tests/compile_tests/paint/tile_noborder_white_bg.bas create mode 100644 tests/compile_tests/paint/tile_noborder_white_bg.bmp create mode 100644 tests/compile_tests/paint/tile_noborder_white_bg.output create mode 100644 tests/compile_tests/utilities/imageassert.bm diff --git a/tests/compile_tests.sh b/tests/compile_tests.sh index bddf194d5..de0e72aaa 100755 --- a/tests/compile_tests.sh +++ b/tests/compile_tests.sh @@ -107,7 +107,7 @@ do pushd . > /dev/null cd "./tests/compile_tests/$category" - testResult=$("../../../$EXE" 2>&1) + testResult=$("../../../$EXE" "../../../$RESULTS_DIR" "$category-$testName" 2>&1) ERR=$? popd > /dev/null diff --git a/tests/compile_tests/paint/tile_border_black_bg.bas b/tests/compile_tests/paint/tile_border_black_bg.bas new file mode 100644 index 000000000..83784e305 --- /dev/null +++ b/tests/compile_tests/paint/tile_border_black_bg.bas @@ -0,0 +1,19 @@ +$Console:Only +ChDir _StartDir$ + +TILE$ = MKL$(&HAAFFBB55) + +' Paint with tiling, white border color. +' background is black, circle is white. +' +' Result should fill the entire circle with pattern +test1& = _NewImage(128, 50, 9) +_Dest test1& + +Circle (64, 25), 25, 7 +Paint (64, 25), TILE$, 7 + +AssertImage test1&, "tile_border_black_bg.bmp" +System + +'$include:'../utilities/imageassert.bm' diff --git a/tests/compile_tests/paint/tile_border_black_bg.bmp b/tests/compile_tests/paint/tile_border_black_bg.bmp new file mode 100644 index 0000000000000000000000000000000000000000..d5a59bc38d5a7619ba28491b2d9d46b18ffc225f GIT binary patch literal 19254 zcmeI0OK!q25J25q^ax#76$fBjb5sn%O=;h$ti6^?v%V@f1* z58L*&qRWlnE({SMO0R8iZRfl!(}m{_jL^(a{ozq;TjZ$P;Ya73zmCh;A<)e4+jFk@ z2Q^^WB2eeyZxsHCyfuHVh#^Y={zi$fY6ZT@Gta#L$NV3y1mJI*{G~j!JU`4XAjxmZ zx%93Ff7`ggAO2l-1^Q1H%ta;eFQVyk0sdVuFDa1_@cy*~Xya?}-xzRR{M3bPpLx=| zK;7JbcL)5{{4Z6| zcQ5#B0CTN^Ui?K7z~=$=4I=6EeubRtgfj6Eald(k|l^=Oob*-dE$a<|NrCCVWe) zOv@ugy(vZOOUttfO*P3Z%S@$WUg|KLGEmDj zI?SfbR3$gnnAwz>O2vGoLn}+mv%)2Rr1hockpq7gV>>429_}dW5d4Y7Q{ls*5gqpPna|oIV6HD1tzv!4UZ&B_brj=JT`jbN{-1 zI`@6QecYULZ`gjZy<>ZEuh^sE{Jw2$BtQZrKmsH{0wh2Js}k71x*Z;`x`^xfC73t2 z{X8C6fZw8|UocomNW!+z$)xP(boS<)5!5a||qVg)D0WzH)AnC4^DYOA)K;4k%x z=}?O<$eh!*ukvM^*gchC_Zzs#$ry{Oab!O*SuKtlAH9*E!wq~0^~1J)bGT~vy%Qzy^E5+ zzUFg_>w@CgujVXPEp!gkB{67E>M~{1dEH(0<9_?6# z#%sHoJJ!S>LQnoHqyT(q>T~EdHpMR~FqEy2Y%9K&`2$X@LX-Z*8Hb;;Ubt&D|ZHN~h6A+|5o#Ux^mIO$E1W14cNPq-LfCOw3_ye>0-bnxe literal 0 HcmV?d00001 diff --git a/tests/compile_tests/paint/tile_border_white_bg.output b/tests/compile_tests/paint/tile_border_white_bg.output new file mode 100644 index 000000000..b86db1d47 --- /dev/null +++ b/tests/compile_tests/paint/tile_border_white_bg.output @@ -0,0 +1 @@ +Success, images are identical! diff --git a/tests/compile_tests/paint/tile_border_white_nocolor.bas b/tests/compile_tests/paint/tile_border_white_nocolor.bas new file mode 100644 index 000000000..ba3f52c57 --- /dev/null +++ b/tests/compile_tests/paint/tile_border_white_nocolor.bas @@ -0,0 +1,23 @@ +$Console:Only +ChDir _StartDir$ + +TILE$ = MKL$(&HAAFFBB55) + +' Paint with tiling, white border color. +' background is colored white, circle is black. +' +' Result should no change anything, as the inside +' of the circle matches the border color. +test1& = _NewImage(128, 50, 9) +_Dest test1& + +' Make the entire image white +Line (0, 0)-(127, 49), 7, BF + +Circle (64, 25), 25, 0 +Paint (64, 25), TILE$, 7 + +AssertImage test1&, "tile_border_white_nocolor.bmp" +System + +'$include:'../utilities/imageassert.bm' diff --git a/tests/compile_tests/paint/tile_border_white_nocolor.bmp b/tests/compile_tests/paint/tile_border_white_nocolor.bmp new file mode 100644 index 0000000000000000000000000000000000000000..bdf2e6b608d07593e211f280a599c5b34a5294d6 GIT binary patch literal 19254 zcmeH}O=`n17(~@u^axp(;saz`vgnO^o}RLTKoE{&BzyGfsf@uvmZkZ5(Zbi;@|Du} zb@^OUN|${9$=4I=6EeubRtgfj6Eald(k|l^=Oob*-dE$a<|NrCCVWe) zOv@ugy(vZOOUttfO*P3Z%S@$WUg|KLGEmDj zI?SfbR3$gnnAwz>O2vGoLn}+mv%)2Rr1hockpq7g5sn%O=;h$ti6^?v%V@f1* z58L*&qRWlnE({SMO0R8iZRfl!(}m{_jL^(a{ozq;TjZ$P;Ya73zmCh;A<)e4+jFk@ z2Q^^WB2eeyZxsHCyfuHVh#^Y={zi$fY6ZT@Gta#L$NV3y1mJI*{G~j!JU`4XAjxmZ zx%93Ff7`ggAO2l-1^Q1H%ta;eFQVyk0sdVuFDa1_@cy*~Xya?}-xzRR{M3bPpLx=| zK;7JbcL)5{{4Z6| zcQ5#B0CTN^Ui?K7z~=V>>429_}dW5d4Y7Q{ls*5gqpPna|oIV6HD1tzv!4UZ&B_brj=JT`jbN{-1 zI`@6QecYULZ`gjZy<>ZEuh^sE{Jw2$BtQZrKmsH{0wh2Js}k71x*Z;`x`^xfC73t2 z{X8C6fZw8|UocomNW!+z$)xP(boS<)5!5a||qVg)D0WzH)AnC4^DYOA)K;4k%x z=}?O<$eh!*ukvM^*gchC_Zzs#$ry{Oab!O*SuKtlAH9*E!wq~0^~1J)bGT~vy%Qzy^E5+ zzUFg_>w@CgujVXPEp!gkB{67E>M~{1dEH(0<9_?6# z#%sHoJJ!S>LQnoHqyT(q>T~EdHpMR~FqEy2Y%9K&`2$X@LX-Z*8Hb;;Ubt&D|ZHN~h6A+|5o#Ux^mIO$E1W14cNPq-LfCOw3_ye>0-bnxe literal 0 HcmV?d00001 diff --git a/tests/compile_tests/paint/tile_noborder_white_bg.output b/tests/compile_tests/paint/tile_noborder_white_bg.output new file mode 100644 index 000000000..b86db1d47 --- /dev/null +++ b/tests/compile_tests/paint/tile_noborder_white_bg.output @@ -0,0 +1 @@ +Success, images are identical! diff --git a/tests/compile_tests/utilities/imageassert.bm b/tests/compile_tests/utilities/imageassert.bm new file mode 100644 index 000000000..c3e3b0786 --- /dev/null +++ b/tests/compile_tests/utilities/imageassert.bm @@ -0,0 +1,111 @@ + +' +' Asserts that a created image, 'originalActualImage', is identical to the provide expected image, 'expectedFileName' +' +' The created image is converted to 32-bit and saved to the results folder. +' We then load the expected image as a 32-bit image, compare file sizes, +' width/height, and each pixel. +' +SUB AssertImage(originalActualImage As Long, expectedFileName As String) + Dim actualImage As Long + Dim ResultsDir As String, TestPrefix As String + + ResultsDir = Command$(1) + TestPrefix = Command$(2) + + ' Make sure the test result will be seen + _Dest _Console + + ' Convert to 32-bit for comparisons + actualImage = _NewImage(_Width(originalActualImage), _Height(originalActualImage), 32) + _PUTIMAGE , originalActualImage, actualImage + + 'First save the result + SaveImage actualImage, ResultsDir + "/" + TestPrefix + "_result.bmp" + + 'Compare both images, print whether they are identical + Dim expectedImage As Long + expectedImage = _LOADIMAGE(expectedFileName, 32) + + If _Width(actualImage) <> _Width(expectedImage) Then + Print "Failure! Image width differs, actual:"; _Width(actualImage);", Expected:"; _Width(expectedImage) + GoTo freeImages + End If + + If _Height(actualImage) <> _Height(expectedImage) Then + Print "Failure! Image height differs, actual:"; _Height(actualImage);", Expected:"; _Height(expectedImage) + GoTo freeImages + End If + + Dim actual As _Mem, expected As _Mem + + actual = _MEMIMAGE(actualImage) + expected = _MEMIMAGE(expectedImage) + + IF actual.SIZE <> expected.SIZE THEN + Print "Failure! Image sizes differ, Actual:"; actual.SIZE; ", Expected:"; expected.SIZE + GoTo freeImages + END IF + + w& = _Width(expectedImage) + h& = _Height(expectedImage) + + For x& = 0 to w& - 1 + For y& = 0 to h& - 1 + pixelOffset = (y& * w& + x&) * 4 + + actualPixel& = _MemGet(actual, actual.OFFSET + pixelOffset, LONG) + expectedPixel& = _MemGet(expected, expected.OFFSET + pixelOffset, LONG) + + If actualPixel& <> expectedPixel& Then + Print "Failure! Image pixels at ("; x&; ","; y&; ") differ, actual: 0x"; HEX$(actualPixel&);", expected: 0x"; HEX$(expectedPixel&) + GoTo freeImages + End If + Next + Next + + PRINT "Success, images are identical!" + +freeImages: + _MEMFREE actual + _MEMFREE expected + _FreeImage actualImage +END SUB + +' From the QB64-PE Wiki: https://qb64phoenix.com/qb64wiki/index.php/SAVEIMAGE +Sub SaveImage (image As Long, filename As String) + bytesperpixel& = _PixelSize(image&) + If bytesperpixel& = 0 Then Print "Text modes unsupported!": End + If bytesperpixel& = 1 Then bpp& = 8 Else bpp& = 24 + x& = _Width(image&) + y& = _Height(image&) + b$ = "BM????QB64????" + MKL$(40) + MKL$(x&) + MKL$(y&) + MKI$(1) + MKI$(bpp&) + MKL$(0) + "????" + String$(16, 0) 'partial BMP header info(???? to be filled later) + If bytesperpixel& = 1 Then + For c& = 0 To 255 ' read BGR color settings from JPG image + 1 byte spacer(CHR$(0)) + cv& = _PaletteColor(c&, image&) ' color attribute to read. + b$ = b$ + Chr$(_Blue32(cv&)) + Chr$(_Green32(cv&)) + Chr$(_Red32(cv&)) + Chr$(0) 'spacer byte + Next + End If + Mid$(b$, 11, 4) = MKL$(Len(b$)) ' image pixel data offset(BMP header) + lastsource& = _Source + _Source image& + If ((x& * 3) Mod 4) Then padder$ = String$(4 - ((x& * 3) Mod 4), 0) + For py& = y& - 1 To 0 Step -1 ' read JPG image pixel color data + r$ = "" + For px& = 0 To x& - 1 + c& = Point(px&, py&) 'POINT 32 bit values are large LONG values + If bytesperpixel& = 1 Then r$ = r$ + Chr$(c&) Else r$ = r$ + Left$(MKL$(c&), 3) + Next px& + d$ = d$ + r$ + padder$ + Next py& + _Source lastsource& + Mid$(b$, 35, 4) = MKL$(Len(d$)) ' image size(BMP header) + b$ = b$ + d$ ' total file data bytes to create file + Mid$(b$, 3, 4) = MKL$(Len(b$)) ' size of data file(BMP header) + If LCase$(Right$(filename$, 4)) <> ".bmp" Then ext$ = ".bmp" + f& = FreeFile + Open filename$ + ext$ For Output As #f&: Close #f& ' erases an existing file + Open filename$ + ext$ For Binary As #f& + Put #f&, , b$ + Close #f& +End Sub From ac8aac39b7b7fb99cb3ce130a87cb37cf0abe581 Mon Sep 17 00:00:00 2001 From: Matthew Kilgore Date: Tue, 8 Nov 2022 01:29:14 -0500 Subject: [PATCH 3/3] Use signed type for xdelta and ydelta arrays I accidentally declared these as uint32_t even though I store -1 in them. It was working anyway due to the implicit conversion that happens when adding it to an int32_t, but it should be fixed regardless. --- internal/c/libqb.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/c/libqb.cpp b/internal/c/libqb.cpp index b37c4fa5c..bf736ac01 100644 --- a/internal/c/libqb.cpp +++ b/internal/c/libqb.cpp @@ -13523,8 +13523,8 @@ void sub_paint(float x, float y, qbs *fillstr, uint32 bordercol, qbs *background // Each index maps to a direction, in the order: // Left, Right, Up, Down - uint32_t xdelta[4] = { -1, 1, 0, 0 }; - uint32_t ydelta[4] = { 0, 0, -1, 1 }; + int32_t xdelta[4] = { -1, 1, 0, 0 }; + int32_t ydelta[4] = { 0, 0, -1, 1 }; // The bits indicate the directions that should be checked for the next // pixel, we ignore the direction we came from.