Created attachment 322261 [details] [review]
GDK W32: Fix the filling of MINMAXINFO

1) MSDN says that the primary display should be used to calculate
   maximized window size, while we were using the display that
   is "closest" to the window
2) MSDN says that max tracking size is a system property, we
   should just call GetSystemMetrics() and use that.

Review of attachment 322261 [details] [review]:

Looks good to me.

Created attachment 322283 [details] [review]
GDK W32: Fix the filling of MINMAXINFO

v2:
* Finally sorted it out. The source of the problem was the fact
  that we used work area of the nearest display as-is, whereas
  W32 expected it the minmax info to be set in terms of the
  primary display.

Review of attachment 322283 [details] [review]:

Hi LRN,

I think the code looks good to me.  We could however use GetMonitorInfo() instead of GetMonitorInfoA(), which will select the appropriate function (GetMonitorInfoA() or GetMonitorInfoW()) at compile time depending on whether UNICODE is defined during the build though (unsure about it on MinGW, which I presume you are using some variant of it, let me know)

(In reply to Fan, Chun-wei from comment #4)
> We could however use GetMonitorInfo()
> instead of GetMonitorInfoA(), which will select the appropriate function
> (GetMonitorInfoA() or GetMonitorInfoW()) at compile time depending on
> whether UNICODE is defined during the build though

I absolutely hate this Unicode/ANSI switch that MS uses. All modern applications must use UTF-8 and (when necessary) UTF-16. ANSI function can be used when no string data is involved (such as GetMonitorInfo() - MONITORINFO structure doesn't have strings, only MONITORINFOEX does). TCHAR, UNICODE and FunctionNameWithoutAOrWSuffix() are absolutely unacceptable, IMO.

> (unsure about it on
> MinGW, which I presume you are using some variant of it, let me know)

MinGW-w64 has:
  typedef struct tagMONITORINFO {
    DWORD cbSize;
    RECT rcMonitor;
    RECT rcWork;
    DWORD dwFlags;
  } MONITORINFO,*LPMONITORINFO;

#define GetMonitorInfo __MINGW_NAME_AW(GetMonitorInfo)

  WINUSERAPI WINBOOL WINAPI GetMonitorInfoA(HMONITOR hMonitor,LPMONITORINFO lpmi);
  WINUSERAPI WINBOOL WINAPI GetMonitorInfoW(HMONITOR hMonitor,LPMONITORINFO lpmi);

so while using GetMonitorInfo() would /technically/ work, i would rather explicitly call GetMonitorInfoA() or GetMonitorInfoW().

If MS PSDK has MONITORINFOA and MONITORINFOW, then i would see why this code could be problematic for you. If that is the case, i would suggest switching to explicit GetMonitorInfoExW() and MONITORINFOEXW.

Okay, that was a bit misinformed. There's no GetMonitorInfoEx[A|W]() function. GetMonitorInfoA() and GetMonitorInfoW() simply accept a pointer to MONITORINFOEXA or MONITORINFOEXW, with cbSize set appropriately. That explains why there's no MONITORINFO[A|W] where GetMonitorInfo[A|W]() does exist.

Hi LRN,
(In reply to LRN from comment #5)
> I absolutely hate this Unicode/ANSI switch that MS uses. All modern
> applications must use UTF-8 and (when necessary) UTF-16. ANSI function can
> be used when no string data is involved (such as GetMonitorInfo() -
> MONITORINFO structure doesn't have strings, only MONITORINFOEX does). TCHAR,
> UNICODE and FunctionNameWithoutAOrWSuffix() are absolutely unacceptable, IMO.

I agree with you on this, totally... I think this has to blame on the backward compatibility of Windows.

As mentioned, though, in the previous comment, I think the patch looks good.

The reason I talked about simply using GetMonitorInfo() is because we don't involve ourselves with strings here (i.e., only MONITORINFO, as you mentioned in the other bug that led to this sub-bug-report), so it's safe for our case here, whether or not we use [/DUNICODE|-DUNICODE].  The [/DUNICODE|-DUNICODE] issue though, would be more like a universal compile-time cflag (i.e. the one we would set in the project file settings or in AMCFLAGS (if I got that name right) in configure.ac).  There is no MONITORINFOA nor MONITORINFOW in the Windows SDK.  As no device name is involved there in our case, so we don't need to use the additional space involved with MONITORINFOEXW.

MS is deprecating the ANSI variants of the Windows APIs though, so that's why I am bringing it up here, but as you mentioned, we could still use A variant to be safer still, and switch to the W variant later to make sure things are clear and ok when we do the transition.  Warning C4133 in Visual Studio would be our friend to find the char<->wchar_t issues when using the Windows APIs when we switch form the ANSI versions to the wide versions, but obviously that would be an issue for another series of bug reports for the transitions...

Thanks though, with blessings, and cheers!

Attachment 322283 [details] pushed as c61764b - GDK W32: Fix the filling of MINMAXINFO