Open AhmedZero opened 2 years ago
Hey, sorry for the late response. Taxes consumed all spare energy I had this weekend.
why don't use the ref keyword in the pointer parameter instead?
By default everything is exposed roughly the same as it is in C++. Since this parameter is a pointer in C++ it's a pointer in C# too.
In theory one could write a transformation to automatically convert all pointer parameters to C# byrefs, but I'd be hesitant to do this because it's not always the best thing to do. (Particularly for APIs where the pointer is allowed to be null since I'd rather not assume people know about Unsafe.NullRef<T>
or force them to use it for common APIs like ImGui::Begin
.)
use
UnmanagedType.LPArray
and use string[];
In general we avoid using .NET's built-in marshaler because it's pretty bad performance-wise.
It's also being deprecated in .NET 7 in favor of more C#-centric approaches like the upcoming LibraryImportGenerator
or Biohazrd.
In this specific case, I simply haven't had a chance to really look at the best way to expose this function in a more developer-friendly way.
In general I want the developer friendly wrappers to guide people towards a pit of success performance-wise, especially for a library where hundreds or even thousands of function calls are being done in a fraction of a 16ms window.
Since this is and ComboBox
are the only APIs like this in Dear ImGui, I'd probably be inclined to just write human-friendly wrappers for them manually. (Probably using a shared UTF8 buffer and the function pointer overload instead of the string array one, but I'd want to sit down and think about it more.)
If you need something more friendly today, I'd probably just use BeginListBox
/Selectable
/EndListBox
directly. (That's basically all ImGui::ListBox
is anyway, minus some ImGuiListClipper
logic which only really matters if your list box has a huge number of items -- in which case you probably need to special case things from C# anyway to avoid the overhead of constant UTF16 -> UTF8 conversions.)
I know about marshaller performance. Anyway, I simply use Unsafe.AsPointer
instead of using a lot of fixed
keywords.
I would be careful about using Unsafe.AsPointer
since it'd be easy to accidentally create a GC hole which can lead to very random crashes. You'd be better off making a helper method which exposes ref
and uses fixed
internally. (fixed
has basically no performance overhead.)
I don't think it has a problem with static variables.
ldarg.0 conv.u ret
I don't think it has a problem with static variables.
Today I learned. Not sure if that's an implementation detail I'd want to rely on, but neat nonetheless.
FYI, according to Pro .NET Memory that assumption only applies to primitive types. All user-defined structs used for static fields are allocated in boxes on the normal GC heap. (You can observe this pretty easily using this snippet.)
Yes it works well with primitive types.
I have an idea to make an option if anyone wants a ref
keyword, generate it with the ref
keyword.
I still don't think it's a good idea to provide overloads that use ref
for all pointer parameters across the whole API surface, so I would hesitate to provide an official option without additional extra care.
That being said if you'd like to experiment with doing so yourself, here's a quick and dirty transformation which adds such overloads:
using Biohazrd;
using Biohazrd.CSharp;
using Biohazrd.CSharp.Trampolines;
using Biohazrd.Transformation;
using System.Linq;
namespace Mochi.DearImGui.Generator;
// This transformation is pretty naive and many of the overloads it generates don't make any sense. Use at your own risk!
internal sealed class MakeRefOverloadsTransformation : TransformationBase
{
protected override TransformationResult TransformFunction(TransformationContext context, TranslatedFunction declaration)
{
if (!declaration.Metadata.TryGet(out TrampolineCollection trampolines))
{ return declaration; }
// Create trampolines which adapt pointers to C# byrefs.
// We don't want to just loop over the entire trampoline collection because we don't want to adapt a non-primary native trampoline
Trampoline? TryMakeTrampoline(Trampoline targetTrampoline)
{
TrampolineBuilder builder = new(targetTrampoline, useAsTemplate: true);
foreach (Adapter adapter in targetTrampoline.Adapters)
{
// Skip parameters which don't accept input
if (!adapter.AcceptsInput)
{ continue; }
// If the parameter isn't a pointer we don't care about it
if (adapter.InputType is not PointerTypeReference pointerType)
{ continue; }
// If the parameter is a `const byte*`, we skip it since it's probably meant to be a string
if (pointerType is { Inner: CSharpBuiltinTypeReference cSharpPointee, InnerIsConst: true } && cSharpPointee == CSharpBuiltinType.Byte)
{ continue; }
// If the parameter is `void*` we skip it since `ref void` isn't valid C#
if (pointerType.Inner is VoidTypeReference)
{ continue; }
// If we got this far, adapt the parameter to a C# byref
builder.AdaptParameter(adapter, new ByRefAdapter(adapter, ByRefKind.Ref));
}
return builder.HasAdapters ? builder.Create() : null;
}
#if false
// Handle pointer parameters in the primary trampoline
{
if (TryMakeTrampoline(trampolines.PrimaryTrampoline) is Trampoline trampoline)
{ declaration = declaration.WithSecondaryTrampoline(trampoline); }
}
// Handle pointer parameters in the secondary trampoline
foreach (Trampoline secondaryTrampoline in trampolines.SecondaryTrampolines)
{
if (TryMakeTrampoline(secondaryTrampoline) is Trampoline trampoline)
{ declaration = declaration.WithSecondaryTrampoline(trampoline); }
}
#else
// Only adapt pointer parameters for the "last" trampoline
// (This should be either the primary trampoline or the interpolated string handler overload. Not the most robust way to identify it but it gets the job done.)
{
if (TryMakeTrampoline(trampolines.Last()) is Trampoline trampoline)
{ declaration = declaration.WithSecondaryTrampoline(trampoline); }
}
#endif
return declaration;
}
}
This transformation should be run immediately after ImGuiCreateStringWrappersTransformation
.
IE: Modify the are around Program.cs:136 to be:
// ...
library = new ImGuiCreateStringWrappersTransformation().Transform(library);
library = new MakeRefOverloadsTransformation().Transform(library);
library = new StripUnreferencedLazyDeclarationsTransformation().Transform(library);
// ...
Thanks 😁.
why don't use the ref keyword in the pointer parameter instead? and function like ImGui.ListBox
use
UnmanagedType.LPArray
and use string[];