fix(): bubble dirty flag to parent

[ShaMan123]

Description

As part of the group rewrite we exposed a parent ref on the object #8951 .
As part of the layout manager fixes an invalidation bug surfaced #9651 .
Fabric relays on Object#_set to bubble cache invalidation to the parent.
I forgot to fix the invalidation logic when I exposed the parent ref so it still invalidated the group ref.
The bug occurs when a nested object is part of an ActiveSelection.
In that case the group ref is the active selection and the parent ref is the group.
ActiveSelection does not render anything, therefore it doesn't cache meaning that it is pointless to flag it as dirty as opposed to the parent.
This PR will fix the mentioned bug.
I also changed the bubbling logic to always bubble a dirty flag regardless of caching state.

In Action

[codesandbox]

Review or Edit in CodeSandbox

Open the branch in Web Editor • VS Code • Insiders

Open Preview

• Run Next Template
• Run Node Template
• Run Vanilla Template
• Run Vue Template

[github-actions]

Build Stats

file / KB (diff)	bundled	minified
fabric	911.285 (-0.208)	304.940 (-0.092)

[ShaMan123]

@asturur please test this with your demo of #9651

[asturur]

most of the time parent is unset, this would break the detection of isOnAcache for the objects that are not part of a multi selection.
This maybe should do something like

const container = this.parent || this.group

And then check the isOnACache of container?

[ShaMan123]

parent is ALWAYS set
In fact group should be used ONLY for calcTransformMatrix and for layout

[asturur]

Ok because ActiveSelection has its own specific exitGroup.
The division between enterGroup, exitGroup, _enterGroup and _exitGroup are confusing at best.

[ShaMan123]

True

[asturur]

Here i have the same exact concern, group is our main property, while parent is just an exception sometimes.
So we need to check parent or group, with precedence to parent.

Also i think is ok to skip const groupNeedsUpdate = this.group && this.group.isOnACache(); is a bit of hit and miss. Is true we don't care if the group is not cached to set it as dirty, since is just a boolean, but if the group is cached on a different cache than the object, at that point the invalidation isn't necessary and is a cost. ( reading the code today this check isn't made, but if say we are going always to flag a group as dirty we are cutting that away )

What is the reason behind changing the dirty flag? what brought you to change it?

[ShaMan123]

but if the group is cached on a different cache than the object

That is not possible in fabric, that is why there is isOnACache to begin with, isn't it?
And it doesn't make sense.
If the group is caching then the object is drawn on that cache so that cache is dirty if the object is dirty.

[asturur]

Yes you are right the separate cache is going to make the lower cache dirty anyway.
So the issue is just parent vs group

[asturur]

@asturur please test this with your demo of #9651

I would not know what you want to test with that demo.

[ShaMan123]

To see that the dirty flag is working as expected with nested selected objects

[asturur]

Just check it out from the branch and add it to this branch.
Probably one or 2 cases are fixed with the set({ top, left }) and this pr.
But this is going to refresh the cache and not adjust the layout too. So you would see the object moving but i m not sure you would see the layout adapting

[asturur]

I m ok with this PR as long the test you want to do are done on that template.

[github-actions]

Coverage after merging fix/dirty-bubbling into master will be

82.69%

Coverage Report

File	Stmts	Branches	Funcs	Lines	Uncovered Lines
index.node.ts	14.29%	100%	0%	25%	23, 26, 29, 41, 44, 47
src
ClassRegistry.ts	100%	100%	100%	100%
Collection.ts	94.85%	94.83%	87.10%	97.14%	109, 112, 215–216, 241–242
CommonMethods.ts	96.55%	87.50%	100%	100%	9
Intersection.ts	100%	100%	100%	100%
Observable.ts	87.76%	85.29%	87.50%	89.58%	134–135, 160–161, 32–33, 41, 50, 78, 89
Point.ts	100%	100%	100%	100%
Shadow.ts	98.55%	95.65%	100%	100%	213
cache.ts	97.06%	90%	100%	100%	57
config.ts	74.14%	65%	66.67%	82.76%	131, 139, 139, 139, 139, 139–141, 152–154, 30
constants.ts	100%	100%	100%	100%
src/LayoutManager
LayoutManager.ts	87.58%	89.29%	76.92%	90.14%	151–152, 154–155, 155, 155, 155, 155, 249, 310–311, 322, 51
constants.ts	100%	100%	100%	100%
src/LayoutManager/LayoutStrategies
ClipPathLayout.ts	82.05%	72.22%	100%	88.89%	30–31, 42, 54, 57–58, 65
FitContentLayout.ts	100%	100%	100%	100%
FixedLayout.ts	20%	0%	0%	40%	20–21, 23, 23, 23, 23, 23
LayoutStrategy.ts	85.11%	64.71%	100%	95.65%	35, 42, 42, 42, 50, 70–71
utils.ts	91.67%	85.71%	100%	100%	28, 34
src/Pattern
Pattern.ts	90.54%	93.94%	80%	90.32%	119, 130, 139, 32, 36
src/brushes
BaseBrush.ts	100%	100%	100%	100%
CircleBrush.ts	0%	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.ts	97.06%	87.50%	100%	100%	21
PencilBrush.ts	91.06%	82.35%	100%	93.81%	122–123, 152, 152–154, 176, 176, 276, 280, 285–286, 68–69, 84–85
SprayBrush.ts	0%	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.ts	78.66%	77.04%	82.14%	79.33%	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.ts	100%	100%	100%	100%
SelectableCanvas.ts	90.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.ts	96.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.ts	100%	100%	100%	100%
TextEditingManager.ts	86%	71.43%	91.67%	91.67%	17, 17, 17–18, 18, 18
src/canvas/DOMManagers
CanvasDOMManager.ts	95.52%	70%	100%	100%	21–22, 29
StaticCanvasDOMManager.ts	97.50%	88.89%	100%	100%	33
util.ts	86.67%	80.56%	83.33%	93.94%	14, 26, 63–64, 67, 67, 74, 93–94
src/color
Color.ts	94.96%	91.67%	96.30%	96.05%	233, 258–259, 267–268, 48
color_map.ts	100%	100%	100%	100%
constants.ts	100%	100%	100%	100%
util.ts	85.71%	76.92%	100%	89.74%	55–56, 56, 58, 58, 58–59, 61–62, 89
src/controls
Control.ts	94.44%	93.10%	91.67%	96.77%	183, 249, 354
changeWidth.ts	100%	100%	100%	100%
commonControls.ts	100%	100%	100%	100%
controlRendering.ts	81.63%	78%	100%	84.78%	106, 111, 121, 121, 45, 50, 61, 61, 65–72, 81–82
drag.ts	100%	100%	100%	100%
fireEvent.ts	88.89%	75%	100%	100%	13
polyControl.ts	10.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.ts	19.57%	12.50%	50%	21.43%	41, 45, 51, 51, 51–52, 55–57, 59, 59

[ShaMan123]

tested