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 max FPS initialization #99149

Merged
merged 1 commit into from
Nov 13, 2024
Merged

Conversation

mrsaturnsan
Copy link
Contributor

Max FPS needs to be initially set after the DisplayServer is created. Otherwise the Renderer will not get the max FPS value propagated to it.

Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

Looks good to me. I think this is a safe change because max_fps is only ever read in two places:

  1. The main loop (which doesn't start until well after this)
  2. The initialization of the Vulkan RDD

With this change we don't need to grab the max_fps from engine when initializing Vulkan because we can trust that it will be updated later.

Accordingly, we could remove the calculation of max_fps and max_time here:

const double max_fps = Engine::get_singleton()->get_max_fps();
const uint64_t max_time = max_fps > 0 ? uint64_t((1000.0 * 1000.0 * 1000.0) / max_fps) : 0;

And just set the swap interval to swap_chain->refresh_duration

@mrsaturnsan mrsaturnsan requested a review from a team as a code owner November 12, 2024 22:53
@mrsaturnsan
Copy link
Contributor Author

Added the requested change :)

Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

Looks great to me! The commits just need to be squashed

Remove unnecessary get_max_fps
@mrsaturnsan
Copy link
Contributor Author

Squashed! :)

Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@Repiteo Repiteo merged commit 15d09a5 into godotengine:master Nov 13, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 13, 2024

Thanks! Congratulations on your first contribution! 🎉

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.

3 participants