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

Fix Android FormattedText and related platforms #7219

Merged
merged 9 commits into from May 17, 2022
Merged

Fix Android FormattedText and related platforms #7219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4eea438
Reduce line length so we can work
mattleibow May 16, 2022
d8cae2a
Set initial Span.FontSize => double.NaN Fixes #6801
mattleibow May 16, 2022
c109fc2
Correctly apply span values to spans Fixes #7220
mattleibow May 16, 2022
3820238
Update src/Controls/src/Core/Platform/Android/Extensions/FormattedStr…
mattleibow May 16, 2022
8268e2b
Merge remote-tracking branch 'origin/main' into dev/span-font-size
mattleibow May 16, 2022
f3dfd7a
Added a test
mattleibow May 16, 2022
68e0574
Update LabelTests.cs
mattleibow May 16, 2022
2d04d6e
make the test test the thing
mattleibow May 16, 2022
b629206
do both tests
mattleibow May 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Controls/src/Core/FontExtensions.cs
View file
Open in desktop
Expand Up @@ -30,7 +30,7 @@ public static FontAttributes GetFontAttributes(this Font font)
public static Font ToFont(this IFontElement element, double? defaultSize = null)
{
var size = element.FontSize;
if (defaultSize.HasValue && (size == 0 || size == double.NaN))
if (defaultSize.HasValue && (size <= 0 || double.IsNaN(size)))
size = defaultSize.Value;

return Font.OfSize(element.FontFamily, size, enableScaling: element.FontAutoScalingEnabled).WithAttributes(element.FontAttributes);
Expand Down
151 changes: 75 additions & 76 deletions src/Controls/src/Core/Platform/Android/Extensions/FormattedStringExtensions.cs
View file
Open in desktop
Expand Up @@ -15,9 +15,27 @@ namespace Microsoft.Maui.Controls.Platform
public static class FormattedStringExtensions
{
public static SpannableString ToSpannableString(this Label label)
=> ToSpannableString(label.FormattedText, label.RequireFontManager(), (label.Handler?.PlatformView as TextView)?.Paint, label.Handler?.MauiContext?.Context, label.LineHeight, label.HorizontalTextAlignment, label.ToFont(), label.TextColor, label.TextTransform);

public static SpannableString ToSpannableString(this FormattedString formattedString, IFontManager fontManager, TextPaint? textPaint = null, Context? context = null, double defaultLineHeight = 0d, TextAlignment defaultHorizontalAlignment = TextAlignment.Start, Font? defaultFont = null, Graphics.Color? defaultColor = null, TextTransform defaultTextTransform = TextTransform.Default)
=> ToSpannableString(
label.FormattedText,
label.RequireFontManager(),
label.Handler?.MauiContext?.Context,
label.CharacterSpacing,
label.HorizontalTextAlignment,
label.ToFont(),
label.TextColor,
label.TextTransform,
label.TextDecorations);

public static SpannableString ToSpannableString(
this FormattedString formattedString,
IFontManager fontManager,
Context? context = null,
double defaultCharacterSpacing = 0d,
TextAlignment defaultHorizontalAlignment = TextAlignment.Start,
Font? defaultFont = null,
Graphics.Color? defaultColor = null,
TextTransform defaultTextTransform = TextTransform.Default,
TextDecorations defaultTextDecorations = TextDecorations.None)
{
if (formattedString == null)
return new SpannableString(string.Empty);
Expand Down Expand Up @@ -53,46 +71,43 @@ public static SpannableString ToSpannableString(this FormattedString formattedSt
int end = start + text.Length;
c = end;

// TextColor
var textColor = span.TextColor ?? defaultColor;

if (textColor != null)
{
if (textColor is not null)
spannable.SetSpan(new ForegroundColorSpan(textColor.ToPlatform()), start, end, SpanTypes.InclusiveExclusive);
}

if (span.BackgroundColor != null)
{
// BackgroundColor
if (span.BackgroundColor is not null)
spannable.SetSpan(new BackgroundColorSpan(span.BackgroundColor.ToPlatform()), start, end, SpanTypes.InclusiveExclusive);
}

var lineHeight = span.LineHeight >= 0
? span.LineHeight
: defaultLineHeight;
// LineHeight
if (span.LineHeight >= 0)
spannable.SetSpan(new LineHeightSpan(span.LineHeight), start, end, SpanTypes.InclusiveExclusive);

if (lineHeight >= 0)
{
spannable.SetSpan(new LineHeightSpan(textPaint, lineHeight), start, end, SpanTypes.InclusiveExclusive);
}
// CharacterSpacing
var characterSpacing = span.CharacterSpacing >= 0
? span.CharacterSpacing
: defaultCharacterSpacing;
if (characterSpacing >= 0)
spannable.SetSpan(new LetterSpacingSpan(characterSpacing.ToEm()), start, end, SpanTypes.InclusiveInclusive);

// Font
var font = span.ToFont(defaultFontSize);
if (font.IsDefault && defaultFont.HasValue)
font = defaultFont.Value;

if (!font.IsDefault)
{
spannable.SetSpan(
new FontSpan(font, context, span.CharacterSpacing.ToEm(), defaultHorizontalAlignment, fontManager),
start,
end,
SpanTypes.InclusiveInclusive);
}

if (span.IsSet(Span.TextDecorationsProperty))
{
spannable.SetSpan(new TextDecorationSpan(span), start, end, SpanTypes.InclusiveInclusive);
}

spannable.SetSpan(new FontSpan(font, fontManager, context), start, end, SpanTypes.InclusiveInclusive);

// TextDecorations
var textDecorations = span.IsSet(Span.TextDecorationsProperty)
? span.TextDecorations
: defaultTextDecorations;
if (textDecorations.HasFlag(TextDecorations.Strikethrough))
spannable.SetSpan(new StrikethroughSpan(), start, end, SpanTypes.InclusiveInclusive);
if (textDecorations.HasFlag(TextDecorations.Underline))
spannable.SetSpan(new UnderlineSpan(), start, end, SpanTypes.InclusiveInclusive);
}

return spannable;
}

Expand All @@ -118,7 +133,6 @@ public static void RecalculateSpanPositions(this TextView textView, Label elemen
int count = 0;
IList<int> totalLineHeights = new List<int>();

#pragma warning disable CA1416 // https://github.com/xamarin/xamarin-android/issues/6962
for (int i = 0; i < spannableString.Length(); i = next)
{
var type = Java.Lang.Class.FromType(typeof(Java.Lang.Object));
Expand All @@ -136,7 +150,7 @@ public static void RecalculateSpanPositions(this TextView textView, Label elemen
// Get all spans in the range - Android can have overlapping spans
var spans = spannableString.GetSpans(i, next, type);

if (spans == null)
if (spans is null || spans.Length == 0)
continue;

var startSpan = spans[0];
Expand Down Expand Up @@ -179,26 +193,21 @@ public static void RecalculateSpanPositions(this TextView textView, Label elemen

((ISpatialElement)span).Region = Region.FromLines(lineHeights, labelWidth, startX, endX, yaxis).Inflate(10);
}
#pragma warning restore CA1416 // 'SpannableString.Length()' is only supported on: 'android' 29.0 and later
}

class FontSpan : MetricAffectingSpan
{
public FontSpan(Font font, Context? context, float characterSpacing, TextAlignment? horizontalTextAlignment, IFontManager fontManager)
readonly Font _font;
readonly IFontManager _fontManager;
readonly Context? _context;

public FontSpan(Font font, IFontManager fontManager, Context? context)
{
Font = font;
Context = context;
CharacterSpacing = characterSpacing;
FontManager = fontManager;
HorizontalTextAlignment = horizontalTextAlignment;
_font = font;
_fontManager = fontManager;
_context = context;
}

public readonly IFontManager FontManager;
public readonly Font Font;
public readonly Context? Context;
public readonly float CharacterSpacing;
public readonly TextAlignment? HorizontalTextAlignment;

public override void UpdateDrawState(TextPaint? tp)
{
if (tp != null)
Expand All @@ -210,28 +219,26 @@ public override void UpdateMeasureState(TextPaint p)
Apply(p);
}

void Apply(Paint paint)
void Apply(TextPaint paint)
{
paint.SetTypeface(Font.ToTypeface(FontManager));
float value = (float)Font.Size;
paint.SetTypeface(_font.ToTypeface(_fontManager));

paint.TextSize = TypedValue.ApplyDimension(
Font.AutoScalingEnabled ? ComplexUnitType.Sp : ComplexUnitType.Dip,
value, Context?.Resources?.DisplayMetrics ?? AAplication.Context.Resources!.DisplayMetrics);

paint.LetterSpacing = CharacterSpacing;
_font.AutoScalingEnabled ? ComplexUnitType.Sp : ComplexUnitType.Dip,
(float)_font.Size,
(_context ?? AAplication.Context)?.Resources?.DisplayMetrics);
}
}

class TextDecorationSpan : MetricAffectingSpan
class LetterSpacingSpan : MetricAffectingSpan
{
public TextDecorationSpan(Span span)
readonly float _letterSpacing;

public LetterSpacingSpan(double letterSpacing)
{
Span = span;
_letterSpacing = (float)letterSpacing;
}

public Span Span { get; }

public override void UpdateDrawState(TextPaint? tp)
{
if (tp != null)
Expand All @@ -243,35 +250,27 @@ public override void UpdateMeasureState(TextPaint p)
Apply(p);
}

void Apply(Paint paint)
void Apply(TextPaint paint)
{
var textDecorations = Span.TextDecorations;
paint.UnderlineText = (textDecorations & TextDecorations.Underline) != 0;
paint.StrikeThruText = (textDecorations & TextDecorations.Strikethrough) != 0;
paint.LetterSpacing = _letterSpacing;
}
}

class LineHeightSpan : Java.Lang.Object, ILineHeightSpan
{
private double _lineHeight;
private int _ascent;
private int _descent;
readonly double _relativeLineHeight;

public LineHeightSpan(TextPaint? paint, double lineHeight)
public LineHeightSpan(double relativeLineHeight)
{
_lineHeight = lineHeight;
var fm = paint?.GetFontMetricsInt();
_ascent = fm?.Ascent ?? 1;
_descent = fm?.Descent ?? 1;
_relativeLineHeight = relativeLineHeight;
}

public void ChooseHeight(Java.Lang.ICharSequence? text, int start, int end, int spanstartv, int v, Paint.FontMetricsInt? fm)
public void ChooseHeight(Java.Lang.ICharSequence? text, int start, int end, int spanstartv, int lineHeight, Paint.FontMetricsInt? fm)
{
if (fm != null)
{
fm.Ascent = (int)(_ascent * _lineHeight);
fm.Descent = (int)(_descent * _lineHeight);
}
if (fm is null)
return;

fm.Ascent = (int)(fm.Top * _relativeLineHeight);
}
}
}
Expand Down
42 changes: 36 additions & 6 deletions src/Controls/src/Core/Platform/Windows/Extensions/FormattedStringExtensions.cs
View file
Open in desktop
Expand Up @@ -11,9 +11,25 @@ namespace Microsoft.Maui.Controls.Platform
public static class FormattedStringExtensions
{
public static void UpdateInlines(this TextBlock textBlock, Label label)
=> UpdateInlines(textBlock, label.RequireFontManager(false), label.FormattedText, label.LineHeight, label.HorizontalTextAlignment, label.ToFont(), label.TextColor, label.TextTransform);

public static void UpdateInlines(this TextBlock textBlock, IFontManager fontManager, FormattedString formattedString, double defaultLineHeight = 0d, TextAlignment defaultHorizontalAlignment = TextAlignment.Start, Font? defaultFont = null, Color? defaultColor = null, TextTransform defaultTextTransform = TextTransform.Default)
=> UpdateInlines(
textBlock,
label.RequireFontManager(false),
label.FormattedText,
label.LineHeight,
label.HorizontalTextAlignment,
label.ToFont(),
label.TextColor,
label.TextTransform);

public static void UpdateInlines(
this TextBlock textBlock,
IFontManager fontManager,
FormattedString formattedString,
double defaultLineHeight = -1d,
TextAlignment defaultHorizontalAlignment = TextAlignment.Start,
Font? defaultFont = null,
Color? defaultColor = null,
TextTransform defaultTextTransform = TextTransform.Default)
{
textBlock.Inlines.Clear();
// Have to implement a measure here, otherwise inline.ContentStart and ContentEnd will be null, when used in RecalculatePositions
Expand Down Expand Up @@ -57,7 +73,14 @@ public static void UpdateInlines(this TextBlock textBlock, IFontManager fontMana
}
}

public static IEnumerable<Tuple<Run, Color, Color>> ToRunAndColorsTuples(this FormattedString formattedString, IFontManager fontManager, double defaultLineHeight = 0d, TextAlignment defaultHorizontalAlignment = TextAlignment.Start, Font? defaultFont = null, Color? defaultColor = null, TextTransform defaultTextTransform = TextTransform.Default)
public static IEnumerable<Tuple<Run, Color, Color>> ToRunAndColorsTuples(
this FormattedString formattedString,
IFontManager fontManager,
double defaultLineHeight = -1d,
TextAlignment defaultHorizontalAlignment = TextAlignment.Start,
Font? defaultFont = null,
Color? defaultColor = null,
TextTransform defaultTextTransform = TextTransform.Default)
{
var runs = new List<Tuple<Run, Color, Color>>();

Expand All @@ -74,15 +97,22 @@ public static void UpdateInlines(this TextBlock textBlock, IFontManager fontMana
return runs;
}

public static Tuple<Run, Color, Color> ToRunAndColorsTuple(this Span span, IFontManager fontManager, Font? defaultFont = null, Color? defaultColor = null, TextTransform defaultTextTransform = TextTransform.Default)
public static Tuple<Run, Color, Color> ToRunAndColorsTuple(
this Span span,
IFontManager fontManager,
Font? defaultFont = null,
Color? defaultColor = null,
TextTransform defaultTextTransform = TextTransform.Default)
{
var defaultFontSize = defaultFont?.Size ?? fontManager.DefaultFontSize;

var transform = span.TextTransform != TextTransform.Default ? span.TextTransform : defaultTextTransform;

var text = TextTransformUtilites.GetTransformedText(span.Text, transform);

var run = new Run { Text = text ?? string.Empty };

var font = span.ToFont();
var font = span.ToFont(defaultFontSize);
if (font.IsDefault && defaultFont.HasValue)
font = defaultFont.Value;

Expand Down
24 changes: 19 additions & 5 deletions src/Controls/src/Core/Platform/iOS/Extensions/FormattedStringExtensions.cs
View file
Open in desktop
Expand Up @@ -26,8 +26,14 @@ public static class FormattedStringExtensions
label.TextColor,
label.TextTransform);


public static NSAttributedString ToNSAttributedString(this FormattedString formattedString, IFontManager fontManager, double defaultLineHeight = 0d, TextAlignment defaultHorizontalAlignment = TextAlignment.Start, Font? defaultFont = null, Color? defaultColor = null, TextTransform defaultTextTransform = TextTransform.Default)
public static NSAttributedString ToNSAttributedString(
this FormattedString formattedString,
IFontManager fontManager,
double defaultLineHeight = -1d,
TextAlignment defaultHorizontalAlignment = TextAlignment.Start,
Font? defaultFont = null,
Color? defaultColor = null,
TextTransform defaultTextTransform = TextTransform.Default)
{
if (formattedString == null)
return new NSAttributedString(string.Empty);
Expand All @@ -45,8 +51,17 @@ public static NSAttributedString ToNSAttributedString(this FormattedString forma
return attributed;
}

public static NSAttributedString ToNSAttributedString(this Span span, IFontManager fontManager, double defaultLineHeight = 0d, TextAlignment defaultHorizontalAlignment = TextAlignment.Start, Font? defaultFont = null, Color? defaultColor = null, TextTransform defaultTextTransform = TextTransform.Default)
public static NSAttributedString ToNSAttributedString(
this Span span,
IFontManager fontManager,
double defaultLineHeight = -1d,
TextAlignment defaultHorizontalAlignment = TextAlignment.Start,
Font? defaultFont = null,
Color? defaultColor = null,
TextTransform defaultTextTransform = TextTransform.Default)
{
var defaultFontSize = defaultFont?.Size ?? fontManager.DefaultFontSize;

var transform = span.TextTransform != TextTransform.Default ? span.TextTransform : defaultTextTransform;

var text = TextTransformUtilites.GetTransformedText(span.Text, transform);
Expand All @@ -71,7 +86,7 @@ public static NSAttributedString ToNSAttributedString(this Span span, IFontManag
_ => UITextAlignment.Left
};

var font = span.ToFont();
var font = span.ToFont(defaultFontSize);
if (font.IsDefault && defaultFont.HasValue)
font = defaultFont.Value;

Expand Down Expand Up @@ -110,6 +125,5 @@ public static NSAttributedString ToNSAttributedString(this Span span, IFontManag

return attrString;
}

}
}
2 changes: 1 addition & 1 deletion src/Controls/src/Core/Span.cs
View file
Open in desktop
Expand Up @@ -159,7 +159,7 @@ void IFontElement.OnFontSizeChanged(double oldValue, double newValue)
}

double IFontElement.FontSizeDefaultValueCreator() =>
this.GetDefaultFontSize();
double.NaN;
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the only "breaking" change, but anyone using this value without it being set by the user is getting a wrong value. I could use 0?

Copy link
Contributor

Choose a reason for hiding this comment

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

GetDefaultFontSize is not returning a good value without it being set by the user?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is, sort of. The method returns 14 if the span is attached and 0 if not. The issue with these default value functions is that it sets the value as if the user set it.

So the OG issue is that the label font was set to a big size, and then the font remains small because by the time the label is rendered the framework overrides the big with the default of 14.

So if you do:

// in ctor
var firstSpan = new Span();
var firstSize = span.FontSize; // <- 0

var secondSpan = new Span();
var label = new Label {
    FontSIze = 100,
    FormattedText = new FormattedString { firstSpan, secondSpan }
};

// after rendering
var secondSize = secondSpan.FontSize; // <- 14
var firstSizeAgain = firstSpan.FontSIze; // <- 0

In both cases, you did not set a font size on the span, and in the second you actually set a size on the label. And that size is not used.

The NaN is a way to tell the rest of the framework that no size has been set, so keep looking.


void IFontElement.OnFontAttributesChanged(FontAttributes oldValue, FontAttributes newValue)
{
Expand Down