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

fix(StaticCanvas): fully clean the cache canvas to avoid leaving trailing pixels #9779

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

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

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
Copy link
Contributor

Build Stats

file / KB (diff) bundled minified
fabric 914.723 (-0.133) 305.397 (-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
Copy link
Contributor

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

82.76%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
index.node.ts14.29%100%0%25%23, 26, 29, 41, 44, 47
src
   ClassRegistry.ts100%100%100%100%
   Collection.ts94.85%94.83%87.10%97.14%109, 112, 215–216, 241–242
   CommonMethods.ts96.55%87.50%100%100%9
   Intersection.ts100%100%100%100%
   Observable.ts87.76%85.29%87.50%89.58%134–135, 160–161, 32–33, 41, 50, 78, 89
   Point.ts100%100%100%100%
   Shadow.ts98.55%95.65%100%100%213
   cache.ts97.06%90%100%100%57
   config.ts74.14%65%66.67%82.76%131, 139, 139, 139, 139, 139–141, 152–154, 30
   constants.ts100%100%100%100%
src/LayoutManager
   ActiveSelectionLayoutManager.ts96.15%100%85.71%100%
   LayoutManager.ts89.03%92.86%76.92%90.41%169–170, 172, 174, 174, 174, 269, 331–332, 343, 51
   constants.ts100%100%100%100%
src/LayoutManager/LayoutStrategies
   ClipPathLayout.ts82.05%72.22%100%88.89%30–31, 42, 54, 57–58, 65
   FitContentLayout.ts100%100%100%100%
   FixedLayout.ts20%0%0%40%20–21, 23, 23, 23, 23, 23
   LayoutStrategy.ts85.11%64.71%100%95.65%37, 44, 44, 44, 52, 72–73
   utils.ts91.67%85.71%100%100%28, 34
src/Pattern
   Pattern.ts90.54%93.94%80%90.32%119, 130, 139, 32, 36
src/brushes
   BaseBrush.ts100%100%100%100%
   CircleBrush.ts0%0%0%0%108, 108, 108, 110, 112, 114–116, 118–121, 128–129, 13, 136, 138, 23–24, 32–36, 40–44, 51–54, 62–66, 68, 76, 76, 76, 76, 76–77, 79, 79, 79–82, 84, 92–93, 95, 97–99
   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.ts78.81%77.33%82.14%79.42%1001–1003, 1006–1007, 1011–1012, 1123–1125, 1128–1129, 1202, 1314, 1435, 158, 183, 289, 289–294, 299, 322–323, 328–333, 353, 353, 353–354, 354, 354–355, 363, 368–369, 369, 369–370, 372, 381, 387–388, 388, 388, 42, 431, 439, 443, 443, 443–444, 446, 46, 528–529, 529, 529–530, 536, 536, 536–538, 558, 560, 560, 560–561, 561, 561, 564, 564, 564–565, 568, 577–578, 580–581, 583, 583–584, 586–587, 599–600, 600, 600–601, 603–608, 614, 621, 658, 658, 658, 660, 662–667, 673, 679, 679, 679–680, 682, 685, 690, 703, 731, 783–784, 784, 784, 784, 784, 784, 787–788, 791, 791–793, 796–797, 879, 886, 886, 886, 899, 928–929, 929, 929–931, 934–935, 935, 935, 937, 945, 945, 945–947, 947, 947, 954–955, 963–964, 964, 964–965, 971, 973
   CanvasOptions.ts100%100%100%100%
   SelectableCanvas.ts90.22%87.30%94.55%91.74%1004, 1012, 1123, 1125, 1127–1128, 1199, 1220–1221, 1239–1240, 460–461, 463–464, 464, 464, 513–514, 591, 596, 666, 671–672, 699–700, 724, 727–730, 730, 730–731, 731, 731, 737–738, 740, 760–761, 766–770, 772, 807–808, 811, 811, 811–812, 931, 931–932, 935, 955, 955
   StaticCanvas.ts96.44%92.72%98.91%98.27%1045, 1055, 1106–1108, 1111, 1146–1147, 1223, 1232, 1232, 1236, 1236, 1283–1284, 188–189, 205, 585, 597–598, 928–929, 929, 929–930
   StaticCanvasOptions.ts100%100%100%100%
   TextEditingManager.ts86%71.43%91.67%91.67%17, 17, 17–18, 18, 18
src/canvas/DOMManagers
   CanvasDOMManager.ts95.52%70%100%100%21–22, 29
   StaticCanvasDOMManager.ts97.50%88.89%100%100%33
   util.ts86.67%80.56%83.33%93.94%14, 26, 63–64, 67, 67, 74, 93–94
src/color
   Color.ts94.96%91.67%96.30%96.05%233, 258–259, 267–268, 48
   color_map.ts100%100%100%100%
   constants.ts100%100%100%100%
   util.ts85.71%76.92%100%89.74%55–56, 56, 58, 58, 58–59, 61–62, 89
src/controls
   Control.ts94.44%93.10%91.67%96.77%183, 249, 354
   changeWidth.ts100%100%100%100%
   commonControls.ts100%100%100%100%
   controlRendering.ts81.63%78%100%84.78%106, 111, 121, 121, 45, 50, 61, 61, 65–72, 81–82
   drag.ts100%100%100%100%
   fireEvent.ts88.89%75%100%100%13
   polyControl.ts10.87%0%0%16.13%114, 114, 114, 114, 114, 116–119, 119, 122, 129, 24–26, 50–52, 58–59, 61, 71, 77–79, 79, 81, 84, 86, 90–92, 94, 99
   rotate.ts19.57%12.50%50%21.43%41, 45, 51, 51, 51–52, 55–57,

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.

None yet

1 participant