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

C#: Optimize Variant conversion callbacks #68310

Merged
merged 1 commit into from Nov 25, 2022
Merged

C#: Optimize Variant conversion callbacks #68310

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 3 additions & 4 deletions modules/mono/editor/bindings_generator.cpp
View file
Open in desktop
Expand Up @@ -2274,7 +2274,7 @@ Error BindingsGenerator::_generate_cs_signal(const BindingsGenerator::TypeInterf
p_output.append(");\n");

// Generate Callable trampoline for the delegate
p_output << MEMBER_BEGIN "private static unsafe void " << p_isignal.proxy_name << "Trampoline"
p_output << MEMBER_BEGIN "private static void " << p_isignal.proxy_name << "Trampoline"
<< "(object delegateObj, NativeVariantPtrArgs args, out godot_variant ret)\n"
<< INDENT1 "{\n"
<< INDENT2 "Callable.ThrowIfArgCountMismatch(args, " << itos(p_isignal.arguments.size()) << ");\n"
Expand All @@ -2289,9 +2289,8 @@ Error BindingsGenerator::_generate_cs_signal(const BindingsGenerator::TypeInterf
p_output << ",";
}

// TODO: We don't need to use VariantConversionCallbacks. We have the type information so we can use [cs_variant_to_managed] and [cs_managed_to_variant].
p_output << "\n" INDENT3 "VariantConversionCallbacks.GetToManagedCallback<"
<< arg_type->cs_type << ">()(args[" << itos(idx) << "])";
p_output << sformat(arg_type->cs_variant_to_managed,
"args[" + itos(idx) + "]", arg_type->cs_type, arg_type->name);

idx++;
}
Expand Down
53 changes: 10 additions & 43 deletions modules/mono/glue/GodotSharp/GodotSharp/Core/Array.cs
View file
Open in desktop
Expand Up @@ -495,35 +495,10 @@ public sealed class Array<[MustBeVariant] T> :
private static Array<T> FromVariantFunc(in godot_variant variant) =>
VariantUtils.ConvertToArrayObject<T>(variant);

// ReSharper disable StaticMemberInGenericType
// Warning is about unique static fields being created for each generic type combination:
// https://www.jetbrains.com/help/resharper/StaticMemberInGenericType.html
// In our case this is exactly what we want.

private static readonly unsafe delegate* managed<in T, godot_variant> ConvertToVariantCallback;
private static readonly unsafe delegate* managed<in godot_variant, T> ConvertToManagedCallback;

// ReSharper restore StaticMemberInGenericType

static unsafe Array()
{
VariantConversionCallbacks.GenericConversionCallbacks[typeof(Array<T>)] =
(
(IntPtr)(delegate* managed<in Array<T>, godot_variant>)&ToVariantFunc,
(IntPtr)(delegate* managed<in godot_variant, Array<T>>)&FromVariantFunc
);

ConvertToVariantCallback = VariantConversionCallbacks.GetToVariantCallback<T>();
ConvertToManagedCallback = VariantConversionCallbacks.GetToManagedCallback<T>();
}

private static unsafe void ValidateVariantConversionCallbacks()
{
if (ConvertToVariantCallback == null || ConvertToManagedCallback == null)
{
throw new InvalidOperationException(
$"The array element type is not supported for conversion to Variant: '{typeof(T).FullName}'.");
}
VariantUtils.GenericConversion<Array<T>>.ToVariantCb = &ToVariantFunc;
VariantUtils.GenericConversion<Array<T>>.FromVariantCb = &FromVariantFunc;
}

private readonly Array _underlyingArray;
Expand All @@ -539,8 +514,6 @@ private static unsafe void ValidateVariantConversionCallbacks()
/// </summary>
public Array()
{
ValidateVariantConversionCallbacks();

_underlyingArray = new Array();
}

Expand All @@ -551,8 +524,6 @@ public Array()
/// <returns>A new Godot Array.</returns>
public Array(IEnumerable<T> collection)
{
ValidateVariantConversionCallbacks();

if (collection == null)
throw new ArgumentNullException(nameof(collection));

Expand All @@ -569,8 +540,6 @@ public Array(IEnumerable<T> collection)
/// <returns>A new Godot Array.</returns>
public Array(T[] array) : this()
{
ValidateVariantConversionCallbacks();

if (array == null)
throw new ArgumentNullException(nameof(array));

Expand All @@ -586,8 +555,6 @@ public Array(T[] array) : this()
/// <param name="array">The untyped array to construct from.</param>
public Array(Array array)
{
ValidateVariantConversionCallbacks();

_underlyingArray = array;
}

Expand Down Expand Up @@ -665,7 +632,7 @@ public void Shuffle()
get
{
_underlyingArray.GetVariantBorrowElementAt(index, out godot_variant borrowElem);
return ConvertToManagedCallback(borrowElem);
return VariantUtils.ConvertTo<T>(borrowElem);
}
set
{
Expand All @@ -675,7 +642,7 @@ public void Shuffle()
godot_variant* ptrw = NativeFuncs.godotsharp_array_ptrw(ref self);
godot_variant* itemPtr = &ptrw[index];
(*itemPtr).Dispose();
*itemPtr = ConvertToVariantCallback(value);
*itemPtr = VariantUtils.CreateFrom(value);
}
}

Expand All @@ -685,9 +652,9 @@ public void Shuffle()
/// </summary>
/// <param name="item">The item to search for.</param>
/// <returns>The index of the item, or -1 if not found.</returns>
public unsafe int IndexOf(T item)
public int IndexOf(T item)
{
using var variantValue = ConvertToVariantCallback(item);
using var variantValue = VariantUtils.CreateFrom(item);
var self = (godot_array)_underlyingArray.NativeValue;
return NativeFuncs.godotsharp_array_index_of(ref self, variantValue);
}
Expand All @@ -700,12 +667,12 @@ public unsafe int IndexOf(T item)
/// </summary>
/// <param name="index">The index to insert at.</param>
/// <param name="item">The item to insert.</param>
public unsafe void Insert(int index, T item)
public void Insert(int index, T item)
{
if (index < 0 || index > Count)
throw new ArgumentOutOfRangeException(nameof(index));

using var variantValue = ConvertToVariantCallback(item);
using var variantValue = VariantUtils.CreateFrom(item);
var self = (godot_array)_underlyingArray.NativeValue;
NativeFuncs.godotsharp_array_insert(ref self, index, variantValue);
}
Expand Down Expand Up @@ -736,9 +703,9 @@ public void RemoveAt(int index)
/// </summary>
/// <param name="item">The item to add.</param>
/// <returns>The new size after adding the item.</returns>
public unsafe void Add(T item)
public void Add(T item)
{
using var variantValue = ConvertToVariantCallback(item);
using var variantValue = VariantUtils.CreateFrom(item);
var self = (godot_array)_underlyingArray.NativeValue;
_ = NativeFuncs.godotsharp_array_add(ref self, variantValue);
}
Expand Down