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

Skip useless handler mappings calls while connecting #27259

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion src/Controls/src/Core/Label/Label.Mapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public partial class Label

// these are really a single property
LabelHandler.Mapper.ReplaceMapping<Label, ILabelHandler>(nameof(Text), MapText);
LabelHandler.Mapper.ReplaceMapping<Label, ILabelHandler>(nameof(FormattedText), MapText);
LabelHandler.Mapper.ReplaceMapping<Label, ILabelHandler>(nameof(FormattedText), MapFormattedText);

LabelHandler.Mapper.ReplaceMapping<Label, ILabelHandler>(nameof(LineBreakMode), MapLineBreakMode);
LabelHandler.Mapper.ReplaceMapping<Label, ILabelHandler>(nameof(MaxLines), MapMaxLines);
Expand Down Expand Up @@ -54,8 +54,17 @@ public static void MapTextType(ILabelHandler handler, Label label) =>
MapTextOrFormattedText(handler, label);
static void MapTextTransform(ILabelHandler handler, Label label) =>
MapTextOrFormattedText(handler, label);
static void MapFormattedText(ILabelHandler handler, Label label)
{
if (label.IsConnectingHandler()) return;

MapText(handler, label);
}

static void MapTextOrFormattedText(ILabelHandler handler, Label label)
{
if (label.IsConnectingHandler()) return;

if (label.HasFormattedTextSpans)
handler.UpdateValue(nameof(FormattedText));
else
Expand Down
32 changes: 32 additions & 0 deletions src/Core/src/Core/Extensions/InternalElementExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
using System.Collections.Generic;
using System.Linq;

namespace Microsoft.Maui
{
internal static class InternalElementExtensions
{
/// <summary>
/// The handler is connecting for the first time to the element and mapping all properties.
/// </summary>
/// <param name="element"></param>
/// <returns></returns>
internal static bool IsMappingProperties(this IElement element) =>
(element.Handler as IElementHandlerStateExhibitor)?.State.HasFlag(ElementHandlerState.MappingProperties) ?? false;

/// <summary>
/// Indicates whether the handler is connecting for the first time to the element and mapping all properties.
/// </summary>
/// <param name="element"></param>
/// <returns></returns>
internal static bool IsConnectingHandler(this IElement element) =>
(element.Handler as IElementHandlerStateExhibitor)?.State.HasFlag(ElementHandlerState.Connecting) ?? false;

/// <summary>
/// Indicates whether the connected handler is now connecting to a new element and updating properties.
/// </summary>
/// <param name="element"></param>
/// <returns></returns>
internal static bool IsReconnectingHandler(this IElement element) =>
(element.Handler as IElementHandlerStateExhibitor)?.State.HasFlag(ElementHandlerState.Reconnecting) ?? false;
}
}
26 changes: 26 additions & 0 deletions src/Core/src/Handlers/Border/BorderHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,25 @@ public static void MapBackground(IBorderHandler handler, IBorderView border)
((PlatformView?)handler.PlatformView)?.UpdateBackground(border);
}

private static bool ShouldSkipStrokeMappings(IBorderHandler handler) {
#if __IOS__ || MACCATALYST || ANDROID
// During the initial connection, the `MapBackground` takes care of updating the stroke properties
// so we can skip the stroke mappings to avoid repetitive and useless updates.
return handler.IsConnectingHandler();
#else
return false;
#endif
}

