System.Drawing.Point instead of POINT?

[mpolicki]

Is there a specific reason System.Drawing.Point isn't being used instead of POINT?

It seems logical to use an existing type when one is available and works just as well.

[AArnott]

Yes: the System.Drawing.Point managed type can't be passed into native interop methods. The native POINT type has very specific layout requirements that our interop type satisfies. So while they semantically seem to be similar, they serve different audiences and require different types.

System.Windows.Forms itself also defines POINT for the same reasons.

[mpolicki]

Actually, I have tried declaring public static extern bool ScreenToClient(IntPtr hWnd, ref Point lpPoint); and public static extern bool ClientToScreen(IntPtr hWnd, ref Point lpPoint);. I have tested them and they have worked in my tests.

The reason it works is that from what I've seen, System.Drawing.Point, this project's POINT, and all those internal POINT types that you linked to have exactly the same structure and layout: two 32-bit int instance fields, x and y, in that order, and no other instance fields (see here).

With that in mind, all having a separate POINT type does is force the user to go through the extra step of converting instances to System.Drawing.Point to be able to work with them.

[AArnott]

The managed Point class lacks the struct attribute to enforce field layout, so in fact whole it might work in your test, the CLR reserves the right to break it in release builds, or on x64, or in a servicing release of the runtime.

When used for native interop, the field layout is critical.

[AArnott]

We could add an implicit conversion operator to our type so that it automatically converts to and from the windows forms type.

[mpolicki]

System.Drawing.Point is a struct, and per documentation

C#, Visual Basic, and C++ compilers apply the Sequential layout value to structures by default.

I very much doubt that's likely to change since backward compatibility is a top priority for the Framework.

[AArnott]

C#, Visual Basic, and C++ compilers apply the Sequential layout value to structures by default.

Whoa. I had no idea. Cool. :) I want to verify that, but if so, then that makes a good case for consolidating the types. If we don't mind a Windows Forms dependency in the library, which I'm not sure about.

[AArnott]

Confirmed. Sequential is default in C#.

That said, we target .NET Standard and PCL profiles with this library, so a WinForms dependency would be incompatible with those targets. I think an implicit conversion operator pair would probably alleviate the pain you're experience though. Do you agree?

[mpolicki]

System.Drawing.Point isn't actually in Windows Forms. Aside from the .NET Framework it's available in .NET Core from 1.0 and .NET Standard from 2.0. I couldn't tell whether the PCL profiles include it (EDIT: actually from the table at the bottom here it looks like they don't).

That being said, wouldn't having the conversion operators just introduce the same dependency?

[AArnott]

It wouldn't be quite the same dependency, since POINT can always exist, and only conditionally define the conversion operators on whatever target framework the assembly reference is allowed. But we can't conditionally define the POINT type itself -- it either has to be present or not, or else we can't use it in the p/invoke method signatures.

That said, as you say, apparently the .NET type is defined as far back as netstandard1.6, so that looks promising.

[mpolicki]

Would it be acceptable to conditionally define Point overloads? Otherwise, even with implicit conversion operators, the user would have to go through an intermediate variable when dealing with ref and out parameters. Plus, it would provide a smooth transition for potential deprecation as the old frameworks lose MS support.

[AArnott]

Good point about the implicit conversion still requiring intermediate locals.

Conditionally defining public API is restricted to only adding API as you move to higher target framework versions or from .NET Standard to compatible target platform, so that folks don't get MissingMethodException at runtime. That may be an option here, I don't know. I'll have to look more into it.