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

Fix flickering on vulkan when drawing to a render target #8698

Merged
merged 4 commits into from Mar 28, 2024

Conversation

Jhonnyg
Copy link
Contributor

@Jhonnyg Jhonnyg commented Mar 22, 2024

Fixed an issue where flickering was happening on the Vulkan and MoltenVK renderers when using a render target.

@Jhonnyg Jhonnyg added bug Something is not working as expected engine Issues related to the Defold engine graphics vulkan labels Mar 22, 2024
Comment on lines +1097 to +1100
if (created_width != context->m_Width || created_height != context->m_Height)
{
dmPlatform::SetWindowSize(context->m_Window, created_width, created_height);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the swapchain has been created at a different resolution we need to make sure the GLFW layer knows about the new updated resoultion.

Comment on lines +1350 to +1353
if (res != VK_SUCCESS && res != VK_SUBOPTIMAL_KHR)
{
CHECK_VK_ERROR(res);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a fix / workaround for android where the swap chain capabilities can have a presentation transform. For now we skip the transform completely by setting the currentTransform = identity, but that causes the presentation function to return a suboptimal result, which we don't care about right now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this be good info as a code comment, or is it easy to figure out?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's probably a good code comment, I will forget it otherwise

@@ -153,6 +153,8 @@ namespace dmGraphics
capabilities.m_SurfaceCapabilities.maxImageCount);
}

swap_chain_image_count = dmMath::Min(swap_chain_image_count, (uint32_t) DM_MAX_FRAMES_IN_FLIGHT);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can only support a fixed amount of frame resources (and don't want to)

@Jhonnyg Jhonnyg requested a review from JCash March 22, 2024 16:26
JCash
JCash previously approved these changes Mar 27, 2024
Copy link
Contributor

@JCash JCash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm
only small note about possible code comment

Comment on lines +1350 to +1353
if (res != VK_SUCCESS && res != VK_SUBOPTIMAL_KHR)
{
CHECK_VK_ERROR(res);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this be good info as a code comment, or is it easy to figure out?

@Jhonnyg Jhonnyg merged commit abdb39b into dev Mar 28, 2024
15 checks passed
@Jhonnyg Jhonnyg deleted the vulkan-fix-culling-with-render-target branch March 28, 2024 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working as expected engine Issues related to the Defold engine graphics vulkan
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants