Conversation
github-actions
bot
added
a: tests
| void dispose() { | ||
| for (final OpacityLayer layer in _createdLayers) { | ||
| layer.dispose(); | ||
| } |
There was a problem hiding this comment.
This doesn't look right. I believe, the caller of pushOpacity assumes ownership of the OpacityLayer and should be in charge of disposing it, not the PaintContext.
There was a problem hiding this comment.
This thing is just for testing.
And, i think it is reasonable to ask test to dispose the instance of created TestRecordingPaintingContext, at the end of the test.
Otherwise, it should return the layer somehow, that means change of contract.
|
|
||
| TestClipPaintingContext._(this._layer) : super(_layer, Rect.zero); | ||
|
|
||
| ContainerLayer _layer = ContainerLayer(); |
There was a problem hiding this comment.
Do you need this default value? Looks like in all code paths it's always overridden? Also, should this field be final?
There was a problem hiding this comment.
Yes, i need just one created in constructor. Thanks.
| final int disabledColorAlpha = disabledColor.alpha; | ||
| if (needsCompositing) { | ||
| context.pushLayer(OpacityLayer(alpha: disabledColorAlpha), paintWithOverlay, offset); | ||
| context.pushOpacity(offset, disabledColorAlpha, paintWithOverlay); |
There was a problem hiding this comment.
I believe you need to hold on to the layer this method returns, see https://master-api.flutter.dev/flutter/rendering/LayerHandle-class.html (and probably other invocations of this method).
polina-c
added
the
a: leak tracking
|
The issue is fixed by other prs. Closing. |
2 participants
_RenderChip should not create new layer to push opacity.
TestRecordingPaintingContext, TestClipPaintingContext should dispose created Layers.