Closed sjnewbury closed 1 year ago
RobertBeckebans
commented
1 year ago neo/renderer/Vulkan/RenderBackend_VK.cpp is only for reference there but it is not used anymore. You could change NUM_FRAME_DATA to 2 and the corresponding code in neo/sys/DeviceManager_VK.cpp
Actually I haven't tested NVRHI with Wayland yet.
RobertBeckebans
commented
1 year ago I already forgot that a user posted something about this:
bluestang — 25.02.2023 17:11 You probably haven't seen this before because MESA's vk wsi code was previously using minimagecount = 3 under XWayland. Under X11 is also set to 3. However, a recent change to their wsi code now sets the minimagecount = 4 under XWayland mirroring what it is set to in Wayland. My assumption this is going to be more of problem once MESA branches to 23.1 and distros pkg that version driver to their users. In addition, SDL2 defaults to the wayland driver (at least on Fedora) and the game doesn't work under that driver either...I have to use SDL_VIDEODRIVER=x11 to make it work as well.
rg3
commented
1 year ago The following two patches allow me to play the game on Wayland with RADV. Not submitting a formal PR since I'm not very familiar with the code. Hope it helps. :crossed_fingers:
commit 322b0d42d6391756bafe5215ba85c58bcc1a0a47
Author: Ricardo Garcia <commits@rg3.name>
Date: Thu May 18 22:36:17 2023 +0200
Allow the game to work on platforms not supporting vk::PresentModeKHR::eImmediate
diff --git a/neo/sys/DeviceManager_VK.cpp b/neo/sys/DeviceManager_VK.cpp
index fb4d5f09..500ee109 100644
--- a/neo/sys/DeviceManager_VK.cpp
+++ b/neo/sys/DeviceManager_VK.cpp
@@ -284,6 +284,7 @@ private:
// SRS - flag indicating support for eFifoRelaxed surface presentation (r_swapInterval = 1) mode
bool enablePModeFifoRelaxed = false;
+ bool enablePModeImmediate = false;
private:
@@ -593,8 +594,7 @@ bool DeviceManager_VK::pickPhysicalDevice()
deviceIsGood = false;
}
- if( ( find( surfacePModes.begin(), surfacePModes.end(), vk::PresentModeKHR::eImmediate ) == surfacePModes.end() ) ||
- ( find( surfacePModes.begin(), surfacePModes.end(), vk::PresentModeKHR::eFifo ) == surfacePModes.end() ) )
+ if( find( surfacePModes.begin(), surfacePModes.end(), vk::PresentModeKHR::eFifo ) == surfacePModes.end() )
{
// can't find the required surface present modes
errorStream << std::endl << " - does not support the requested surface present modes";
@@ -908,6 +908,7 @@ bool DeviceManager_VK::createDevice()
// SRS - Determine if "smart" (r_swapInterval = 1) vsync mode eFifoRelaxed is supported by device and surface
auto surfacePModes = m_VulkanPhysicalDevice.getSurfacePresentModesKHR( m_WindowSurface );
enablePModeFifoRelaxed = find( surfacePModes.begin(), surfacePModes.end(), vk::PresentModeKHR::eFifoRelaxed ) != surfacePModes.end();
+ enablePModeImmediate = find( surfacePModes.begin(), surfacePModes.end(), vk::PresentModeKHR::eImmediate ) != surfacePModes.end();
// stash the renderer string
auto prop = m_VulkanPhysicalDevice.getProperties();
@@ -1020,7 +1021,7 @@ bool DeviceManager_VK::createSwapChain()
.setPQueueFamilyIndices( enableSwapChainSharing ? queues.data() : nullptr )
.setPreTransform( vk::SurfaceTransformFlagBitsKHR::eIdentity )
.setCompositeAlpha( vk::CompositeAlphaFlagBitsKHR::eOpaque )
- .setPresentMode( m_DeviceParams.vsyncEnabled > 0 ? ( m_DeviceParams.vsyncEnabled == 2 || !enablePModeFifoRelaxed ? vk::PresentModeKHR::eFifo : vk::PresentModeKHR::eFifoRelaxed ) : vk::PresentModeKHR::eImmediate )
+ .setPresentMode( m_DeviceParams.vsyncEnabled > 0 ? ( m_DeviceParams.vsyncEnabled == 2 || !enablePModeFifoRelaxed ? vk::PresentModeKHR::eFifo : vk::PresentModeKHR::eFifoRelaxed ) : (enablePModeImmediate ? vk::PresentModeKHR::eImmediate : vk::PresentModeKHR::eFifo ) )
.setClipped( true )
.setOldSwapchain( nullptr );
commit 7a9549c739fc719081d52df258a06d748380a112
Author: Ricardo Garcia <commits@rg3.name>
Date: Thu May 18 22:36:17 2023 +0200
Make swapChainBufferCount more flexible for all platforms
diff --git a/neo/sys/DeviceManager_VK.cpp b/neo/sys/DeviceManager_VK.cpp
index 5d084155..fb4d5f09 100644
--- a/neo/sys/DeviceManager_VK.cpp
+++ b/neo/sys/DeviceManager_VK.cpp
@@ -30,6 +30,8 @@
#include <string>
#include <queue>
#include <unordered_set>
+#include <limits>
+#include <algorithm>
#include "renderer/RenderCommon.h"
#include <sys/DeviceManager.h>
@@ -559,13 +561,8 @@ bool DeviceManager_VK::pickPhysicalDevice()
auto surfaceFmts = dev.getSurfaceFormatsKHR( m_WindowSurface );
auto surfacePModes = dev.getSurfacePresentModesKHR( m_WindowSurface );
- if( surfaceCaps.minImageCount > m_DeviceParams.swapChainBufferCount ||
- ( surfaceCaps.maxImageCount < m_DeviceParams.swapChainBufferCount && surfaceCaps.maxImageCount > 0 ) )
- {
- errorStream << std::endl << " - cannot support the requested swap chain image count:";
- errorStream << " requested " << m_DeviceParams.swapChainBufferCount << ", available " << surfaceCaps.minImageCount << " - " << surfaceCaps.maxImageCount;
- deviceIsGood = false;
- }
+ const auto upperImageCountLimit = ( surfaceCaps.maxImageCount > 0 ? surfaceCaps.maxImageCount : std::numeric_limits<uint32_t>::max() );
+ m_DeviceParams.swapChainBufferCount = std::clamp( m_DeviceParams.swapChainBufferCount, surfaceCaps.minImageCount, upperImageCountLimit );
if( surfaceCaps.minImageExtent.width > requestedExtent.width ||
surfaceCaps.minImageExtent.height > requestedExtent.height ||
SRSaunders
commented
1 year ago Interesting that eImmediate mode is sometimes not available. I know the Vulkan spec only guarantees eFifo, but I thought that was mainly historical. Live and learn...
This defect and your code above comes at a good time since I am currently working on latency optimization, and was thinking about implementing eMailbox mode (if available) for Vulkan. And the issue of variations in minImageCount across platforms was bugging me, since swapChainBufferCount is currently hardcoded to 3 mainly for linux, but it also affects latency on multiple platforms (Windows, linux, macOS). It would be better to have it match the number of GPU frames by default, but enable overrides as necessary by the renderer.
Would you mind if I took some of your code above and incorporated into a new PR? I will credit you as appropriate.
Also, can you tell me if eMailbox is available on Wayland with RADV?
rg3
commented
1 year ago Interesting that
eImmediatemode is sometimes not available. I know the Vulkan spec only guaranteeseFifo, but I thought that was mainly historical. Live and learn...
For reference, I'm using RADV on Wayland with an RX 6700 card, and I see the following as part of "vulkaninfo":
GPU id : 0 (AMD Radeon Graphics (RADV NAVI22)):
Surface type = VK_KHR_wayland_surface
Formats: count = 7
SurfaceFormat[0]:
format = FORMAT_B8G8R8A8_SRGB
colorSpace = COLOR_SPACE_SRGB_NONLINEAR_KHR
SurfaceFormat[1]:
format = FORMAT_B8G8R8A8_UNORM
colorSpace = COLOR_SPACE_SRGB_NONLINEAR_KHR
SurfaceFormat[2]:
format = FORMAT_R8G8B8A8_SRGB
colorSpace = COLOR_SPACE_SRGB_NONLINEAR_KHR
SurfaceFormat[3]:
format = FORMAT_R8G8B8A8_UNORM
colorSpace = COLOR_SPACE_SRGB_NONLINEAR_KHR
SurfaceFormat[4]:
format = FORMAT_A2R10G10B10_UNORM_PACK32
colorSpace = COLOR_SPACE_SRGB_NONLINEAR_KHR
SurfaceFormat[5]:
format = FORMAT_A2B10G10R10_UNORM_PACK32
colorSpace = COLOR_SPACE_SRGB_NONLINEAR_KHR
SurfaceFormat[6]:
format = FORMAT_R5G6B5_UNORM_PACK16
colorSpace = COLOR_SPACE_SRGB_NONLINEAR_KHR
Present Modes: count = 2
PRESENT_MODE_MAILBOX_KHR
PRESENT_MODE_FIFO_KHR
VkSurfaceCapabilitiesKHR:
-------------------------
minImageCount = 4
maxImageCount = 0
currentExtent:
width = 4294967295
height = 4294967295
minImageExtent:
width = 1
height = 1
maxImageExtent:
width = 16384
height = 16384
maxImageArrayLayers = 1
supportedTransforms: count = 1
SURFACE_TRANSFORM_IDENTITY_BIT_KHR
currentTransform = SURFACE_TRANSFORM_IDENTITY_BIT_KHR
supportedCompositeAlpha: count = 2
COMPOSITE_ALPHA_OPAQUE_BIT_KHR
COMPOSITE_ALPHA_PRE_MULTIPLIED_BIT_KHR
supportedUsageFlags: count = 6
IMAGE_USAGE_TRANSFER_SRC_BIT
IMAGE_USAGE_TRANSFER_DST_BIT
IMAGE_USAGE_SAMPLED_BIT
IMAGE_USAGE_STORAGE_BIT
IMAGE_USAGE_COLOR_ATTACHMENT_BIT
IMAGE_USAGE_INPUT_ATTACHMENT_BIT
VkSurfaceCapabilities2EXT:
--------------------------
supportedSurfaceCounters:
None
VkSurfaceProtectedCapabilitiesKHR:
----------------------------------
supportsProtected = falseAs you can see, it supports Mailbox and FIFO, but not inmediate mode. In my code I went for FIFO as a fallback, but perhaps Mailbox maps to some form of synchronization (I'm guessing this is popularly known as Fast Sync?). Perhaps, in addition, the game should detect which modes are available at the start and give the user only the available options.
This defect and your code above comes at a good time since I am currently working on latency optimization, and was thinking about implementing
eMailboxmode (if available) for Vulkan.
I think it makes sense, thanks!
And the issue of variations in
minImageCountacross platforms was bugging me, sinceswapChainBufferCountis currently hardcoded to 3 mainly for linux, but it also affects latency on multiple platforms (Windows, linux, macOS). It would be better to have it match the number of GPU frames by default, but enable overrides as necessary by the renderer.
If you go with clamping, as my code above does, you can set it to your ideal value (2 perhaps?) and then clamp, resulting in the minimum latency allowed by the platform.
Would you mind if I took some of your code above and incorporated into a new PR? I will credit you as appropriate.
Of course not! Feel free to do whatever you please with the patches above.
Also, can you tell me if
eMailboxis available on Wayland with RADV?
Replied above. :)
Way back before NVHRI was introduced I had written a patch to make RBDOOM-3-BFG work with the Wayland SDL backend. It just worked. I also had made a patch to make it work with hidpi properly.
Since then, the changes I made in my patch are now in the tree, namely removing dependency on X11/XCB and using the generic SDL Vulkan surface creation.
Now however, launching RBDOOM-3-BFG results in the following output:
What's weird here is current mesa requires 4 images for MAILBOX present mode under Wayland, but only 2 for FIFO mode. Yet if I try to change the code to force FIFO mode it changes nothing. Previous mesa versions always required 4 images under Wayland. I tried changing NUM_FRAME_DATA to 2 and separately forcing the images to 4, but it also changed nothing.
The code I tried changing was here:
neo/renderer/Vulkan/RenderBackend_VK.cppFWIW retroarch works fine with 2 or 3 swap chain images, presumably using FIFO.