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

Optimize Matrix.Elements #47932

Merged
merged 3 commits into from
Feb 9, 2021
Merged

Optimize Matrix.Elements #47932

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0ef8b9d
Optimize Matrix.Elements
JeremyKuhne Feb 5, 2021
8ec48a6
Move more element usage to the stack.
JeremyKuhne Feb 6, 2021
9d1119b
Remove DeleteObject call
JeremyKuhne Feb 6, 2021
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
40 changes: 26 additions & 14 deletions src/libraries/System.Drawing.Common/src/System/Drawing/Drawing2D/Matrix.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Runtime.InteropServices;
using Gdip = System.Drawing.SafeNativeMethods.Gdip;
Expand Down Expand Up @@ -86,25 +87,35 @@ public float[] Elements
{
get
{
IntPtr buf = Marshal.AllocHGlobal(6 * 8); // 6 elements x 8 bytes (float)
JeremyKuhne marked this conversation as resolved.
Show resolved Hide resolved
float[] elements = new float[6];
GetElements(elements);
return elements;
}
}

try
{
Gdip.CheckStatus(Gdip.GdipGetMatrixElements(new HandleRef(this, NativeMatrix), buf));
internal unsafe void GetElements(Span<float> elements)
{
Debug.Assert(elements.Length >= 6);

float[] m = new float[6];
Marshal.Copy(buf, m, 0, 6);
return m;
}
finally
{
Marshal.FreeHGlobal(buf);
}
fixed (float* m = elements)
{
Gdip.CheckStatus(Gdip.GdipGetMatrixElements(new HandleRef(this, NativeMatrix), m));
}
}

public float OffsetX => Elements[4];
public float OffsetY => Elements[5];
public unsafe float OffsetX => Offset.X;

public unsafe float OffsetY => Offset.Y;

internal unsafe PointF Offset
{
get
{
Span<float> elements = stackalloc float[6];
GetElements(elements);
return new PointF(elements[4], elements[5]);
}
}

public void Reset()
{
Expand Down Expand Up @@ -263,6 +274,7 @@ public bool IsIdentity
return isIdentity != 0;
}
}

public override bool Equals([NotNullWhen(true)] object? obj)
{
if (!(obj is Matrix matrix2))
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Drawing.Common/src/System/Drawing/GdiplusNative.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -800,7 +800,7 @@ internal static unsafe partial class Gdip
internal static extern int GdipVectorTransformMatrixPointsI(HandleRef matrix, Point* pts, int count);

[DllImport(LibraryName, ExactSpelling = true)]
internal static extern int GdipGetMatrixElements(HandleRef matrix, IntPtr m);
internal static extern unsafe int GdipGetMatrixElements(HandleRef matrix, float* m);

[DllImport(LibraryName, ExactSpelling = true)]
internal static extern int GdipIsMatrixInvertible(HandleRef matrix, out int boolean);
Expand Down
4 changes: 1 addition & 3 deletions src/libraries/System.Drawing.Common/src/System/Drawing/Graphics.Windows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -697,9 +697,7 @@ public object GetContextInfo()

if (!cumulTransform.IsIdentity)
{
float[] elements = cumulTransform.Elements;
currentOffset.X = elements[4];
currentOffset.Y = elements[5];
currentOffset = cumulTransform.Offset;
}

GraphicsContext? context = _previousContext;
Expand Down
4 changes: 1 addition & 3 deletions src/libraries/System.Drawing.Common/src/System/Drawing/GraphicsContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,7 @@ public GraphicsContext(Graphics g)
Matrix transform = g.Transform;
if (!transform.IsIdentity)
{
float[] elements = transform.Elements;
_transformOffset.X = elements[4];
_transformOffset.Y = elements[5];
_transformOffset = transform.Offset;
}
transform.Dispose();

Expand Down
15 changes: 11 additions & 4 deletions src/libraries/System.Drawing.Common/src/System/Drawing/Icon.Windows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Buffers;
using System.ComponentModel;
using System.Diagnostics;
using System.Drawing.Drawing2D;
using System.Drawing.Imaging;
using System.Drawing.Internal;
using System.IO;
Expand Down Expand Up @@ -380,6 +381,7 @@ private void DrawIcon(IntPtr dc, Rectangle imageRect, Rectangle targetRect, bool
finally
{
RestoreClipRgn(dc, hSaveRgn);
Interop.Gdi32.DeleteObject(hSaveRgn);
JeremyKuhne marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down Expand Up @@ -416,8 +418,11 @@ internal void Draw(Graphics graphics, int x, int y)
internal void Draw(Graphics graphics, Rectangle targetRect)
{
Rectangle copy = targetRect;
copy.X += (int)graphics.Transform.OffsetX;
copy.Y += (int)graphics.Transform.OffsetY;

using Matrix transform = graphics.Transform;
Copy link
Member

@stephentoub stephentoub Feb 7, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Wow, a property that allocates and returns a new finalizable object on every access... awesome ;-)

Copy link
Member Author

Choose a reason for hiding this comment

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

I've got a proposal up to help address this, um, suboptimal pattern: #47940

PointF offset = transform.Offset;
copy.X += (int)offset.X;
copy.Y += (int)offset.Y;

using (WindowsGraphics wg = WindowsGraphics.FromGraphics(graphics, ApplyGraphicsProperties.Clipping))
{
Expand All @@ -433,8 +438,10 @@ internal void Draw(Graphics graphics, Rectangle targetRect)
internal void DrawUnstretched(Graphics graphics, Rectangle targetRect)
{
Rectangle copy = targetRect;
copy.X += (int)graphics.Transform.OffsetX;
copy.Y += (int)graphics.Transform.OffsetY;
using Matrix transform = graphics.Transform;
PointF offset = transform.Offset;
copy.X += (int)offset.X;
copy.Y += (int)offset.Y;

using (WindowsGraphics wg = WindowsGraphics.FromGraphics(graphics, ApplyGraphicsProperties.Clipping))
{
Expand Down
9 changes: 5 additions & 4 deletions src/libraries/System.Drawing.Common/src/misc/GDI/WindowsGraphics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ public static WindowsGraphics FromGraphics(Graphics g, ApplyGraphicsProperties p
Debug.Assert(g != null, "null Graphics object.");

WindowsRegion? wr = null;
float[]? elements = null;

PointF offset = default;

Region? clipRgn = null;
Matrix? worldTransf = null;
Expand All @@ -71,7 +72,7 @@ public static WindowsGraphics FromGraphics(Graphics g, ApplyGraphicsProperties p
{
if ((properties & ApplyGraphicsProperties.TranslateTransform) != 0)
{
elements = worldTransf.Elements;
offset = worldTransf.Offset;
}

worldTransf.Dispose();
Expand Down Expand Up @@ -110,10 +111,10 @@ public static WindowsGraphics FromGraphics(Graphics g, ApplyGraphicsProperties p
}
}

if (elements != null)
if (offset != default)
JeremyKuhne marked this conversation as resolved.
Show resolved Hide resolved
{
// elements (XFORM) = [eM11, eM12, eM21, eM22, eDx, eDy], eDx/eDy specify the translation offset.
wg.DeviceContext.TranslateTransform((int)elements[4], (int)elements[5]);
wg.DeviceContext.TranslateTransform((int)offset.X, (int)offset.Y);
}

return wg;
Expand Down