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

Corruption of Table.topIndex when deleting items off the bottom of the list, and using BeginUpdate(). #39

Open
antiduh opened this issue Feb 28, 2017 · 2 comments
Labels

Comments

@antiduh
Copy link
Contributor

antiduh commented Feb 28, 2017

I've found a somewhat complicated bug involving Table.TopIndex - under certain usages, it seems that the value gets corrupted, causing the table to layout rows in the wrong order.

Specifically, the circumstances to reproduce it are as follows:

  • Add enough rows to a table such that there are rows below the bottom visible edge of the table.
  • Scroll down and remove the last row.
  • Note that sometimes the table does not scroll up to remove empty whitespace at the bottom of the table created by removing the last row.
  • Repeat removing the last row until rows are deleted offscreen.
  • Table.topIndex is now corrupted, and the scrollbars/row layout act very strangely.

It seems that to reproduce this it is necessary to use BeginUpdate() and EndUpdate(). Specifically, whenever my "data model" changes, I "update" the table by clearing it entirely and refilling it:

private void ReloadTable()
{
    int selectedRow = SelectedRowIndex;

    table.BeginUpdate();

    tableModel.Rows.Clear();

    foreach( int value in data )
    {
        Row row = new Row();
        row.Cells.Add( new Cell( value ) );
        tableModel.Rows.Add( row );
    }

    table.EndUpdate();

    // Restore selection.
    if( selectedRow != -1 )
    {
        int row = Math.Min( selectedRow, tableModel.Rows.Count - 1 );

        tableModel.Selections.Clear();
        tableModel.Selections.SelectCell( row, 0 );
    }
}

Since this is a slightly complicated bug to understand, I've attached a demo program and a video of me reproducing the bug. In order to make the demo program work, I've had to make a few mods to Table.cs for debugging (be able to modify TopIndex and also catch an event when it changes).

I've attached a zip file containing my demo program and a video of me reproducing the bug, as well as a patch file showing the changes I needed to make to Table.cs to make the demo program.

XPTable TopIndex.zip

@antiduh
Copy link
Contributor Author

antiduh commented Mar 1, 2017

I was able to mitigate this by changing the implementation of vScrollBar_ValueChanged:

private void vScrollBar_ValueChanged( object sender, EventArgs e )
{
    // Work around for https://github.com/schoetbi/XPTable/issues/39
    // Instead of the complicated GetNewTopRowIndex/FindNextVisibleCell bs, 
    // just use the scrollbar value; that's what its there for.
    //int newtopIndex = GetNewTopRowIndex(topIndex, vScrollBar.Value - lastVScrollValue);
    //topIndex = newtopIndex;

    this.topIndex = vScrollBar.Value;

    this.Invalidate();

    if( this.EnableWordWrap )
        UpdateScrollBars();

    lastVScrollValue = vScrollBar.Value;
}

I'm not sure why my implementation isn't what is used to start with - my understanding is that the scrollbar's value and topIndex should always be in sync. Perhaps it has to do with hidden rows or parent-child rows?

It works for me, but I'm not sure the above fix can be committed.

@schalpat schalpat added the bug label Mar 2, 2017
@schoetbi
Copy link
Owner

schoetbi commented Mar 2, 2017

I also have an odd feeling when changing scrollbar issues since the code is not easy understandable and clear. Please give me some time to look at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants