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

UWP DataGrid.RowDetailsVisibilityMode.VisibleWhenSelected performance issue #2842

Closed
4 of 15 tasks
bent-rasmussen opened this issue Mar 4, 2019 · 4 comments
Closed
4 of 15 tasks
Labels
DataGrid 🔠 Issues on DataGrid control

Comments

@bent-rasmussen
Copy link

bent-rasmussen commented Mar 4, 2019

I'm submitting a...

Bug report (I searched for similar issues and did not find one)

See reproduction of issue at this github bug repro:
DataGridRowDetailsPerfTest

Current behavior

Scrolling performance degrades dramatically when the DataGrid property RowDetailsVisibilityMode is set to VisibleWhenSelected. There is no issue when it is set to Collapsed or Visible. I have tested this with a relatively modest datagrid of 20,000 items, which should pose no issue with a UI virtualized datagrid.

datagridrowdetailsperftest

Expected behavior

Smooth scrolling performance regardless of which RowDetailsVisibilityMode is chosen.

Minimal reproduction of the problem with instructions

There is a minimal repro here:
DataGridRowDetailsPerfTest

Environment

Nuget Package(s):

  • Microsoft.NETCore.UniversalWindowsPlatform
  • Microsoft.Toolkit.Uwp
  • Microsoft.Toolkit.Uwp.UI
  • Microsoft.Toolkit.Uwp.Controls
  • Microsoft.Toolkit.Uwp.UI.Controls.DataGrid

Package Version(s):

  • Microsoft.NETCore.UniversalWindowsPlatform = 6.2.3
  • Microsoft.Toolkit.* = 5.1.0

Windows 10 Build Number:

  • Fall Creators Update (16299)
  • April 2018 Update (17134)
  • October 2018 Update (17763)
  • Insider Build (build number: )

App min and target version:

  • Fall Creators Update (16299)
  • April 2018 Update (17134)
  • October 2018 Update (17763)
  • Insider Build (xxxxx)

Device form factor:

  • Desktop
  • Mobile
  • Xbox
  • Surface Hub
  • IoT

Visual Studio

  • 2017 (version: Professional 15.9.7)
  • 2017 Preview (version: )
@michael-hawker
Copy link
Member

@RBrid?

@RBrid RBrid added the DataGrid 🔠 Issues on DataGrid control label Mar 6, 2019
@christian-clausen
Copy link

I have done some performance analysis and implemented an experimental code change that at least works for our scenario.

The code causing bad performance is in DataGrid.ScrollSlotsByHeight (from DataGridRows.cs). There is a check for 'very large scroll' that excludes the case DataGridRowDetailsVisibilityMode.VisibleWhenSelected:

if (height > 2 * this.CellsHeight &&
    (this.RowDetailsVisibilityMode != DataGridRowDetailsVisibilityMode.VisibleWhenSelected || this.RowDetailsTemplate == null

Consequently, the large scroll optimization is by-passed for VisibleWhenSelected.

I have implemented an experimental code change, that works well at least in our usage scenario:

When there is a large scroll, we calculate an average Row Detail Height, see averageRowDetailHeight below: Then scrolling is smooth also with VisibleWhenSelected.

Here follows the code change (only implemented for the 'scrolling down' case):

if (height > 2 * this.CellsHeight /* ORIG CODE: &&
    (this.RowDetailsVisibilityMode != DataGridRowDetailsVisibilityMode.VisibleWhenSelected || this.RowDetailsTemplate == null */)
{
    // Very large scroll occurred. Instead of determining the exact number of scrolled off rows,
    // let's estimate the number based on this.RowHeight.

    /* ORIG CODE:
    ResetDisplayedRows();
    double singleRowHeightEstimate = this.RowHeightEstimate + (this.RowDetailsVisibilityMode == DataGridRowDetailsVisibilityMode.Visible ? this.RowDetailsHeightEstimate : 0);
    int scrolledToSlot = newFirstScrollingSlot + (int)(height / singleRowHeightEstimate);
    scrolledToSlot += _collapsedSlotsTable.GetIndexCount(newFirstScrollingSlot, newFirstScrollingSlot + scrolledToSlot);
    newFirstScrollingSlot = Math.Min(GetNextVisibleSlot(scrolledToSlot), lastVisibleSlot);
    */
    // NEW CODE Start
    double averageRowDetailHeight()
    {
        double result = 0;
        switch (this.RowDetailsVisibilityMode)
        {
            case DataGridRowDetailsVisibilityMode.Collapsed:
                result = 0;
                break;
            case DataGridRowDetailsVisibilityMode.Visible:
                result = this.RowDetailsHeightEstimate;
                break;
            case DataGridRowDetailsVisibilityMode.VisibleWhenSelected:
                if (this.RowDetailsTemplate == null)
                {
                    result = 0;
                }
                else
                {
                    int totalRows = this.DataConnection.Count;
                    int totalOpenRowDetails = GetDetailsCountInclusive(0, totalRows);
                    result = this.RowDetailsHeightEstimate * ((double)totalOpenRowDetails / totalRows);
                }

                break;
            default:
                break; // Error, cannot happen
        }

        return result;
    }

    ResetDisplayedRows();
    double singleRowHeightEstimate = this.RowHeightEstimate + averageRowDetailHeight();
    int scrolledToSlot = newFirstScrollingSlot + (int)(height / singleRowHeightEstimate);
    scrolledToSlot += _collapsedSlotsTable.GetIndexCount(newFirstScrollingSlot, newFirstScrollingSlot + scrolledToSlot);
    newFirstScrollingSlot = Math.Min(GetNextVisibleSlot(scrolledToSlot), lastVisibleSlot);

    // NEW CODE End

@anawishnoff
Copy link

We are not adding features to this DataGrid at this time. See possible workaround above. If you would like to see this feature in the newly proposed WinUI DataGrid, please add a comment in the WinUI discussion issue. Thanks!

@Kyaa-dost
Copy link
Contributor

Closing this issue as this is already responded by @anawishnoff

@CommunityToolkit CommunityToolkit locked as resolved and limited conversation to collaborators Aug 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
DataGrid 🔠 Issues on DataGrid control
Projects
None yet
Development

No branches or pull requests

6 participants