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

Improve tiny performance #1472

Merged
merged 11 commits into from Apr 17, 2018

Conversation

Projects
None yet
4 participants
@lindexi
Copy link
Contributor

lindexi commented Apr 4, 2018

I found us set the dashes first but we may waste it.

When we enter this code that we will reset this value.

dashes = pen.DashStyle?.Dashes.Select(x => (float)x).ToArray();

As a frequently used code I think this optimize is necessary.

And I fix a comment. That it should rename renderTarget.

/// <param name="target">The render target.</param>

lindexi added some commits Apr 4, 2018

Lasy new the dashes for if we new an array first that we may waste it…
… when we use `dashes = pen.DashStyle.Dashes.Select(x => (float)x).ToArray()`. As a frequently used code I think this optimize is necessary.
@grokys

grokys approved these changes Apr 10, 2018

Copy link
Member

grokys left a comment

The change looks good to me, but could you remove the comments and undo the formatting change on line 134? Comments at this low level are't really necessary I don't think.

}

//If we don't enter the code above that it is null. We should set it a default value to solve the null exception.
if (dashes == null)

This comment has been minimized.

@grokys

grokys Apr 10, 2018

Member

Also rather than this block you can just do return new StrokeStyle(factory, properties, dashes ?? new float[0]));

This comment has been minimized.

@lindexi

lindexi Apr 12, 2018

Contributor

@grokys I will modify.

//If we don't enter the code above that it is null. We should set it a default value to solve the null exception.
if (dashes == null)
{
dashes = new float[0];

This comment has been minimized.

@kronic

kronic Apr 11, 2018

use Array.Empty<float>

This comment has been minimized.

@lindexi

lindexi Apr 12, 2018

Contributor

@kronic Thx.

properties.DashOffset = (float)pen.DashStyle.Offset;
dashes = pen.DashStyle?.Dashes.Select(x => (float)x).ToArray();
properties.DashOffset = (float) pen.DashStyle.Offset;
dashes = pen.DashStyle.Dashes.Select(x => (float)x).ToArray();

This comment has been minimized.

@kronic

kronic Apr 12, 2018

use dashes = pen.DashStyle.Dashes.Cast<float>.ToArray()

This comment has been minimized.

@lindexi

lindexi Apr 12, 2018

Contributor

@kronic Good, but there are no type in the solution which is inherited between double and float that grokys use select to cast it.

This comment has been minimized.

@lindexi

lindexi Apr 12, 2018

Contributor

@kronic

        List<double> titHruxvrvaa = new List<double>()
        {
            1d,
            2d,
            3d
        };

        var traStqjq = titHruxvrvaa.Cast<float>().ToArray();//System.InvalidCastException:“Unable to cast object of type 'System.Double' to type 'System.Single'.”

        foreach (var temp in traStqjq)
        {
            Console.WriteLine(temp);
        }

This comment has been minimized.

@kronic

kronic Apr 12, 2018

@lindexi
I'm sorry, you're right.

@@ -127,13 +127,17 @@ public static StrokeStyle ToDirect2DStrokeStyle(this Avalonia.Media.Pen pen, Fac
EndCap = pen.EndLineCap.ToDirect2D(),
DashCap = pen.DashCap.ToDirect2D()
};
var dashes = new float[0];
float[] dashes = null;//If we new an array first that we may waste it when we use `dashes = pen.DashStyle.Dashes.Select(x => (float)x).ToArray()`. As a frequently used code I think this optimize is necessary.

This comment has been minimized.

@grokys

grokys Apr 12, 2018

Member

Could you remove this comment too? We really don't need a comment explaining what this is doing, it's pretty obvious ;)

This comment has been minimized.

@lindexi
}

//If we don't enter the code above that it is null. We should set it a default value to solve the null exception.

This comment has been minimized.

@grokys

grokys Apr 12, 2018

Member

And this one.

This comment has been minimized.

@lindexi
if (pen.DashStyle?.Dashes != null && pen.DashStyle.Dashes.Count > 0)
{
properties.DashStyle = DashStyle.Custom;
properties.DashOffset = (float)pen.DashStyle.Offset;
dashes = pen.DashStyle?.Dashes.Select(x => (float)x).ToArray();
properties.DashOffset = (float) pen.DashStyle.Offset;

This comment has been minimized.

@grokys

grokys Apr 12, 2018

Member

There's still a whitespace change here.

lindexi added some commits Apr 13, 2018

@grokys

grokys approved these changes Apr 16, 2018

grokys added some commits Apr 16, 2018

@grokys grokys merged commit 6eb380b into AvaloniaUI:master Apr 17, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@grokys

This comment has been minimized.

Copy link
Member

grokys commented Apr 17, 2018

Thanks @lindexi !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment