Removing from one list changing another list unexpectedly

63 views Asked by At

I am trying to implement an undo functionality in my app, part of the code that gets executed when the undo button is pushed is as follows :

if (this.threadStep > 0) 
{
    Debug.WriteLine($"thread remove");

    this._AllCurrentLinesInCanvas.RemoveAt(this._AllCurrentLinesInCanvas.Count - 1);

    _renderTargetDrawingSession.Clear(Colors.Transparent);

    foreach (var line in this._AllCurrentLinesInCanvas)
    {
        _renderTargetDrawingSession.DrawInk(line);
    }

    this.threadStep -= 1;
}

After putting a debugger and then examining, I have found out that when this line gets executed :

this._AllCurrentLinesInCanvas.RemoveAt(this._AllCurrentLinesInCanvas.Count - 1);

not only is _AllCurrentLinesInCanvas changing but another list that is not supposed to change is also being altered:

enter image description here

This is how the lists are constructed when a stroke is made :

private int threadStep = 0; 
private bool isDeepInThread = true;
private int currentTimeline = 0;
private IReadOnlyList<InkStroke> SingleWetLine;
private List<IReadOnlyList<InkStroke>> _AllCurrentLinesInCanvas = new List<IReadOnlyList<InkStroke>>(); 
private List<List<IReadOnlyList<InkStroke>>> _DrawingHistoryThread = new List<List<IReadOnlyList<InkStroke>>>();
private List<List<List<IReadOnlyList<InkStroke>>>> Timelines = new List<List<List<IReadOnlyList<InkStroke>>>>();
private bool clearedByUndoing = false;

private void InkPresenter_StrokesCollected(InkPresenter sender, InkStrokesCollectedEventArgs args) 
{
    this.SingleWetLine = myInkCanvasObject.myInkSynchronizer.BeginDry(); 

    myInkCanvasObject.myInkSynchronizer.EndDry(); 

    this._AllCurrentLinesInCanvas.Add(this.SingleWetLine.ToList()); 
     
    if (this.threadStep == this._DrawingHistoryThread.Count) 
    {
        this._DrawingHistoryThread.Add(new List<IReadOnlyList<InkStroke>>(this._AllCurrentLinesInCanvas));

        if (this.Timelines.Count == 0 || this.clearedByUndoing) 
        {
            this.Timelines.Add(new List<List<IReadOnlyList<InkStroke>>>(this._DrawingHistoryThread));

            this.currentTimeline = this.Timelines.Count;

            this.clearedByUndoing = false;
        }

        else
        {
            this.Timelines[this.Timelines.Count - 1] = new List<List<IReadOnlyList<InkStroke>>>(this._DrawingHistoryThread);

        }

        this.isDeepInThread = true;

        this.threadStep += 1;
    }

    else // create a new timeline when a fork occurs
    {
        Debug.WriteLine("Fork State Hit");
        
        this._DrawingHistoryThread.Clear();

        this._DrawingHistoryThread.Add(new List<IReadOnlyList<InkStroke>>(this._AllCurrentLinesInCanvas));

        this.Timelines.Add(new List<List<IReadOnlyList<InkStroke>>>(this._DrawingHistoryThread));

        this.isDeepInThread = true;

        this.threadStep = 1;

        this.currentTimeline = this.Timelines.Count; 
    }

    myWin2DCanvas.Invalidate();
}

And this is the entire undo function

public void Undo(object sender, RoutedEventArgs e)
{

    using (CanvasDrawingSession _renderTargetDrawingSession = myRenderTarget.CreateDrawingSession())
    {

        if (this.threadStep > 0) 
        {
            Debug.WriteLine($"thread remove");

            this._AllCurrentLinesInCanvas.RemoveAt(this._AllCurrentLinesInCanvas.Count - 1);

            _renderTargetDrawingSession.Clear(Colors.Transparent);

            foreach (var line in this._AllCurrentLinesInCanvas)
            {
                _renderTargetDrawingSession.DrawInk(line);
            }

            this.threadStep -= 1;
        }

        else if (this.currentTimeline > 1) // you are at a node
        {
            Debug.WriteLine("node remove");

            this.currentTimeline -= 1;

            this.threadStep = this.Timelines[this.currentTimeline - 1].Count;

            this._AllCurrentLinesInCanvas.Clear();

            Debug.WriteLine($"_All Current builder : {this.currentTimeline - 1}, {this.threadStep - 1}");

            this._AllCurrentLinesInCanvas = this.Timelines[this.currentTimeline - 1][this.threadStep - 1];

            _renderTargetDrawingSession.Clear(Colors.Transparent);

            foreach (var line in this._AllCurrentLinesInCanvas)
            {
                _renderTargetDrawingSession.DrawInk(line);
            }

            this.threadStep -= 1;
        }

        else // at origin -> first line
        {
            Debug.WriteLine("Cleared through undoing");

            _renderTargetDrawingSession.Clear(Colors.Transparent);

            // clear everything

            this.threadStep = 0;

            this._AllCurrentLinesInCanvas.Clear();

            this._DrawingHistoryThread.Clear();

            this.clearedByUndoing = true;

        }

    }   
    myWin2DCanvas.Invalidate();
}
1

There are 1 answers

3
Developer-Mike On BEST ANSWER

I think your problem is that the nested lists still have References to each other.

this.Timelines[this.Timelines.Count - 1] = new List<List<IReadOnlyList<InkStroke>>>(this._DrawingHistoryThread);

Here you don't make a deep copy of the _DrawingHistoryThread object. To make a deep copy you should use this:

new List<List<IReadOnlyList<InkStroke>>>(this._DrawingHistoryThread.Select(line => line.ToList()))

(Untested code)