[PATCH 4/5] win32u: Move null user driver implementation from user32.

[Rémi Bernon]

On 11/10/21 17:33, Jacek Caban wrote:
> On 11/10/21 3:19 PM, Rémi Bernon wrote:
>> On 11/10/21 15:01, Jacek Caban wrote:
>>> On 11/10/21 12:47 AM, Rémi Bernon wrote:
>>>> On 11/10/21 00:41, Rémi Bernon wrote:
>>>>> On 11/9/21 13:55, Jacek Caban wrote:
>>>>>> Signed-off-by: Jacek Caban <jacek at codeweavers.com>
>>>>>> ---
>>>>>>   dlls/user32/driver.c | 209 +----------------------------
>>>>>>   dlls/win32u/driver.c | 310 
>>>>>> ++++++++++++++++++++++++++++++++++++++++++-
>>>>>>   2 files changed, 310 insertions(+), 209 deletions(-)
>>>>>>
>>>>>>
>>>>>
>>>>> A little bit related to my previous comment, this patch introduces 
>>>>> a reset to "null" graphics driver on driver unload.
>>>>>
>>>>> I think it could also add the same
>>>>>
>>>>>    __wine_set_display_driver( &null_driver, WINE_GDI_DRIVER_VERSION );
>>>>>
>>>>> call when the "null" driver is explicitly requested and loaded, 
>>>>> possibly addressing my previous comment too (as the pointer would 
>>>>> not be lasy_load_driver anymore).
>>>>
>>>> Also, regarding EnumDisplayMonitors and GetMonitorInfo, for which 
>>>> you added some FIXME, I have some patches to move the default 
>>>> implementation back to the corresponding user32 functions.
>>>>
>>>> I intended to send them after the previous nulldrv display device 
>>>> cache series I sent a week ago but I don't think they actually 
>>>> depend on it.
>>>>
>>>> (Similarly, I used the monitor cache to keep the default and initial 
>>>> monitor for GetMonitorInfo implementation)
>>>>
>>>> I rebased and pushed the patches there if you're interested, 
>>>> although only the commit up to 
>>>> c998fbadab6aa33e3bd2740b0651a0c0509d4c02 are really relevant for this: 
>>>
>>>
>>> I have null driver functions and some other sysparams.c functions 
>>> moved to win32u in my tree. Those needed to be changed due to to 
>>> advapi32 and setupapi calls. I used a cache as well, but instead of 
>>> having two caches, I extended adapters cache to contain missing 
>>> monitor rects informations. There are a few functions that need to 
>>> enumerate monitors, they would look like this:
>>>
>>>
>>>      update_display_cache();
>>>
>>>      pthread_mutex_lock( &display_lock );
>>>
>>>      LIST_FOR_EACH_ENTRY( adapter, &adapters, struct display_device, 
>>> entry )
>>>      {
>>>          LIST_FOR_EACH_ENTRY( monitor, &adapter->children, struct 
>>> monitor, dev.entry )
>>>          {
>>>
>>>           /* .... */
>>>
>>>          }
>>>      }
>>>
>>>      pthread_mutex_unlock( &display_lock );
>>>
>>>
>>> What do you think about such approach?
>>>
>>
>> Do you actually need to move the display device cache to win32u or is 
>> it just because of the non-trivial nulldrv functions being moved there 
>> too?
> 
> 
> It all needs to be moved. A number of other win32u functionality depends 
> on those functions.
> 
> 
>>
>> In the tree I linked above, in the end after all the changes, the 
>> nulldrv functions are all mostly no-op and the default implementation 
>> lives in user32, updating the cache by enumerating setupapi devices.
>>
>> It also moves several things out of graphics drivers, such as reading 
>> / writing current display mode, to the user32 EnumDisplaySettingsExW / 
>> ChangeDisplaySettingsEx generic code.
>>
>> Overall I think it makes everything much simpler, and reduce the 
>> amount of entry points graphics driver would need, and reduce the 
>> amount of changes needed if you need to replace all setupapi / 
>> advapi32 calls. 
> 
> 
> Yes, I think that those are mostly the right direction. I could even 
> imagine setting registry in wineandroid.drv and remove 
> pEnumDisplayMonitors entry point.
> 
> 
> I already replaced problematic setupapi / advapi32 calls from 
> sysparams.c in my tree. I'd prefer cache layout like I mentioned, but it 
> should be easy to adopt to a different cache layout as well.
> 
I prefer a flat array for its simplicity, as we don't really need any 
custom struct, as it also allows some initial values to be set 
conveniently through its initialized, and because I expect the number of 
display devices / monitors to be bounded, but it probably doesn't matter 
much. And it can very well be changed later.
-- 
Rémi Bernon <rbernon at codeweavers.com>