/// <summary>
/// Maps the abstract <see cref="IBorderStroke.Shape"/> property to the platform-specific implementations.
/// </summary>
/// <param name="handler">The associated handler.</param>
/// <param name="border">The associated <see cref="IBorderView"/> instance.</param>
public static void MapStrokeShape(IBorderHandler handler, IBorderView border)
{
if (ShouldSkipStrokeMappings(handler)) return;
Copy link
Contributor

@MartyIX MartyIX Jan 24, 2025

Choose a reason for hiding this comment

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

Nit but this makes putting breakpoints easier:

Suggested change
if (ShouldSkipStrokeMappings(handler)) return;
if (ShouldSkipStrokeMappings(handler))
{
return;
}


((PlatformView?)handler.PlatformView)?.UpdateStrokeShape(border);
MapBackground(handler, border);
}
Expand All @@ -104,6 +116,8 @@ public static void MapStrokeShape(IBorderHandler handler, IBorderView border)
/// <param name="border">The associated <see cref="IBorderView"/> instance.</param>
public static void MapStroke(IBorderHandler handler, IBorderView border)
{
if (ShouldSkipStrokeMappings(handler)) return;

((PlatformView?)handler.PlatformView)?.UpdateStroke(border);
MapBackground(handler, border);
}
Expand All @@ -115,6 +129,8 @@ public static void MapStroke(IBorderHandler handler, IBorderView border)
/// <param name="border">The associated <see cref="IBorderView"/> instance.</param>
public static void MapStrokeThickness(IBorderHandler handler, IBorderView border)
{
if (ShouldSkipStrokeMappings(handler)) return;

((PlatformView?)handler.PlatformView)?.UpdateStrokeThickness(border);
MapBackground(handler, border);
}
Expand All @@ -126,6 +142,8 @@ public static void MapStrokeThickness(IBorderHandler handler, IBorderView border
/// <param name="border">The associated <see cref="IBorderView"/> instance.</param>
public static void MapStrokeLineCap(IBorderHandler handler, IBorderView border)
{
if (ShouldSkipStrokeMappings(handler)) return;

((PlatformView?)handler.PlatformView)?.UpdateStrokeLineCap(border);
}

Expand All @@ -136,6 +154,8 @@ public static void MapStrokeLineCap(IBorderHandler handler, IBorderView border)
/// <param name="border">The associated <see cref="IBorderView"/> instance.</param>
public static void MapStrokeLineJoin(IBorderHandler handler, IBorderView border)
{
if (ShouldSkipStrokeMappings(handler)) return;

((PlatformView?)handler.PlatformView)?.UpdateStrokeLineJoin(border);
}

Expand All @@ -146,6 +166,8 @@ public static void MapStrokeLineJoin(IBorderHandler handler, IBorderView border)
/// <param name="border">The associated <see cref="IBorderView"/> instance.</param>
public static void MapStrokeDashPattern(IBorderHandler handler, IBorderView border)
{
if (ShouldSkipStrokeMappings(handler)) return;

((PlatformView?)handler.PlatformView)?.UpdateStrokeDashPattern(border);
}

Expand All @@ -156,6 +178,8 @@ public static void MapStrokeDashPattern(IBorderHandler handler, IBorderView bord
/// <param name="border">The associated <see cref="IBorderView"/> instance.</param>
public static void MapStrokeDashOffset(IBorderHandler handler, IBorderView border)
{
if (ShouldSkipStrokeMappings(handler)) return;

((PlatformView?)handler.PlatformView)?.UpdateStrokeDashOffset(border);
}

Expand All @@ -166,6 +190,8 @@ public static void MapStrokeDashOffset(IBorderHandler handler, IBorderView borde
/// <param name="border">The associated <see cref="IBorderView"/> instance.</param>
public static void MapStrokeMiterLimit(IBorderHandler handler, IBorderView border)
{
if (ShouldSkipStrokeMappings(handler)) return;

((PlatformView?)handler.PlatformView)?.UpdateStrokeMiterLimit(border);
}

Expand Down
25 changes: 23 additions & 2 deletions src/Core/src/Handlers/Element/ElementHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

namespace Microsoft.Maui.Handlers
{
public abstract partial class ElementHandler : IElementHandler
public abstract partial class ElementHandler : IElementHandler, IElementHandlerStateExhibitor
{
public static IPropertyMapper<IElement, IElementHandler> ElementMapper = new PropertyMapper<IElement, IElementHandler>()
{
Expand All @@ -15,6 +15,9 @@ public abstract partial class ElementHandler : IElementHandler
internal readonly IPropertyMapper _defaultMapper;
internal readonly CommandMapper? _commandMapper;
internal IPropertyMapper _mapper;
ElementHandlerState _handlerState;

ElementHandlerState IElementHandlerStateExhibitor.State => _handlerState;

protected ElementHandler(IPropertyMapper mapper, CommandMapper? commandMapper = null)
{
Expand All @@ -40,24 +43,38 @@ public virtual void SetVirtualView(IElement view)
_ = view ?? throw new ArgumentNullException(nameof(view));

if (VirtualView == view)
{
return;
}

var oldVirtualView = VirtualView;

bool setupPlatformView = oldVirtualView == null;

VirtualView = view;
PlatformView ??= CreatePlatformElement();
if (PlatformView is null)
{
_handlerState = ElementHandlerState.Connecting;
PlatformView = CreatePlatformElement();
}
else
{
_handlerState = ElementHandlerState.Reconnecting;
}

if (VirtualView.Handler != this)
{
VirtualView.Handler = this;
}

// We set the previous virtual view to null after setting it on the incoming virtual view.
// This makes it easier for the incoming virtual view to have influence
// on how the exchange of handlers happens.
// We will just set the handler to null ourselves as a last resort cleanup
if (oldVirtualView?.Handler != null)
{
oldVirtualView.Handler = null;
}

if (setupPlatformView)
{
Expand All @@ -77,6 +94,8 @@ public virtual void SetVirtualView(IElement view)
}

_mapper.UpdateProperties(this, VirtualView);

_handlerState = ElementHandlerState.Connected;
}

public virtual void UpdateValue(string property)
Expand Down Expand Up @@ -129,6 +148,8 @@ void IElementHandler.DisconnectHandler()
PlatformView = null;
DisconnectHandler(oldPlatformView);
}

_handlerState = ElementHandlerState.Disconnected;
}
}
}
32 changes: 32 additions & 0 deletions src/Core/src/Handlers/ElementHandlerState.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
using System;

namespace Microsoft.Maui
{
/// <summary>
/// Exposes the state of an element handler.
/// </summary>
[Flags]
internal enum ElementHandlerState : byte
{
/// <summary>
/// The handler is not connected to an element.
/// </summary>
Disconnected = 0x0,
/// <summary>
/// The handler is mapping all properties to the element.
/// </summary>
MappingProperties = 0x1,
/// <summary>
/// The handler is connecting for the first time to the element and mapping all properties.
/// </summary>
Connecting = MappingProperties | 0x2,
/// <summary>
/// The connected handler is now connecting to a new element and updating properties.
/// </summary>
Reconnecting = MappingProperties | 0x4,
/// <summary>
/// The handler is connected to an element.
/// </summary>
Connected = 0x8
}
}
16 changes: 16 additions & 0 deletions src/Core/src/Handlers/IElementHandlerStateExhibitor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
namespace Microsoft.Maui
{
/// <summary>
/// Exposes the state of an element handler.
/// </summary>
/// <remarks>
/// To be migrated to a public API.
/// </remarks>
internal interface IElementHandlerStateExhibitor
{
/// <summary>
/// Gets the state of the element handler.
/// </summary>
ElementHandlerState State { get; }
}
}
32 changes: 32 additions & 0 deletions src/Core/src/Handlers/InternalElementHandlerExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
using System.Collections.Generic;
using System.Linq;

namespace Microsoft.Maui
{
internal static class InternalElementHandlerExtensions
{
/// <summary>
/// The handler is connecting for the first time to the element and mapping all properties.
/// </summary>
/// <param name="handler"></param>
/// <returns></returns>
internal static bool IsMappingProperties(this IElementHandler handler) =>
(handler as IElementHandlerStateExhibitor)?.State.HasFlag(ElementHandlerState.MappingProperties) ?? false;
Comment on lines +13 to +14
Copy link
Contributor

Choose a reason for hiding this comment

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

The method as-is is user-friendly. However, the result is perhaps unexpected when handler is not IElementHandlerStateExhibitor. In that case, I would expect to get null (and not false) as it would tell me "perhaps the handler is actually mapping properties but it does not provide the information". Though, I'm aware that that might lead to less friendly API.

Anyway, feel free to ignore if you feel like it does not add any value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is only temporary for .NET9, and the ?. is there for type safety.
Hopefully these methods will be public and moved to IElementHandler in .NET10.


/// <summary>
/// Indicates whether the handler is connecting for the first time to the element and mapping all properties.
/// </summary>
/// <param name="handler"></param>
/// <returns></returns>
internal static bool IsConnectingHandler(this IElementHandler handler) =>
(handler as IElementHandlerStateExhibitor)?.State.HasFlag(ElementHandlerState.Connecting) ?? false;

/// <summary>
/// Indicates whether the connected handler is now connecting to a new element and updating properties.
/// </summary>
/// <param name="handler"></param>
/// <returns></returns>
internal static bool IsReconnectingHandler(this IElementHandler handler) =>
(handler as IElementHandlerStateExhibitor)?.State.HasFlag(ElementHandlerState.Reconnecting) ?? false;
}
}
2 changes: 2 additions & 0 deletions src/Core/src/Handlers/View/ViewHandler.Windows.cs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ public static void MapContextFlyout(IViewHandler handler, IView view)
{
if (view is IContextFlyoutElement contextFlyoutContainer)
{
if (handler.IsConnectingHandler() && contextFlyoutContainer.ContextFlyout is null) return;

MapContextFlyout(handler, contextFlyoutContainer);
}
}
Expand Down
Loading
Loading