amerkoleci / Vortice.Windows

.NET bindings for Direct3D12, Direct3D11, WIC, Direct2D1, XInput, XAudio, X3DAudio, DXC, Direct3D9 and DirectInput.
MIT License
1.01k stars 73 forks source link

ID2D1DeviceContext.CreateImageBrush() ref and Nullable struct arguments #238

Closed gerhard17 closed 2 years ago

gerhard17 commented 2 years ago

In Vortice.D2D1 v2.1.2 / .net6.0 exists an unsymmetry in the parameters of the ID2D1DeviceContext.CreateImageBrush() method

Vortice.Direct2D1.ID2D1DeviceContext
public unsafe ID2D1ImageBrush CreateImageBrush(ID2D1Image image, ref ImageBrushProperties imageBrushProperties, BrushProperties? brushProperties)

which has the C syntax

HRESULT CreateImageBrush(
  [in]                ID2D1Image                          *image,
  [in, ref]           const D2D1_IMAGE_BRUSH_PROPERTIES & imageBrushProperties,
  [in, ref, optional] const D2D1_BRUSH_PROPERTIES &       brushProperties,
  [out]               ID2D1ImageBrush                     **imageBrush
);

I think the [in, ref] & syntax is used due to performance reasons. The structs can be large, so passing by reference may be of value.

Question:

In C# the imageBrushProperties are passed by ref vs. the brushProperties are passed by value. Why this difference?

I think neither of the both is really correct:

  1. imageBrushProperties are passed performant by reference, but it cant be used with readonly fields.
  2. brushProperties are passed by value, which can handle readonly fields, but is not as performant as passing a reference.

Idea:

Would it be possible to pass both arguments as C# 'in' parameters, which matches the C [in, ref] syntax closer? This is performant in the large struct case and can also handle usages with readonly fields. At least imageBrushProperties should be an 'in' argument.

Remark 1:

To get the 'ref' out of the 'in' without struct copy

System.Runtime.CompilerServices.Unsafe
public static ref T AsRef<T>(in T source)

could be used.

Remark 2:

Another potential performance problem (also with other APIs which are using Nullable) is that Nullable involves multiple struct copies. A first one at the caller side when creating the Nullable and a second one when extracting the value out of it. Maybe the extraction can be done with unsafe code and struct aliasing to get a pinnable ref directly.

Regards, Gerhard

gerhard17 commented 2 years ago

Same ref argument code gen in

ID2D1Bitmap1 CreateBitmap(SizeI size, IntPtr sourceData, int pitch, ref BitmapProperties1 bitmapProperties)
HRESULT CreateBitmap(
  D2D1_SIZE_U                    size,
  const D2D1_BITMAP_PROPERTIES & bitmapProperties,
  ID2D1Bitmap                    **bitmap
);
amerkoleci commented 2 years ago

This commit https://github.com/amerkoleci/Vortice.Windows/commit/4cf53b63f3900c8b98840028e958b07fc1aa6749 fixes those issues.

gerhard17 commented 2 years ago

All IWICImageEncoder methods have also a ref WICImageParameters imageParameters argument, which should be 'in'