[PATCH 5/5] winevulkan: Implement VK_EXT_full_screen_exclusive

[Zebediah Figura]

On 7/16/20 2:42 AM, Joshua Ashton wrote:
>

Why not do this the normal way, i.e. by modifying the code in winex11
and winemac?

Also, what's the point of faking the extension like this? (I'd also note
that "implement" is rather misleading here...)

[Brendan Shanks]

> On Jul 16, 2020, at 12:42 AM, Joshua Ashton  wrote:
>
> +VkResult WINAPI wine_vkGetPhysicalDeviceSurfacePresentModes2EXT(
> - VkPhysicalDevice                            physicalDevice,
> - const VkPhysicalDeviceSurfaceInfo2KHR*      pSurfaceInfo,
> - uint32_t*                                   pPresentModeCount,
> - VkPresentModeKHR*                           pPresentModes)
>
> +{
> - TRACE("%p, %p, %p, %p", physicalDevice, pSurfaceInfo, pPresentModeCount, pPresentModes);
> - return thunk_vkGetPhysicalDeviceSurfacePresentModesKHR(physicalDevice, pSurfaceInfo->surface, pPresentModeCount, pPresentModes);
>
> +}
> -
>
> +VkResult WINAPI wine_vkGetDeviceGroupSurfacePresentModes2EXT(
> - VkDevice                                    device,
> - const VkPhysicalDeviceSurfaceInfo2KHR*      pSurfaceInfo,
> - VkDeviceGroupPresentModeFlagsKHR*           pModes)
>
> +{
> - TRACE("%p, %p, %p", device, pSurfaceInfo, pModes);
> - return thunk_vkGetDeviceGroupSurfacePresentModesKHR(device, pSurfaceInfo->surface, pModes);
>
> +}

Could you just call wine_vkGetPhysicalDeviceSurfacePresentModesKHR() and wine_vkGetDeviceGroupSurfacePresentModesKHR() here rather than having to make those private thunks?
Similar to how wine_vkGetPhysicalDeviceProperties() is called elsewhere in the file (with a prototype manually added).

Brendan

[Joshua Ashton]

Yes, that would be do-able. I will make that change.
I thought making them private thunks would be preferred but in
retrospect it does seem a bit poop.

- Josh

On Thu, 16 Jul 2020 at 17:32, Brendan Shanks  wrote:
> > On Jul 16, 2020, at 12:42 AM, Joshua Ashton  wrote:
> >
> > +VkResult WINAPI wine_vkGetPhysicalDeviceSurfacePresentModes2EXT(
> > - VkPhysicalDevice                            physicalDevice,
> > - const VkPhysicalDeviceSurfaceInfo2KHR*      pSurfaceInfo,
> > - uint32_t*                                   pPresentModeCount,
> > - VkPresentModeKHR*                           pPresentModes)
> >
> > +{
> > - TRACE("%p, %p, %p, %p", physicalDevice, pSurfaceInfo, pPresentModeCount, pPresentModes);
> > - return thunk_vkGetPhysicalDeviceSurfacePresentModesKHR(physicalDevice, pSurfaceInfo->surface, pPresentModeCount, pPresentModes);
> >
> > +}
> > -
> >
> > +VkResult WINAPI wine_vkGetDeviceGroupSurfacePresentModes2EXT(
> > - VkDevice                                    device,
> > - const VkPhysicalDeviceSurfaceInfo2KHR*      pSurfaceInfo,
> > - VkDeviceGroupPresentModeFlagsKHR*           pModes)
> >
> > +{
> > - TRACE("%p, %p, %p", device, pSurfaceInfo, pModes);
> > - return thunk_vkGetDeviceGroupSurfacePresentModesKHR(device, pSurfaceInfo->surface, pModes);
> >
> > +}
>
> Could you just call wine_vkGetPhysicalDeviceSurfacePresentModesKHR() and wine_vkGetDeviceGroupSurfacePresentModesKHR() here rather than having to make those private thunks?
> Similar to how wine_vkGetPhysicalDeviceProperties() is called elsewhere in the file (with a prototype manually added).
>
> Brendan

[Liam Middlebrook]

> 47 +    /* Toss out VkSurfaceFullScreenExclusiveInfoEXT                                                                                        
>  48 +     * and VkSurfaceFullScreenExclusiveWin32InfoEXT */                                                                                    
>  49 +    VkPhysicalDeviceSurfaceInfo2KHR surface_info;                                                                                                                                                      
>  50 +    surface_info.sType   = pSurfaceInfo->sType;                                                                                                                                                              
>  51 +    surface_info.pNext   = NULL;                                                                                                                                 
>  52 +    surface_info.surface = pSurfaceInfo->surface;

I think we should have a more robust solution for filtering out faked 
extension structures from pNext chains. I also think that this filtering 
mechanism will need to be accessible from winex11 and winemac.

We'll need to handle the pNext chains which can be passed into 
vkCreateSwapchainKHR() to make sure we don't pass invalid pNext chains 
into the driver.

I agree with Zebediah that this should also include changes to winex11 
and winemac. Is your plan currently that because fullscreen exclusive 
isn't a thing on Linux, we should just say it's supported and that's 
good enough? Is there any concept of FSE currently in WINE that we could 
reference as prior art for a more full implementation here (even if we 
step towards that incrementally)?

It seems like at a minimum there should be some fixme messages for the 
various VkFullScreenExclusiveEXT values which can be used in conjunction 
with vkCreateSwapchainKHR, as well as other parts of this extension that 
we're not really fully supporting, but rather are spoofing to 
applications which require it.

Thanks,

Liam Middlebrook

On 7/16/20 12:42 AM, Joshua Ashton wrote:
>