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

Propagate previously registered scopes when creating GlobalFrameView #206

Merged
merged 1 commit into from
Jul 31, 2024

Conversation

insertt
Copy link
Contributor

@insertt insertt commented May 6, 2024

GlobalFrameView might be created after some scopes were already registered and frame sink won't see them without prior propagation with GlobalProfiler::emit_scope_snapshot().

Checklist

  • I have read the Contributor Guide
  • I have read and agree to the Code of Conduct
  • I have added a description of my changes and why I'd like them included in the section below

Description of Changes

I've modified init function of GlobalFrameView so that it calls GlobalProfiler::emit_scope_snapshot() after the frame sink is registered so that it can see previously registered scopes.

Related Issues

I've noticed this issue when trying to render profiler UI conditionally with being disabled by default and in result scopes that were registered before the first call to the GlobalProfiler::new_frame() were not propagated to the frame sink that was registered afterwards.

Simple reproduction code:

let mut render_profiler_ui = false;

[..]

if render_profiler_ui {
  puffin_egui::profiler_window(&ctx);
}

[..]

render_profiler_ui |= input.key_pressed(Key::F12);

GlobalFrameView might be created after some scopes were already created
and our registered sink won't see them without prior propagation with
GlobalProfiler::emit_scope_snapshot().
@insertt insertt requested review from emilk and TimonPost as code owners May 6, 2024 16:33
@emilk emilk merged commit 0b0ad3f into EmbarkStudios:main Jul 31, 2024
1 check passed
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.

2 participants