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

StaggeredLayout crashes on ARM64 build #542

Closed
3 of 14 tasks
euju-ms opened this issue Sep 20, 2024 · 9 comments · Fixed by #574
Closed
3 of 14 tasks

StaggeredLayout crashes on ARM64 build #542

euju-ms opened this issue Sep 20, 2024 · 9 comments · Fixed by #574
Labels
bug Something isn't working Priority-1 regression What was working is now broke

Comments

@euju-ms
Copy link

euju-ms commented Sep 20, 2024

Describe the bug

The StaggeredLayout from the new namespace CommunityToolkit.WinUI.Controls.Primitives crashes on ARM64 build when used in an ItemsView with the following StackTace:

Message = "Array dimensions exceeded supported range."

   at CommunityToolkit.WinUI.Controls.StaggeredLayout.MeasureOverride(VirtualizingLayoutContext context, Size availableSize)
   at Microsoft.UI.Xaml.Controls.VirtualizingLayout.Microsoft.UI.Xaml.Controls.IVirtualizingLayoutOverrides.MeasureOverride(VirtualizingLayoutContext context, Size availableSize)
   at ABI.Microsoft.UI.Xaml.Controls.IVirtualizingLayoutOverrides.Do_Abi_MeasureOverride_2(IntPtr thisPtr, IntPtr context, Size availableSize, Size* result)

Note that the sample app also crashes if you go to the StaggeredLayout page (IF you are on ARM64 device).

Regression

No response

Reproducible in sample app?

  • This bug can be reproduced in the sample app.

Steps to reproduce

1. Build and run the sample app on ARM64.
2. Observe the crash.

Here's the link for the sample app: https://github.com/euju-ms/StaggeredLayoutArm64

The code basically uses the StaggeredLayout in ItemsView.

<ItemsView ...>

    <ItemsView.Layout>
        <tkControls:StaggeredLayout />
    </ItemsView.Layout>
</ItemsView>

Expected behavior

Expected to see three items show up instead of crashing.

Screenshots

Following three blue squares should show when it works; but using the newest StaggeredLayout crashes the app immediately.

image

Windows Build Number

  • Windows 10 1809 (Build 17763)
  • Windows 10 1903 (Build 18362)
  • Windows 10 1909 (Build 18363)
  • Windows 10 2004 (Build 19041)
  • Windows 10 20H2 (Build 19042)
  • Windows 10 21H1 (Build 19043)
  • Windows 11 21H2 (Build 22000)
  • Other (specify)

Other Windows Build number

Windows 11 24H2 (Build 26100.1742)

App minimum and target SDK version

  • Windows 10, version 1809 (Build 17763)
  • Windows 10, version 1903 (Build 18362)
  • Windows 10, version 1909 (Build 18363)
  • Windows 10, version 2004 (Build 19041)
  • Other (specify)

Other SDK version

No response

Visual Studio Version

2022

Visual Studio Build Number

Version 17.11.3

Device form factor

Desktop

Nuget packages

<PackageReference Include="Microsoft.Windows.SDK.BuildTools" Version="10.0.22621.756" />
<PackageReference Include="Microsoft.WindowsAppSDK" Version="1.6.240829007" />
<PackageReference Include="CommunityToolkit.WinUI.Controls.Primitives" Version="8.1.240916" />

Additional context

I set WindowsSdkPackageVersion of 10.0.22621.45 for WinAppSdk 1.6 to work (but I verified that the issue persists without this in WinAppSdk 1.5.240802000).

I have also verified that the StaggeredLayout from the old package CommunityToolkit.WinUI.UI.Controls version 7.1.2 works.

Help us help you

No.

@euju-ms
Copy link
Author

euju-ms commented Sep 24, 2024

Just updated the original post to say that the sample app crashes if you go to StaggeredLayout page from ARM64 device.

Here's the log message from Event Viewer

Faulting application name: CommunityToolkit.App.Uwp.exe, version: 0.0.0.0, time stamp: 0x66b3a419
Faulting module name: Windows.UI.Xaml.dll, version: 10.0.26100.1742, time stamp: 0x4d542ab4
Exception code: 0xc000027b
Fault offset: 0x000000000087dd8c
Faulting process id: 0x627C
Faulting application start time: 0x1DB0ED3BE9D2735
Faulting application path: C:\Program Files\WindowsApps\Microsoft.UWPCommunityToolkitSampleApp_8.1.0.0_arm64__8wekyb3d8bbwe\CommunityToolkit.App.Uwp.exe
Faulting module path: C:\Windows\System32\Windows.UI.Xaml.dll
Report Id: 5cdfe141-e798-4c5c-b463-7b7985bd673f
Faulting package full name: Microsoft.UWPCommunityToolkitSampleApp_8.1.0.0_arm64__8wekyb3d8bbwe
Faulting package-relative application ID: App

@Arlodotexe Arlodotexe transferred this issue from CommunityToolkit/WindowsCommunityToolkit Oct 31, 2024
@Arlodotexe Arlodotexe added bug Something isn't working regression What was working is now broke labels Nov 13, 2024
@Arlodotexe Arlodotexe moved this to 📋 Backlog in Toolkit 8.x Nov 13, 2024
@Arlodotexe
Copy link
Member

@michael-hawker any ideas here?

@michael-hawker
Copy link
Member

@Arlodotexe I think the only change here has been #371 and the update of the underlying WindowsAppSDK. PowerToys was seeing this as well: microsoft/PowerToys#35139 - reverting back to 8.0 - though they said they also saw it on x64. (The UWP sample app from the store is running for me though on my x64 box.)

I don't have immediate access to an ARM64 device though for testing. It's odd that it'd be architecture specific here.

@euju-ms can you try the latest 8.2-preview on NuGet we have and see if that changes anything? For the sample gallery, are you referring to the one directly from the store or did you build it locally? As the store build is UWP, which would indicate it's across framework and maybe something in our code then.

Otherwise, based on the error message pointing out arrays, and only one main code change happening, I would wonder if it has something to do with the change to this logic here from #371:

double columnWidth;
int numColumns;
if (ItemsStretch is StaggeredLayoutItemsStretch.None)
{
columnWidth = double.IsNaN(DesiredColumnWidth) ? availableWidth : Math.Min(DesiredColumnWidth, availableWidth);
numColumns = Math.Max(1, (int)Math.Floor(availableWidth / state.ColumnWidth));
}
else
{
if (double.IsNaN(DesiredColumnWidth) || DesiredColumnWidth > availableWidth)
{
columnWidth = availableWidth;
numColumns = 1;
}
else
{
var tempAvailableWidth = availableWidth + ColumnSpacing;
numColumns = (int)Math.Floor(tempAvailableWidth / DesiredColumnWidth);
columnWidth = tempAvailableWidth / numColumns - ColumnSpacing;
}
}

And it's downstream effect here:

var columnHeights = new double[numColumns];
var itemsPerColumn = new int[numColumns];
var deadColumns = new HashSet<int>();
for (int i = 0; i < context.ItemCount; i++)
{
var columnIndex = GetColumnIndex(columnHeights);
bool measured = false;
StaggeredItem item = state.GetItemAt(i);
if (item.Height == 0)
{
// Item has not been measured yet. Get the element and store the values
item.Element = context.GetOrCreateElementAt(i);
item.Element.Measure(new Size((float)state.ColumnWidth, (float)availableHeight));
item.Height = item.Element.DesiredSize.Height;
measured = true;
}
double spacing = itemsPerColumn[columnIndex] > 0 ? RowSpacing : 0;
item.Top = columnHeights[columnIndex] + spacing;
double bottom = item.Top + item.Height;
columnHeights[columnIndex] = bottom;
itemsPerColumn[columnIndex]++;
state.AddItemToColumn(item, columnIndex);
if (bottom < context.RealizationRect.Top)
{
// The bottom of the element is above the realization area
if (item.Element != null)
{
context.RecycleElement(item.Element);
item.Element = null;
}
}
else if (item.Top > context.RealizationRect.Bottom)
{
// The top of the element is below the realization area
if (item.Element != null)
{
context.RecycleElement(item.Element);
item.Element = null;
}
deadColumns.Add(columnIndex);
}
else if (measured == false)
{
// We ALWAYS want to measure an item that will be in the bounds
item.Element = context.GetOrCreateElementAt(i);
item.Element.Measure(new Size((float)state.ColumnWidth, (float)availableHeight));
if (item.Height != item.Element.DesiredSize.Height)
{
// this item changed size; we need to recalculate layout for everything after this
state.RemoveFromIndex(i + 1);
item.Height = item.Element.DesiredSize.Height;
columnHeights[columnIndex] = item.Top + item.Height;
}
}
if (deadColumns.Count == numColumns)
{
break;
}
}

But in the default case, as far as I can tell, this should behave the same between 8.0 and 8.1...

@Poker-sang you wouldn't happen to have an ARM64 device would you and able to assist in debugging this issue?

I was able to test on my x64 machine and see that things work fine for both UWP and Uno WASM here, though hitting a different AOT related issue on main now for testing WinUI 3 (with setting ItemsSource), so I'm trying to resolve that first to continue testing. See #517, CommunityToolkit/Tooling-Windows-Submodule#213, #516.

@michael-hawker
Copy link
Member

Updating to CSWinRT 2.1.6 allows me to run the StaggeredLayout sample on my x64 machine again, submitting a PR for that to the Tooling repo: CommunityToolkit/Tooling-Windows-Submodule#232

@Poker-sang
Copy link
Contributor

sorry i don't have an arm64 machine🤕

@Poker-sang
Copy link
Contributor

I am totally sorry. I mistakenly moved

numColumns = Math.Max(1, (int)Math.Floor(availableWidth / state.ColumnWidth)); 

before state.ColumnWidth's assginment. That's a stupid bug. And not always appears. I fixed it along with #574

@michael-hawker
Copy link
Member

@Poker-sang thanks for jumping in here and taking a look. 🦙❤️ I see that now, definitely easy to miss.

Thank you for submitting a fix so quickly. This explains the inconsistency we may have been seeing in reproducing the issue. We'll help sort out and review the PRs after the holiday weekend here next week and get them into the next preview for 8.2.

@euju-ms
Copy link
Author

euju-ms commented Dec 2, 2024

@michael-hawker

  1. I tried using CommunityToolkit.WinUI.Controls.Primitives 8.2.241112-preview1 in my sample test app, but it still crashes.
  2. I realize that I overused the term 'sample app', but to clarify: The Windows Community Toolkit Gallery app crashes when I go to the StaggeredLayout page. This is the store build :(

@michael-hawker
Copy link
Member

@euju-ms no worries, looks like we have a fix incoming, which we haven't merged yet. It should be in the next preview that we'll hopefully push up to NuGet later this week 🤞.

@github-project-automation github-project-automation bot moved this from 📋 Backlog to ✅ Done in Toolkit 8.x Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Priority-1 regression What was working is now broke
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants