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

[Merged by Bors] - Disable Vsync for stress tests. #5187

Closed

Conversation

Elabajaba
Copy link
Contributor

Objective

Currently stress tests are vsynced. This is undesirable for a stress test, as you want to run them with uncapped framerates.

Solution

Ensure all stress tests are using PresentMode::Immediate if they render anything.

@IceSentry
Copy link
Contributor

I think you should add bevy::window::PrresentMode to the use block.

Also, while I don't necessarily disagree with the idea, on g-sync monitors the framerate will still be capped to the refresh rate while the app is windowed and focused.

@alice-i-cecile alice-i-cecile added A-Rendering Drawing game state to the screen A-Diagnostics Logging, crash handling, error reporting and performance analysis labels Jul 3, 2022
@alice-i-cecile
Copy link
Member

alice-i-cecile commented Jul 3, 2022

I'm of the same opinion as @IceSentry above :) Clean that up and I'd be fully on board.

We should consider refactoring these examples to reduce duplication, but that's a different PR.

@Elabajaba
Copy link
Contributor Author

I think you should add bevy::window::PrresentMode to the use block.

Done

Also, while I don't necessarily disagree with the idea, on g-sync monitors the framerate will still be capped to the refresh rate while the app is windowed and focused.

Unless something has changed in the past ~1.5 years, I don't think enabling Gsync caps your framerate? I know freesync doesn't currently cap your framerate.

@IceSentry
Copy link
Contributor

It caps it, but only when the window is not full screen and in focus. It's just not noticeable for most people because most people play games in fullscreen. I've confirmed this with multiple people including some people working on wgpu. It's a weird edge case, but it definitely happens.

@alice-i-cecile
Copy link
Member

Should we full screen the stress tests by default then?

@IceSentry
Copy link
Contributor

No, I really don't think it matters that much and I'd rather not have some examples going fullscreen when developing. I was just pointing it out because it was really weird when I discovered it. You can easily push the tests to go bellow the refresh rate anyway.

Most people don't have a g-sync monitor anyway so it's very much an edge case and it's easy to bypass, just don't focus the window while the test is running or put it on another monitor. (Yes, I know not everyone has a second monitor, but people with g-sync monitors are already a minority and I wouldn't be surprised if most of those people also have a second monitor)

@mockersf
Copy link
Member

mockersf commented Jul 3, 2022

I guess this never came up because most of us are below async rates on those examples anyway 😄

@mockersf mockersf added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Jul 3, 2022
Copy link
Contributor

@superdump superdump left a comment

Choose a reason for hiding this comment

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

bors r+

bors bot pushed a commit that referenced this pull request Jul 3, 2022
# Objective

Currently stress tests are vsynced. This is undesirable for a stress test, as you want to run them with uncapped framerates.

## Solution

Ensure all stress tests are using PresentMode::Immediate if they render anything.
@bors bors bot changed the title Disable Vsync for stress tests. [Merged by Bors] - Disable Vsync for stress tests. Jul 3, 2022
@bors bors bot closed this Jul 3, 2022
inodentry pushed a commit to IyesGames/bevy that referenced this pull request Aug 8, 2022
# Objective

Currently stress tests are vsynced. This is undesirable for a stress test, as you want to run them with uncapped framerates.

## Solution

Ensure all stress tests are using PresentMode::Immediate if they render anything.
james7132 pushed a commit to james7132/bevy that referenced this pull request Oct 28, 2022
# Objective

Currently stress tests are vsynced. This is undesirable for a stress test, as you want to run them with uncapped framerates.

## Solution

Ensure all stress tests are using PresentMode::Immediate if they render anything.
ItsDoot pushed a commit to ItsDoot/bevy that referenced this pull request Feb 1, 2023
# Objective

Currently stress tests are vsynced. This is undesirable for a stress test, as you want to run them with uncapped framerates.

## Solution

Ensure all stress tests are using PresentMode::Immediate if they render anything.
@Elabajaba Elabajaba deleted the stress-test-disable-vsync branch October 8, 2023 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Diagnostics Logging, crash handling, error reporting and performance analysis A-Rendering Drawing game state to the screen S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants