-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[xamlc] add compiled StrokeShapeTypeConverter #8849
Merged
StephaneDelcroix
merged 1 commit into
dotnet:main
from
jonathanpeppers:StrokeShapeTypeConverter
Jul 28, 2022
Merged
[xamlc] add compiled StrokeShapeTypeConverter #8849
StephaneDelcroix
merged 1 commit into
dotnet:main
from
jonathanpeppers:StrokeShapeTypeConverter
Jul 28, 2022
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Context: dotnet#4293 (comment) The `dotnet new maui` template was spending some time in: 5.13ms (0.02%) microsoft.maui.controls!Microsoft.Maui.Controls.Shapes.StrokeShapeTypeConverter.ConvertFrom(System.ComponentModel.ITypeDe This amount of time is spent by a single value: <Style TargetType="Border"> <Setter Property="StrokeShape" Value="Rectangle"/> </Style> I went ahead and implemented a compiled converter for all "shape" syntax, except for one: PathShape="Path M16.484421,0.73799322C20.831404,0.7379931 24.353395,1.1259904 24.353395,1.6049905 24.353395,2.0839829 20.831404,2.4719803 16.484421,2.47198 12.138443,2.4719803 8.6154527,2.0839829 8.6154527,1.6049905 8.6154527,1.1259904 12.138443,0.7379931 16.484421,0.73799322z M1.9454784,0.061995983C2.7564723,5.2449602 12.246436,11.341911 12.246436,11.341911 13.248431,19.240842 9.6454477,17.915854 9.6454477,17.915854 7.9604563,18.897849 6.5314603,17.171859 6.5314603,17.171859 4.1084647,18.29585 3.279473,15.359877 3.2794733,15.359877 0.82348057,15.291876 1.2804796,11.362907 1.2804799,11.362907 -1.573514,10.239915 1.2344746,6.3909473 1.2344746,6.3909473 -1.3255138,4.9869594 1.9454782,0.061996057 1.9454784,0.061995983z M30.054371,0C30.054371,9.8700468E-08 33.325355,4.9249634 30.765367,6.3289513 30.765367,6.3289513 33.574364,10.177919 30.71837,11.30191 30.71837,11.30191 31.175369,15.22988 28.721384,15.297872 28.721384,15.297872 27.892376,18.232854 25.468389,17.110862 25.468389,17.110862 24.040392,18.835847 22.355402,17.853852 22.355402,17.853852 18.752417,19.178845 19.753414,11.279907 19.753414,11.279907 29.243385,5.1829566 30.054371,0z" Digging into the details, I found it would be a lot of work to implement this for `Path` string syntax. For now it is at least slightly better for `Path`, in that we'll generate: compiledTypeConverter.PathShape = new Path { Data = (Geometry)new PathGeometryConverter().ConvertFromInvariantString("M16.484421,0.73799322C20.831404,0.7379931 24.353395,1.1259904 24.353395,1.6049905 24.353395,2.0839829 20.831404,2.4719803 16.484421,2.47198 12.138443,2.4719803 8.6154527,2.0839829 8.6154527,1.6049905 8.6154527,1.1259904 12.138443,0.7379931 16.484421,0.73799322z M1.9454784,0.061995983C2.7564723,5.2449602 12.246436,11.341911 12.246436,11.341911 13.248431,19.240842 9.6454477,17.915854 9.6454477,17.915854 7.9604563,18.897849 6.5314603,17.171859 6.5314603,17.171859 4.1084647,18.29585 3.279473,15.359877 3.2794733,15.359877 0.82348057,15.291876 1.2804796,11.362907 1.2804799,11.362907 -1.573514,10.239915 1.2344746,6.3909473 1.2344746,6.3909473 -1.3255138,4.9869594 1.9454782,0.061996057 1.9454784,0.061995983z M30.054371,0C30.054371,9.8700468E-08 33.325355,4.9249634 30.765367,6.3289513 30.765367,6.3289513 33.574364,10.177919 30.71837,11.30191 30.71837,11.30191 31.175369,15.22988 28.721384,15.297872 28.721384,15.297872 27.892376,18.232854 25.468389,17.110862 25.468389,17.110862 24.040392,18.835847 22.355402,17.853852 22.355402,17.853852 18.752417,19.178845 19.753414,11.279907 19.753414,11.279907 29.243385,5.1829566 30.054371,0z") }; In writing test cases and reviewing `StrokeShapeTypeConverter`, I found: * Missing test case for `Line` string syntax * Missing test cases for many shapes with no numbers -- many of these actually would fail with `IndexOutOfRangeException`. After fixing up the failing tests, I updated the `CompiledTypeConverter` test for each case of "shape" string syntax that is supported: * Ellipse * Line * Polygon * Polyline * Rectangle * RoundRectangle * Path In the `CompiledConverters` test, previously we'd generate: compiledTypeConverter.EllipseShape = (IShape)new StrokeShapeTypeConverter().ConvertFromInvariantString("Ellipse"); compiledTypeConverter.LineShape = (IShape)new StrokeShapeTypeConverter().ConvertFromInvariantString("Line"); compiledTypeConverter.LineShapeTwo = (IShape)new StrokeShapeTypeConverter().ConvertFromInvariantString("Line 1,2"); compiledTypeConverter.LineShapeFour = (IShape)new StrokeShapeTypeConverter().ConvertFromInvariantString("Line 1,2,3,4"); compiledTypeConverter.PolygonShape = (IShape)new StrokeShapeTypeConverter().ConvertFromInvariantString("Polygon 10,110 60,10 110,110"); compiledTypeConverter.PolylineShape = (IShape)new StrokeShapeTypeConverter().ConvertFromInvariantString("Polyline 0 48, 0 144, 96 150, 100 0, 192 0, 192 96, 50 96, 48 192, 150 200 144 48"); compiledTypeConverter.RectangleShape = (IShape)new StrokeShapeTypeConverter().ConvertFromInvariantString("Rectangle"); compiledTypeConverter.RoundRectangleShape = (IShape)new StrokeShapeTypeConverter().ConvertFromInvariantString("RoundRectangle 1,2,3,4"); compiledTypeConverter.PathShape = (IShape)new StrokeShapeTypeConverter().ConvertFromInvariantString("Path M16.484421,0.73799322C20.831404,0.7379931 24.353395,1.1259904 24.353395,1.6049905 24.353395,2.0839829 20.831404,2.4719803 16.484421,2.47198 12.138443,2.4719803 8.6154527,2.0839829 8.6154527,1.6049905 8.6154527,1.1259904 12.138443,0.7379931 16.484421,0.73799322z M1.9454784,0.061995983C2.7564723,5.2449602 12.246436,11.341911 12.246436,11.341911 13.248431,19.240842 9.6454477,17.915854 9.6454477,17.915854 7.9604563,18.897849 6.5314603,17.171859 6.5314603,17.171859 4.1084647,18.29585 3.279473,15.359877 3.2794733,15.359877 0.82348057,15.291876 1.2804796,11.362907 1.2804799,11.362907 -1.573514,10.239915 1.2344746,6.3909473 1.2344746,6.3909473 -1.3255138,4.9869594 1.9454782,0.061996057 1.9454784,0.061995983z M30.054371,0C30.054371,9.8700468E-08 33.325355,4.9249634 30.765367,6.3289513 30.765367,6.3289513 33.574364,10.177919 30.71837,11.30191 30.71837,11.30191 31.175369,15.22988 28.721384,15.297872 28.721384,15.297872 27.892376,18.232854 25.468389,17.110862 25.468389,17.110862 24.040392,18.835847 22.355402,17.853852 22.355402,17.853852 18.752417,19.178845 19.753414,11.279907 19.753414,11.279907 29.243385,5.1829566 30.054371,0z"); And now we generate: compiledTypeConverter.EllipseShape = new Ellipse(); compiledTypeConverter.LineShape = new Line(); compiledTypeConverter.LineShapeTwo = new Line(1.0, 2.0, 0.0, 0.0); compiledTypeConverter.LineShapeFour = new Line(1.0, 2.0, 3.0, 4.0); compiledTypeConverter.PolygonShape = new Polygon(new PointCollection(new Point[3] { new Point(10.0, 110.0), new Point(60.0, 10.0), new Point(110.0, 110.0) })); compiledTypeConverter.PolylineShape = new Polyline(new PointCollection(new Point[10] { new Point(0.0, 48.0), new Point(0.0, 144.0), new Point(96.0, 150.0), new Point(100.0, 0.0), new Point(192.0, 0.0), new Point(192.0, 96.0), new Point(50.0, 96.0), new Point(48.0, 192.0), new Point(150.0, 200.0), new Point(144.0, 48.0) })); compiledTypeConverter.RectangleShape = new Rectangle(); compiledTypeConverter.RoundRectangleShape = new RoundRectangle { CornerRadius = new CornerRadius(1.0, 2.0, 3.0, 4.0) }; compiledTypeConverter.PathShape = new Path { Data = (Geometry)new PathGeometryConverter().ConvertFromInvariantString("M16.484421,0.73799322C20.831404,0.7379931 24.353395,1.1259904 24.353395,1.6049905 24.353395,2.0839829 20.831404,2.4719803 16.484421,2.47198 12.138443,2.4719803 8.6154527,2.0839829 8.6154527,1.6049905 8.6154527,1.1259904 12.138443,0.7379931 16.484421,0.73799322z M1.9454784,0.061995983C2.7564723,5.2449602 12.246436,11.341911 12.246436,11.341911 13.248431,19.240842 9.6454477,17.915854 9.6454477,17.915854 7.9604563,18.897849 6.5314603,17.171859 6.5314603,17.171859 4.1084647,18.29585 3.279473,15.359877 3.2794733,15.359877 0.82348057,15.291876 1.2804796,11.362907 1.2804799,11.362907 -1.573514,10.239915 1.2344746,6.3909473 1.2344746,6.3909473 -1.3255138,4.9869594 1.9454782,0.061996057 1.9454784,0.061995983z M30.054371,0C30.054371,9.8700468E-08 33.325355,4.9249634 30.765367,6.3289513 30.765367,6.3289513 33.574364,10.177919 30.71837,11.30191 30.71837,11.30191 31.175369,15.22988 28.721384,15.297872 28.721384,15.297872 27.892376,18.232854 25.468389,17.110862 25.468389,17.110862 24.040392,18.835847 22.355402,17.853852 22.355402,17.853852 18.752417,19.178845 19.753414,11.279907 19.753414,11.279907 29.243385,5.1829566 30.054371,0z") };
eaad6ce
to
27fce8e
Compare
I updated this to be I see some other things, but I'll file issues for those. |
StephaneDelcroix
approved these changes
Jul 28, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
area-xaml
XAML, CSS, Triggers, Behaviors
fixed-in-7.0.0-rc.1.6683
t/perf
The issue affects performance (runtime speed, memory usage, startup time, etc.) (sub: perf)
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #4293
The
dotnet new maui
template was spending some time in:This amount of time is spent by a single value:
I went ahead and implemented a compiled converter for all "shape"
syntax, except for one:
Digging into the details, I found it would be a lot of work to
implement this for
Path
string syntax. For now it is at leastslightly better for
Path
, in that we'll generate:In writing test cases and reviewing
StrokeShapeTypeConverter
, I found:Line
string syntaxactually would fail with
IndexOutOfRangeException
.After fixing up the failing tests, I updated the
CompiledTypeConverter
test for each case of "shape" string syntaxthat is supported:
In the
CompiledConverters
test, previously we'd generate:And now we generate: