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

Fixes VisualLine leak #263

Merged
merged 1 commit into from
Oct 6, 2022
Merged

Fixes VisualLine leak #263

merged 1 commit into from
Oct 6, 2022

Conversation

Gillibald
Copy link
Contributor

@Gillibald Gillibald commented Oct 6, 2022

This PR introduces a fix that removes a VisualLineDrawingVisual from the logical tree on VisualLine.Dispose

@@ -890,7 +890,7 @@ protected override Size MeasureOverride(Size availableSize)
{
// no document -> create empty list of lines
_allVisualLines = new List<VisualLine>();
_visibleVisualLines = new ReadOnlyCollection<VisualLine>(_allVisualLines.ToArray());
_visibleVisualLines = new ReadOnlyCollection<VisualLine>(_allVisualLines);
Copy link
Collaborator

@danipen danipen Oct 6, 2022

Choose a reason for hiding this comment

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

@Gillibald be careful with this change. I think we need to assign a copy of the collection. I already fixed that issue in this PR #137. Please take a look.

Also, see the upstream project implementation. I'd say that is needed otherwise causes the issue described in #137:

@Gillibald Gillibald force-pushed the fixes/visualLineLeak branch from 2a1f2c1 to 0a052a3 Compare October 6, 2022 13:38
@Gillibald Gillibald force-pushed the fixes/visualLineLeak branch from 0a052a3 to b284999 Compare October 6, 2022 13:39
@Gillibald Gillibald changed the title [WIP] Fixes VisualLine leak Fixes VisualLine leak Oct 6, 2022
@Gillibald Gillibald enabled auto-merge October 6, 2022 13:41
@Gillibald Gillibald merged commit 3342450 into master Oct 6, 2022
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