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

Rework how TextBlock skips redundant measure and arrange calls #17271

Merged
merged 12 commits into from
Oct 21, 2024

Conversation

Gillibald
Copy link
Contributor

@Gillibald Gillibald commented Oct 14, 2024

What does the pull request do?

This PR attempts to improve the logic that skips the creation of new TextLayouts when Measure or Arrange is called.

Previously we only tested for smaller widths. This PR tests for changes to the constraining size in general.

What is the current behavior?

What is the updated/expected behavior with this PR?

How was the solution implemented (if it's not obvious)?

Checklist

Breaking changes

Obsoletions / Deprecations

Fixed issues

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.3.999-cibuild0052636-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.3.999-cibuild0052638-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.3.999-cibuild0052710-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@Gillibald Gillibald changed the title [WIP] Rework how TextBlock skips redundant measure and arrange calls Rework how TextBlock skips redundant measure and arrange calls Oct 17, 2024
@Gillibald Gillibald added backport-candidate-11.2.x Consider this PR for backporting to 11.2 branch customer-priority Issue reported by a customer with a support agreement. labels Oct 17, 2024
Copy link
Member

@MrJul MrJul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@MrJul MrJul enabled auto-merge October 21, 2024 08:36
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.3.999-cibuild0052737-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@MrJul MrJul added this pull request to the merge queue Oct 21, 2024
Merged via the queue into AvaloniaUI:master with commit 82b048d Oct 21, 2024
10 checks passed
@Gillibald Gillibald deleted the fixes/textBlockMeasureArrange branch October 21, 2024 12:00
maxkatz6 pushed a commit that referenced this pull request Oct 27, 2024
* Rework how TextBlock skips redundant measure and arrange calls
Add some tests

* Adjust tests

* Try this

* Make sure the TextBlock is arranged after it has been measured with a different availableSize

* Make it more clear that we are resetting and recreating the TextLayout

* Capture textLayout after inlines have been processed
@maxkatz6 maxkatz6 added backported-11.2.x and removed backport-candidate-11.2.x Consider this PR for backporting to 11.2 branch labels Oct 27, 2024
@Gillibald Gillibald added the backport-candidate-11.1.x Consider this PR for backporting to 11.1 branch label Nov 1, 2024
@adirh3
Copy link
Contributor

adirh3 commented Nov 3, 2024

@Gillibald FYI since this PR I am getting crashes on InvalidOperationException with message Invalid size returned for Measure. in Layoutable line 387.
I can't properly reproduce it, it happens randomly when I show my window when a TextBlock with opacity=0.5 cause a re-render of it's parent DockPanel (that is inside a ListBox) which contains another TextBlock with FontSize="{DynamicBinding IconSize}" and during Measure have FontSize is set to NaN which cause it to crash.

Stack -

Application: FluentSearch.exe
CoreCLR Version: 8.0.1024.46610
.NET Version: 8.0.10
Description: The process was terminated due to an unhandled exception.
Exception Info: System.InvalidOperationException: Invalid size returned for Measure.
   at Avalonia.Layout.Layoutable.Measure(Size availableSize)
   at Avalonia.Layout.Layoutable.MeasureOverride(Size availableSize)
   at Avalonia.Layout.Layoutable.MeasureCore(Size availableSize)
   at Avalonia.Layout.Layoutable.Measure(Size availableSize)
   at Avalonia.Layout.Layoutable.MeasureOverride(Size availableSize)
   at Avalonia.Layout.Layoutable.MeasureCore(Size availableSize)
   at Avalonia.Layout.Layoutable.Measure(Size availableSize)
   at Avalonia.Controls.DockPanel.MeasureOverride(Size constraint)
   at Avalonia.Layout.Layoutable.MeasureCore(Size availableSize)
   at Avalonia.Layout.Layoutable.Measure(Size availableSize)
   at Avalonia.Layout.LayoutManager.Measure(Layoutable control)
   at Avalonia.Layout.LayoutManager.Measure(Layoutable control)
   at Avalonia.Layout.LayoutManager.Measure(Layoutable control)
   at Avalonia.Layout.LayoutManager.Measure(Layoutable control)
   at Avalonia.Layout.LayoutManager.ExecuteLayoutPass()
   at Avalonia.Media.MediaContext.FireInvokeOnRenderCallbacks()
   at Avalonia.Media.MediaContext.RenderCore()
   at Avalonia.Media.MediaContext.Render()
   at Avalonia.Threading.DispatcherOperation.InvokeCore()
   at Avalonia.Threading.DispatcherOperation.Execute()
   at Avalonia.Threading.Dispatcher.ExecuteJob(DispatcherOperation job)
   at Avalonia.Threading.Dispatcher.ExecuteJobsCore(Boolean fromExplicitBackgroundProcessingCallback)
   at Avalonia.Threading.Dispatcher.Signaled()
   at Avalonia.Win32.Win32Platform.WndProc(IntPtr hWnd, UInt32 msg, IntPtr wParam, IntPtr lParam)
   at Avalonia.Win32.Interop.UnmanagedMethods.DispatchMessage(MSG& lpmsg)
   at Avalonia.Win32.Win32DispatcherImpl.RunLoop(CancellationToken cancellationToken)
   at Avalonia.Threading.DispatcherFrame.Run(IControlledDispatcherImpl impl)
   at Avalonia.Threading.Dispatcher.PushFrame(DispatcherFrame frame)
   at Avalonia.Threading.Dispatcher.MainLoop(CancellationToken cancellationToken)
   at Avalonia.Controls.ApplicationLifetimes.ClassicDesktopStyleApplicationLifetime.StartCore(String[] args)
   at Avalonia.Controls.ApplicationLifetimes.ClassicDesktopStyleApplicationLifetime.Start(String[] args)
   at Blast.Program.Main(String[] args) in C:\Projects\Blast\Blast\Program.cs:line 364

@Gillibald
Copy link
Contributor Author

We need to cover this in a unit tests that sets the FontSize to NaN

@adirh3
Copy link
Contributor

adirh3 commented Nov 3, 2024

We need to cover this in a unit tests that sets the FontSize to NaN

So you know how to repro this? If not I can try make something or capture process dump if it helps

@adirh3
Copy link
Contributor

adirh3 commented Nov 4, 2024

@Gillibald if it helps, it seems like the control is not even attached to visual tree.
But I don't understand why it's being measure, I caught it during debug -
image
It crashes internally in Measure when the DesiredSize is invalid, due to it not being attached -> bindings are not working.

Gillibald added a commit to Gillibald/Avalonia that referenced this pull request Nov 14, 2024
…niaUI#17271)

* Rework how TextBlock skips redundant measure and arrange calls
Add some tests

* Adjust tests

* Try this

* Make sure the TextBlock is arranged after it has been measured with a different availableSize

* Make it more clear that we are resetting and recreating the TextLayout

* Capture textLayout after inlines have been processed
@grokys grokys added backported-11.1.x and removed backport-candidate-11.1.x Consider this PR for backporting to 11.1 branch labels Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants