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

Many ImageEx controls causing Windows.UI.Xaml.LayoutCycleException #1328

Closed
darshanrampatel opened this issue Jul 20, 2017 · 34 comments
Closed

Comments

@darshanrampatel
Copy link
Contributor

darshanrampatel commented Jul 20, 2017

I have a ListView (containing around ~180 items) where the DataTemplate contains an ImageEx control:

<uwpcontrols:ImageEx
	Grid.Row="1"
	Grid.Column="0"
	Width="128"
	Height="128"
	HorizontalAlignment="Center"
	VerticalAlignment="Center"
	x:Phase="1"
	ImageExFailed="Image_ImageExFailed"
	Source="{x:Bind PictureURL_thumbnail, Mode=OneTime}" />

If I try and load these all at once, I get one of the below exceptions:

Exception Type:  System.Exception
Crashed Thread:  2

Application Specific Information:
Layout cycle detected.  Layout could not complete.
Layout cycle detected.  Layout could not complete.
Exception Type:  System.Exception
Crashed Thread:  3

Application Specific Information:
No installed components were detected.

Multiple animations in the same containing Storyboard cannot target the same property on a single element.

Changing the ImageEx control to a standard Image control doesn't cause this exception:

 <Image
	Grid.Row="1"
	Grid.Column="0"
	Width="128"
	Height="128"
	HorizontalAlignment="Center"
	VerticalAlignment="Center"
	x:Phase="1"
	ImageFailed="Image_ImageFailed"
	Source="{x:Bind PictureURL_thumbnail, Mode=OneTime}" />

Is there anything I can do to work around this apart from switching back to the Image control?

@Code-ScottLe
Copy link
Contributor

ListView will use virtualization on the item container (ItemsStackPanel to be exact), so it is definitely not loading up all 180+ items at the same time. Could you post a project that we can reproduce?

cc @hermitdave .

@avknaidu
Copy link
Member

@Code-ScottLe @hermitdave @darshanrampatel My stack overflow app loads around 30 users per page and it has no issue. Maybe it is something related to number of ListViewItems? Can you try decreasing the loading count and see if it still crashes?

@hermitdave
Copy link
Contributor

Can you post a repro?

@darshanrampatel
Copy link
Contributor Author

darshanrampatel commented Jul 22, 2017 via email

@darshanrampatel
Copy link
Contributor Author

@hermitdave @Code-ScottLe @avknaidu

Repro solution

If you uncomment one of the below Image or ImageEx controls in MainPage.xaml, you should see the same behaviour as me:

<DataTemplate x:DataType="vm:Picture">
	<Grid>
		<TextBlock Text="{x:Bind ID}" />
		<!--<Image
			Width="128"
			Height="128"
			HorizontalAlignment="Center"
			VerticalAlignment="Center"
			x:Phase="1"
			ImageFailed="Image_ImageFailed"
			Source="{x:Bind URL}" />-->
		<!--<uwpcontrols:ImageEx
			Width="128"
			Height="128"
			HorizontalAlignment="Center"
			VerticalAlignment="Center"
			x:Phase="1"
			ImageExFailed="Image_ImageExFailed"
			Source="{x:Bind URL}" />-->
	</Grid>
</DataTemplate>

@Code-ScottLe
Copy link
Contributor

Few things, your project above doesn't have enough stuffs (missing base Microsoft.Toolkit.Uwp and Microsoft.Toolkit.Uwp.UI).

And I can run your project no problem

@hermitdave
Copy link
Contributor

@darshanrampatel how did you add a reference to the controls lib? Nuget is good because it automatically downloads dependencies

@darshanrampatel
Copy link
Contributor Author

darshanrampatel commented Jul 23, 2017 via email

@darshanrampatel
Copy link
Contributor Author

darshanrampatel commented Jul 23, 2017 via email

@Code-ScottLe
Copy link
Contributor

