Skip to content

Commit dec6626

Browse files
h3xds1nzh3xds1nz
authored
Optimize parsing/conversion of TextDecorations from string, reduce allocations (#9778)
* Simplify CanConvertTo * Simplify CanConvertFrom * Simplify ConvertFrom * Simplify ConvertTo * Simplify ConvertFromString * Remove obsolete Match; AdvanceToNextNameStart; AdvanceToNextNonWhiteSpace * Remove TextDecorationNames and PredefinedTextDecorations arrays * Remove redundant usings * Add documentation, remove whitespaces, final cleanup * Abstract away the use of bit flags for matches with an enum * Fix post-merge issues * Fix original typo
1 parent 1f21aef commit dec6626

File tree

1 file changed

+93
-206
lines changed

1 file changed

+93
-206
lines changed

‎src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/TextDecorationCollectionConverter.cs‎

Lines changed: 93 additions & 206 deletions
Original file line numberDiff line numberDiff line change
@@ -1,272 +1,159 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System.ComponentModel;
54
using System.ComponentModel.Design.Serialization;
5+
using System.ComponentModel;
66
using System.Globalization;
77
using System.Reflection;
88

99
namespace System.Windows
1010
{
1111
/// <summary>
12-
/// TypeConverter for TextDecorationCollection
13-
/// </summary>
12+
/// Provides a type converter to convert from <see cref="string"/> to <see cref="TextDecorationCollection"/> only.
13+
/// </summary>
1414
public sealed class TextDecorationCollectionConverter : TypeConverter
1515
{
1616
/// <summary>
17-
/// CanConvertTo method
17+
/// Returns whether this converter can convert the object to the specified
18+
/// <paramref name="destinationType"/>, using the specified <paramref name="context"/>.
1819
/// </summary>
19-
/// <param name="context"> ITypeDescriptorContext </param>
20-
/// <param name="destinationType"> Type to convert to </param>
21-
/// <returns> false will always be returned because TextDecorations cannot be converted to any other type. </returns>
20+
/// <param name="context">Context information used for conversion.</param>
21+
/// <param name="destinationType">Type being evaluated for conversion.</param>
22+
/// <returns>
23+
/// <see langword="false"/> will always be returned because <see cref="TextDecorations"/> cannot be converted to any other type.
24+
/// </returns>
2225
public override bool CanConvertTo(ITypeDescriptorContext context, Type destinationType)
2326
{
24-
if (destinationType == typeof(InstanceDescriptor))
25-
{
26-
return true;
27-
}
28-
29-
// return false for any other target type. Don't call base.CanConvertTo() because it would be confusing
30-
// in some cases. For example, for destination typeof(String), base convertor just converts the TDC to the
31-
// string full name of the type.
32-
return false;
27+
// Return false for any other target type. Don't call base.CanConvertTo() because it would be confusing
28+
// in some cases. For example, for destination typeof(string), base TypeConverter just converts the
29+
// ITypeDescriptorContext to the full name string of the given type.
30+
return destinationType == typeof(InstanceDescriptor);
3331
}
3432

3533
/// <summary>
36-
/// CanConvertFrom
34+
/// Returns whether this class can convert specific <see cref="Type"/> into <see cref="TextDecorationCollection"/>.
3735
/// </summary>
3836
/// <param name="context"> ITypeDescriptorContext </param>
39-
/// <param name="sourceType">Type to convert to </param>
40-
/// <returns> true if it can convert from sourceType to TextDecorations, false otherwise </returns>
37+
/// <param name="sourceType">Type being evaluated for conversion.</param>
38+
/// <returns>
39+
/// <see langword="true"/> if <paramref name="sourceType"/> is <see langword="string"/>, otherwise <see langword="false"/>.
40+
/// </returns>
4141
public override bool CanConvertFrom(ITypeDescriptorContext context, Type sourceType)
4242
{
43-
if (sourceType == typeof(string))
44-
{
45-
return true;
46-
}
47-
48-
return false;
43+
return sourceType == typeof(string);
4944
}
5045

5146
/// <summary>
52-
/// ConvertFrom
47+
/// Converts <paramref name="input"/> of <see langword="string"/> type to its <see cref="TextDecorationCollection"/> representation.
5348
/// </summary>
54-
/// <param name="context"> ITypeDescriptorContext </param>
55-
/// <param name="culture"> CultureInfo </param>
56-
/// <param name="input"> The input object to be converted to TextDecorations </param>
57-
/// <returns> the converted value of the input object </returns>
49+
/// <param name="context">Context information used for conversion, ignored currently.</param>
50+
/// <param name="culture">The culture specifier to use, ignored currently.</param>
51+
/// <param name="input">The string to convert from.</param>
52+
/// <returns>A <see cref="TextDecorationCollection"/> representing the <see langword="string"/> specified by <paramref name="input"/>.</returns>
5853
public override object ConvertFrom(ITypeDescriptorContext context, CultureInfo culture, object input)
5954
{
60-
if (input == null)
61-
{
55+
if (input is null)
6256
throw GetConvertFromException(input);
63-
}
6457

65-
string value = input as string;
66-
67-
if (null == value)
68-
{
58+
if (input is not string value)
6959
throw new ArgumentException(SR.Format(SR.General_BadType, "ConvertFrom"), nameof(input));
70-
}
71-
72-
return ConvertFromString(value);
60+
61+
return ConvertFromString(value);
7362
}
7463

7564
/// <summary>
76-
/// ConvertFromString
65+
/// Converts <paramref name="text"/> to its <see cref="TextDecorationCollection"/> representation.
7766
/// </summary>
78-
/// <param name="text"> The string to be converted into TextDecorationCollection object </param>
79-
/// <returns> the converted value of the string flag </returns>
67+
/// <param name="text">The string to be converted into TextDecorationCollection object.</param>
68+
/// <returns>A <see cref="TextDecorationCollection"/> representing the <see cref="string"/> specified by <paramref name="text"/>.</returns>
8069
/// <remarks>
81-
/// The text parameter can be either string "None" or a combination of the predefined
82-
/// TextDecoration names delimited by commas (,). One or more blanks spaces can precede
83-
/// or follow each text decoration name or comma. There can't be duplicate TextDecoration names in the
84-
/// string. The operation is case-insensitive.
70+
/// The text parameter can be either be <see langword="null"/>; <see cref="string.Empty"/>; the string "None"
71+
/// or a combination of the predefined <see cref="TextDecorations"/> names delimited by commas (,).
72+
/// One or more blanks spaces can precede or follow each text decoration name or comma.
73+
/// There can't be duplicate TextDecoration names in the string. The operation is case-insensitive.
8574
/// </remarks>
8675
public static new TextDecorationCollection ConvertFromString(string text)
87-
{
88-
if (text == null)
89-
{
90-
return null;
91-
}
76+
{
77+
if (text is null)
78+
return null;
79+
80+
// Flags indicating which pre-defined TextDecoration have been matched
81+
Decorations matchedDecorations = Decorations.None;
82+
83+
// Sanitize the input
84+
ReadOnlySpan<char> decorationsSpan = text.AsSpan().Trim();
9285

93-
TextDecorationCollection textDecorations = new TextDecorationCollection();
86+
// Test for "None", which equals to empty collection and needs to be specified alone
87+
if (decorationsSpan.IsEmpty || decorationsSpan.Equals("None", StringComparison.OrdinalIgnoreCase))
88+
return new TextDecorationCollection();
9489

95-
// Flags indicating which Predefined textDecoration has alrady been added.
96-
byte MatchedTextDecorationFlags = 0;
97-
98-
// Start from the 1st non-whitespace character
99-
// Negative index means error is encountered
100-
int index = AdvanceToNextNonWhiteSpace(text, 0);
101-
while (index >= 0 && index < text.Length)
90+
// Create new collection, save re-allocations
91+
TextDecorationCollection textDecorations = new(1 + decorationsSpan.Count(','));
92+
foreach (Range segment in decorationsSpan.Split(','))
10293
{
103-
if (Match(None, text, index))
94+
ReadOnlySpan<char> decoration = decorationsSpan[segment].Trim();
95+
96+
if (decoration.Equals("Overline", StringComparison.OrdinalIgnoreCase) && !matchedDecorations.HasFlag(Decorations.OverlineMatch))
97+
{
98+
textDecorations.Add(TextDecorations.OverLine[0]);
99+
matchedDecorations |= Decorations.OverlineMatch;
100+
}
101+
else if (decoration.Equals("Baseline", StringComparison.OrdinalIgnoreCase) && !matchedDecorations.HasFlag(Decorations.BaselineMatch))
104102
{
105-
// Matched "None" in the input
106-
index = AdvanceToNextNonWhiteSpace(text, index + None.Length);
107-
if (textDecorations.Count > 0 || index < text.Length)
108-
{
109-
// Error: "None" can only be specified by its own
110-
index = -1;
111-
}
103+
textDecorations.Add(TextDecorations.Baseline[0]);
104+
matchedDecorations |= Decorations.BaselineMatch;
105+
}
106+
else if (decoration.Equals("Underline", StringComparison.OrdinalIgnoreCase) && !matchedDecorations.HasFlag(Decorations.UnderlineMatch))
107+
{
108+
textDecorations.Add(TextDecorations.Underline[0]);
109+
matchedDecorations |= Decorations.UnderlineMatch;
110+
}
111+
else if (decoration.Equals("Strikethrough", StringComparison.OrdinalIgnoreCase) && !matchedDecorations.HasFlag(Decorations.StrikethroughMatch))
112+
{
113+
textDecorations.Add(TextDecorations.Strikethrough[0]);
114+
matchedDecorations |= Decorations.StrikethroughMatch;
112115
}
113116
else
114117
{
115-
// Match the input with one of the predefined text decoration names
116-
int i;
117-
for(i = 0;
118-
i < TextDecorationNames.Length
119-
&& !Match(TextDecorationNames[i], text, index);
120-
i++
121-
);
122-
123-
if (i < TextDecorationNames.Length)
124-
{
125-
// Found a match within the predefined names
126-
if ((MatchedTextDecorationFlags & (1 << i)) > 0)
127-
{
128-
// Error: The matched value is duplicated.
129-
index = -1;
130-
}
131-
else
132-
{
133-
// Valid match. Add to the collection and remember that this text decoration
134-
// has been added
135-
textDecorations.Add(PredefinedTextDecorations[i]);
136-
MatchedTextDecorationFlags |= (byte)(1 << i);
137-
138-
// Advance to the start of next name
139-
index = AdvanceToNextNameStart(text, index + TextDecorationNames[i].Length);
140-
}
141-
}
142-
else
143-
{
144-
// Error: no match found in the predefined names
145-
index = -1;
146-
}
118+
throw new ArgumentException(SR.Format(SR.InvalidTextDecorationCollectionString, text));
147119
}
148120
}
149121

150-
if (index < 0)
151-
{
152-
throw new ArgumentException(SR.Format(SR.InvalidTextDecorationCollectionString, text));
153-
}
154-
155-
return textDecorations;
156-
}
122+
return textDecorations;
123+
}
157124

158125
/// <summary>
159-
/// ConvertTo
126+
/// Converts a <paramref name="value"/> of <see cref="TextDecorationCollection"/> to the specified <paramref name="destinationType"/>.
160127
/// </summary>
161-
/// <param name="context"> ITypeDescriptorContext </param>
162-
/// <param name="culture"> CultureInfo </param>
163-
/// <param name="value"> the object to be converted to another type </param>
164-
/// <param name="destinationType"> The destination type of the conversion </param>
165-
/// <returns> null will always be returned because TextDecorations cannot be converted to any other type. </returns>
128+
/// <param name="context">Context information used for conversion.</param>
129+
/// <param name="culture">The culture specifier to use.</param>
130+
/// <param name="value">Duration value to convert from.</param>
131+
/// <param name="destinationType">Type being evaluated for conversion.</param>
132+
/// <returns><see langword="null"/> will always be returned because <see cref="TextDecorations"/> cannot be converted to any other type.</returns>
166133
public override object ConvertTo(ITypeDescriptorContext context, CultureInfo culture, object value, Type destinationType)
167134
{
168135
if (destinationType == typeof(InstanceDescriptor) && value is IEnumerable<TextDecoration>)
169136
{
170-
ConstructorInfo ci = typeof(TextDecorationCollection).GetConstructor(
171-
new Type[]{typeof(IEnumerable<TextDecoration>)}
172-
);
173-
174-
return new InstanceDescriptor(ci, new object[]{value});
175-
}
176-
177-
// Pass unhandled cases to base class (which will throw exceptions for null value or destinationType.)
178-
return base.ConvertTo(context, culture, value, destinationType);
179-
}
180-
181-
//---------------------------------
182-
// Private methods
183-
//---------------------------------
184-
/// <summary>
185-
/// Match the input against a predefined pattern from certain index onwards
186-
/// </summary>
187-
private static bool Match(string pattern, string input, int index)
188-
{
189-
int i = 0;
190-
for (;
191-
i < pattern.Length
192-
&& index + i < input.Length
193-
&& pattern[i] == Char.ToUpperInvariant(input[index + i]);
194-
i++) ;
195-
196-
return (i == pattern.Length);
197-
}
198-
199-
/// <summary>
200-
/// Advance to the start of next name
201-
/// </summary>
202-
private static int AdvanceToNextNameStart(string input, int index)
203-
{
204-
// Two names must be seperated by a comma and optionally spaces
205-
int separator = AdvanceToNextNonWhiteSpace(input, index);
137+
ConstructorInfo ci = typeof(TextDecorationCollection).GetConstructor(new Type[] { typeof(IEnumerable<TextDecoration>) });
206138

207-
int nextNameStart;
208-
if (separator >= input.Length)
209-
{
210-
// reach the end
211-
nextNameStart = input.Length;
212-
}
213-
else
214-
{
215-
if (input[separator] == Separator)
216-
{
217-
nextNameStart = AdvanceToNextNonWhiteSpace(input, separator + 1);
218-
if (nextNameStart >= input.Length)
219-
{
220-
// Error: Separator is at the end of the input
221-
nextNameStart = -1;
222-
}
223-
}
224-
else
225-
{
226-
// Error: There is a non-whitespace, non-separator character following
227-
// the matched value
228-
nextNameStart = -1;
229-
}
139+
return new InstanceDescriptor(ci, new object[] { value });
230140
}
231141

232-
return nextNameStart;
142+
// Pass unhandled cases to base class (which will throw exceptions for null value or destinationType.)
143+
return base.ConvertTo(context, culture, value, destinationType);
233144
}
234145

235146
/// <summary>
236-
/// Advance to the next non-whitespace character
147+
/// Abstraction helper of matched decorations during conversion.
237148
/// </summary>
238-
private static int AdvanceToNextNonWhiteSpace(string input, int index)
149+
[Flags]
150+
private enum Decorations : byte
239151
{
240-
for (; index < input.Length && Char.IsWhiteSpace(input[index]); index++) ;
241-
return (index > input.Length) ? input.Length : index;
152+
None = 0,
153+
OverlineMatch = 1 << 0,
154+
BaselineMatch = 1 << 1,
155+
UnderlineMatch = 1 << 2,
156+
StrikethroughMatch = 1 << 3,
242157
}
243-
244-
//---------------------------------
245-
// Private members
246-
//---------------------------------
247-
248-
//
249-
// Predefined valid names for TextDecorations
250-
// Names should be normalized to be upper case
251-
//
252-
private const string None = "NONE";
253-
private const char Separator = ',';
254-
255-
private static readonly string[] TextDecorationNames = new string[] {
256-
"OVERLINE",
257-
"BASELINE",
258-
"UNDERLINE",
259-
"STRIKETHROUGH"
260-
};
261-
262-
// Predefined TextDecorationCollection values. It should match
263-
// the TextDecorationNames array
264-
private static readonly TextDecorationCollection[] PredefinedTextDecorations =
265-
new TextDecorationCollection[] {
266-
TextDecorations.OverLine,
267-
TextDecorations.Baseline,
268-
TextDecorations.Underline,
269-
TextDecorations.Strikethrough
270-
};
271-
}
158+
}
272159
}

0 commit comments

Comments
 (0)