Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(): rm fabric.filterBackend => getFilterBackend #8487

Merged
merged 9 commits into from
Jan 1, 2023

Conversation

ShaMan123
Copy link
Contributor

@ShaMan123 ShaMan123 commented Dec 4, 2022

prepare for dropping fabric NS

Motivation

Prepare for dropping fabric namespace

Description

The ref fabric.filterBakend is gone, I wonder if the dev needs to set the backend for any reason because it is now impossible.
getFilterBackend is a lazy initializer.

Changes

Gist

In Action

@@ -181,7 +181,7 @@ export class Image extends FabricObject {
* Delete a single texture if in webgl mode
*/
removeTexture(key: string) {
const backend = fabric.filterBackend;
const backend = getFilterBackend(false);
if (backend && backend.evictCachesForKey) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we use instanceof WebGL?

@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2022

Build Stats

file / KB (diff) bundled minified
fabric 948.365 (+0.164) 305.565 (-0.012)

@github-actions
Copy link
Contributor

github-actions bot commented Dec 4, 2022

Coverage after merging chore-filterbackend into master will be

82.89%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
HEADER.js54%48.15%0%63.64%12, 14, 14, 14, 14, 14, 16–17, 21, 21–22, 22, 22, 24, 24, 26, 28, 30–31, 77, 77, 77
src
   cache.ts97.06%90%100%100%56
   config.ts77.27%66.67%66.67%84.62%130, 138–140, 151–153
   constants.ts100%100%100%100%
   intersection.class.ts100%100%100%100%
   pattern.class.ts92.31%85.71%100%96.43%120, 126, 137, 146, 98
   point.class.ts100%100%100%100%
   shadow.class.ts98.51%96%100%100%157
src/brushes
   base_brush.class.ts100%100%100%100%
   circle_brush.class.ts1.52%0%0%2%101, 103–105, 114, 114, 114, 116, 118, 120–122, 124–127, 135, 142, 144, 24, 29–30, 38–42, 46–50, 57–60, 68–72, 74, 82, 82, 82, 82, 82–83, 85, 85, 85–88, 90, 98–99
   pattern_brush.class.ts97.14%87.50%100%100%22
   pencil_brush.class.ts91.91%85.42%100%93.64%123–124, 153, 153–155, 277, 281, 286–287, 69–70, 85–86
   spray_brush.class.ts1.16%0%0%1.56%100–102, 104–105, 113, 113, 113, 113, 113–114, 116–117, 124–125, 127, 129–133, 142, 146–147, 147, 155, 155, 155–158, 160–163, 167–168, 170, 172–175, 178, 185–186, 188, 190–191, 193, 200–201, 203–204, 207, 207, 214, 214, 218, 23–24, 26–28, 28, 28–30, 34, 43, 50, 57, 64, 71, 78, 90–92
src/canvas
   canvas.class.ts92.52%88.15%94.12%95.45%1150, 1150–1151, 1154, 1174, 1174, 1236, 1272–1273, 1292–1293, 1301–1302, 1323, 1331, 1444–1445, 1447–1448, 1468–1469, 1632–1633, 1637, 509, 576–577, 582, 592, 721–722, 724–725, 725, 725, 771–772, 833–834, 837, 839, 884–886, 916, 921–922, 951–952
   canvas_events.ts78.10%75%82.81%79.52%1003–1004, 1004, 1004–1006, 1008–1009, 1009, 1009, 1011, 1019, 1019, 1019–1021, 1021, 1021, 1027–1028, 1036–1037, 1037, 1037–1038, 1043, 1045, 1075–1077, 1080–1081, 1085–1086, 1184, 1204–1206, 1209–1210, 1282, 1369, 1402, 145, 1496–1497, 1503, 1507–1508, 1524, 1546, 1593, 1598, 1637, 170, 318–319, 319, 319–320, 320, 323–327, 332, 334, 346–348, 351, 368, 368, 368–369, 369, 369–370, 378, 383–384, 384, 384–385, 387, 396, 402–403, 403, 403, 439, 443, 443, 443, 443, 443–444, 446, 521, 523, 523, 523–525, 545, 547, 547, 547–548, 548, 548, 551, 551, 551–552, 555, 564–565, 567–568, 570, 570–571, 573–574, 586–587, 587, 587–588, 590–594, 600, 607, 647, 647, 647, 649, 651–655, 661, 667, 667, 667–668, 670, 673, 678, 691, 718, 774–775, 775, 775–776, 778, 781–782, 782, 782–783, 785–786, 789, 789–791, 794–795, 805, 887, 901, 908, 929, 961, 982–983, 989
   canvas_gestures.mixin.ts9.21%5.56%7.14%11.36%101, 114, 127, 139, 148, 157–161, 170, 172, 172, 172–173, 175–176, 189, 198, 207, 211, 30, 30, 30–31, 31, 31, 31, 36, 39–40, 40, 40–41, 47, 50, 58, 58, 58, 58, 58–59, 62–64, 66–67, 69, 71, 71, 71–73, 76, 78, 88
   static_canvas.class.ts94.57%88.89%97.92%97.13%1130–1131, 1131, 1131–1132, 1252, 1262, 1316–1317, 1320, 1355–1356, 1434, 1443, 1448, 1497–1498, 1726, 1726–1727, 1777, 1780, 1783, 1783, 1783, 1786, 1789, 1789, 1789, 309, 345, 358, 414–415, 417–418, 427, 431–432, 434–435, 890
src/color
   color.class.ts92.16%86.49%100%94.29%330–331, 335–336, 339–340, 58, 88–89, 89, 91, 91, 91–92, 94–95
   color_map.ts100%100%100%100%
   constants.ts100%100%100%100%
   index.ts100%100%100%100%
   util.ts100%100%100%100%
src/controls
   actions.ts100%100%100%100%
   changeWidth.ts100%100%100%100%
   control.class.ts93.98%88.89%90.91%97.78%236, 320, 320, 355
   controls.render.ts81.63%78%100%84.78%106, 111, 121, 121, 45, 50, 61, 61, 65–72, 81–82
   default_controls.ts86.67%66.67%100%100%125, 132
   drag.ts100%100%100%100%
   rotate.ts20%12.50%50%22.22%45, 51, 51, 51–52, 55–57, 59, 59, 59, 59, 59–61, 61, 61–63, 65, 65, 65–67, 67, 67–68, 73, 73, 73–74, 76, 78, 80–81
   scale.ts93.41%92.68%100%93.59%129–130, 132–134, 148–149, 181–183, 42
   scaleSkew.ts81.25%64.29%100%92.31%27, 29, 29, 29, 31, 33
   skew.ts91.03%79.31%100%97.67%130&ndash

export function getFilterBackend(): FilterBackend {
if (!fabric.filterBackend) {
fabric.filterBackend = initFilterBackend();
export function getFilterBackend(strict = true): FilterBackend {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we want strict option here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image#removeTexture: we don't want it to create the backend, only to probe it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok so is the issue that initElement calls removeTexture? and that we don't want to strictly couple Image with having a filter Backend?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems correct

@asturur
Copy link
Member

asturur commented Jan 1, 2023

Merging this and coming back with

  • setFilterBackend ( when you want to swap imperatively from one to other )
  • eventually getFilterBackend and getOrInitFilterBackend insted of strict because i really don't like strict = true
    (cant see the strictly get it or not strictly get it meaning )

@asturur asturur merged commit 22dba1c into master Jan 1, 2023
@ShaMan123
Copy link
Contributor Author

cant see the strictly get it or not strictly get it meaning

strict in terms of existence
But whatever. It is a matter of taste.

@ShaMan123 ShaMan123 deleted the chore-filterbackend branch January 2, 2023 10:56
@ShaMan123
Copy link
Contributor Author

Maybe it is only a naming issue.

frankrousseau pushed a commit to cgwire/fabric.js that referenced this pull request Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants