Skip to content

Conversation

@asturur
Copy link
Member

@asturur asturur commented Mar 29, 2024

Description

This PR changes the code that cleans the cache canvas between one frame and another when there is no need to fully wipe the canvas element.
Fabric relies on the knowledge of how large is the drawing and how much is zoomed to clean the canvas correctly.
When we loose understanding of the size of the element ( the case with strange strokes and acute angles ) we don't clean good enough.

This code simplify the cleaning procedure with simply a full canvas wipe in a non transformed state.
It also deletes cacheWidth and cacheHeight that seems like they were around for this exact purpose and nothing else.

This bug wasn't really common before, but with the modifyPoly controls and the nested selections became more common.
It is also very easy to fix and reduces code instead of adding it, so probably a win/win

in Master:
image

in this branch:
image

As you can see from this picture the triangle will still bleed out but won't leave the trail when moved.

Does it perform differently?
I'm not sure how to trigger it or measure it properly.
This change triggers when we invalidate the cache but we don't rebuild the canvas.
So i probably have to build a specific use case just to be sure

in this branch:

In Action

@codesandbox
Copy link

codesandbox bot commented Mar 29, 2024

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@asturur asturur marked this pull request as draft March 29, 2024 12:04
@github-actions
Copy link
Contributor

github-actions bot commented Mar 29, 2024

Build Stats

file / KB (diff) bundled minified
fabric 913.225 (-0.133) 304.706 (-0.145)

@asturur asturur changed the title DRAFT - test run fix(StaticCanvas): fully clean the cache canvas to avoid leaving trailing pixels Mar 29, 2024
@asturur asturur marked this pull request as ready for review June 24, 2024 06:54
@asturur asturur merged commit 0c647ff into master Jun 24, 2024
@asturur asturur deleted the remove-the-dirty-pixels-cache branch June 24, 2024 07:37
@github-actions
Copy link
Contributor

Coverage after merging remove-the-dirty-pixels-cache into master will be

84.86%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
index.node.ts14.29%100%0%25%23, 26, 29, 41, 44, 47
src
   ClassRegistry.ts90.32%61.54%100%97.73%31, 52
   Collection.ts78.47%42.62%87.10%85.82%130, 138, 153, 155–157, 159, 169–170, 181, 197, 215, 217, 228, 243, 254, 265, 270, 279, 281, 286–287, 302, 304, 309–310, 329, 333–334, 338–344, 346–348, 350
   CommonMethods.ts91.43%71.43%100%96%50, 52
   Intersection.ts85.25%48.91%100%97.30%184–188, 190, 228, 237, 239, 289, 297, 297
   Observable.ts79.89%54.55%93.75%87.10%136, 145, 148, 160, 162, 167, 68–70, 72, 76, 80, 84–85, 87–91
   Point.ts90.27%61.22%100%93.60%104, 117, 148, 157, 179, 197, 206, 216, 225, 236–239, 259, 285, 297, 317, 328, 341, 349, 359, 95
   Shadow.ts89.39%78.26%100%90.19%145, 148, 150–155, 164, 201, 204, 211, 228–235, 239–240
   cache.ts84.88%45.45%100%90.14%57, 59, 71–72, 74–77
   config.ts87.73%55%66.67%94.03%132, 134–137, 139, 142–143, 147, 152
   constants.ts100%100%100%100%
src/LayoutManager
   ActiveSelectionLayoutManager.ts93.33%76.92%85.71%100%
   LayoutManager.ts90.54%65.06%76.92%99.29%269, 333, 344
   constants.ts100%100%100%100%
   index.ts48.57%37.50%80%66.67%1, 1, 1–2, 2, 2–3, 3, 3–4, 4, 4–5, 5, 5, 5–6
   types.ts100%100%100%100%
src/LayoutManager/LayoutStrategies
   ClipPathLayout.ts73.08%50%100%78.95%39, 41–44, 46–48, 57–58, 66–69
   FitContentLayout.ts100%100%100%100%
   FixedLayout.ts85.71%20%100%100%23, 23
   LayoutStrategy.ts87.60%55.56%100%96.55%46, 54, 72, 72, 74
   utils.ts72.58%50%100%78.72%29–32, 34–35, 40–44
src/Pattern
   Pattern.ts70.18%90.91%80%65.95%105–107, 114, 118–119, 119–122, 130–138, 140–141, 143, 153–164, 174, 176–181, 183–188, 190–199, 204–205, 207–209, 211, 33, 37
src/brushes
   BaseBrush.ts89.33%91.67%100%88.55%110, 115, 124–125, 130, 135, 143, 146, 155–160, 99
   CircleBrush.ts52.10%12.50%12.50%58.25%100–108, 108–118, 122, 130–139, 55, 67, 69, 76, 76, 78–79, 79, 83, 85–86, 92–98
   PatternBrush.ts97.06%87.50%100%100%21
   PencilBrush.ts91.06%82.35%100%93.81%122–123, 152, 152–154, 176, 176, 276, 280, 285–286, 68–69, 84–85
   SprayBrush.ts0%0%0%0%107, 107, 107, 107, 107–108, 110–111, 118–119, 121, 123–127, 136, 140–141, 141, 148, 148, 148–151, 153–156, 160–161, 163, 165–168, 17, 171, 178–179, 18, 181, 183–184, 186, 193–194, 196–197, 20, 200, 200, 207, 207, 21, 211, 22, 22, 22–24, 28, 32, 39, 46, 53, 60, 67, 84–86, 94–96, 98–99
src/canvas
   Canvas.ts75.53%40%92.86%85%1004, 1007–1008, 1012–1013, 1018, 1063–1068, 1105, 1126, 1128, 1130, 1137, 1140, 1203–1207, 1286–1290, 1317, 1334, 1380–1397, 1403–1408, 1411–1412, 1414, 1418, 1420–1421, 1423–1425, 1429, 1431, 1433–1435, 1438–1443, 1446–1448, 1451, 1453, 1467, 1474, 1476–1487, 1489–1492, 1492, 1494, 1498–1499, 1502–1503, 1506–1508, 1511, 354, 369, 388, 443, 559–563, 566–567, 569, 579, 582–583, 585, 588–590, 602, 609–613, 615–620, 622–626, 659, 661, 668–672, 674–679, 681, 683–684, 686–689, 691–692, 747, 783–786, 789, 791, 794, 796, 798, 824, 886–887, 932–933, 935–936, 938, 947–953, 956, 963–964, 966–970, 972, 974, 995–996, 999
   CanvasOptions.ts100%100%100%100%
   SelectableCanvas.ts85.70%53.99%100%94.72%1012, 1021, 1089–1093, 1139–1140, 1142–1143, 1145, 1172, 1174–1175, 1175, 1177, 1216, 1218, 1220, 1236, 1238, 1266, 1269, 1312–1319, 1322, 1325, 1334–1335, 1339–1348, 1352, 1354–1355, 1361–1366, 1370, 362, 384, 462, 513, 515, 591, 596, 673, 701, 992, 994–995
   StaticCanvas.ts75.87%70.27%98.91%75.30%1005, 1008, 1010, 1012, 1020, 1022–1028, 1038, 1041, 1044–1050, 1052–1053, 1055–1076, 1083–1085, 1087, 1095, 1101–1103, 1103–1104, 1104–1105, 1107, 1107–1112, 1125, 1130–1132, 1136, 1140, 1142, 1144, 1149–1153, 1155, 1157–1160, 1163, 1165, 1174, 1176–1177, 1184–1187, 1189, 1195–1198, 1202–1203, 1213, 1219–1221, 1223–1228, 1228, 1228–1232, 1232, 1232–1237, 1239–1246, 1275–1276, 1278–1283, 1285–1291, 1295, 1297–1310, 1318–1319, 1329, 1340, 1381–1385, 1387, 1389, 1391–1395, 1414, 1430, 1447, 1467, 1500, 1504, 1515–1517, 424, 435, 443–448, 456, 495, 497, 549, 553,

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