Skip to content

Add Continuous mode to ctx.memory.options#4214

Draft
rustbasic wants to merge 33 commits intoemilk:mainfrom
rustbasic:patch29
Draft

Add Continuous mode to ctx.memory.options#4214
rustbasic wants to merge 33 commits intoemilk:mainfrom
rustbasic:patch29

Conversation

@rustbasic
Copy link
Copy Markdown
Contributor

Add Continuous mode to ctx.memory.options

let continuous_mode = egui_ctx.options(|options| options.continuous_mode);
if continuous_mode {
let viewport_id = egui_ctx.viewport_id();
// TODO : Do not recall until the next repaint.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The contributing guidelines say:

Add your github handle to the TODO:s you write, e.g: TODO(emilk): clean this up

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The contributing guidelines say:

Add your github handle to the TODO:s you write, e.g: TODO(emilk): clean this up

thank you.

Copy link
Copy Markdown
Contributor Author

@rustbasic rustbasic Mar 23, 2024

Choose a reason for hiding this comment

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

The contributing guidelines say:

Add your github handle to the TODO:s you write, e.g: TODO(emilk): clean this up

If you know how to pass the doc test, please let me know.
It doesn't work with the "no_run" option.

It would be better to exclude what is in egui_demo_app because it seems that we do not want to write it in the doc.
Still, I'd like to know how.

// TODO(rustbasic) : Do not recall until the next repaint.
// 1000 millis / 60 fps = 16.67 millis
self.egui_ctx
.request_repaint_after_for(std::time::Duration::from_millis(8), viewport_id);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

why 8 ms delay?

Copy link
Copy Markdown
Contributor Author

@rustbasic rustbasic Mar 25, 2024

Choose a reason for hiding this comment

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

why 8 ms delay?

It takes about two attempts.
It is the most efficient.
If you do 16.67ms, 60 fps cannot be achieved.

In conclusion, this method solves everything with only 1% CPU usage.
However, having subviewports increases usage by about 10%.
Currently, if you keep calling request_repaint(), more than 10% of your CPU is being used even without doing anything.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

But why not request an immediate repaint? eframe has vsync anyways

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

But why not request an immediate repaint? eframe has vsync anyways

But why not request an immediate repaint? eframe has vsync anyways

This is an unnecessary waste of CPU usage.
It would be a good idea to apply this and continue to monitor and upgrade.
It can't be perfect from the beginning.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

It's not a waste of CPU, because of vsync.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's not a waste of CPU, because of vsync.

It will be more effective if you actually use it and check the CPU usage.

There is also the advantage that the user does not have to worry about request_repaint().

@rustbasic rustbasic requested review from YgorSouza and emilk March 25, 2024 11:29

/// If `true`, This will call `egui::Context::request_repaint()` at the end of each frame
/// If `false` (default), egui is only updated if are input events (like mouse movements) or there are some animations in the GUI.
pub continuous_mode: bool,
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Maybe we should just move enum RunMode { into egui from crates/egui_demo_app/src/backend_panel.rs. At least we should copy its documentation.

self.frame_history
.on_new_frame(ctx.input(|i| i.time), frame.info().cpu_usage);

match self.run_mode {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

self.run_mode can be replaced with options.continuous_mode

@rustbasic rustbasic requested a review from emilk March 30, 2024 08:59
@emilk emilk marked this pull request as draft April 21, 2024 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants