Skip to content
This repository has been archived by the owner on Jul 26, 2023. It is now read-only.

System.Drawing.Point instead of POINT? #420

Closed
mpolicki opened this issue Jan 14, 2019 · 11 comments
Closed

System.Drawing.Point instead of POINT? #420

mpolicki opened this issue Jan 14, 2019 · 11 comments
Labels

Comments

@mpolicki
Copy link

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
Copy link
Collaborator

AArnott commented Jan 15, 2019

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
Copy link
Author

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
Copy link
Collaborator

AArnott commented Jan 16, 2019

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
Copy link
Collaborator

AArnott commented Jan 16, 2019

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

@mpolicki
Copy link
Author

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
Copy link
Collaborator

AArnott commented Jan 16, 2019

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
Copy link
Collaborator

AArnott commented Jan 16, 2019

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
Copy link
Author

mpolicki commented Jan 16, 2019

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
Copy link
Collaborator

AArnott commented Jan 16, 2019

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.

@AArnott AArnott reopened this Jan 16, 2019
@mpolicki
Copy link
Author

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
Copy link
Collaborator

AArnott commented Jan 17, 2019

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants