User32.GetMonitorInfo() has the wrong signature.

[HavenDV]

Correct code contains [In, Out] instead out keyword.
Tested on Windows 10 latest version.

[DllImport("User32.dll", CharSet = CharSet.Auto)]
internal static extern bool GetMonitorInfo(IntPtr hmonitor, [In, Out] MonitorInfoEx info);

[AArnott]

I see that our definition of it is:

pinvoke/src/User32/User32.cs

Lines 1814 to 1816 in bb42d00

	public static extern unsafe bool GetMonitorInfo(
	IntPtr hMonitor,
	[Friendly(FriendlyFlags.Out)] MONITORINFO* lpmi);

This is wrong because the Friendly attribute indicates the second parameter is only an out parameter when in fact the docs indicate the struct must be preinitialized with its own size prior to the call. As such, we need to be bidirectional about it.

@HavenDV, I haven't tested your proposed fix, but I would be surprised if that worked because you are passing a copy of the struct as the second parameter instead of a pointer to the struct as the header file and documentation indicate.
AFAIK the [In, Out] attributes do nothing to modify how the method behaves -- it only sets bits in the metadata that are for documentation purposes. They are no substitute for using the C# in, out or ref modifiers. ref would be the appropriate choice here, I believe.
You said you tested your fix though, so I find that fascinating, and will do the same.

[AArnott]

Confirmed: ref or a pointer works, but [In, Out] MonitorInfoEx results in an AV crashing the process.
So there is a bug here, but the resolution prescribed in the issue description is not it.
Thank you for calling out the bug, @HavenDV.

[HavenDV]

I think it's a little more complicated.
For me, and for some other users, the ref version of the signature does not work.
GetLastError returns: 'The parameter is incorrect'
https://stackoverflow.com/a/54501256/4792221

And as far as I know [Out] might be important:
https://stackoverflow.com/questions/33815276/what-is-the-difference-between-in-out-and-ref-when-using-pinvoke-in-c

P.S. What's interesting is that if I pass your structure to the ref method, then no error occurs. Unfortunately the DeviceName is of type char *, so I cannot use that.

P.S.S. I found a difference that requires the [Out] attribute instead of the ref. I used a class, not a struct.
In fact, using a class has the advantage that the user does not need to set initial parameters like this:

var info = new User32.MONITORINFOEX
{
    cbSize = Marshal.SizeOf(typeof(User32.MONITORINFOEX)),
};
GetMonitorInfo(hDesktop, ref info);

Class usage(In this case, the use of [Out] is necessary.):

[StructLayout(LayoutKind.Sequential, CharSet = CharSet.Auto, Pack = 4)]
internal class MonitorInfoEx
{
    public uint cbSize = (uint)Marshal.SizeOf(typeof(MonitorInfoEx));
    public RECT rcMonitor;
    public RECT rcWork;
    public uint dwFlags;
    [MarshalAs(UnmanagedType.ByValArray, SizeConst = 32)]
    public char[] szDevice = new char[32];
}

[DllImport("user32", CharSet = CharSet.Auto, SetLastError = true)]
internal static extern bool GetMonitorInfo(
    IntPtr hmonitor, 
    [In, Out] MonitorInfoEx info);

var info = new MonitorInfoEx();
GetMonitorInfo(hDesktop, info);

[HavenDV]

User32.GetCursorInfo should also be ref instead of out.

[Lakritzator]

It's not that complicated, ref works just fine with structs, but only if it's defined correctly.

Preferably do not use class for interop calls, although it might seem to work it causes a lot of headaches.
The behavior of a class, particularly when it comes to the memory location it occupies, is much less defined as a struct.
This would mean you will need to pin the memory location before most win32 APIs you would call, otherwise you might run into the odd case where the garbage collector decides to move the class without you know it, and passing a wrong pointer.
It might work mostly, but not always, which for me is a no-go on classes, a struct solves this.

I myself have a convenience method on every struct for creating, like here: https://github.com/dapplo/Dapplo.Windows/blob/master/src/Dapplo.Windows.User32/Structs/MonitorInfoEx.cs#L75

It's important to know that the DllImports here are trying to create as less overhead as possible, converting from a native char * to a .NET string costs time and memory, even marshalling this... The idea is only convert when something is used, which is more or less up to you.

I add convenience methods for dealing with most things like having a char * like here: https://github.com/dapplo/Dapplo.Windows/blob/master/src/Dapplo.Windows.User32/Structs/MonitorInfoEx.cs#L60
But they have the disadvantage that the conversion is done every time the Name property is used, also bad in some cases.
This is why the solution in this project, is called low level and tries to satisfy most generic needs, you will need to build your own stuff on top of it.

I created my project Dapplo.Windows a bit before this one, and it has a completely different focus (more modern & functional usage of Win32 APIs). I planned to refactor to use this with the great work of @AArnott and all contributors "under the covers", and remove all the overhead which is done double. Unfortunately I lack time 🤷‍♂️

Btw. You can read about interop best practices here: https://docs.microsoft.com/en-us/dotnet/standard/native-interop/best-practices

[HavenDV]

Thanks for the explanation. I do not use classes in Interop interactions, but it was interesting to know about their use and the [Out] attribute.

By the way, it seems to me that you are very experienced in WinAPI, perhaps you can answer my question, which I could not find on the Internet - User32.GetCursorPos sometimes (1-5%) returns a value that seems to take into account some factor, like DPI ... This happens inside a transparent WPF window when the Rectangle is created using mouse coordinates.

[Lakritzator]

@HavenDV
I rather not hijack an issue, but it might be an interesting discussion nevertheless.
Just to make sure I understand, you are requesting the GetCursorPos during the lifetime of your application, and sometimes a few calls just return different values? And what do you mean with the last part (rectangle created) of "This happens inside a transparent WPF window when the Rectangle is created using mouse coordinates."

Maybe this is on a system with displays which have different DPI settings?

I don't recall any Greenshot issues which confirm such a thing, but let me check and get back to you.

[HavenDV]

Apologies in advance for spamming this issue.

Maybe this is on a system with displays which have different DPI settings?

Yes, this happens on a triple screen system (1 HD 100% scale, 2 Primary 4K 150% scale, 3 HD 150%).
I am using a transparent window that covers all three screens.
I successfully translate the coordinates of the mouse into physical, and then into the coordinates of the window in order to select a rectangle in it by event.
Everything works fine except for rare artifacts when the rectangle jerks to the side. Trace of User32.GetCursorPos values ​​shows that sometimes (less than 5%, but often during initialization for the first few milliseconds) they can change x times (approximately by DPI scaleFactor). The mouse cursor remains in the same place.

P.S. The User32.GetCursorPos values ​​depend on the window under the cursor.
Trace values ​​User32.WindowFromPhysicalPoint (point) issues another window handle if the values ​​are incorrect.

// Trace.WriteLine($"{point.x}, {point.y}, {hwnd}, {dpiOfHWND}");
-678, 1213, 31787824, 96
-678, 1213, 31787824, 96
-452, 808, 221119456, 144
-676, 1214, 31787824, 96
-676, 1214, 31787824, 96

P.S.S
Unfortunately, I am testing and sometimes there are also incorrect values with dpi 96.

-1076, 1438, 18549032, 144
-717, 959, 31787824, 96
-1077, 1441, 31787824, 96
-1078, 1440, 18549032, 144

P.S.S.S
I fix the coordinates if dpiAwareness is greater than 0. But when moving the cursor to a new window, it seems to return one or more incorrect value.

Trace.WriteLine($"{point.x}, {point.y}, {hwnd}, {dpi}, {dpiFromAwarennessHwndContext}, {text}");
-634, 1199, 65992, 144, 0, FolderView
-634, 1199, 65992, 144, 0, FolderView
-634, 1199, 65992, 144, 0, FolderView
-634, 1198, 14814430, 144, 144, RectangleView
-423, 799, 65992, 144, 0, FolderView
-633, 1202, 65992, 144, 0, FolderView
-631, 1202, 65992, 144, 0, FolderView
-630, 1202, 65992, 144, 0, FolderView

[Lakritzator]

That is so recognizable, tracking an issue like that... The odd things I have seen in my time supporting and building Greenshot 😁

Just in case: Your application is correctly set to multi-monitor DPI Aware?

From my experience is working with transparent windows very tricky, events seem to pass through, this might explain why you suddenly get information from another window.

Try changing the alpha to not 100% transparent, I know this might not be what you want but just humor me!
(Instead of A=0, use A=1, which might be slightly visible...)

Another thing I have seen, is that some applications create & destroy windows all the time (yes really!!)
This might not be noticeable to the user, as they are not visible, but have some weird effects.

In the Win32 world, I sometimes have to come up with weird hacks, to make things work... 😞
Some workaround were just due to not knowing what happens, and might have been mitigated over time, some are still there.

[AArnott]

User32.GetCursorInfo should also be ref instead of out.

No it isn't. POINT has no size field, and per the docs, GetCursorInfo does not read the struct passed in prior to initializing its two fields, so there is no reason for the caller in initialize them, and thus no reason to use ref rather than out. These two modifiers are equivalent in terms of runtime behavior but ref makes c# require the caller to initialize the value first.

Unfortunately the DeviceName is of type char *, so I cannot use that.

Why not? All you need to do is new string(info.DeviceName) and you get a string from it. Are you unable or unwilling to use the unsafe keyword for some reason?

We generally avoid using marshal attributes in our structs because that causes .NET to see them as managed types and that severely limits where and how they can be used.

I found a difference that requires the [Out] attribute instead of the ref. I used a class, not a struct.

ah, ok that teaches me something. I have never tried using classes for interop when the native layer uses structs, but what you say makes sense.

In fact, using a class has the advantage that the user does not need to set initial parameters like this:

I guess so, since the ctor on your class runs first and can initialize the field. But on a struct in C#, the default ctor cannot be defined so the user has to initialize it themselves. To make this easier we usually define a Create static factory method on the struct that does the same as a ctor would. Such is the case on this very struct, BTW.
We use structs rather than classes to avoid forcing object allocations for interop.

more later...

[AArnott]

Some interop calls do very particular memory management that .NET marshaling cannot deal with as well, which is another reason we prefer non-managed/marshaled structs in general.

[HavenDV]

User32.GetCursorInfo should also be ref instead of out.

No it isn't. POINT has no size field, and per the docs, GetCursorInfo does not read the struct passed in prior to initializing its two fields, so there is no reason for the caller in initialize them, and thus no reason to use ref rather than out. These two modifiers are equivalent in terms of runtime behavior but ref makes c# require the caller to initialize the value first.

GetCursorInfo, not GetCursorPos

[HavenDV]

In the Win32 world, I sometimes have to come up with weird hacks, to make things work... 😞
Some workaround were just due to not knowing what happens, and might have been mitigated over time, some are still there.

In the end, I just started using LowLevelMouseHook instead of GetCursorPos and everything works as it should.

[Lakritzator]

I'm not sure if that is supported with pinvoke (this) but I have you covered with Dapplo.Windows.Input 😉

var mouseHookSubscription = MouseHook.MouseEvents.Subscribe(mouseEvent => your handler code);

[Lakritzator]

User32.GetCursorInfo definitively should be a ref, it needs a initialized struct.

[AArnott]

Ah, I see my mistake. I'll send another PR to fix GetCursorInfo.