Limit Yoga layout to relevant root on resize #10580
Conversation
rozele
force-pushed
the
resizeOptimization
branch
from
2d3854a to
195232b
Compare
rozele
force-pushed
the
resizeOptimization
branch
from
195232b to
af30530
Compare
|
Marking as draft until #10671 merges and we can rebase this PR. |
rozele
force-pushed
the
resizeOptimization
branch
from
af30530 to
2c8a5b0
Compare
Recalculation of Yoga layout due to root view size change does not require that we re-run layout on all roots. It also does not require that we invoke UpdateLayout on any new nodes (as a resize operation is independent of a batch of UIManager operations). This change limits the scope of Yoga calculation and application of Yoga results to XAML views to only the root view that changed size. This optimization primarily impacts apps with multiple windows, but can also benefit any React Native Windows app with multiple roots.
rozele
force-pushed
the
resizeOptimization
branch
from
2c8a5b0 to
dedbbac
Compare
| m_sizeChangedVector.push_back( | ||
| view.as<xaml::FrameworkElement>().SizeChanged(winrt::auto_revoke, [this](auto &&, auto &&) { DoLayout(); })); | ||
| m_sizeChangedVector.push_back(view.as<xaml::FrameworkElement>().SizeChanged( | ||
| winrt::auto_revoke, [this, rootTag](auto &&, xaml::SizeChangedEventArgs const &args) { |
There was a problem hiding this comment.
Is it safe to capture rootTag, or should it be a weak_ref? Is there a scenario where this thing gets torn down and we need to care about this SizeChanged event firing? Probably not?
I noticed that m_sizeChangedVector doesn't have the element removed in removeRootView. Preexisting, but... given that you're capturing a value now it made me wonder if that's all buttoned up.
| view.as<xaml::FrameworkElement>().SizeChanged(winrt::auto_revoke, [this](auto &&, auto &&) { DoLayout(); })); | ||
| m_sizeChangedVector.push_back(view.as<xaml::FrameworkElement>().SizeChanged( | ||
| winrt::auto_revoke, [this, rootTag](auto &&, xaml::SizeChangedEventArgs const &args) { | ||
| ApplyLayout(rootTag, args.NewSize().Width, args.NewSize().Height); |
There was a problem hiding this comment.
This is slightly different than what was there before. This takes the size right from the size changed event. The previous implementation (via DoLayout) would use the ActualSize of the XAML type. Are those for sure the same? Sounds like... sure? In which case this is fine. Just checking.
4 participants
Description
Type of Change
Erase all that don't apply.
Why
Recalculation of Yoga layout due to root view size change does not require that we re-run layout on all roots. It also does not require that we invoke UpdateLayout on any new nodes (as a resize operation is independent of a batch of UIManager operations).
What
This change limits the scope of Yoga calculation and application of Yoga results to XAML views to only the root view that changed size. This optimization primarily impacts apps with multiple windows, but can also benefit any React Native Windows app with multiple roots.
In a future PR, we could also track impacted roots in view manager operations by looking up the rootTag field on the ShadowNode, and only running Yoga layout on windows with actual updates in a given batch.
Testing
Ran RNTester, window resize still adapts correctly.
2022-09-14.15-30-18.mp4
Microsoft Reviewers: Open in CodeFlow