From a1244eb96d8562fd42043506eb2c3631cc33500a Mon Sep 17 00:00:00 2001 From: Dave Pagurek Date: Fri, 24 Jan 2025 14:47:01 -0500 Subject: [PATCH 1/4] Fix closed curves having an extra loop --- src/shape/custom_shapes.js | 2 +- test/unit/visual/cases/shapes.js | 10 ++++++++++ .../2D mode/Drawing closed curves/000.png | Bin 749 -> 857 bytes .../Drawing simple closed curves/000.png | Bin 0 -> 701 bytes .../Drawing simple closed curves/metadata.json | 3 +++ .../WebGL mode/Drawing closed curves/000.png | Bin 769 -> 654 bytes .../Drawing simple closed curves/000.png | Bin 0 -> 612 bytes .../Drawing simple closed curves/metadata.json | 3 +++ test/unit/visual/visualTest.js | 2 +- 9 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 test/unit/visual/screenshots/Shape drawing/2D mode/Drawing simple closed curves/000.png create mode 100644 test/unit/visual/screenshots/Shape drawing/2D mode/Drawing simple closed curves/metadata.json create mode 100644 test/unit/visual/screenshots/Shape drawing/WebGL mode/Drawing simple closed curves/000.png create mode 100644 test/unit/visual/screenshots/Shape drawing/WebGL mode/Drawing simple closed curves/metadata.json diff --git a/src/shape/custom_shapes.js b/src/shape/custom_shapes.js index 58465a2298..f054275ba8 100644 --- a/src/shape/custom_shapes.js +++ b/src/shape/custom_shapes.js @@ -395,7 +395,7 @@ class SplineSegment extends Segment { points.unshift(prevVertex); points.push(this.vertices.at(-1)); } else if (this._splineProperties.ends === constants.JOIN) { - points.unshift(this.vertices.at(-1), prevVertex); + points.unshift(this.vertices.at(-1)); points.push(prevVertex, this.vertices.at(0)); } diff --git a/test/unit/visual/cases/shapes.js b/test/unit/visual/cases/shapes.js index 8e7835a604..83ec314049 100644 --- a/test/unit/visual/cases/shapes.js +++ b/test/unit/visual/cases/shapes.js @@ -150,6 +150,16 @@ visualSuite('Shape drawing', function() { screenshot(); }); + visualTest('Drawing simple closed curves', function(p5, screenshot) { + setup(p5); + p5.beginShape(); + p5.splineVertex(10, 10); + p5.splineVertex(15, 40); + p5.splineVertex(40, 35); + p5.endShape(p5.CLOSE); + screenshot(); + }); + visualTest('Drawing with curves with tightness', function(p5, screenshot) { setup(p5); p5.splineProperty('tightness', -1); diff --git a/test/unit/visual/screenshots/Shape drawing/2D mode/Drawing closed curves/000.png b/test/unit/visual/screenshots/Shape drawing/2D mode/Drawing closed curves/000.png index 9260c77d472cc16ed953cffaf20cf5a470acbcb7..80536c3fec45666ca76d09905e54705c955df869 100644 GIT binary patch delta 822 zcmV-61IhgD1=$9WFn2)q5J+KXmkSUC3ky*p#8iS8U?B(= zRtkdH*_a>*DMS!rWhYozS_p!mBB&@>2zHja0t>++cz6dUY&`1D?CyvpJ0SQsGw(Cs z@6O$9HapNS7U_yQFH=-%e9uvB|Ar-&wr=rJk2jFWRXUrL95k@ zYPFjA9Tlx03<8+hY^F$t`DLX*>h=1~y4T+W*=#nn+wG*{1n$U3DwQ(tKNnUIwg^D> z`#n{wRb4^Ia?NIw?(gp@91hd#>nmk48A>D)x^s>GK?nrGTCEm+eSI-8dTBTi2r%)* zV)0wo+uIxaPJdOLNr5;V4vNKM7eTVwEJdTy9}t{|!@$j~b~P!GQmI7we4d@^sezbq zxm;8(m(P00a5$uwmlpsBMg#Gr>Rn@SStJ&-c0ZdgCG_jA0LI&t5FNV#d^J-P*11bLl^}8 zR=eG%!GB<2RHN{`r>7@!yWLVQCIy1V(Tc#j@wVg@RPOA`PO_y?+1zcfxIr$R&rN%C4PcbhP&+O_yXaIy7zO&=d0?7r&x`T1mL%tenz zBkK42>~3s2oifq5MbWz>6HSkXLHGu6Gu$XH77Los=l|^+30b6l|rRZDpV4LVxv%~m7YMUar135H_P2+&X|#P?@n@?8@tYLzVG~*@nu=omHxo` z3P60OVxWl61ff8Yk5nX;N|9w*evADk9w}=9WH=mBB9ZVdjDHkJtyZIQx%~McqtS>C zhXV};gU?oEzu!}}TJ?3FNLfpO_&HDhCrC1xWaSBRORP$zVl=;`6hQL=M zllie1*HUhPfSJ$dtY&-v9%&E^jlEvaZg9fiXpo^$h<`SljjRBkp99S4bW(1b(jfOc z4;UDjhxxGTgf})sk zO0q9OM6WuZ&o_?JCe;XdPRyw9FxfuM3tr&Gc>|Bl6J zl5CK7lz(97K}@#Wvq10#@5&SIn9l}5o)nA4ho`_-Z3l=(qd~D)OnO{lgP82_0KmcR za=EbYn%N-2US1arDrz_Z;_Wg4k%=FSloe6Q1(VMN vASQQL<=ufID!XR#nLrVfyQ}iPx%cS%G+RA@u(nK7!uKoEu}Sa<^OU~M5z^O^eneyD(4K{A;P-EP0jF#tiOu&Co1M7dR1fIfmCKBm(tt=H?1;SdHG5s*i9 z=JPoXheP9eKomqJgKxx+I0#@?s}&WCMWczpcd&o*p;oI!r_+fll}g~Q7AVN0tBppZ zK&t{*kVd0HgTa6bg@R|Rn#gKKKupAqF%SjQZnr6!Op4cPt{~&_nD+ZUEtgBruJ!z2 zE|(LBaRuS-?z)tSo8$2)meE)sDC1uhj=GC~9uF9)O28EaFGmFo*YcCO-a$*jus|eI z@F(>oU|1lCli6%08t~F?#s=Y(^WTga7KnMb4Sav zNT<+bgP3$LRn9b37rWOdSWJ(ovT()hS zmDRODOsJwD0t@4}zu^$qeoA%Gs6A1mv)t2l_x%O%G~Ad=RUm@865 j!ith}M2bjSQ(~?kp%6$DIY3ASDw}QNjY0OhCmFkO62@F##nD z09`63pkx9T$kRX4J^A^CUnx<*h2ecxCB&K3kBr!o$RwOGD z$OGROTKD^1&1SPe{&b&~rO5GkQ~;^#+Wn1zW?+K=rfHf$W`DR(D*}NcsfL#bWV6{Q zJSn!IAP7e^s2EyFHb_~PYQNvJ@&tupyWOhAV$nU<>s7s8ub{=kI|xAldbivCMqyP| zog+f!84Co)old9DyU*uyFi)VwYPI?UK{O#@yn-Mn9*>8*TrO|x0m5v#Tz1azSvW)> zFt8|!-aMg@LVq79H`GGFh(X}Z^?LoU3Jrx+9~8m(${7|244Kd8s%=|^I5+cza%d=| zLc@qbV2~MJP~&hojD}Pn3>5&^?T;0Rt_l(yYm8oMr7J(?JD<-^>kJ#j^g2|*YCV}u z2CEbosxM@@W4kEf-fNj4NLIA53MDZHg=9LNT9sush<}d|;y}GC?_*<=1F;RUs5+hG&M=Uu-ib3ckh_gwEW&5js@$ZRu&df4ki}PW;A0M!OjAMG=~WfbbjN%|=(H`e%si zQ)@*eVHBfp28rQi$vd(l5^u%mn-z)SWyw1}Mv)&F-62oETF@L*BL_t(&L+zNcio!q;h9?NVft?RvWnrbDpkU=Y*jXtEwl*q) zU}IrnWg!SYf|Z~mXt1!b@&WAZ>=eYiU)aOTC2?mj3Gpu5dD+{YZ)fJ8+0}GiztbNW zS^(lRF9sg*nL{Y>$Xj|OnM{(d>wdHSn>|vY1(3mDK#4@cw|_8FAj9GC0Z=NH-XCPY z-&4Qee}52o#m{*n6_e-uk(Zjf*kQ{p0GeFl?oM$Md>^dA_28?Tk)hoU^yfh zLnco|D5leC%IEVmo6YEYy}BwEj)Mq@X0u6+M&rwStyYWhH@E5$3j~H7k4M`)saC60 zC=|Xy2pTtG9DfBtP8<#gTCG-=d9+w8D3{AU4#P>|V1eB4chWS?G*3iG&F6EVJ$hh|yqpC6!81C=_Ccl$B>* zNxLH+F@hjjF;v3I%AiapL$O#)NtTs8A{#c`c~{Kb8eAr5OEW9 ztDdMpuqyj=pd(Mvt~|ezC?~>adjaFm=QH(sJ>|PR+atnYymJ|!d^j*L9s_IO;qP*} zuv!x1UdkW{vC(Km;c%GR?Y3o0FWP-vkzp);Z&fy%FQYmAlq2^Mt3?D5zHhhNAJcI8 zy4`LRkH;V1+{Jpzw#PRonAfk(VJWzHUd#v>m@dm&GnkSHkt O0000Px%9!W$&RA@u(nZM1#Fc8I^1t?QdG67XeSRf@6P_YCsKw2s$pk#qSmx?KvfCc#U zB3)!f5dX=YWAWKiB&3kozkAQ;F$D@tq za=8?W>A2S#6#>9*w^OUts-5Zeda3Pp8y%1l1Q1nKbs)FfO&yQN=zxqMMNz1_t~(Gi z0D#u(b*m1psfQKF`~AJHggS@AL7h&gN$oZWg8gy1Tv|njMhxn(fDk0CHdQiU1p&}h zRvW!XK3R@3+hVZ@mIy>Z`c^|CkRGBSWVP${Dt?HHgM`{QMv(r=?rRb06m0}KpU-MO zpR4_T@5|6VCz%9-g6K(rJEIvvY)^Mrxp3Yf32CRv=Ovz7I79#tKAo!+(N#KA+CTrZq?id+FKC z6(&j$0A*R`)DN%*3286A&}^E<>X>o@3285a$@AQ*sSz~@dH|cvh989Nh4v3eHEh$F z5wR8~4q-29V7N5@oUBC=lqy+t05r>88s=UgoEF34F<@6}z529Zy+Kg6z-F`AtlKZt zvH#|M{6f6n@9k}T#zm3$S-d`vHwe81K>F2(!D4CXc5$_i=`a}$(w~6-5YiWKS_H>^ yb`Xw5`Me{6_(V Date: Fri, 24 Jan 2025 14:56:25 -0500 Subject: [PATCH 2/4] Try using pixelmatch for testing --- package-lock.json | 22 ++++++++++++++++++ package.json | 1 + test/unit/visual/visualTest.js | 42 +++++++++++----------------------- 3 files changed, 36 insertions(+), 29 deletions(-) diff --git a/package-lock.json b/package-lock.json index 3adf94f078..161bbe4f44 100644 --- a/package-lock.json +++ b/package-lock.json @@ -37,6 +37,7 @@ "i18next-browser-languagedetector": "^4.0.1", "lint-staged": "^15.1.0", "msw": "^2.6.3", + "pixelmatch": "^6.0.0", "rollup": "^4.9.6", "rollup-plugin-string": "^3.0.0", "rollup-plugin-visualizer": "^5.12.0", @@ -8161,6 +8162,18 @@ "url": "https://github.com/sponsors/sindresorhus" } }, + "node_modules/pixelmatch": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/pixelmatch/-/pixelmatch-6.0.0.tgz", + "integrity": "sha512-FYpL4XiIWakTnIqLqvt3uN4L9B3TsuHIvhLILzTiJZMJUsGvmKNeL4H3b6I99LRyerK9W4IuOXw+N28AtRgK2g==", + "dev": true, + "dependencies": { + "pngjs": "^7.0.0" + }, + "bin": { + "pixelmatch": "bin/pixelmatch" + } + }, "node_modules/pkg-dir": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/pkg-dir/-/pkg-dir-5.0.0.tgz", @@ -8231,6 +8244,15 @@ "semver-compare": "^1.0.0" } }, + "node_modules/pngjs": { + "version": "7.0.0", + "resolved": "https://registry.npmjs.org/pngjs/-/pngjs-7.0.0.tgz", + "integrity": "sha512-LKWqWJRhstyYo9pGvgor/ivk2w94eSjE3RGVuzLGlr3NmD8bf7RcYGze1mNdEHRP6TRP6rMuDHk5t44hnTRyow==", + "dev": true, + "engines": { + "node": ">=14.19.0" + } + }, "node_modules/postcss": { "version": "8.4.44", "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.4.44.tgz", diff --git a/package.json b/package.json index 978e7d8703..afd0f256b1 100644 --- a/package.json +++ b/package.json @@ -52,6 +52,7 @@ "i18next-browser-languagedetector": "^4.0.1", "lint-staged": "^15.1.0", "msw": "^2.6.3", + "pixelmatch": "^6.0.0", "rollup": "^4.9.6", "rollup-plugin-string": "^3.0.0", "rollup-plugin-visualizer": "^5.12.0", diff --git a/test/unit/visual/visualTest.js b/test/unit/visual/visualTest.js index 594bb302ae..c076da33fd 100644 --- a/test/unit/visual/visualTest.js +++ b/test/unit/visual/visualTest.js @@ -1,5 +1,6 @@ import p5 from '../../../src/app.js'; import { server } from '@vitest/browser/context' +import pixelmatch from 'pixelmatch' import { THRESHOLD, DIFFERENCE, ERODE } from '../../../src/core/constants.js'; const { readFile, writeFile } = server.commands @@ -104,39 +105,22 @@ export async function checkMatch(actual, expected, p5) { ); } - const expectedWithBg = p5.createGraphics(expected.width, expected.height); - expectedWithBg.pixelDensity(1); - expectedWithBg.background(BG); - expectedWithBg.image(expected, 0, 0); + const diffData = actual.drawingContext.createImageData(actual.width, actual.height); + const diffCount = pixelmatch( + actual.drawingContext.getImageData(0, 0, actual.width, actual.height).data, + expected.drawingContext.getImageData(0, 0, actual.width, actual.height).data, + diffData.data, + actual.width, + actual.height, + { threshold: 0.4 } + ); const cnv = p5.createGraphics(actual.width, actual.height); - cnv.pixelDensity(1); - cnv.background(BG); - cnv.image(actual, 0, 0); - cnv.blendMode(DIFFERENCE); - cnv.image(expectedWithBg, 0, 0); - for (let i = 0; i < shiftThreshold; i++) { - cnv.filter(ERODE, false); - } - const diff = cnv.get(); + cnv.drawingContext.putImageData(diffData, 0, 0) + const diff = cnv.get() cnv.remove(); - diff.loadPixels(); - expectedWithBg.remove(); - - let ok = true; - for (let i = 0; i < diff.pixels.length; i += 4) { - let diffSum = 0; - for (let off = 0; off < 3; off++) { - diffSum += diff.pixels[i+off] - } - diffSum /= 3; - if (diffSum > COLOR_THRESHOLD) { - ok = false; - break; - } - } - return { ok, diff }; + return { ok: diffCount === 0, diff }; } /** From 24ffe886d3af37de8c0627428c92f7b80cbbbef6 Mon Sep 17 00:00:00 2001 From: Dave Pagurek Date: Fri, 24 Jan 2025 15:03:29 -0500 Subject: [PATCH 3/4] Revert "Try using pixelmatch for testing" This reverts commit 2cb6a0a64a4fb0a50012dd0d3ebbadaf34748e0d. --- package-lock.json | 22 ------------------ package.json | 1 - test/unit/visual/visualTest.js | 42 +++++++++++++++++++++++----------- 3 files changed, 29 insertions(+), 36 deletions(-) diff --git a/package-lock.json b/package-lock.json index 161bbe4f44..3adf94f078 100644 --- a/package-lock.json +++ b/package-lock.json @@ -37,7 +37,6 @@ "i18next-browser-languagedetector": "^4.0.1", "lint-staged": "^15.1.0", "msw": "^2.6.3", - "pixelmatch": "^6.0.0", "rollup": "^4.9.6", "rollup-plugin-string": "^3.0.0", "rollup-plugin-visualizer": "^5.12.0", @@ -8162,18 +8161,6 @@ "url": "https://github.com/sponsors/sindresorhus" } }, - "node_modules/pixelmatch": { - "version": "6.0.0", - "resolved": "https://registry.npmjs.org/pixelmatch/-/pixelmatch-6.0.0.tgz", - "integrity": "sha512-FYpL4XiIWakTnIqLqvt3uN4L9B3TsuHIvhLILzTiJZMJUsGvmKNeL4H3b6I99LRyerK9W4IuOXw+N28AtRgK2g==", - "dev": true, - "dependencies": { - "pngjs": "^7.0.0" - }, - "bin": { - "pixelmatch": "bin/pixelmatch" - } - }, "node_modules/pkg-dir": { "version": "5.0.0", "resolved": "https://registry.npmjs.org/pkg-dir/-/pkg-dir-5.0.0.tgz", @@ -8244,15 +8231,6 @@ "semver-compare": "^1.0.0" } }, - "node_modules/pngjs": { - "version": "7.0.0", - "resolved": "https://registry.npmjs.org/pngjs/-/pngjs-7.0.0.tgz", - "integrity": "sha512-LKWqWJRhstyYo9pGvgor/ivk2w94eSjE3RGVuzLGlr3NmD8bf7RcYGze1mNdEHRP6TRP6rMuDHk5t44hnTRyow==", - "dev": true, - "engines": { - "node": ">=14.19.0" - } - }, "node_modules/postcss": { "version": "8.4.44", "resolved": "https://registry.npmjs.org/postcss/-/postcss-8.4.44.tgz", diff --git a/package.json b/package.json index afd0f256b1..978e7d8703 100644 --- a/package.json +++ b/package.json @@ -52,7 +52,6 @@ "i18next-browser-languagedetector": "^4.0.1", "lint-staged": "^15.1.0", "msw": "^2.6.3", - "pixelmatch": "^6.0.0", "rollup": "^4.9.6", "rollup-plugin-string": "^3.0.0", "rollup-plugin-visualizer": "^5.12.0", diff --git a/test/unit/visual/visualTest.js b/test/unit/visual/visualTest.js index c076da33fd..594bb302ae 100644 --- a/test/unit/visual/visualTest.js +++ b/test/unit/visual/visualTest.js @@ -1,6 +1,5 @@ import p5 from '../../../src/app.js'; import { server } from '@vitest/browser/context' -import pixelmatch from 'pixelmatch' import { THRESHOLD, DIFFERENCE, ERODE } from '../../../src/core/constants.js'; const { readFile, writeFile } = server.commands @@ -105,22 +104,39 @@ export async function checkMatch(actual, expected, p5) { ); } - const diffData = actual.drawingContext.createImageData(actual.width, actual.height); - const diffCount = pixelmatch( - actual.drawingContext.getImageData(0, 0, actual.width, actual.height).data, - expected.drawingContext.getImageData(0, 0, actual.width, actual.height).data, - diffData.data, - actual.width, - actual.height, - { threshold: 0.4 } - ); + const expectedWithBg = p5.createGraphics(expected.width, expected.height); + expectedWithBg.pixelDensity(1); + expectedWithBg.background(BG); + expectedWithBg.image(expected, 0, 0); const cnv = p5.createGraphics(actual.width, actual.height); - cnv.drawingContext.putImageData(diffData, 0, 0) - const diff = cnv.get() + cnv.pixelDensity(1); + cnv.background(BG); + cnv.image(actual, 0, 0); + cnv.blendMode(DIFFERENCE); + cnv.image(expectedWithBg, 0, 0); + for (let i = 0; i < shiftThreshold; i++) { + cnv.filter(ERODE, false); + } + const diff = cnv.get(); cnv.remove(); + diff.loadPixels(); + expectedWithBg.remove(); + + let ok = true; + for (let i = 0; i < diff.pixels.length; i += 4) { + let diffSum = 0; + for (let off = 0; off < 3; off++) { + diffSum += diff.pixels[i+off] + } + diffSum /= 3; + if (diffSum > COLOR_THRESHOLD) { + ok = false; + break; + } + } - return { ok: diffCount === 0, diff }; + return { ok, diff }; } /** From c4fc9130da7b329edeb19ac99b99a86d4c5f3206 Mon Sep 17 00:00:00 2001 From: Dave Pagurek Date: Fri, 24 Jan 2025 15:04:21 -0500 Subject: [PATCH 4/4] Keep current thresholding for now --- test/unit/visual/visualTest.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/visual/visualTest.js b/test/unit/visual/visualTest.js index 594bb302ae..068e8c7247 100644 --- a/test/unit/visual/visualTest.js +++ b/test/unit/visual/visualTest.js @@ -37,7 +37,7 @@ let namePrefix = ''; // By how many pixels can the snapshot shift? This is // often useful to accommodate different text rendering // across environments. -let shiftThreshold = 1; +let shiftThreshold = 2; /** * A helper to define a category of visual tests.