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.

Sign up for GitHub

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

Jump to bottom

[iOS] Improve background layer frame mapping performance #24848

Merged
merged 10 commits into from
Dec 10, 2024
Merged

[iOS] Improve background layer frame mapping performance #24848

merged 10 commits into from
Dec 10, 2024

Conversation

albyrock87
Copy link
Contributor

@albyrock87 albyrock87 commented Sep 20, 2024 •
edited by jfversluis
Loading

Important

This PR Obsoletes a couple of methods.

static Microsoft.Maui.Controls.Platform.BrushExtensions.UpdateBackgroundLayer(this UIKit.UIView view) -> void
static Microsoft.Maui.Platform.ViewExtensions.UpdateBackgroundLayerFrame(this UIKit.UIView! view) -> void

Description of Change

Using https://github.com/davidortinau/AllTheLists app, and scrolling through collection view with Border inside we can clearly see that a good amount of time is spent in UpdateBackgroundLayerFrame which is needed to sync the sublayer with the UIView.Layer.

image

This has an impact on both MappingFrame and ContentView.LayoutSubviews:
image

Which translated to these timings
image
image

This PR gets rid of all mapping frame code and simply adds an observer on the sublayer.

As a result we get a lot of improvement (80%):
image
image

While the new observer is super fast
image

Issues Fixed

Fixes #24847
Fixes #26057
Fixes #20218
Closes #26222

@albyrock87 albyrock87 requested a review from a team as a code owner September 20, 2024 14:56
@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Sep 20, 2024
@PureWeen
Copy link
Member

PureWeen commented Sep 20, 2024 •
edited by jfversluis
Loading

/azp run

@azure-pipelines Azure Pipelines

This comment was marked as outdated.

Sign in to view
@albyrock87
Copy link
Contributor Author

Failing tests definitely unrelated :)

rmarinho

This comment was marked as outdated.

Sign in to view

@PureWeen

This comment was marked as outdated.

Sign in to view
@albyrock87

This comment was marked as outdated.

Sign in to view
@albyrock87 albyrock87 changed the base branch from main to net9.0 September 25, 2024 12:51
@albyrock87 albyrock87 force-pushed the sync-sublayer-frame-differently branch from 8947b0b to 9d24253 Compare September 25, 2024 12:51
@albyrock87

This comment was marked as outdated.

Sign in to view
@rmarinho

This comment was marked as outdated.

Sign in to view
@azure-pipelines Azure Pipelines

This comment was marked as outdated.

Sign in to view
@rmarinho rmarinho self-requested a review September 26, 2024 13:28
@rmarinho rmarinho added this to the .NET 9 SR1 milestone Sep 27, 2024
@PureWeen

This comment was marked as outdated.

Sign in to view
@github-actions github-actions bot force-pushed the sync-sublayer-frame-differently branch from 9d24253 to b235d2a Compare October 3, 2024 22:08
@PureWeen

This comment was marked as outdated.

Sign in to view
@azure-pipelines Azure Pipelines

This comment was marked as outdated.

Sign in to view
@PureWeen

This comment was marked as outdated.

Sign in to view
@github-actions github-actions bot force-pushed the sync-sublayer-frame-differently branch from b235d2a to 9eae070 Compare October 10, 2024 17:10
@PureWeen

This comment was marked as outdated.

Sign in to view
@azure-pipelines Azure Pipelines

This comment was marked as outdated.

Sign in to view
@rmarinho rmarinho changed the base branch from net9.0 to main October 19, 2024 23:55
@PureWeen PureWeen removed this from the .NET 9 SR1 milestone Oct 23, 2024
@azure-pipelines Azure Pipelines

This comment was marked as outdated.

Sign in to view
@albyrock87
Copy link
Contributor Author

Failed test is unrelated.

@albyrock87 albyrock87 mentioned this pull request Dec 2, 2024
Closed
4 tasks
@albyrock87
Copy link
Contributor Author

I've included @Tamilarasan-Paranthaman UI test from #26222

@PureWeen PureWeen modified the milestones: .NET 9 SR2, .NET 9 SR3 Dec 6, 2024
@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

PureWeen commented Dec 9, 2024

/rebase

@PureWeen PureWeen assigned PureWeen and jsuarezruiz and unassigned jsuarezruiz Dec 9, 2024
@jfversluis jfversluis added the area-drawing Shapes, Borders, Shadows, Graphics, BoxView, custom drawing label Dec 10, 2024
@PureWeen PureWeen requested a review from Copilot December 10, 2024 15:53
Copilot
Copilot AI reviewed Dec 10, 2024
View reviewed changes

Choose a reason for hiding this comment

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

Copilot reviewed 13 out of 28 changed files in this pull request and generated no suggestions.

Files not reviewed (15)
  • src/Controls/tests/TestCases.HostApp/Issues/Issue26057.xaml: Language not supported
  • src/Compatibility/Core/src/iOS/Renderers/FrameRenderer.cs: Evaluated as low risk
  • src/Compatibility/Core/src/iOS/Renderers/PageRenderer.cs: Evaluated as low risk
  • src/Compatibility/Core/src/iOS/VisualElementRenderer.cs: Evaluated as low risk
  • src/Controls/src/Core/Compatibility/Handlers/iOS/FrameRenderer.cs: Evaluated as low risk
  • src/Controls/src/Core/Platform/iOS/Extensions/BrushExtensions.cs: Evaluated as low risk
  • src/Controls/tests/TestCases.HostApp/Issues/Issue24847.cs: Evaluated as low risk
  • src/Controls/tests/TestCases.HostApp/Issues/Issue26057.xaml.cs: Evaluated as low risk
  • src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue24847.cs: Evaluated as low risk
  • src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue26057.cs: Evaluated as low risk
  • src/Core/src/Handlers/View/ViewHandler.iOS.cs: Evaluated as low risk
  • src/Core/src/Platform/iOS/CALayerAutosizeObserver.cs: Evaluated as low risk
  • src/Core/src/Platform/iOS/ContentView.cs: Evaluated as low risk
  • src/Core/src/Platform/iOS/IAutoSizableCALayer.cs: Evaluated as low risk
  • src/Core/src/Platform/iOS/LayerExtensions.cs: Evaluated as low risk
@albyrock87
Revert compatibility changes
f00bfe9
@albyrock87
Merge branch 'main' into sync-sublayer-frame-differently
8a9c659 
# Conflicts:
#	src/Core/src/PublicAPI/net-ios/PublicAPI.Unshipped.txt
#	src/Core/src/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt
@PureWeen
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen
Copy link
Member

  • failing test on iOS/Catalyst is a known issue on main and not related to this PR

@PureWeen PureWeen dismissed stale reviews from rmarinho and jsuarezruiz December 10, 2024 22:14

Changes applied

@PureWeen PureWeen merged commit ffb0db2 into dotnet:main Dec 10, 2024
101 of 104 checks passed
@albyrock87 albyrock87 deleted the sync-sublayer-frame-differently branch December 10, 2024 22:45
@samhouts samhouts added fixed-in-net9.0-nightly This may be available in a nightly release! fixed-in-net8.0-nightly This may be available in a nightly release! labels Dec 16, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jan 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

Copilot code review Copilot Copilot left review comments

@PureWeen PureWeen PureWeen approved these changes

@jfversluis jfversluis Awaiting requested review from jfversluis jfversluis is a code owner automatically assigned from dotnet/dotnet-maui-reviewers

@tj-devel709 tj-devel709 Awaiting requested review from tj-devel709 tj-devel709 is a code owner automatically assigned from dotnet/dotnet-maui-reviewers

@rmarinho rmarinho Awaiting requested review from rmarinho

@jsuarezruiz jsuarezruiz Awaiting requested review from jsuarezruiz

Assignees

@PureWeen PureWeen

@jsuarezruiz jsuarezruiz

Labels
area-drawing Shapes, Borders, Shadows, Graphics, BoxView, custom drawing community ✨ Community Contribution fixed-in-net8.0-nightly This may be available in a nightly release! fixed-in-net9.0-nightly This may be available in a nightly release! p/1 Work that is important, and has been scheduled for release in this or an upcoming sprint platform/iOS 🍎 t/perf The issue affects performance (runtime speed, memory usage, startup time, etc.) (sub: perf)
Projects
MAUI SDK Ongoing
Status: Done
Milestone
.NET 9 SR3
7 participants
@albyrock87 @PureWeen @rmarinho @jsuarezruiz @jfversluis @samhouts @Tamilarasan-Paranthaman