update time in render stage #4735

hymm · 2022-05-12T20:46:03Z

Objective

Reduce variability of Time resource
Alternative to move time update to after everything #4728. See that PR for more detail on what causes most of the variation. This PR should be closer to what we want to do when we have pipelined rendering.

Solution

Move time updates into the render system. This puts the time update as close in time to frame presentation as possible.
Add a second extract stage after the render cleanup stage. This is required to move the time from the render world into the app world. Once we have pipelined rendering the two extract stages can be combined.

Histograms of the different approaches

main branch has 10x the standard deviation of the 2 PRs. The 2 PRs are very similar in terms of median and standard deviation, but the distributions are a little different. Not sure what effect the difference in distributions would have.

TODO

Figure out how to update the time when there is not a render sub app
Changelog
Migration Guide

Nilirad · 2022-05-12T20:56:05Z

Wouldn't moving Time update to the render stage break it for apps that don't use the render feature (e.g. apps made with MinimalPlugins)?

hymm · 2022-05-12T20:59:59Z

Wouldn't moving Time update to the render stage break it for apps that don't use the render feature (e.g. apps made with MinimalPlugins)?

definitely. See TODO items. But I think this is the most "correct" way of doing it when the render feature is enabled.

hymm · 2022-05-14T04:15:26Z

closing in favor of #4744

@aevyrie

# Objective - The time update is currently done in the wrong part of the schedule. For a single frame the current order of things is update input, update time (First stage), other stages, render stage (frame presentation). So when we update the time it includes the input processing of the current frame and the frame presentation of the previous frame. This is a problem when vsync is on. When input processing takes a longer amount of time for a frame, the vsync wait time gets shorter. So when these are not paired correctly we can potentially have a long input processing time added to the normal vsync wait time in the previous frame. This leads to inaccurate frame time reporting and more variance of the time than actually exists. For more details of why this is an issue see the linked issue below. - Helps with #4669 - Supercedes #4728 and #4735. This PR should be less controversial than those because it doesn't add to the API surface. ## Solution - The most accurate frame time would come from hardware. We currently don't have access to that for multiple reasons, so the next best thing we can do is measure the frame time as close to frame presentation as possible. This PR gets the Instant::now() for the time immediately after frame presentation in the render system and then sends that time to the app world through a channel. - implements suggestion from @aevyrie from here #4728 (comment) ## Statistics ![image](https://user-images.githubusercontent.com/2180432/168410265-f249f66e-ea9d-45d1-b3d8-7207a7bc536c.png) --- ## Changelog - Make frame time reporting more accurate. ## Migration Guide `time.delta()` now reports zero for 2 frames on startup instead of 1 frame.

@aevyrie

# Objective - The time update is currently done in the wrong part of the schedule. For a single frame the current order of things is update input, update time (First stage), other stages, render stage (frame presentation). So when we update the time it includes the input processing of the current frame and the frame presentation of the previous frame. This is a problem when vsync is on. When input processing takes a longer amount of time for a frame, the vsync wait time gets shorter. So when these are not paired correctly we can potentially have a long input processing time added to the normal vsync wait time in the previous frame. This leads to inaccurate frame time reporting and more variance of the time than actually exists. For more details of why this is an issue see the linked issue below. - Helps with bevyengine#4669 - Supercedes bevyengine#4728 and bevyengine#4735. This PR should be less controversial than those because it doesn't add to the API surface. ## Solution - The most accurate frame time would come from hardware. We currently don't have access to that for multiple reasons, so the next best thing we can do is measure the frame time as close to frame presentation as possible. This PR gets the Instant::now() for the time immediately after frame presentation in the render system and then sends that time to the app world through a channel. - implements suggestion from @aevyrie from here bevyengine#4728 (comment) ## Statistics ![image](https://user-images.githubusercontent.com/2180432/168410265-f249f66e-ea9d-45d1-b3d8-7207a7bc536c.png) --- ## Changelog - Make frame time reporting more accurate. ## Migration Guide `time.delta()` now reports zero for 2 frames on startup instead of 1 frame.

@aevyrie

# Objective - The time update is currently done in the wrong part of the schedule. For a single frame the current order of things is update input, update time (First stage), other stages, render stage (frame presentation). So when we update the time it includes the input processing of the current frame and the frame presentation of the previous frame. This is a problem when vsync is on. When input processing takes a longer amount of time for a frame, the vsync wait time gets shorter. So when these are not paired correctly we can potentially have a long input processing time added to the normal vsync wait time in the previous frame. This leads to inaccurate frame time reporting and more variance of the time than actually exists. For more details of why this is an issue see the linked issue below. - Helps with bevyengine#4669 - Supercedes bevyengine#4728 and bevyengine#4735. This PR should be less controversial than those because it doesn't add to the API surface. ## Solution - The most accurate frame time would come from hardware. We currently don't have access to that for multiple reasons, so the next best thing we can do is measure the frame time as close to frame presentation as possible. This PR gets the Instant::now() for the time immediately after frame presentation in the render system and then sends that time to the app world through a channel. - implements suggestion from @aevyrie from here bevyengine#4728 (comment) ## Statistics ![image](https://user-images.githubusercontent.com/2180432/168410265-f249f66e-ea9d-45d1-b3d8-7207a7bc536c.png) --- ## Changelog - Make frame time reporting more accurate. ## Migration Guide `time.delta()` now reports zero for 2 frames on startup instead of 1 frame.

@aevyrie

# Objective - The time update is currently done in the wrong part of the schedule. For a single frame the current order of things is update input, update time (First stage), other stages, render stage (frame presentation). So when we update the time it includes the input processing of the current frame and the frame presentation of the previous frame. This is a problem when vsync is on. When input processing takes a longer amount of time for a frame, the vsync wait time gets shorter. So when these are not paired correctly we can potentially have a long input processing time added to the normal vsync wait time in the previous frame. This leads to inaccurate frame time reporting and more variance of the time than actually exists. For more details of why this is an issue see the linked issue below. - Helps with bevyengine#4669 - Supercedes bevyengine#4728 and bevyengine#4735. This PR should be less controversial than those because it doesn't add to the API surface. ## Solution - The most accurate frame time would come from hardware. We currently don't have access to that for multiple reasons, so the next best thing we can do is measure the frame time as close to frame presentation as possible. This PR gets the Instant::now() for the time immediately after frame presentation in the render system and then sends that time to the app world through a channel. - implements suggestion from @aevyrie from here bevyengine#4728 (comment) ## Statistics ![image](https://user-images.githubusercontent.com/2180432/168410265-f249f66e-ea9d-45d1-b3d8-7207a7bc536c.png) --- ## Changelog - Make frame time reporting more accurate. ## Migration Guide `time.delta()` now reports zero for 2 frames on startup instead of 1 frame.

alice-i-cecile added C-Bug An unexpected or incorrect behavior C-Performance A change motivated by improving speed, memory usage or compile times A-Core labels May 12, 2022

update time in render stage

c17ef16

hymm force-pushed the update-time-in-render-stage branch from 190544a to c17ef16 Compare May 14, 2022 02:36

hymm mentioned this pull request May 14, 2022

[Merged by Bors] - Update time by sending frame instant through a channel #4744

Closed

hymm closed this May 14, 2022

hymm deleted the update-time-in-render-stage branch October 5, 2023 17:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update time in render stage #4735

update time in render stage #4735

hymm commented May 12, 2022 •

edited

Loading

Nilirad commented May 12, 2022

hymm commented May 12, 2022 •

edited

Loading

hymm commented May 14, 2022

update time in render stage #4735

update time in render stage #4735

Conversation

hymm commented May 12, 2022 • edited Loading

Objective

Solution

Histograms of the different approaches

TODO

Nilirad commented May 12, 2022

hymm commented May 12, 2022 • edited Loading

hymm commented May 14, 2022

hymm commented May 12, 2022 •

edited

Loading

hymm commented May 12, 2022 •

edited

Loading