IWICBitmapSource.CopyPixels last parameter should allow `byte*` and/or `Span<byte>`

[harborsiem]

Actual behavior

HRESULT Windows.Win32.Graphics.Imaging.IWICBitmapSource.CopyPixels(WICRect* prc, uint cbStride, uint cbBufferSize, byte[] pbBuffer).
I think the byte array is wrong here, because the user have no information about the size of the array before calling CopyPixels.
Also the extended versions for this call seems to be wrong.

Expected behavior

unsafe HRESULT Windows.Win32.Graphics.Imaging.IWICBitmapSource.CopyPixels(WICRect* prc, uint cbStride, uint cbBufferSize, byte* pbBuffer).

Repro steps

1. NativeMethods.txt content:

IWICBitmapSource

2. NativeMethods.json content (if present):

  "$schema": "https://aka.ms/CsWin32.schema.json",
  "emitSingleFile": true,
  "public": true,
  "comInterop": {
    "useIntPtrForComOutPointers": true,
    "preserveSigMethods": [
      "IWICBitmapSource",
    ]
  },
  "useSafeHandles": false

3. Any of your own code that should be shared?

Context

• CsWin32 version: 0.2.219-beta
• Win32Metadata version (if explicitly set by project):
• Target Framework: [e.g. netstandard2.0]
• LangVersion (if explicitly set by project): [e.g. 9]

[AArnott]

I think the byte array is wrong here, because the user have no information about the size of the array before calling CopyPixels.
Also the extended versions for this call seems to be wrong.

Are you actually seeing a failure here, or is this just a speculative concern?

I see the generated method is:

void CopyPixels(winmdroot.Graphics.Imaging.WICRect* prc, uint cbStride, uint cbBufferSize, [Out] [MarshalAs(UnmanagedType.LPArray,SizeParamIndex = 2)] byte[] pbBuffer);

This looks correct to me, since the documentation for this method shows the user should allocate the array and then pass its length in as the 3rd parameter so the native side knows the length of the array. The native function can write to the array as documented, then you can read it afterward. So I don't see the problem.

That said, I am actually a bit surprised that the original method uses byte[] instead of byte* since we tend to emit the most performant methods and then add a friendly overload that would take the array or Span<byte> as a convenience.

[harborsiem]

It is just a speculative concern.
Now I noticed that I have to do some more calculation for the buffer size from user side before I can call CopyPixels. The documentation for this function didn't say how to get the cbStride or how to calculate cbBufferSize, so I am a little bit confused about using this function.

[AArnott]

I've never used this function myself and generally this repo can't provide guidance for individual functions. General purpose win32 forums would be better for that. Although you can try using the Discussions tab to ask other community members about how to use CopyPixels if you want.
Can we close the issue for now since it sounds like there is no defect here?

[harborsiem]

I think you are right, that there is no defect here. But for a performance standpoint I think it will be better to have a byte* for the last parameter. If you want to create a System.Drawing.Bitmap from this function you can do this

uint stride, bufferSize;
IWICBitmap wicBitmap;
uint width, height;
//fill stride, bufferSize from some WIC functions I do not know yet, create the wicBitmap
wicBitmap.GetSize(out width, out height);                    
Bitmap alpha = new Bitmap((int)width, (int)height, PixelFormat.Format32bppArgb);
BitmapData alphaData = alpha.LockBits(new Rectangle(new Point(), alpha.Size), ImageLockMode.ReadWrite, alpha.PixelFormat);
wicBitmap.CopyPixels(null, stride, bufferSize, (byte*)alphaData.Scan0);
alpha.UnlockBits(alphaData);

If you only have the byte[], you have to do a Marshal.Copy to copy the byte[] to alphaData.Scan0. In this case you have two marshalling functions.

[AArnott]

As a workaround for now, for performance intensive applications consider setting allowMarshaling: false in NativeMethods.json, which will get you the pointer-typed parameter you want.

[AArnott]

So I did a little more research into this. I wanted to get back to a byte* parameter type with a Span<byte> friendly overload for cases like yours. But with #878 I learned that pointers on COM interfaces are generally not desirable because they block remoting interfaces, and CsWin32 cannot predict which interfaces may be used in a remoting setting.

So I think we'll have to keep the byte[] parameter type here. Of course the workaround I mentioned in my prior message about allowMarshaling: false can avoid that requirement if you're willing to give up interfaces completely.