[PATCH v3 2/3] winex11.drv: Add EDID to the virtual desktop monitor device.

[Eduard Permyakov]

On 2021-09-07 5:54 a.m., Zhiyi Zhang wrote:
>
> On 9/6/21 11:24 PM, Eduard Permyakov wrote:
>> Some applications use the EDID property of the monitor device
>> to detect supported resolutions, and set the viewport size. Add
>> a computed EDID to the virtual desktop monitor device to allow
>> the virtual desktop driver to be better compatible with such
>> applications.
>>
>> Most fields of the EDID contain details that are of no relevance
>> to applications, so populate only the native resolution and physical
>> monitor dimensions. This should cover the vast majority of use
>> cases.
>>
>> Signed-off-by: Eduard Permyakov <epermyakov at codeweavers.com>
>> ---
>>   dlls/winex11.drv/desktop.c | 40 ++++++++++++++++++++++++++++++++++++--
>>   1 file changed, 38 insertions(+), 2 deletions(-)
>>
>> diff --git a/dlls/winex11.drv/desktop.c b/dlls/winex11.drv/desktop.c
>> index 71b3a0a5a27..0abd0ddf58b 100644
>> --- a/dlls/winex11.drv/desktop.c
>> +++ b/dlls/winex11.drv/desktop.c
>> @@ -20,6 +20,8 @@
>>    */
>>   
>>   #include "config.h"
>> +#include <stdlib.h>
>> +#include <stdint.h>
> This remain unchanged. Please see my previous comment.
>
>>   #include <X11/cursorfont.h>
>>   #include <X11/Xlib.h>
>>   
>> @@ -111,6 +113,38 @@ static void add_desktop_mode( DEVMODEW *mode, DWORD depth, DWORD width, DWORD he
>>       mode->dmDisplayFrequency = 60;
>>   }
>>   
>> +static void get_virtual_edid( unsigned char **edid, unsigned long *len )
>> +{
>> +    static const size_t desc_size = 128;
>> +    unsigned int width_mm, height_mm;
>> +    unsigned char *ret;
>> +
>> +    *edid = NULL;
>> +    *len = 0;
>> +
>> +    ret = calloc( desc_size, 1 );
>> +    if (!ret)
>> +        return;
>> +
>> +    /* Set the native resolution in the Preferred Timing Mode descriptor */
>> +    ret[0x38] = max_width & 0xff;
>> +    ret[0x3A] = ((max_width >> 8) & 0xf) << 4;
>> +
>> +    ret[0x3B] = max_height & 0xff;
>> +    ret[0x3D] = ((max_height >> 8) & 0xf) << 4;
>> +
>> +    /* Set the monitor size (mm) in the Preferred Timing Mode descriptor */
>> +    height_mm = 200;
>> +    width_mm = height_mm / ((float)max_height) * max_width;
>> +
>> +    ret[0x42] = width_mm & 0xff;
>> +    ret[0x43] = height_mm & 0xff;
>> +    ret[0x44] = (((width_mm >> 8) & 0xf) << 4) | ((height_mm >> 8) & 0xf);
> Are you sure this EDID data is enough for the game? It doesn't look valid to me.
> For example, according to VESA ENHANCED EXTENDED DISPLAY IDENTIFICATION
> DATA STANDARD (Defines EDID Structure Version 1, Revision 4) Release A, Revision 2,
>
> 3.9 Standard Timings
>> Unused Standard Timing data fields shall be set to 01h, 01h
> And 3.10.3.2 Alphanumeric Data String Descriptor Definition
>
>> If there are less than 13 characters in the
>> string, then terminate the alphanumeric data string with ASCII code ‘0Ah’ (line feed) and pad the
>> unused bytes in the field with ASCII code ‘20h’ (space)
> And this EDID doesn't have EDID version and revision, even if you're using EDID version 1 revision 0.
>
> Maybe you should base the EDID on one of the sample EDID in the VESA EDID standard.


The game I used for testing (Industries of Titan) reads the native 
resolution and physical size
(for computing DPI values) and does so by indexing fixed fields in the 
EDID. I haven't studied
in depth what the app tries to do with the DPI values. It does not do a 
full parse of the EDID.
The vast majority of the fields in it are low-level details of the 
monitor which you can't really
"fake" if you don't have an actual physical monitor. Anyway, if an app 
really wants to know these
details, then it probably wants to be driving the monitor which I think 
is incompatible with the
use case of using the virtual desktop driver. Based on your suggestion, 
I have just hard-coded
a valid "default" EDID for a generic 320mm x 427mm monitor. Then I 
patched the native
resolution. I feel this is sufficient for this niche use case.

>
>
>> +
>> +    *edid = ret;
>> +    *len = desc_size;
>> +}
>> +
>>   static BOOL X11DRV_desktop_get_modes( ULONG_PTR id, DWORD flags, DEVMODEW **new_modes, UINT *mode_count )
>>   {
>>       UINT depth_idx, size_idx, mode_idx = 0;
>> @@ -265,8 +299,7 @@ static BOOL X11DRV_desktop_get_monitors( ULONG_PTR adapter_id, struct x11drv_mon
>>       SetRect( &monitor->rc_work, 0, 0, desktop_width, desktop_height );
>>       query_desktop_work_area( &monitor->rc_work );
>>       monitor->state_flags = DISPLAY_DEVICE_ATTACHED;
>> -    monitor->edid_len = 0;
>> -    monitor->edid = NULL;
>> +    get_virtual_edid( &monitor->edid, &monitor->edid_len );
>>       if (desktop_width && desktop_height)
>>           monitor->state_flags |= DISPLAY_DEVICE_ACTIVE;
>>   
>> @@ -277,6 +310,9 @@ static BOOL X11DRV_desktop_get_monitors( ULONG_PTR adapter_id, struct x11drv_mon
>>   
>>   static void X11DRV_desktop_free_monitors( struct x11drv_monitor *monitors, int count )
>>   {
>> +    int i;
>> +    for (i = 0; i < count; i++)
>> +        free( monitors[i].edid );
>>       heap_free( monitors );
>>   }
>>