removed ZST for Ui and changed shared font atlas to be based on UnsafeCell

CHANGELOG.markdown

@@ -6,6 +6,10 @@
 
 - BREAKING: Removed `push_style_colors` and `push_style_vars`. Instead, use `push_style_color` in a loop. This was deprecated in `0.7.0` and should have been removed in `0.8.0`. This also removes their associated tokens.
 
+- BREAKING: Ui now does not have a lifetime associated with it, but is only ever given to users in the form of `&mut Ui`. Additionally, the `render` function has been moved to the `Context` instead of `Ui`.
+
+- BREAKING: `SharedFontAtlas` now uses `UnsafeCell` rather than `Rc<RefCell>` as its wrapper -- this simplifies the codebase and more accurately reflects how we expect `SharedFontAtlas` to be used (ie, you're probably going to set it up once, and then give it around, rather than constantly edit it). `SharedFontAtlas` users, if this change is very bad for you, please let us know with issues!
+
 ## [0.8.0] - 2021-09-17
 
 Welcome to the `0.8.0` update. This is one of the largest updates imgui-rs has ever seen; it will generate errors in a `0.7` project, but hopefully it should be both quick to fix, and enjoyable to update. See our [release page](https://github.com/imgui-rs/imgui-rs/releases/tag/v0.8.0) for more information and a list of contributors to this cycle. Thank you to everyone who uses `imgui-rs`, files issues, and spend their time and effort to PR new changes into the codebase. Because of all that effort, this is by far the best `imgui-rs` has looked!

imgui/src/context.rs

@@ -1,13 +1,12 @@
 use parking_lot::ReentrantMutex;
-use std::cell::{RefCell, UnsafeCell};
+use std::cell::UnsafeCell;
 use std::ffi::{CStr, CString};
 use std::ops::Drop;
 use std::path::PathBuf;
 use std::ptr;
-use std::rc::Rc;
 
 use crate::clipboard::{ClipboardBackend, ClipboardContext};
-use crate::fonts::atlas::{FontAtlas, FontAtlasRefMut, FontId, SharedFontAtlas};
+use crate::fonts::atlas::{FontAtlas, FontId, SharedFontAtlas};
 use crate::io::Io;
 use crate::style::Style;
 use crate::Ui;
@@ -50,7 +49,7 @@ use crate::{sys, DrawData};
 #[derive(Debug)]
 pub struct Context {
     raw: *mut sys::ImGuiContext,
-    shared_font_atlas: Option<Rc<RefCell<SharedFontAtlas>>>,
+    shared_font_atlas: Option<UnsafeCell<SharedFontAtlas>>,
     ini_filename: Option<CString>,
     log_filename: Option<CString>,
     platform_name: Option<CString>,
@@ -94,7 +93,7 @@ impl Context {
     ///
     /// Panics if an active context already exists
     #[doc(alias = "CreateContext")]
-    pub fn create_with_shared_font_atlas(shared_font_atlas: Rc<RefCell<SharedFontAtlas>>) -> Self {
+    pub fn create_with_shared_font_atlas(shared_font_atlas: UnsafeCell<SharedFontAtlas>) -> Self {
         Self::create_internal(Some(shared_font_atlas))
     }
     /// Suspends this context so another context can be the active context.
@@ -217,7 +216,7 @@ impl Context {
         io.clipboard_user_data = clipboard_ctx.get() as *mut _;
         self.clipboard_ctx = clipboard_ctx;
     }
-    fn create_internal(shared_font_atlas: Option<Rc<RefCell<SharedFontAtlas>>>) -> Self {
+    fn create_internal(shared_font_atlas: Option<UnsafeCell<SharedFontAtlas>>) -> Self {
         let _guard = CTX_MUTEX.lock();
         assert!(
             no_current_context(),
@@ -226,8 +225,8 @@ impl Context {
 
         let shared_font_atlas_ptr = match &shared_font_atlas {
             Some(shared_font_atlas) => {
-                let borrowed_font_atlas = shared_font_atlas.borrow();
+                let borrowed_font_atlas = shared_font_atlas.get();
-                borrowed_font_atlas.0
+                unsafe { &*borrowed_font_atlas }.0
             }
             None => ptr::null_mut(),
         };
@@ -243,7 +242,9 @@ impl Context {
             platform_name: None,
             renderer_name: None,
             clipboard_ctx: Box::new(ClipboardContext::dummy().into()),
-            ui: Ui(().into()),
+            ui: Ui {
+                buffer: UnsafeCell::new(crate::string::UiBuffer::new(1024)),
+            },
         }
     }
     fn is_current_context(&self) -> bool {
@@ -297,7 +298,7 @@ impl SuspendedContext {
         Self::create_internal(None)
     }
     /// Creates a new suspended imgui-rs context with a shared font atlas.
-    pub fn create_with_shared_font_atlas(shared_font_atlas: Rc<RefCell<SharedFontAtlas>>) -> Self {
+    pub fn create_with_shared_font_atlas(shared_font_atlas: UnsafeCell<SharedFontAtlas>) -> Self {
         Self::create_internal(Some(shared_font_atlas))
     }
     /// Attempts to activate this suspended context.
@@ -318,7 +319,7 @@ impl SuspendedContext {
             Err(self)
         }
     }
-    fn create_internal(shared_font_atlas: Option<Rc<RefCell<SharedFontAtlas>>>) -> Self {
+    fn create_internal(shared_font_atlas: Option<UnsafeCell<SharedFontAtlas>>) -> Self {
         let _guard = CTX_MUTEX.lock();
         let raw = unsafe { sys::igCreateContext(ptr::null_mut()) };
         let ctx = Context {
@@ -329,7 +330,9 @@ impl SuspendedContext {
             platform_name: None,
             renderer_name: None,
             clipboard_ctx: Box::new(ClipboardContext::dummy().into()),
-            ui: Ui(().into()),
+            ui: Ui {
+                buffer: UnsafeCell::new(crate::string::UiBuffer::new(1024)),
+            },
         };
         if ctx.is_current_context() {
             // Oops, the context was activated -> deactivate
@@ -411,9 +414,9 @@ fn test_suspend_failure() {
 #[test]
 fn test_shared_font_atlas() {
     let _guard = crate::test::TEST_MUTEX.lock();
-    let atlas = Rc::new(RefCell::new(SharedFontAtlas::create()));
+    let atlas = SharedFontAtlas::create();
-    let suspended1 = SuspendedContext::create_with_shared_font_atlas(atlas.clone());
+    let suspended1 = SuspendedContext::create_with_shared_font_atlas(atlas.clone().into());
-    let mut ctx2 = Context::create_with_shared_font_atlas(atlas);
+    let mut ctx2 = Context::create_with_shared_font_atlas(atlas.into());
     {
         let _borrow = ctx2.fonts();
     }
@@ -422,17 +425,6 @@ fn test_shared_font_atlas() {
     let _borrow = ctx.fonts();
 }
 
-#[test]
-#[should_panic]
-fn test_shared_font_atlas_borrow_panic() {
-    let _guard = crate::test::TEST_MUTEX.lock();
-    let atlas = Rc::new(RefCell::new(SharedFontAtlas::create()));
-    let _suspended = SuspendedContext::create_with_shared_font_atlas(atlas.clone());
-    let mut ctx = Context::create_with_shared_font_atlas(atlas.clone());
-    let _borrow1 = atlas.borrow();
-    let _borrow2 = ctx.fonts();
-}
-
 #[test]
 fn test_ini_load_save() {
     let (_guard, mut ctx) = crate::test::test_ctx();
@@ -506,19 +498,11 @@ impl Context {
         }
     }
     /// Returns a mutable reference to the font atlas.
-    ///
+    pub fn fonts(&mut self) -> &mut FontAtlas {
-    /// # Panics
+        // we take this with an `&mut Self` here, which means
-    ///
+        // that we can't get the sharedfontatlas through safe code
-    /// Panics if the context uses a shared font atlas that is already borrowed
+        // otherwise
-    pub fn fonts(&mut self) -> FontAtlasRefMut<'_> {
+        unsafe { &mut *(self.io_mut().fonts as *mut FontAtlas) }
-        match self.shared_font_atlas {
-            Some(ref font_atlas) => FontAtlasRefMut::Shared(font_atlas.borrow_mut()),
-            None => unsafe {
-                // safe because FontAtlas is a transparent wrapper around sys::ImFontAtlas
-                let fonts = &mut *(self.io_mut().fonts as *mut FontAtlas);
-                FontAtlasRefMut::Owned(fonts)
-            },
-        }
     }
 
     /// Starts a new frame.
@@ -539,10 +523,6 @@ impl Context {
         if !default_font.is_null() && self.fonts().get_font(FontId(default_font)).is_none() {
             self.io_mut().font_default = ptr::null_mut();
         }
-        // NewFrame/Render/EndFrame mutate the font atlas so we need exclusive access to it
-        if let Some(font_atlas) = self.shared_font_atlas.as_ref() {
-            assert!(font_atlas.try_borrow_mut().is_ok());
-        }
         // TODO: precondition checks
         unsafe {
             sys::igNewFrame();

imgui/src/fonts/atlas.rs

@@ -433,7 +433,7 @@ pub struct FontAtlasTexture<'a> {
 }
 
 /// A font atlas that can be shared between contexts
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct SharedFontAtlas(pub(crate) *mut sys::ImFontAtlas);
 
 impl SharedFontAtlas {

imgui/src/lib.rs

@@ -116,15 +116,12 @@ impl Context {
     }
 }
 
-/// A temporary reference for building the user interface for one frame
+/// A reference for building the user interface for one frame
 #[derive(Debug)]
-pub struct Ui(pub(crate) cell::UnsafeCell<()>);
+pub struct Ui {
-
+    // our scratch sheet
-/// This is our internal buffer that we use for the Ui object.
+    buffer: cell::UnsafeCell<string::UiBuffer>,
-///
+}
-/// We edit this buffer
-static mut BUFFER: cell::UnsafeCell<string::UiBuffer> =
-    cell::UnsafeCell::new(string::UiBuffer::new(100));
 
 impl Ui {
     /// This provides access to the backing scratch buffer that we use to write
@@ -141,13 +138,13 @@ impl Ui {
     /// We otherwise make no assumptions about the size or keep state in this buffer between calls,
     /// so editing the `UiBuffer` is fine.
     pub unsafe fn scratch_buffer(&self) -> &cell::UnsafeCell<string::UiBuffer> {
-        &BUFFER
+        &self.buffer
     }
 
     /// Internal method to push a single text to our scratch buffer.
     fn scratch_txt(&self, txt: impl AsRef<str>) -> *const sys::cty::c_char {
         unsafe {
-            let handle = &mut *BUFFER.get();
+            let handle = &mut *self.buffer.get();
             handle.scratch_txt(txt)
         }
     }
@@ -155,7 +152,7 @@ impl Ui {
     /// Internal method to push an option text to our scratch buffer.
     fn scratch_txt_opt(&self, txt: Option<impl AsRef<str>>) -> *const sys::cty::c_char {
         unsafe {
-            let handle = &mut *BUFFER.get();
+            let handle = &mut *self.buffer.get();
             handle.scratch_txt_opt(txt)
         }
     }
@@ -166,7 +163,7 @@ impl Ui {
         txt_1: impl AsRef<str>,
     ) -> (*const sys::cty::c_char, *const sys::cty::c_char) {
         unsafe {
-            let handle = &mut *BUFFER.get();
+            let handle = &mut *self.buffer.get();
             handle.scratch_txt_two(txt_0, txt_1)
         }
     }
@@ -177,7 +174,7 @@ impl Ui {
         txt_1: Option<impl AsRef<str>>,
     ) -> (*const sys::cty::c_char, *const sys::cty::c_char) {
         unsafe {
-            let handle = &mut *BUFFER.get();
+            let handle = &mut *self.buffer.get();
             handle.scratch_txt_with_opt(txt_0, txt_1)
         }
     }