Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

On/off VSync support for vulkan #36862

Closed
wants to merge 1 commit into from

Conversation

giarve
Copy link
Contributor

@giarve giarve commented Mar 6, 2020

Being a boolean setting, the code will choose the best available present mode to fit user's choice.
Presentation modes are ordered from no tearing to non-sync modes (will probably have tearing).

Fixes #36467.

@giarve giarve force-pushed the add-vulkan-vsync-x11-win branch 2 times, most recently from 65ec8f4 to 9a831a5 Compare March 6, 2020 17:51
Being a boolean setting, the code will choose the best available present mode to fit user's choice.
Presentation modes are ordered from no tearing to non-sync modes (will probably have tearing).
@ghost
Copy link

ghost commented Jul 13, 2020

Any chance we can implement this so i can test it,
Am trying to fix this #35899

@Calinou
Copy link
Member

Calinou commented Jul 31, 2020

@giarve Out of curiosity, how difficult would it be to adapt this PR to expose adaptive V-Sync? (In other words, V-Sync that automatically disables itself when the FPS is below the monitor refresh rate to avoid stuttering at low FPS.)

@giarve
Copy link
Contributor Author

giarve commented Jul 31, 2020

@giarve Out of curiosity, how difficult would it be to adapt this PR to expose adaptive V-Sync? (In other words, V-Sync that automatically disables itself when the FPS is below the monitor refresh rate to avoid stuttering at low FPS.)

According to the specification:

VK_PRESENT_MODE_FIFO_RELAXED_KHR specifies that the presentation engine generally waits for the next vertical blanking period to update the current image. If a vertical blanking period has already passed since the last update of the current image then the presentation engine does not wait for another vertical blanking period for the update, meaning this mode may result in visible tearing in this case. This mode is useful for reducing visual stutter with an application that will mostly present a new image before the next vertical blanking period, but may occasionally be late, and present a new image just after the next vertical blanking period. An internal queue is used to hold pending presentation requests. New requests are appended to the end of the queue, and one request is removed from the beginning of the queue and processed during or after each vertical blanking period in which the queue is non-empty.

Looks like what you're asking for. If you have a project in which you're able to reproduce stuttering issues, recompile the engine changing:

VkPresentModeKHR swapchainPresentMode = VK_PRESENT_MODE_FIFO_KHR;

to

VkPresentModeKHR swapchainPresentMode = VK_PRESENT_MODE_FIFO_RELAXED_KHR;

in
drivers/vulkan/vulkan_context.cpp:874

and rerun the game and check if it solves the stuttering.

Depending on the GPU you may not have this presentation mode available, in that case I won't be of much help. In fact, this PR was first created to address compatibility issues with different presentation modes, because this change set what only does is selecting the most differentiating mode available for VSync enabled or disabled.

The main problem of this PR is that it doesn't allow you to select which exact mode you want to try, so it could be interesting (if you find the VK_PRESENT_MODE_FIFO_RELAXED_KHR to work) to expose the available presentation mode list to settings, or just exposing what you suggested, toggling adaptive V-Sync.

I wouldn't be able to make a custom adaptive Vsync because it looks like a more advanced topic.

When I last checked the code the default mode was full VSync enabled, with three buffers but without the MAILBOX presentation mode enabled (which would be required to enable triple buffering and minimize latency). By default, the VK_PRESENT_MODE_FIFO_KHR is the one enabled, which is good for mobile because it has the least battery consumption.

Take into account that this PR is Vulkan only and wouldn't solve the problems with Godot stable may have (if there are any, which I don't know).

@nathanfranke
Copy link
Contributor

I would like to see this merged soon since disabling VSync helps with profiling.

@nathanfranke
Copy link
Contributor

Needs to be rebased

@Geometror
Copy link
Member

I am currently trying to reimplement this with a newer version of Godot since I failed to rebase giarve's branch(too many changes since then).
But for some reason even setting the presentation mode directly to VK_PRESENT_MODE_IMMEDIATE_KHR by passing it to the VkSwapchainCreateInfoKHR constructor does not work as expected (still running at monitors refresh rate). Could someone explain that behavior?
see drivers/vulkan/vulkan_context.cpp:

VkSwapchainCreateInfoKHR swapchain_ci = {
		/*sType*/ VK_STRUCTURE_TYPE_SWAPCHAIN_CREATE_INFO_KHR,
		/*pNext*/ nullptr,
		/*flags*/ 0,
		/*surface*/ window->surface,
		/*minImageCount*/ desiredNumOfSwapchainImages,
		/*imageFormat*/ format,
		/*imageColorSpace*/ color_space,
		/*imageExtent*/ {
				/*width*/ swapchainExtent.width,
				/*height*/ swapchainExtent.height,
		},
		/*imageArrayLayers*/ 1,
		/*imageUsage*/ VK_IMAGE_USAGE_COLOR_ATTACHMENT_BIT,
		/*imageSharingMode*/ VK_SHARING_MODE_EXCLUSIVE,
		/*queueFamilyIndexCount*/ 0,
		/*pQueueFamilyIndices*/ nullptr,
		/*preTransform*/ (VkSurfaceTransformFlagBitsKHR)preTransform,
		/*compositeAlpha*/ compositeAlpha,
		/*presentMode*/ VK_PRESENT_MODE_IMMEDIATE_KHR,
		/*clipped*/ true,
		/*oldSwapchain*/ VK_NULL_HANDLE,
	};```

@Calinou Calinou added this to the 4.0 milestone Jan 5, 2021
@Calinou
Copy link
Member

Calinou commented Jan 22, 2021

Closing per discussion on IRC, as this PR needs to be redone following recent DisplayServer changes:

<Calinou> I think it's good. Maybe add https://github.com/godotengine/godot/pull/36862 for review?
<reduz> Calinou: no.. still not completely sure how this has to work and I have to do a few more changes to vulkan
<Calinou> yes, it probably has to be redone from scratch
<reduz> Calinou: plus the PR should be redone for Displayservers
<Calinou> should we close the PR and mark it as salvageable?
<reduz> probably salvagable but needs a ton of work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Vulkan: disabling V-Sync not reimplemented yet
4 participants