CsWin32 generates broken wrapper for ExtTextOut

DJm00n commented 2 years ago

Actual behavior

Seems CsWin32 generates ReadOnlySpan where it should't: I seening string lpString, ReadOnlySpan<int> lpDx where in original there is winmdroot.Foundation.PCWSTR lpString, uint c, [Optional] int* lpDx where c is The length of the string pointed to by lpString. but it get receives (uint )lpDx.Length in wrapper instead.

public static partial class PInvoke
{
    /// <inheritdoc cref="ExtTextOut(winmdroot.Graphics.Gdi.HDC, int, int, winmdroot.Graphics.Gdi.ETO_OPTIONS, winmdroot.Foundation.RECT*, winmdroot.Foundation.PCWSTR, uint, int*)"/>
    public static unsafe winmdroot.Foundation.BOOL ExtTextOut(SafeHandle hdc, int x, int y, winmdroot.Graphics.Gdi.ETO_OPTIONS options, winmdroot.Foundation.RECT? lprect, string lpString, ReadOnlySpan<int> lpDx)
    {
        bool hdcAddRef = false;
        try
        {
            fixed (int* lpDxLocal = lpDx)
            {
                fixed (char* lpStringLocal = lpString)
                {
                    winmdroot.Graphics.Gdi.HDC hdcLocal;
                    if (hdc is object)
                    {
                        hdc.DangerousAddRef(ref hdcAddRef);
                        hdcLocal = (winmdroot.Graphics.Gdi.HDC)hdc.DangerousGetHandle();
                    }
                    else
                        hdcLocal = default(winmdroot.Graphics.Gdi.HDC);
                    winmdroot.Foundation.RECT lprectLocal = lprect.HasValue ? lprect.Value : default(winmdroot.Foundation.RECT);
                    winmdroot.Foundation.BOOL __result = PInvoke.ExtTextOut(hdcLocal, x, y, options, lprect.HasValue ? &lprectLocal : null, lpStringLocal, (uint )lpDx.Length, lpDxLocal);
                    return __result;
                }
            }
        }
        finally
        {
            if (hdcAddRef)
                hdc.DangerousRelease();
        }
    }

    /// <summary>The ExtTextOut function draws text using the currently selected font, background color, and text color. You can optionally provide dimensions to be used for clipping, opaquing, or both.</summary>
    /// <param name="hdc">A handle to the device context.</param>
    /// <param name="x">The x-coordinate, in logical coordinates, of the reference point used to position the string.</param>
    /// <param name="y">The y-coordinate, in logical coordinates, of the reference point used to position the string.</param>
    /// <param name="options"></param>
    /// <param name="lprect">A pointer to an optional <a href="https://docs.microsoft.com/windows/desktop/api/windef/ns-windef-rect">RECT</a> structure that specifies the dimensions, in logical coordinates, of a rectangle that is used for clipping, opaquing, or both.</param>
    /// <param name="lpString">A pointer to a string that specifies the text to be drawn. The string does not need to be zero-terminated, since <i>cbCount</i> specifies the length of the string.</param>
    /// <param name="c">
    /// <para>The <a href="https://docs.microsoft.com/windows/desktop/gdi/specifying-length-of-text-output-string">length of the string</a> pointed to by <i>lpString</i>. This value may not exceed 8192.</para>
    /// <para><see href="https://docs.microsoft.com/windows/win32/api//wingdi/nf-wingdi-exttextoutw#parameters">Read more on docs.microsoft.com</see>.</para>
    /// </param>
    /// <param name="lpDx">A pointer to an optional array of values that indicate the distance between origins of adjacent character cells. For example, lpDx[<i>i</i>] logical units separate the origins of character cell <i>i</i> and character cell <i>i</i> + 1.</param>
    /// <returns>
    /// <para>If the string is drawn, the return value is nonzero. However, if the ANSI version of <b>ExtTextOut</b> is called with ETO_GLYPH_INDEX, the function returns <b>TRUE</b> even though the function does nothing. If the function fails, the return value is zero.</para>
    /// </returns>
    /// <remarks>
    /// <para><see href="https://docs.microsoft.com/windows/win32/api//wingdi/nf-wingdi-exttextoutw">Learn more about this API from docs.microsoft.com</see>.</para>
    /// </remarks>
    [DllImport("Gdi32", ExactSpelling = true, EntryPoint = "ExtTextOutW")]
    [DefaultDllImportSearchPaths(DllImportSearchPath.System32)]
    public static extern unsafe winmdroot.Foundation.BOOL ExtTextOut(winmdroot.Graphics.Gdi.HDC hdc, int x, int y, winmdroot.Graphics.Gdi.ETO_OPTIONS options, [Optional] winmdroot.Foundation.RECT* lprect, winmdroot.Foundation.PCWSTR lpString, uint c, [Optional] int* lpDx);
}

Expected behavior

uint c and int* lpDx should have separate parameters in generated ExtTextOut wrapper.

Repro steps

NativeMethods.txt content:
```
ExtTextOut
```

NativeMethods.json content:

{
"$schema": "https://aka.ms/CsWin32.schema.json",
"wideCharOnly": true,
"emitSingleFile": true,
"public": true
}

Context

CsWin32 version: 0.1.635-beta
Win32Metadata version (if explicitly set by project): implicit
Target Framework: net461;net472;netcoreapp3.1
LangVersion (if explicitly set by project): 9.0

AArnott commented 2 years ago

The way I read the docs for this API, we see that c is defined to be:

The length of the string pointed to by lpString.

So we know c == lpString.Length. But reading on...

For the ANSI version of ExtTextOut, the lpDx array has the same number of INT values as there are bytes in lpString. For DBCS characters, you can apportion the dx in the lpDx entries between the lead byte and the trail byte, as long as the sum of the two bytes adds up to the desired dx. For DBCS characters with the Unicode version of ExtTextOut, each Unicode glyph gets a single pdx entry.

We see that lpDx is tied to the length of lpString. That means by the transitive property, that c can be derived from the length of either lpString or lpDx because they are supposed to be equal. In fact this is what the underlying metadata for the API suggests as well:

public unsafe static extern BOOL ExtTextOutW([In] HDC hdc, [In] int x, [In] int y, [In] ETO_OPTIONS options, [Optional][In][Const] RECT lprect, [Optional][In][Const][NativeArrayInfo(CountParamIndex = 6)] PWSTR lpString, [In] uint c, [Optional][In][Const][NativeArrayInfo(CountParamIndex = 6)] int lpDx)

We could improve the wrapper to throw in such a case if the two arguments don't have equal lengths to help make this more clear, but I don't think we'll get around to that.

DJm00n commented 2 years ago

Thank you @AArnott! Seems that API is a bit more trickery than I thought. But I still didn’t get how to call current wrapper properly in case when I want to omit lpDx.

AArnott commented 2 years ago

Ooh! Good point. I see from the docs that lpDx is:

A pointer to an optional array of values

I'll have to review to see how we can best accommodate this.

microsoft / CsWin32