I only minimized this so I can show you how long it was running. Showing no problem at all.

@darshanrampatel
Copy link
Contributor Author

@Code-ScottLe Hmm, I always get a crash instantly.

See this video: ImageExIssue.zip

@Code-ScottLe
Copy link
Contributor

I can't playback your video for some reason. Make sure you have all of the packages. Even the Animation one. Some of the controls does use some of the animations from our own toolkit.

@hermitdave
Copy link
Contributor

I'll take a look in a few minutes

@hermitdave
Copy link
Contributor

I can't seem to reproduce the issue. I can maximize and restore back to original size without any issues.

@hermitdave
Copy link
Contributor

@darshanrampatel can you please try and run the sample app (download from store) and see if you have the same issue ?
I use ImageEx extensively and whilst I haven't upgraded to 1.5.1, there have been no significant changes for a long time.

@darshanrampatel
Copy link
Contributor Author

darshanrampatel commented Jul 25, 2017

@hermitdave I can run the store app with no issue. I am also using ImageEx in my apps extensively and, just so it's clear, this is only an issue when many ImageEx controls on the same page are all being loaded at the same time.

Just to confirm, I am maximising the repro app before I press the refresh button so that the maximum number of ImageEx controls load at the same time.

I have tested this and I can load 120 ImageEx controls (I am changing the number of iterations here) but not 125.

How many ImageEx controls fit on your maximised screen at once?

@hermitdave
Copy link
Contributor

@darshanrampatel my daily mail online app has around 400 imageex controls on channel pages. I have a simplified template to not include progress ring.

@darshanrampatel
Copy link
Contributor Author

darshanrampatel commented Jul 26, 2017

@hermitdave Interesting.

If I adjust the template to not include the progress ring in the repro app, I don't get any exceptions and can load 200+ controls at once.

@Code-ScottLe
Copy link
Contributor

So if you don't have the progress ring, you can load more stuffs? That is a little bit odd.

@hermitdave
Copy link
Contributor

@darshanrampatel progress ring itself has tons of animations and lots of those would imply UI thread load.

@hermitdave
Copy link
Contributor

@Code-ScottLe not really.. Less stress on UI thread..

@Code-ScottLe
Copy link
Contributor

@hermitdave But shouldn't the ListView would just virtualize the entire list? Unless all 200+ items are tiny enough to be displayed all at once.

@hermitdave
Copy link
Contributor

@Code-ScottLe in this instance its a grid view and default virtualization would be 4 pages + current which mean lots of images in this instance.. 128x128

@Code-ScottLe
Copy link
Contributor

@hermitdave I guess that could reduce the amount of time spending on the UI thread for optimization purpose. Not sure how that is breaking @darshanrampatel in this case with the exception though.

@darshanrampatel can you try your project with reference to the actual toolkit project? It might help you debug this further.

@darshanrampatel
Copy link
Contributor Author

@Code-ScottLe @hermitdave I was playing around with the ImageEx.xaml and came up with a way that I could still keep the ProgressRing but not see any crashes.

All I did was add the Visibility property to the ProgressRing and then set in to Visible/Collapsed as appropariate in the Visual States. According to the documentation for ProgressRing:

If IsActive is false, the ProgressRing is not shown, but space is reserved for it in the UI layout. To not reserve space for the ProgressRing, set its Visibility property to Collapsed.

I can't see any downsides to doing this.

Can you guys please test this? I have update the repro project with this new style.

    <Style TargetType="controls:ImageEx">
        <Setter Property="Background" Value="Transparent" />
        <Setter Property="Foreground" Value="{ThemeResource ApplicationForegroundThemeBrush}" />
        <Setter Property="Template">
            <Setter.Value>
                <ControlTemplate TargetType="controls:ImageEx">
                    <Grid Background="{TemplateBinding Background}">
                        <Image
                            Name="PlaceholderImage"
                            HorizontalAlignment="{TemplateBinding HorizontalAlignment}"
                            VerticalAlignment="{TemplateBinding VerticalAlignment}"
                            Opacity="1.0"
                            Source="{TemplateBinding PlaceholderSource}"
                            Stretch="{TemplateBinding PlaceholderStretch}" />
                        <Image
                            Name="Image"
                            HorizontalAlignment="{TemplateBinding HorizontalAlignment}"
                            VerticalAlignment="{TemplateBinding VerticalAlignment}"
                            NineGrid="{TemplateBinding NineGrid}"
                            Opacity="0.0"
                            Stretch="{TemplateBinding Stretch}" />
                        <ProgressRing
                            Name="Progress"
                            Margin="16"
                            HorizontalAlignment="Center"
                            VerticalAlignment="Center"
                            Background="Transparent"
                            Foreground="{TemplateBinding Foreground}"
                            IsActive="False"
                            Visibility="Collapsed" />
                        <VisualStateManager.VisualStateGroups>
                            <VisualStateGroup x:Name="CommonStates">
                                <VisualState x:Name="Failed">
                                    <Storyboard>
                                        <ObjectAnimationUsingKeyFrames Storyboard.TargetName="Image" Storyboard.TargetProperty="Opacity">
                                            <DiscreteObjectKeyFrame KeyTime="0" Value="0" />
                                        </ObjectAnimationUsingKeyFrames>
                                        <ObjectAnimationUsingKeyFrames Storyboard.TargetName="PlaceholderImage" Storyboard.TargetProperty="Opacity">
                                            <DiscreteObjectKeyFrame KeyTime="0" Value="1" />
                                        </ObjectAnimationUsingKeyFrames>
                                    </Storyboard>
                                </VisualState>
                                <VisualState x:Name="Loading">
                                    <Storyboard>
                                        <ObjectAnimationUsingKeyFrames Storyboard.TargetName="Progress" Storyboard.TargetProperty="IsActive">
                                            <DiscreteObjectKeyFrame KeyTime="0" Value="True" />
                                        </ObjectAnimationUsingKeyFrames>
                                        <ObjectAnimationUsingKeyFrames Storyboard.TargetName="Progress" Storyboard.TargetProperty="Visibility">
                                            <DiscreteObjectKeyFrame KeyTime="0" Value="Visible" />
                                        </ObjectAnimationUsingKeyFrames>
                                        <ObjectAnimationUsingKeyFrames Storyboard.TargetName="Image" Storyboard.TargetProperty="Opacity">
                                            <DiscreteObjectKeyFrame KeyTime="0" Value="0" />
                                        </ObjectAnimationUsingKeyFrames>
                                        <ObjectAnimationUsingKeyFrames Storyboard.TargetName="PlaceholderImage" Storyboard.TargetProperty="Opacity">
                                            <DiscreteObjectKeyFrame KeyTime="0" Value="1" />
                                        </ObjectAnimationUsingKeyFrames>
                                    </Storyboard>
                                </VisualState>
                                <VisualState x:Name="Loaded">
                                    <Storyboard>
                                        <ObjectAnimationUsingKeyFrames Storyboard.TargetName="Progress" Storyboard.TargetProperty="IsActive">
                                            <DiscreteObjectKeyFrame KeyTime="0" Value="False" />
                                        </ObjectAnimationUsingKeyFrames>
                                        <ObjectAnimationUsingKeyFrames Storyboard.TargetName="Progress" Storyboard.TargetProperty="Visibility">
                                            <DiscreteObjectKeyFrame KeyTime="0" Value="Collapsed" />
                                        </ObjectAnimationUsingKeyFrames>
                                        <DoubleAnimation
                                            AutoReverse="False"
                                            BeginTime="0"
                                            Storyboard.TargetName="Image"
                                            Storyboard.TargetProperty="Opacity"
                                            From="0"
                                            To="1" />
                                        <DoubleAnimation
                                            AutoReverse="False"
                                            BeginTime="0"
                                            Storyboard.TargetName="PlaceholderImage"
                                            Storyboard.TargetProperty="Opacity"
                                            From="1"
                                            To="0" />
                                    </Storyboard>
                                </VisualState>
                                <VisualState x:Name="Unloaded" />
                            </VisualStateGroup>
                        </VisualStateManager.VisualStateGroups>
                    </Grid>
                </ControlTemplate>
            </Setter.Value>
        </Setter>
    </Style>

@Swifter
Copy link

Swifter commented Sep 15, 2017

I have a project wherein I use an ImageEx control inside an item template and when this template is used in a FlipView with many ImageEx controls on a page, they never load when IsCacheEnabled="True". Removing this makes them work again, but this was one of the reasons I wanted to use ImageEx. Boo.

I tried the style template above, hoping it might fix the issue but of course it did not. Oh, yes, before you ask, I did enable the cache. :)

@hermitdave
Copy link
Contributor

@Swifter could you please post a repro ?

@Swifter
Copy link

Swifter commented Sep 15, 2017

@hermitdave This is a HUGE project, but I will try to make a little one for you.

@darshanrampatel
Copy link
Contributor Author

@Swifter I use ImageEx controls within a FlipView as well and have no issues with the IsCacheEnabled property. My ViewModel.Pictures list contains over 200+ pictures.

 <FlipView
            x:Name="PicturesFlipView"
            Background="Transparent"
            ItemsSource="{x:Bind ViewModel.Pictures, Mode=OneWay}"
            SelectedItem="{Binding SelectedItem, Mode=TwoWay}"
            SizeChanged="PicturesFlipView_SizeChanged">
            <FlipView.ItemTemplate>
                <DataTemplate x:DataType="models:PicturesToPass">
                    <Grid>
                        <ScrollViewer
                            HorizontalScrollBarVisibility="Auto"
                            MaxZoomFactor="3"
                            MinZoomFactor="1"
                            VerticalScrollBarVisibility="Auto"                          
                            ZoomMode="Enabled">
                            <Viewbox
                                MaxWidth="{Binding DataContext.DialogWidth, ElementName=PicturesGrid, Mode=OneWay}"
                                MaxHeight="{Binding DataContext.DialogHeight, ElementName=PicturesGrid, Mode=OneWay}"
                                StretchDirection="DownOnly">
                                <uwpcontrols:ImageEx IsCacheEnabled="True"
                                    DoubleTapped="ImageEx_DoubleTapped"
                                    ImageExFailed="ImageEx_ImageExFailed"
                                    Source="{x:Bind URL, Mode=OneTime}"
                                    Stretch="Uniform" />
                            </Viewbox>
                        </ScrollViewer>                       
                    </Grid>
                </DataTemplate>
            </FlipView.ItemTemplate>
        </FlipView>

@windowstoolkitbot
Copy link

This issue seems inactive. It will automatically be closed in 14 days if there is no activity.

@windowstoolkitbot
Copy link

This issue seems inactive. It will automatically be closed in 7 days if there is no activity.

@darshanrampatel
Copy link
Contributor Author

@hermitdave @Code-ScottLe Do you want to make a PR to adjust the template to add the Visibility property changing in the VisualStates? I can't see any negative impact from this.

@hermitdave
Copy link
Contributor

I'll have a go when i get back to UK.. Getting little computer time while on holiday

@nmetulev nmetulev added this to the v2.1 milestone Oct 27, 2017
hermitdave added a commit that referenced this issue Oct 31, 2017
Setting Visibility of ProgressRing to Collapsed by default.
Modify VisualState to make ProgressRing Visible when Loading.
Removed un-necessary modification in Loaded state.
@nmetulev
Copy link
Contributor

PR merged

@nmetulev nmetulev closed this as completed Nov 2, 2017
@ghost ghost locked as resolved and limited conversation to collaborators Nov 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants