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

Put repaint callback and tex manager behind separate mutexes #1389

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ NOTE: [`epaint`](epaint/CHANGELOG.md), [`eframe`](eframe/CHANGELOG.md), [`egui_w
### Added ⭐
* Added `Shape::Callback` for backend-specific painting ([#1351](https://github.com/emilk/egui/pull/1351)).
* Added `Frame::canvas` ([#1362](https://github.com/emilk/egui/pull/1362)).
* `Context::request_repaint` will wake up UI thread, if integrations has called `Context::set_request_repaint_callback` ([#1366](https://github.com/emilk/egui/pull/1366)).
* `Context::request_repaint` will wake up UI thread, if integrations has called `Context::with_repaint_callback` ([#1366](https://github.com/emilk/egui/pull/1366)).
* Added `Ui::push_id` ([#1374](https://github.com/emilk/egui/pull/1374)).

### Changed 🔧
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions egui-winit/src/epi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,12 +229,11 @@ pub struct EpiIntegration {
impl EpiIntegration {
pub fn new(
integration_name: &'static str,
egui_ctx: egui::Context,
max_texture_side: usize,
window: &winit::window::Window,
persistence: crate::epi::Persistence,
) -> Self {
let egui_ctx = egui::Context::default();

*egui_ctx.memory() = persistence.load_memory().unwrap_or_default();

let prefer_dark_mode = prefer_dark_mode();
Expand Down
1 change: 1 addition & 0 deletions egui/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ epaint = { version = "0.17.0", path = "../epaint", default-features = false }

ahash = "0.7"
nohash-hasher = "0.2"
parking_lot = "0.12"

# Optional:
ron = { version = "0.7", optional = true }
Expand Down
111 changes: 70 additions & 41 deletions egui/src/context.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
// #![warn(missing_docs)]

use std::sync::atomic::AtomicU32;
use std::sync::atomic::Ordering::SeqCst;

use crate::{
animation_manager::AnimationManager, data::output::PlatformOutput, frame_state::FrameState,
input_state::*, layers::GraphicLayers, memory::Options, output::FullOutput, TextureHandle, *,
Expand All @@ -8,7 +11,10 @@ use epaint::{mutex::*, stats::*, text::Fonts, TessellationOptions, *};

// ----------------------------------------------------------------------------

struct WrappedTextureManager(Arc<RwLock<epaint::TextureManager>>);
// NOTE: we always use `parking_lot` here so we can
// use `WrappedTextureManager` from any thread.
#[derive(Clone)]
struct WrappedTextureManager(Arc<parking_lot::RwLock<epaint::TextureManager>>);

impl Default for WrappedTextureManager {
fn default() -> Self {
Expand All @@ -21,7 +27,26 @@ impl Default for WrappedTextureManager {
);
assert_eq!(font_id, TextureId::default());

Self(Arc::new(RwLock::new(tex_mngr)))
Self(Arc::new(parking_lot::RwLock::new(tex_mngr)))
}
}

// ----------------------------------------------------------------------------

struct RepaintInfo {
/// While positive, keep requesting repaints. Decrement at the end of each frame.
repaint_requests: AtomicU32,
request_repaint_callbacks: Option<Box<dyn Fn() + Send + Sync>>,
}

impl Default for RepaintInfo {
fn default() -> Self {
Self {
// Start with painting an extra frame to compensate for some widgets
// that take two frames before they "settle":
repaint_requests: 1.into(),
request_repaint_callbacks: None,
}
}
}

Expand All @@ -33,7 +58,6 @@ struct ContextImpl {
fonts: Option<Fonts>,
memory: Memory,
animation_manager: AnimationManager,
tex_manager: WrappedTextureManager,

input: InputState,

Expand All @@ -45,10 +69,6 @@ struct ContextImpl {
output: PlatformOutput,

paint_stats: PaintStats,

/// While positive, keep requesting repaints. Decrement at the end of each frame.
repaint_requests: u32,
request_repaint_callbacks: Option<Box<dyn Fn() + Send + Sync>>,
}

impl ContextImpl {
Expand Down Expand Up @@ -142,33 +162,46 @@ impl ContextImpl {
/// paint(full_output.textures_delta, clipped_primitives);
/// }
/// ```
#[derive(Clone)]
pub struct Context(Arc<RwLock<ContextImpl>>);
#[derive(Clone, Default)]
pub struct Context {
ctx: Arc<RwLock<ContextImpl>>,
// These are not inside of `ContextImpl` because we want to have thread-safe access to them
// even without running with the multi_threaded feature flag.
// See https://github.com/emilk/egui/issues/1379.
repaint_info: Arc<RepaintInfo>,
tex_manager: WrappedTextureManager,
}

impl std::cmp::PartialEq for Context {
fn eq(&self, other: &Context) -> bool {
Arc::ptr_eq(&self.0, &other.0)
Arc::ptr_eq(&self.ctx, &other.ctx)
}
}

impl Default for Context {
fn default() -> Self {
Self(Arc::new(RwLock::new(ContextImpl {
// Start with painting an extra frame to compensate for some widgets
// that take two frames before they "settle":
repaint_requests: 1,
..ContextImpl::default()
})))
impl Context {
pub fn new() -> Self {
Self::default()
}

/// Construct a [`Context`] with a callback that is called when the user calls
/// [`Context::request_repaint`].
pub fn with_repaint_callback(callback: impl Fn() + Send + Sync + 'static) -> Self {
Self {
ctx: Default::default(),
repaint_info: Arc::new(RepaintInfo {
request_repaint_callbacks: Some(Box::new(callback)),
..Default::default()
}),
tex_manager: Default::default(),
}
}
}

impl Context {
fn read(&self) -> RwLockReadGuard<'_, ContextImpl> {
self.0.read()
self.ctx.read()
}

fn write(&self) -> RwLockWriteGuard<'_, ContextImpl> {
self.0.write()
self.ctx.write()
}

/// Run the ui code for one frame.
Expand Down Expand Up @@ -538,26 +571,19 @@ impl Context {
/// If this is called at least once in a frame, then there will be another frame right after this.
/// Call as many times as you wish, only one repaint will be issued.
///
/// It is safe to call this from any thread at any time.
///
/// If called from outside the UI thread, the UI thread will wake up and run,
/// provided the egui integration has set that up via [`Self::set_request_repaint_callback`]
/// provided the egui integration has set that up via [`Self::with_repaint_callback`]
/// (this will work on `eframe`).
pub fn request_repaint(&self) {
// request two frames of repaint, just to cover some corner cases (frame delays):
let mut ctx = self.write();
ctx.repaint_requests = 2;
if let Some(callback) = &ctx.request_repaint_callbacks {
self.repaint_info.repaint_requests.store(2, SeqCst);
if let Some(callback) = &self.repaint_info.request_repaint_callbacks {
(callback)();
}
}

/// For integrations: this callback will be called when an egui user calls [`Self::request_repaint`].
///
/// This lets you wake up a sleeping UI thread.
pub fn set_request_repaint_callback(&self, callback: impl Fn() + Send + Sync + 'static) {
let callback = Box::new(callback);
self.write().request_repaint_callbacks = Some(callback);
}

/// Tell `egui` which fonts to use.
///
/// The default `egui` fonts only support latin and cyrillic alphabets,
Expand Down Expand Up @@ -661,6 +687,8 @@ impl Context {
///
/// For how to load an image, see [`ImageData`] and [`ColorImage::from_rgba_unmultiplied`].
///
/// This is safe to call from any thread at any time.
///
/// ```
/// struct MyImage {
/// texture: Option<egui::TextureHandle>,
Expand Down Expand Up @@ -706,8 +734,10 @@ impl Context {
/// In general it is easier to use [`Self::load_texture`] and [`TextureHandle`].
///
/// You can show stats about the allocated textures using [`Self::texture_ui`].
pub fn tex_manager(&self) -> Arc<RwLock<epaint::textures::TextureManager>> {
self.read().tex_manager.0.clone()
///
/// This is safe to call from any thread at any time.
pub fn tex_manager(&self) -> Arc<parking_lot::RwLock<epaint::textures::TextureManager>> {
self.tex_manager.0.clone()
}

// ---------------------------------------------------------------------
Expand Down Expand Up @@ -764,20 +794,19 @@ impl Context {

let font_image_delta = ctx_impl.fonts.as_ref().unwrap().font_image_delta();
if let Some(font_image_delta) = font_image_delta {
ctx_impl
.tex_manager
self.tex_manager
.0
.write()
.set(TextureId::default(), font_image_delta);
}

textures_delta = ctx_impl.tex_manager.0.write().take_delta();
textures_delta = self.tex_manager.0.write().take_delta();
};

let platform_output: PlatformOutput = std::mem::take(&mut self.output());

let needs_repaint = if self.read().repaint_requests > 0 {
self.write().repaint_requests -= 1;
let needs_repaint = if self.repaint_info.repaint_requests.load(SeqCst) > 0 {
self.repaint_info.repaint_requests.fetch_sub(1, SeqCst);
true
} else {
false
Expand Down
15 changes: 8 additions & 7 deletions egui_glow/src/epi_backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,20 +46,21 @@ pub fn run(app_name: &str, native_options: &epi::NativeOptions, app_creator: epi
let mut painter = crate::Painter::new(gl.clone(), None, "")
.unwrap_or_else(|error| panic!("some OpenGL error occurred {}\n", error));

let event_loop_proxy = parking_lot::Mutex::new(event_loop.create_proxy());
let egui_ctx = egui::Context::with_repaint_callback({
move || {
event_loop_proxy.lock().send_event(RequestRepaintEvent).ok();
}
});

let mut integration = egui_winit::epi::EpiIntegration::new(
"egui_glow",
egui_ctx,
painter.max_texture_side(),
gl_window.window(),
persistence,
);

{
let event_loop_proxy = parking_lot::Mutex::new(event_loop.create_proxy());
integration.egui_ctx.set_request_repaint_callback(move || {
event_loop_proxy.lock().send_event(RequestRepaintEvent).ok();
});
}

let mut app = app_creator(&epi::CreationContext {
egui_ctx: integration.egui_ctx.clone(),
integration_info: integration.frame.info(),
Expand Down
10 changes: 4 additions & 6 deletions egui_web/src/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,14 +159,12 @@ impl AppRunner {

let needs_repaint: std::sync::Arc<NeedRepaint> = Default::default();

let egui_ctx = egui::Context::default();

{
let egui_ctx = egui::Context::with_repaint_callback({
let needs_repaint = needs_repaint.clone();
egui_ctx.set_request_repaint_callback(move || {
move || {
needs_repaint.0.store(true, SeqCst);
});
}
}
});

load_memory(&egui_ctx);
if prefer_dark_mode == Some(true) {
Expand Down
4 changes: 2 additions & 2 deletions epaint/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ single_threaded = ["atomic_refcell"]

# Only needed if you plan to use the same fonts from multiple threads.
# It comes with a minor performance impact.
multi_threaded = ["parking_lot"]
multi_threaded = []


[dependencies]
Expand All @@ -63,7 +63,7 @@ atomic_refcell = { version = "0.1", optional = true } # Used
bytemuck = { version = "1.7.2", optional = true, features = ["derive"] }
cint = { version = "^0.2.2", optional = true }
nohash-hasher = "0.2"
parking_lot = { version = "0.12", optional = true } # Using parking_lot over std::sync::Mutex gives 50% speedups in some real-world scenarios.
parking_lot = "0.12"
serde = { version = "1", optional = true, features = ["derive", "rc"] }

[dev-dependencies]
Expand Down
8 changes: 3 additions & 5 deletions epaint/src/texture_handle.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
use crate::{
emath::NumExt,
mutex::{Arc, RwLock},
ImageData, ImageDelta, TextureId, TextureManager,
};
use crate::{emath::NumExt, mutex::Arc, ImageData, ImageDelta, TextureId, TextureManager};

use parking_lot::RwLock;

/// Used to paint images.
///
Expand Down