[PATCH 2/9] winemac.drv: EnumDisplayDevices placeholder implementation

[Donat Enikeev]

Hi Ken,

> This doesn't build:

Thanks, will have a deeper look. Though since I don't have mac (even VM), it was aimed to bring just backward compatibility with the migration, while actual devices/displays enumeration, flags, etc -- are kept for someone having experience with Cocoa, with an ability to check results live on multi-display setup. 

> Beyond that, can you explain the rationale of moving the implementation of EnumDisplayDevices to the drivers?

Primarily - centralization and integrity: user32 knows nothing about device names, ids, etc and fully relies on driver implementation. All others function in the row are there and sharing same data that is as close to actual device information as possible. Whenever it is decided to refactor the whole approach -- it is supposed to be much easier from the one place. So it looks reasonable to me

> It may be somewhat clunky, but has the advantage that it ensures all three functions return mutually consistent results. Since you appear to have dropped my patches which caused GetMonitorInfoW() to return unique names for the displays, they are no longer consistent. 

I am probably missing something, but they supposed to be consistent (at least for winex11.drv) 

Within initialization we fill structure with proper name for each device

-            /* FIXME: using the same device name for all monitors for now */
-            lstrcpyW( monitors[i].szDevice, default_monitor.szDevice );
+            sprintfW( monitors[i].szDevice, adapter_name, i + 1);

While GetMonitorInfo() still does nothing other than returning whatever is there (another advantage of the moving)

    if (info->cbSize >= sizeof(MONITORINFOEXW))
        lstrcpyW( ((MONITORINFOEXW *)info)->szDevice, monitors[i].szDevice );

> because Alexandre wanted the tracking and management of displays and display modes centralized in the explorer process[1]. 

Not sure what was meant exactly here, but the patch making things work with certain respect to surrounding approach - looks to me as a small but good first step. 

>I have my reservations about that approach, mind you. ;)

Could you please clarify this point, do you mean you are going to work all that out in the nearest future? 

> Patch 1: \ No newline at end of file

Thanks, will fix

> Patches 1 and 2 ought to come after 3. In the current order, they introduce dead code.

Yes, the patches 1 and 2 don't make sense without 3, so the order is not that important probably,  but no problem to change it

Best,
Donnie

25.04.2017, 07:07, "Ken Thomases" <ken at codeweavers.com>:
> Hi,
>
> Thanks for taking this up again…
>
> On Apr 24, 2017, at 3:48 PM, Donat Enikeev <donat at enikeev.net> wrote:
>>  Based on Ken Thomases <ken at codeweavers.com> staging patches
>>
>>  Signed-off-by: Donat Enikeev <donat at enikeev.net>
>>  ---
>>  dlls/winemac.drv/display.c | 51 +++++++++++++++++++++++++++++++++++++++
>>  dlls/winemac.drv/winemac.drv.spec | 1 +
>>  2 files changed, 52 insertions(+)
>
> This doesn't build:
>
> display.c:1007:13: error: use of undeclared identifier 'monitors'
>         if (monitors[iDevNum].dwFlags & MONITORINFOF_PRIMARY)
>             ^
> display.c:1010:80: error: use of undeclared identifier 'monitors'
>         memcpy(lpDisplayDevice->DeviceName, monitors[iDevNum].szDevice, sizeof(monitors[iDevNum].szDevice));
>                                                                                ^
> display.c:1010:45: error: use of undeclared identifier 'monitors'
>         memcpy(lpDisplayDevice->DeviceName, monitors[iDevNum].szDevice, sizeof(monitors[iDevNum].szDevice));
>                                             ^
>
> Is this patch written on the assumption that some other staging patches have already been applied?
>
> Beyond that, can you explain the rationale of moving the implementation of EnumDisplayDevices to the drivers? My patches implemented it in user32 based on EnumDisplayMonitors() and GetMonitorInfoW(). It may be somewhat clunky, but has the advantage that it ensures all three functions return mutually consistent results. Since you appear to have dropped my patches which caused GetMonitorInfoW() to return unique names for the displays, they are no longer consistent. GetMonitorInfoW() always uses "\\.\DISPLAY1" for all monitors while this implementation of EnumDisplayDevices() attempts to use "\\.\DISPLAY2", etc., for secondary and subsequent monitors.
>
> For what it's worth, my original patches were rejected because Alexandre wanted the tracking and management of displays and display modes centralized in the explorer process[1]. I have my reservations about that approach, mind you. ;)
> [1] https://www.winehq.org/pipermail/wine-devel/2014-March/103680.html
>
> Issues with other patches in the series:
>
> Patch 1: \ No newline at end of file
>
> Patches 1 and 2 ought to come after 3. In the current order, they introduce dead code.
>
> -Ken