fix(`WebGLProbe`): regression `enableGLFiltering` config + init bug #8301

ShaMan123 · 2022-09-19T08:24:58Z

This PR fixes 2 regression from #8199

enableWebGLFiltering see https://github.com/fabricjs/fabric.js/pull/8199/files#r973981355
bug in negation

and adds tests

see https://github.com/fabricjs/fabric.js/pull/8199/files#r973981355

github-actions · 2022-09-19T08:28:07Z

Coverage after merging restore-enableGLFiltering-flag into master will be

82.44%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
HEADER.js	54.90%	48.15%	0%	65.22%	105, 105, 105, 12, 14, 14, 14, 14, 14, 16–17, 21, 21–22, 22, 22, 24, 24, 26, 28, 30–31
src
cache.ts	97.06%	90%	100%	100%	57
canvas.class.ts	93.44%	90.36%	94.23%	95.58%	1077, 1077–1078, 1081, 1101, 1101, 1136, 1169–1170, 1198–1199, 1232, 1240, 1350–1351, 1353–1354, 1374–1375, 1533, 1538, 1548, 1552, 485–486, 491, 500, 649–651, 696–697, 761–762, 765, 767, 814–816, 858, 863–864, 892–893
config.ts	77.27%	66.67%	66.67%	84.62%	130, 138–140, 151–153
constants.ts	100%	100%	100%	100%
intersection.class.ts	100%	100%	100%	100%
pattern.class.ts	92.19%	85.71%	100%	96.30%	110, 116, 127, 136, 88
point.class.ts	100%	100%	100%	100%
shadow.class.ts	95.95%	90%	100%	100%	172, 239, 8
static_canvas.class.ts	90.10%	83.79%	96.70%	92.75%	1152–1153, 1153, 1153–1154, 1288, 1354–1355, 1358, 1407–1408, 1501, 1516, 1520, 1546–1547, 1576–1577, 1610–1611, 1652–1653, 1656, 1658, 1658, 1658, 1658, 1662, 1662, 1662–1664, 1686–1687, 1728–1729, 1732, 1734, 1734, 1734, 1734, 1738, 1738, 1738–1740, 1813, 1813–1814, 1873, 1875, 1875, 1875, 1875, 1875–1876, 1879–1880, 1880, 1880–1881, 1884, 1884, 1884, 1886, 1889, 1895, 1897–1898, 1898, 1898, 1901–1902, 1902, 1902, 1905, 278–279, 281–282, 284–285, 298–299, 301–302, 616, 642, 698–701, 901
src/brushes
base_brush.class.ts	100%	100%	100%	100%
circle_brush.class.ts	2.99%	0%	0%	3.92%	100–101, 103, 105–107, 116, 116, 116, 118, 120, 122–124, 126–129, 137, 144, 146, 26, 31–32, 40–44, 48–52, 59–62, 70–74, 76, 84, 84, 84, 84, 84–85, 87, 87, 87–90, 92
pattern_brush.class.ts	5.26%	0%	0%	8.33%	16, 20–23, 25–26, 26, 26–29, 37–38, 40, 44, 55, 55, 55, 63–65, 65, 65, 72–73, 75–76, 76, 80
pencil_brush.class.ts	91.95%	85.42%	100%	93.69%	125–126, 155, 155–157, 279, 283, 288–289, 71–72, 87–88
spray_brush.class.ts	2.30%	0%	0%	3.08%	102–104, 106–107, 115, 115, 115, 115, 115–116, 118–119, 126–127, 129, 131–135, 144, 148–149, 149, 157, 157, 157–160, 162–165, 169–170, 172, 174–177, 180, 187–188, 190, 192–193, 195, 202–203, 205–206, 209, 209, 216, 216, 220, 25–26, 28–30, 30, 30–32, 36, 45, 52, 59, 66, 73, 80, 92–94
src/color
color.class.ts	91.67%	84.51%	100%	94.44%	325–326, 330–331, 334–335, 41, 45, 72–73, 73, 75, 75, 75–76, 78–79
color_map.ts	100%	100%	100%	100%
constants.ts	100%	100%	100%	100%
index.ts	100%	100%	100%	100%
util.ts	100%	100%	100%	100%
src/controls
control.class.ts	90.24%	81.48%	86.67%	97.50%	210, 304, 304, 348, 372, 6
controls.actions.ts	76.80%	69.20%	93.75%	80.67%	161, 163, 163, 163, 165, 167, 26, 28, 28, 28, 290–293, 321, 323, 330–331, 333, 333–334, 336–337, 341, 373, 375, 382–383, 385, 385–386, 388–389, 393, 421–422, 424, 426, 431, 434, 434, 434–435, 435, 435, 437, 437, 437–438, 438, 438, 441, 441, 441–442, 442, 442, 475–476, 478, 480, 485, 488, 488, 488–489, 489, 489, 491, 491, 491–492, 492, 492, 495, 495, 495–496, 496, 496, 520–522, 528, 528, 528–529, 532–535, 537, 537, 537–539, 539, 539–541, 543, 543, 543–545, 545, 545–546, 551, 551, 551–552, 554, 556–558, 57, 589–590, 592–594, 641–643, 8, 841
controls.render.ts	85.11%	84.78%	100%	84.78%	23, 27, 42–49, 58–59, 82, 86
default_controls.ts	94.29%	50%	100%	100%	115, 83
src/filters
2d_backend.class.ts	96.43%	83.33%	100%	100%	78
WebGLProbe.ts	40%	40%	57.14%	34.78%	38–39, 49–53, 61, 64, 66, 66, 66–67, 67, 67–70, 72, 74, 78
base_filter.class.ts	28.90%	31.82%	36.36%	26.17%	100–102, 102, 102–103, 112–116, 116, 116–117, 124–125, 125, 125–128, 143, 159, 169–174, 178, 181, 181, 181–184, 184, 184–185, 185, 188–189, 195, 204–205, 210–214, 257–260, 273, 273, 273–274, 276, 292–294, 294, 294, 294, 294–295, 297, 299–300, 306–307, 309–311, 315–316, 318, 322–324, 328, 332, 352, 352, 352–356, 393, 78, 78, 78–79, 79, 79–80, 80, 80–81, 86–89, 89, 89–90,

asturur · 2022-09-19T09:04:43Z

src/filters/WebGLProbe.ts

@@ -57,10 +58,10 @@ class WebGLProbe {
   * @returns config object if true
   */
  private queryWebGL() {
-    if (this.initialized || !fabric.isLikelyNode) {
+    if (this.initialized || fabric.isLikelyNode) {


mmmm when this changed?

or it was just a typo in the conversion?

asturur · 2022-09-19T09:05:34Z

Add the changelog please!

ShaMan123

fixed regressions from #8199

enableWebGLFiltering
bug in negation
added tests

To Do

Once my ci PRs are in I can test the WebGLProbe with sinon
In a next PR we should make config changes restored after each test

ShaMan123 · 2022-09-19T09:06:21Z

src/filters/WebGLProbe.ts

@@ -57,10 +58,10 @@ class WebGLProbe {
   * @returns config object if true
   */
  private queryWebGL() {
-    if (this.initialized || !fabric.isLikelyNode) {
+    if (this.initialized || fabric.isLikelyNode) {


😵
😲
regression found

asturur · 2022-09-19T09:20:52Z

what do you mean Once my ci PRs are in I can test the WebGLProbe with sinon ? We can test webGlProbe right now probably launching a browser with acceleration disabled.

I'm not familiar with sinon anymore, but i would avoid adding more testing frameworks right now. if i have to differentiate testing i want to put down all the choices and pick reasonably. ( node specific and browser specific )

ShaMan123 · 2022-09-19T09:24:56Z

sinon is good for both... it is meant for stubbing and spying after objects, classes, modules etc.
Without it I can't access the webGLProbe because it isn't and shouldn't be exported by fabric

ShaMan123 · 2022-09-19T09:25:15Z

I remember adding sinon to a ci pr

ShaMan123

added changelog

asturur · 2022-09-19T10:20:10Z

i used sinon a lot, i don't use it anymore, i m not sure i want to re-learn it.
Adding Sinon and committing to it is something that needs to be weighted against moving to jest for unit testing and something else for browser-needed testing.

I remember adding sinon to a ci pr

Well is not in master yet.

But i m sure we can test this one without mocking

commit 6a868f9 Author: ShaMan123 <shacharnen@gmail.com> Date: Tue Sep 20 19:32:39 2022 +0300 ws fix commit e18d5c6 Author: ShaMan123 <shacharnen@gmail.com> Date: Tue Sep 20 19:26:33 2022 +0300 rename + ws commit 7987763 Author: ShaMan123 <shacharnen@gmail.com> Date: Tue Sep 20 19:20:56 2022 +0300 rename commit 00468ca Author: ShaMan123 <shacharnen@gmail.com> Date: Tue Sep 20 17:38:00 2022 +0300 Update group.js commit 2bb6551 Author: ShaMan123 <shacharnen@gmail.com> Date: Tue Sep 20 17:37:53 2022 +0300 Update object.svg_export.ts commit c716e64 Author: ShaMan123 <shacharnen@gmail.com> Date: Tue Sep 20 17:24:39 2022 +0300 ws commit 6b2e167 Author: ShaMan123 <shacharnen@gmail.com> Date: Tue Sep 20 17:19:32 2022 +0300 Update object.svg_export.ts commit 83d3755 Author: ShaMan123 <shacharnen@gmail.com> Date: Tue Sep 20 17:14:30 2022 +0300 imports commit 2c3ea4c Author: ShaMan123 <shacharnen@gmail.com> Date: Tue Sep 20 17:08:58 2022 +0300 cleaner impl commit a4a9013 Author: ShaMan123 <shacharnen@gmail.com> Date: Tue Sep 20 16:47:46 2022 +0300 fix(): group svg export expose `createClipPathSVGMarkup` for group to override commit 6e21b15 Author: ShaMan123 <shacharnen@gmail.com> Date: Tue Sep 20 09:45:07 2022 +0300 cleanup commit 3001858 Author: ShaMan123 <shacharnen@gmail.com> Date: Tue Sep 20 09:36:57 2022 +0300 fix(): group svg export commit 4b0130b Author: ShaMan123 <shacharnen@gmail.com> Date: Tue Sep 20 09:08:23 2022 +0300 Update object.svg_export.ts commit 44d7b0b Author: Andrea Bogazzi <andreabogazzi79@gmail.com> Date: Tue Sep 20 15:21:04 2022 +0200 Update CHANGELOG.md commit 4f481af Author: Shachar <34343793+ShaMan123@users.noreply.github.com> Date: Tue Sep 20 13:41:01 2022 +0300 ci(): add `build.lock` (#8290) commit e84f9a0 Author: Shachar <34343793+ShaMan123@users.noreply.github.com> Date: Tue Sep 20 06:25:26 2022 +0300 BREAKING: rename `data-fabric-hiddentextarea` to `data-fabric` (#8292) aligns with the rest of the data attribute usage commit f291945 Author: Shachar <34343793+ShaMan123@users.noreply.github.com> Date: Mon Sep 19 13:21:25 2022 +0300 fix(`WebGLProbe`): regression `enableGLFiltering` config + init bug (#8301) commit 71193d3 Author: Andrea Bogazzi <andreabogazzi79@gmail.com> Date: Mon Sep 19 08:35:26 2022 +0200 chore() Updating changelog (#8300) commit d77dc9b Author: Shachar <34343793+ShaMan123@users.noreply.github.com> Date: Mon Sep 19 01:55:39 2022 +0300 fix(fabric.utils) added missing import in dom_misc (#8293)

…abricjs#8301)

fix(): restore lost enableGLFiltering config

f9c43a7

see https://github.com/fabricjs/fabric.js/pull/8199/files#r973981355

ShaMan123 requested a review from asturur September 19, 2022 08:25

ShaMan123 added 4 commits September 19, 2022 11:48

import

d3d946e

test

b4b97d2

fix(): bug!

caa95a8

prettier

ba84f3f

ShaMan123 changed the title ~~fix(): restore lost enableGLFiltering config~~ fix(WebGLProbe): restore lost enableGLFiltering config + init bug Sep 19, 2022

asturur reviewed Sep 19, 2022

View reviewed changes

asturur previously approved these changes Sep 19, 2022

View reviewed changes

ShaMan123 commented Sep 19, 2022

View reviewed changes

ShaMan123 changed the title ~~fix(WebGLProbe): restore lost enableGLFiltering config + init bug~~ fix(WebGLProbe): regression enableGLFiltering config + init bug Sep 19, 2022

Update CHANGELOG.md

e69bbb6

ShaMan123 dismissed asturur’s stale review via e69bbb6 September 19, 2022 09:23

ShaMan123 requested a review from asturur September 19, 2022 09:25

ShaMan123 commented Sep 19, 2022

View reviewed changes

asturur approved these changes Sep 19, 2022

View reviewed changes

asturur merged commit f291945 into master Sep 19, 2022

asturur deleted the restore-enableGLFiltering-flag branch November 27, 2022 00:15

frankrousseau pushed a commit to cgwire/fabric.js that referenced this pull request Jan 6, 2023

fix(WebGLProbe): regression enableGLFiltering config + init bug (f…

b24dd45

…abricjs#8301)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(`WebGLProbe`): regression `enableGLFiltering` config + init bug #8301

fix(`WebGLProbe`): regression `enableGLFiltering` config + init bug #8301

ShaMan123 commented Sep 19, 2022 •

edited

Loading

github-actions bot commented Sep 19, 2022 •

edited

Loading

asturur Sep 19, 2022

asturur Sep 19, 2022

ShaMan123 Sep 19, 2022

asturur commented Sep 19, 2022

ShaMan123 left a comment

ShaMan123 Sep 19, 2022

asturur commented Sep 19, 2022

ShaMan123 commented Sep 19, 2022

ShaMan123 commented Sep 19, 2022

ShaMan123 left a comment

asturur commented Sep 19, 2022 •

edited

Loading

fix(WebGLProbe): regression enableGLFiltering config + init bug #8301

fix(WebGLProbe): regression enableGLFiltering config + init bug #8301

Conversation

ShaMan123 commented Sep 19, 2022 • edited Loading

github-actions bot commented Sep 19, 2022 • edited Loading

asturur Sep 19, 2022

Choose a reason for hiding this comment

asturur Sep 19, 2022

Choose a reason for hiding this comment

ShaMan123 Sep 19, 2022

Choose a reason for hiding this comment

asturur commented Sep 19, 2022

ShaMan123 left a comment

Choose a reason for hiding this comment

To Do

ShaMan123 Sep 19, 2022

Choose a reason for hiding this comment

asturur commented Sep 19, 2022

ShaMan123 commented Sep 19, 2022

ShaMan123 commented Sep 19, 2022

ShaMan123 left a comment

Choose a reason for hiding this comment

asturur commented Sep 19, 2022 • edited Loading

fix(`WebGLProbe`): regression `enableGLFiltering` config + init bug #8301

fix(`WebGLProbe`): regression `enableGLFiltering` config + init bug #8301

ShaMan123 commented Sep 19, 2022 •

edited

Loading

github-actions bot commented Sep 19, 2022 •

edited

Loading

asturur commented Sep 19, 2022 •

edited

Loading