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

Expose desired_maximum_frame_latency through window creation #12954

Merged
merged 1 commit into from
Apr 19, 2024

Conversation

Kanabenki
Copy link
Contributor

Objective

Solution

  • Add a corresponding optional field on Window and ExtractedWindow

Changelog

Added

  • wgpu's desired_maximum_frame_latency is exposed through window creation. This can be used to override the default maximum number of queued frames on the GPU (currently 2).

Migration Guide

  • The desired_maximum_frame_latency field must be added to instances of Window and ExtractedWindow where all fields are explicitly specified.

@Kanabenki Kanabenki added A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times C-Usability A simple quality-of-life change that makes Bevy easier to use labels Apr 13, 2024
// 1 is the minimum, but may cause lower framerates due to the cpu waiting for the gpu to finish
// all work for the previous frame before starting work on the next frame, which then means the gpu
// has to wait for the cpu to finish to start on the next frame.
const DEFAULT_DESIRED_MAXIMUM_FRAME_LATENCY: u32 = 2;
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this change depending on if pipelined rendering is enabled as the initial issue suggests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll try to do some general latency benchmarking to see the impact of changing from 2 to 1 when pipelined rending is disabled. From some quick testing it seemed that this had a neutral to negative impact on the average frame rate when running some stress tests, but I didn't test latency yet and that's probably the main thing to check for.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just want to note that I was wrong about 2 being what we we're using previously (wgpu defaulted to 3 (triple buffering), but changed their default to 2 in the PR that added this setting), though we've been using 2 for bevy 0.13 so not really sure if it matters.

Copy link
Member

@alice-i-cecile alice-i-cecile left a comment

Choose a reason for hiding this comment

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

I'm fine to defer changing the default for pipelined rendering to a follow-up, but I am curious about the results.

Copy link
Contributor

@Elabajaba Elabajaba left a comment

Choose a reason for hiding this comment

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

Code looks good.

I did some unscientific latency benchmarking using presentmon's click to photon latency measurements, and 1 frame was quite a bit worse than 2 with pipelined rendering when I tested it with scene viewer (probably due to the lower framerate, or maybe inaccuracies in presentmon's latency measurements?)

@alice-i-cecile alice-i-cecile 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 Apr 19, 2024
@alice-i-cecile alice-i-cecile added this pull request to the merge queue Apr 19, 2024
Merged via the queue into bevyengine:main with commit 1df41b7 Apr 19, 2024
32 checks passed
@Kanabenki
Copy link
Contributor Author

Sorry for getting back late, did some quick testing with pipelined rendering disabled to see if switching to one frame for that case might make sense, running on this scene I got way worse frame times (80 against 50 fps), and worse latency (40 against 50ms) using Presentmon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times C-Usability A simple quality-of-life change that makes Bevy easier to use 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.

Expose maximum_frame_latency from wgpu to Bevy users
4 participants