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

Ability to skip rendering a frame / ask for second pass #4976

Closed
ficoos opened this issue Aug 17, 2024 · 2 comments · Fixed by #5059
Closed

Ability to skip rendering a frame / ask for second pass #4976

ficoos opened this issue Aug 17, 2024 · 2 comments · Fixed by #5059
Assignees
Labels
egui layout Related to widget layout

Comments

@ficoos
Copy link

ficoos commented Aug 17, 2024

Is your feature request related to a problem? Please describe.

I would like to be able to specifically skip a frame because I know it will cause a graphical stutter due to sizing layouting.
Being able to do ctx.skip_frame() or something similar to prompt the system to skip displaying the next frame and immediately request another repaint.

Describe the solution you'd like

Describe alternatives you've considered

Additional context

@emilk
Copy link
Owner

emilk commented Aug 27, 2024

I like this idea for covering up frame jitter issues, and have been considering it myself.

There's some trickyness to this to figure out:

  • We need to avoid an infinite loop, if skip_frame is repeatedly called. We should probably only ever skip one frame.
  • We need to handle the output of the skipped frame, just skip the rendering
  • Should we skip painting and request a new animation frame, or just call App::update (or viewport callback) again?
  • We must clear the input for the repeated frames (see Investigate a multipass (two-pass) version of egui #843 for more)

@emilk emilk added this to the Next Major Release milestone Aug 27, 2024
@emilk emilk changed the title Ability to skip frame Ability to skip rendering a frame / ask for second pass Aug 27, 2024
@ficoos
Copy link
Author

ficoos commented Aug 29, 2024

Avoiding an infinite loop is a non-issue IMHO because you can do any number of things to cause an infinite loop when programming. It is also difficult to detect if the skipping is due to the same cause or two different causes that happened to be in consecutive frames. There could maybe be a warning if it is called consecutively in debug by putting this information in a temp.

I think that the simplest thing would be to just skip painting and request a new frame. This makes the semantics almost identical to if the frame had been presented (the current behaviour) which would simplify the integration and backward compatibility.

The major issues with this that I see is actually that building this in to Widgets has a global effect (some operations can cause the UI to take twice as long to update) which some users may prefer jitter to lost frames.

This means it should probably be configurable whether to respect skip_frame requests.

@emilk emilk self-assigned this Sep 2, 2024
@emilk emilk added layout Related to widget layout egui style visuals and theming and removed style visuals and theming labels Sep 10, 2024
emilk added a commit that referenced this issue Sep 13, 2024
* Closes #4976
* Part of #4378 
* Implements parts of #843

### Background
Some widgets (like `Grid` and `Table`) needs to know the width of future
elements in order to properly size themselves. For instance, the width
of the first column of a grid may not be known until all rows of the
grid has been added, at which point it is too late. Therefore these
widgets store sizes from the previous frame. This leads to "first-frame
jitter", were the content is placed in the wrong place for one frame,
before being accurately laid out in subsequent frames.

### What
This PR adds the function `ctx.request_discard` which discards the
visual output and does another _pass_, i.e. calls the whole app UI code
once again (in eframe this means calling `App::update` again). This will
thus discard the shapes produced by the wrongly placed widgets, and
replace it with new shapes. Note that only the visual output is
discarded - all other output events are accumulated.

Calling `ctx.request_discard` should only be done in very rare
circumstances, e.g. when a `Grid` is first shown. Calling it every frame
will mean the UI code will become unnecessarily slow.

Two safe-guards are in place:

* `Options::max_passes` is by default 2, meaning egui will never do more
than 2 passes even if `request_discard` is called on every pass
* If multiple passes is done for multiple frames in a row, a warning
will be printed on the screen in debug builds:


![image](https://github.com/user-attachments/assets/c2c1e4a4-b7c9-4d7a-b3ad-abdd74bf449f)

### Breaking changes
A bunch of things that had "frame" in the name now has "pass" in them
instead:

* Functions called `begin_frame` and `end_frame` are now called
`begin_pass` and `end_pass`
* `FrameState` is now `PassState`
* etc


### TODO
* [x] Figure out good names for everything (`ctx.request_discard`)
* [x] Add API to query if we're gonna repeat this frame (to early-out
from expensive rendering)
* [x] Clear up naming confusion (pass vs frame) e.g. for `FrameState`
* [x] Figure out when to call this
* [x] Show warning on screen when there are several frames in a row with
multiple passes
* [x] Document
* [x] Default on or off?
* [x] Change `Context::frame_nr` name/docs
* [x] Rename `Context::begin_frame/end_frame` and deprecate the old ones
* [x] Test with Rerun
* [x] Document breaking changes
hacknus pushed a commit to hacknus/egui that referenced this issue Oct 30, 2024
* Closes emilk#4976
* Part of emilk#4378 
* Implements parts of emilk#843

### Background
Some widgets (like `Grid` and `Table`) needs to know the width of future
elements in order to properly size themselves. For instance, the width
of the first column of a grid may not be known until all rows of the
grid has been added, at which point it is too late. Therefore these
widgets store sizes from the previous frame. This leads to "first-frame
jitter", were the content is placed in the wrong place for one frame,
before being accurately laid out in subsequent frames.

### What
This PR adds the function `ctx.request_discard` which discards the
visual output and does another _pass_, i.e. calls the whole app UI code
once again (in eframe this means calling `App::update` again). This will
thus discard the shapes produced by the wrongly placed widgets, and
replace it with new shapes. Note that only the visual output is
discarded - all other output events are accumulated.

Calling `ctx.request_discard` should only be done in very rare
circumstances, e.g. when a `Grid` is first shown. Calling it every frame
will mean the UI code will become unnecessarily slow.

Two safe-guards are in place:

* `Options::max_passes` is by default 2, meaning egui will never do more
than 2 passes even if `request_discard` is called on every pass
* If multiple passes is done for multiple frames in a row, a warning
will be printed on the screen in debug builds:


![image](https://github.com/user-attachments/assets/c2c1e4a4-b7c9-4d7a-b3ad-abdd74bf449f)

### Breaking changes
A bunch of things that had "frame" in the name now has "pass" in them
instead:

* Functions called `begin_frame` and `end_frame` are now called
`begin_pass` and `end_pass`
* `FrameState` is now `PassState`
* etc


### TODO
* [x] Figure out good names for everything (`ctx.request_discard`)
* [x] Add API to query if we're gonna repeat this frame (to early-out
from expensive rendering)
* [x] Clear up naming confusion (pass vs frame) e.g. for `FrameState`
* [x] Figure out when to call this
* [x] Show warning on screen when there are several frames in a row with
multiple passes
* [x] Document
* [x] Default on or off?
* [x] Change `Context::frame_nr` name/docs
* [x] Rename `Context::begin_frame/end_frame` and deprecate the old ones
* [x] Test with Rerun
* [x] Document breaking changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
egui layout Related to widget layout
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants