Fix buggy text withviewports on monitors with different scales (#3666)

* Closes https://github.com/emilk/egui/issues/3664

Bonus: optimize color conversions and font atlas upload, especially in
debug builds.

crates/ecolor/src/lib.rs

@@ -93,7 +93,7 @@ pub fn linear_u8_from_linear_f32(a: f32) -> u8 {
 }
 
 fn fast_round(r: f32) -> u8 {
-    (r + 0.5).floor() as _ // rust does a saturating cast since 1.45
+    (r + 0.5) as _ // rust does a saturating cast since 1.45
 }
 
 #[test]

crates/eframe/src/lib.rs

@@ -221,8 +221,6 @@ pub fn run_native(
     mut native_options: NativeOptions,
     app_creator: AppCreator,
 ) -> Result<()> {
-    let renderer = native_options.renderer;
-
     #[cfg(not(feature = "__screenshot"))]
     assert!(
         std::env::var("EFRAME_SCREENSHOT_TO").is_err(),
@@ -233,6 +231,17 @@ pub fn run_native(
         native_options.viewport.title = Some(app_name.to_owned());
     }
 
+    let renderer = native_options.renderer;
+
+    #[cfg(all(feature = "glow", feature = "wgpu"))]
+    {
+        match renderer {
+            Renderer::Glow => "glow",
+            Renderer::Wgpu => "wgpu",
+        };
+        log::info!("Both the glow and wgpu renderers are available. Using {renderer}.");
+    }
+
     match renderer {
         #[cfg(feature = "glow")]
         Renderer::Glow => {

crates/eframe/src/native/glow_integration.rs

@@ -65,6 +65,8 @@ struct GlowWinitRunning {
 
     // These needs to be shared with the immediate viewport renderer, hence the Rc/Arc/RefCells:
     glutin: Rc<RefCell<GlutinWindowContext>>,
+
+    // NOTE: one painter shared by all viewports.
     painter: Rc<RefCell<egui_glow::Painter>>,
 }
 

crates/eframe/src/native/wgpu_integration.rs

@@ -50,7 +50,9 @@ struct WgpuWinitRunning {
     shared: Rc<RefCell<SharedState>>,
 }
 
-/// Everything needed by the immediate viewport renderer.
+/// Everything needed by the immediate viewport renderer.\
+///
+/// This is shared by all viewports.
 ///
 /// Wrapped in an `Rc<RefCell<…>>` so it can be re-entrantly shared via a weak-pointer.
 pub struct SharedState {
@@ -161,7 +163,11 @@ impl WgpuWinitApp {
             ),
             self.native_options.viewport.transparent.unwrap_or(false),
         );
-        pollster::block_on(painter.set_window(ViewportId::ROOT, Some(&window)))?;
+
+        {
+            crate::profile_scope!("set_window");
+            pollster::block_on(painter.set_window(ViewportId::ROOT, Some(&window)))?;
+        }
 
         let wgpu_render_state = painter.render_state();
 
@@ -921,11 +927,14 @@ fn render_immediate_viewport(
         return;
     };
 
-    if let Err(err) = pollster::block_on(painter.set_window(ids.this, Some(window))) {
-        log::error!(
-            "when rendering viewport_id={:?}, set_window Error {err}",
-            ids.this
-        );
+    {
+        crate::profile_scope!("set_window");
+        if let Err(err) = pollster::block_on(painter.set_window(ids.this, Some(window))) {
+            log::error!(
+                "when rendering viewport_id={:?}, set_window Error {err}",
+                ids.this
+            );
+        }
     }
 
     let clipped_primitives = egui_ctx.tessellate(shapes, pixels_per_point);
@@ -997,6 +1006,8 @@ fn initialize_or_update_viewport<'vp>(
     viewport_ui_cb: Option<Arc<dyn Fn(&egui::Context) + Send + Sync>>,
     focused_viewport: Option<ViewportId>,
 ) -> &'vp mut Viewport {
+    crate::profile_function!();
+
     if builder.icon.is_none() {
         // Inherit icon from parent
         builder.icon = viewports

crates/egui-wgpu/src/renderer.rs

@@ -527,12 +527,14 @@ impl Renderer {
                     image.pixels.len(),
                     "Mismatch between texture size and texel count"
                 );
-                Cow::Owned(image.srgba_pixels(None).collect::<Vec<_>>())
+                crate::profile_scope!("font -> sRGBA");
+                Cow::Owned(image.srgba_pixels(None).collect::<Vec<egui::Color32>>())
             }
         };
         let data_bytes: &[u8] = bytemuck::cast_slice(data_color32.as_slice());
 
         let queue_write_data_to_texture = |texture, origin| {
+            crate::profile_scope!("write_texture");
             queue.write_texture(
                 wgpu::ImageCopyTexture {
                     texture,
@@ -570,16 +572,19 @@ impl Renderer {
             // Use same label for all resources associated with this texture id (no point in retyping the type)
             let label_str = format!("egui_texid_{id:?}");
             let label = Some(label_str.as_str());
-            let texture = device.create_texture(&wgpu::TextureDescriptor {
-                label,
-                size,
-                mip_level_count: 1,
-                sample_count: 1,
-                dimension: wgpu::TextureDimension::D2,
-                format: wgpu::TextureFormat::Rgba8UnormSrgb, // Minspec for wgpu WebGL emulation is WebGL2, so this should always be supported.
-                usage: wgpu::TextureUsages::TEXTURE_BINDING | wgpu::TextureUsages::COPY_DST,
-                view_formats: &[wgpu::TextureFormat::Rgba8UnormSrgb],
-            });
+            let texture = {
+                crate::profile_scope!("create_texture");
+                device.create_texture(&wgpu::TextureDescriptor {
+                    label,
+                    size,
+                    mip_level_count: 1,
+                    sample_count: 1,
+                    dimension: wgpu::TextureDimension::D2,
+                    format: wgpu::TextureFormat::Rgba8UnormSrgb, // Minspec for wgpu WebGL emulation is WebGL2, so this should always be supported.
+                    usage: wgpu::TextureUsages::TEXTURE_BINDING | wgpu::TextureUsages::COPY_DST,
+                    view_formats: &[wgpu::TextureFormat::Rgba8UnormSrgb],
+                })
+            };
             let sampler = self
                 .samplers
                 .entry(image_delta.options)

crates/egui-wgpu/src/winit.rs

@@ -74,6 +74,8 @@ impl BufferPadding {
 /// Everything you need to paint egui with [`wgpu`] on [`winit`].
 ///
 /// Alternatively you can use [`crate::renderer`] directly.
+///
+/// NOTE: all egui viewports share the same painter.
 pub struct Painter {
     configuration: WgpuConfiguration,
     msaa_samples: u32,

crates/egui/src/context.rs

@@ -3,7 +3,7 @@
 use std::{borrow::Cow, cell::RefCell, sync::Arc, time::Duration};
 
 use ahash::HashMap;
-use epaint::{mutex::*, stats::*, text::Fonts, TessellationOptions, *};
+use epaint::{mutex::*, stats::*, text::Fonts, util::OrderedFloat, TessellationOptions, *};
 
 use crate::{
     animation_manager::AnimationManager,
@@ -194,10 +194,23 @@ impl Default for ViewportRepaintInfo {
 
 #[derive(Default)]
 struct ContextImpl {
-    /// `None` until the start of the first frame.
-    fonts: Option<Fonts>,
+    /// Since we could have multiple viewport across multiple monitors with
+    /// different `pixels_per_point`, we need a `Fonts` instance for each unique
+    /// `pixels_per_point`.
+    /// This is because the `Fonts` depend on `pixels_per_point` for the font atlas
+    /// as well as kerning, font sizes, etc.
+    fonts: std::collections::BTreeMap<OrderedFloat<f32>, Fonts>,
+    font_definitions: FontDefinitions,
+
     memory: Memory,
     animation_manager: AnimationManager,
+
+    /// All viewports share the same texture manager and texture namespace.
+    ///
+    /// In all viewports, [`TextureId::default`] is special, and points to the font atlas.
+    /// The font-atlas texture _may_ be different across viewports, as they may have different
+    /// `pixels_per_point`, so we do special book-keeping for that.
+    /// See <https://github.com/emilk/egui/issues/3664>.
     tex_manager: WrappedTextureManager,
 
     /// Set during the frame, becomes active at the start of the next frame.
@@ -335,23 +348,37 @@ impl ContextImpl {
         let max_texture_side = input.max_texture_side;
 
         if let Some(font_definitions) = self.memory.new_font_definitions.take() {
-            crate::profile_scope!("Fonts::new");
-            let fonts = Fonts::new(pixels_per_point, max_texture_side, font_definitions);
-            self.fonts = Some(fonts);
+            // New font definition loaded, so we need to reload all fonts.
+            self.fonts.clear();
+            self.font_definitions = font_definitions;
+            #[cfg(feature = "log")]
+            log::debug!("Loading new font definitions");
         }
 
-        let fonts = self.fonts.get_or_insert_with(|| {
-            let font_definitions = FontDefinitions::default();
-            crate::profile_scope!("Fonts::new");
-            Fonts::new(pixels_per_point, max_texture_side, font_definitions)
-        });
+        let mut is_new = false;
+
+        let fonts = self
+            .fonts
+            .entry(pixels_per_point.into())
+            .or_insert_with(|| {
+                #[cfg(feature = "log")]
+                log::debug!("Creating new Fonts for pixels_per_point={pixels_per_point}");
+
+                is_new = true;
+                crate::profile_scope!("Fonts::new");
+                Fonts::new(
+                    pixels_per_point,
+                    max_texture_side,
+                    self.font_definitions.clone(),
+                )
+            });
 
         {
             crate::profile_scope!("Fonts::begin_frame");
             fonts.begin_frame(pixels_per_point, max_texture_side);
         }
 
-        if self.memory.options.preload_font_glyphs {
+        if is_new && self.memory.options.preload_font_glyphs {
             crate::profile_scope!("preload_font_glyphs");
             // Preload the most common characters for the most common fonts.
             // This is not very important to do, but may save a few GPU operations.
@@ -379,6 +406,10 @@ impl ContextImpl {
         builders.get_mut(&id).unwrap()
     }
 
+    fn pixels_per_point(&mut self) -> f32 {
+        self.viewport().input.pixels_per_point
+    }
+
     /// Return the `ViewportId` of the current viewport.
     ///
     /// For the root viewport this will return [`ViewportId::ROOT`].
@@ -578,7 +609,7 @@ impl Context {
     /// ```
     #[inline]
     pub fn input<R>(&self, reader: impl FnOnce(&InputState) -> R) -> R {
-        self.input_for(self.viewport_id(), reader)
+        self.write(move |ctx| reader(&ctx.viewport().input))
     }
 
     /// This will create a `InputState::default()` if there is no input state for that viewport
@@ -666,10 +697,11 @@ impl Context {
     /// That's because since we don't know the proper `pixels_per_point` until then.
     #[inline]
     pub fn fonts<R>(&self, reader: impl FnOnce(&Fonts) -> R) -> R {
-        self.read(move |ctx| {
+        self.write(move |ctx| {
+            let pixels_per_point = ctx.pixels_per_point();
             reader(
                 ctx.fonts
-                    .as_ref()
+                    .get(&pixels_per_point.into())
                     .expect("No fonts available until first call to Context::run()"),
             )
         })
@@ -677,8 +709,12 @@ impl Context {
 
     /// Read-write access to [`Fonts`].
     #[inline]
-    pub fn fonts_mut<R>(&self, writer: impl FnOnce(&mut Option<Fonts>) -> R) -> R {
-        self.write(move |ctx| writer(&mut ctx.fonts))
+    #[deprecated = "This function will be removed"]
+    pub fn fonts_mut<R>(&self, writer: impl FnOnce(Option<&mut Fonts>) -> R) -> R {
+        self.write(move |ctx| {
+            let pixels_per_point = ctx.pixels_per_point();
+            writer(ctx.fonts.get_mut(&pixels_per_point.into()))
+        })
     }
 
     /// Read-only access to [`Options`].
@@ -1296,12 +1332,18 @@ impl Context {
     ///
     /// The new fonts will become active at the start of the next frame.
     pub fn set_fonts(&self, font_definitions: FontDefinitions) {
-        let update_fonts = self.fonts_mut(|fonts| {
-            if let Some(current_fonts) = fonts {
+        crate::profile_function!();
+
+        let pixels_per_point = self.pixels_per_point();
+
+        let mut update_fonts = true;
+
+        self.read(|ctx| {
+            if let Some(current_fonts) = ctx.fonts.get(&pixels_per_point.into()) {
                 // NOTE: this comparison is expensive since it checks TTF data for equality
-                current_fonts.lock().fonts.definitions() != &font_definitions
-            } else {
-                true
+                if current_fonts.lock().fonts.definitions() == &font_definitions {
+                    update_fonts = false; // no need to update
+                }
             }
         });
 
@@ -1575,14 +1617,33 @@ impl ContextImpl {
         self.memory
             .end_frame(&viewport.input, &viewport.frame_state.used_ids);
 
-        let font_image_delta = self.fonts.as_ref().unwrap().font_image_delta();
-        if let Some(font_image_delta) = font_image_delta {
-            self.tex_manager
-                .0
-                .write()
-                .set(TextureId::default(), font_image_delta);
+        if let Some(fonts) = self.fonts.get(&pixels_per_point.into()) {
+            let tex_mngr = &mut self.tex_manager.0.write();
+            if let Some(font_image_delta) = fonts.font_image_delta() {
+                // A partial font atlas update, e.g. a new glyph has been entered.
+                tex_mngr.set(TextureId::default(), font_image_delta);
+            }
+
+            if 1 < self.fonts.len() {
+                // We have multiple different `pixels_per_point`,
+                // e.g. because we have many viewports spread across
+                // monitors with different DPI scaling.
+                // All viewports share the same texture namespace and renderer,
+                // so the all use `TextureId::default()` for the font texture.
+                // This is a problem.
+                // We solve this with a hack: we always upload the full font atlas
+                // every frame, for all viewports.
+                // This ensures it is up-to-date, solving
+                // https://github.com/emilk/egui/issues/3664
+                // at the cost of a lot of performance.
+                // (This will override any smaller delta that was uploaded above.)
+                crate::profile_scope!("full_font_atlas_update");
+                let full_delta = ImageDelta::full(fonts.image(), TextureAtlas::texture_options());
+                tex_mngr.set(TextureId::default(), full_delta);
+            }
         }
 
+        // Inform the backend of all textures that have been updated (including font atlas).
         let textures_delta = self.tex_manager.0.write().take_delta();
 
         #[cfg_attr(not(feature = "accesskit"), allow(unused_mut))]
@@ -1705,6 +1766,24 @@ impl ContextImpl {
             self.memory.set_viewport_id(viewport_id);
         }
 
+        let active_pixels_per_point: std::collections::BTreeSet<OrderedFloat<f32>> = self
+            .viewports
+            .values()
+            .map(|v| v.input.pixels_per_point.into())
+            .collect();
+        self.fonts.retain(|pixels_per_point, _| {
+            if active_pixels_per_point.contains(pixels_per_point) {
+                true
+            } else {
+                #[cfg(feature = "log")]
+                log::debug!(
+                    "Freeing Fonts with pixels_per_point={} because it is no longer needed",
+                    pixels_per_point.into_inner()
+                );
+                false
+            }
+        });
+
         FullOutput {
             platform_output,
             textures_delta,
@@ -1736,7 +1815,7 @@ impl Context {
             let tessellation_options = ctx.memory.options.tessellation_options;
             let texture_atlas = ctx
                 .fonts
-                .as_ref()
+                .get(&pixels_per_point.into())
                 .expect("tessellate called before first call to Context::run()")
                 .texture_atlas();
             let (font_tex_size, prepared_discs) = {

crates/egui/src/data/output.rs

@@ -14,6 +14,8 @@ pub struct FullOutput {
     ///
     /// The backend needs to apply [`crate::TexturesDelta::set`] _before_ painting,
     /// and free any texture in [`crate::TexturesDelta::free`] _after_ painting.
+    ///
+    /// It is assumed that all egui viewports share the same painter and texture namespace.
     pub textures_delta: epaint::textures::TexturesDelta,
 
     /// What to paint.

crates/egui_glow/src/painter.rs

@@ -59,6 +59,8 @@ impl From<String> for PainterError {
 ///
 /// This struct must be destroyed with [`Painter::destroy`] before dropping, to ensure OpenGL
 /// objects have been properly deleted and are not leaked.
+///
+/// NOTE: all egui viewports share the same painter.
 pub struct Painter {
     gl: Rc<glow::Context>,
 

crates/epaint/src/image.rs

@@ -350,7 +350,7 @@ impl From<FontImage> for ImageData {
 
 #[inline]
 fn fast_round(r: f32) -> u8 {
-    (r + 0.5).floor() as _ // rust does a saturating cast since 1.45
+    (r + 0.5) as _ // rust does a saturating cast since 1.45
 }
 
 // ----------------------------------------------------------------------------

crates/epaint/src/text/fonts.rs

@@ -425,6 +425,12 @@ impl Fonts {
         self.lock().fonts.atlas.clone()
     }
 
+    /// The full font atlas image.
+    #[inline]
+    pub fn image(&self) -> crate::FontImage {
+        self.lock().fonts.atlas.lock().image().clone()
+    }
+
     /// Current size of the font image.
     /// Pass this to [`crate::Tessellator`].
     pub fn font_image_size(&self) -> [usize; 2] {
@@ -590,7 +596,7 @@ impl FontsImpl {
         );
 
         let texture_width = max_texture_side.at_most(8 * 1024);
-        let initial_height = 64;
+        let initial_height = 32; // Keep initial font atlas small, so it is fast to upload to GPU. This will expand as needed anyways.
         let atlas = TextureAtlas::new([texture_width, initial_height]);
 
         let atlas = Arc::new(Mutex::new(atlas));

crates/epaint/src/texture_atlas.rs

@@ -97,7 +97,7 @@ impl TextureAtlas {
         // for r in [1, 2, 4, 8, 16, 32, 64] {
         //     let w = 2 * r + 3;
         //     let hw = w as i32 / 2;
-        const LARGEST_CIRCLE_RADIUS: f32 = 64.0;
+        const LARGEST_CIRCLE_RADIUS: f32 = 8.0; // keep small so that the initial texture atlas is small
         for i in 0.. {
             let r = 2.0_f32.powf(i as f32 / 2.0 - 1.0);
             if r > LARGEST_CIRCLE_RADIUS {
@@ -172,9 +172,21 @@ impl TextureAtlas {
         }
     }
 
+    /// The texture options suitable for a font texture
+    #[inline]
+    pub fn texture_options() -> crate::textures::TextureOptions {
+        crate::textures::TextureOptions::LINEAR
+    }
+
+    /// The full font atlas image.
+    #[inline]
+    pub fn image(&self) -> &FontImage {
+        &self.image
+    }
+
     /// Call to get the change to the image since last call.
     pub fn take_delta(&mut self) -> Option<ImageDelta> {
-        let texture_options = crate::textures::TextureOptions::LINEAR;
+        let texture_options = Self::texture_options();
 
         let dirty = std::mem::replace(&mut self.dirty, Rectu::NOTHING);
         if dirty == Rectu::NOTHING {

examples/puffin_profiler/src/main.rs

@@ -1,26 +1,45 @@
 #![cfg_attr(not(debug_assertions), windows_subsystem = "windows")] // hide console window on Windows in release
 
+use std::sync::{
+    atomic::{AtomicBool, Ordering},
+    Arc,
+};
+
 use eframe::egui;
 
 fn main() -> Result<(), eframe::Error> {
     env_logger::init(); // Log to stderr (if you run with `RUST_LOG=debug`).
     start_puffin_server(); // NOTE: you may only want to call this if the users specifies some flag or clicks a button!
-    let options = eframe::NativeOptions::default();
+
     eframe::run_native(
         "My egui App",
-        options,
+        eframe::NativeOptions {
+            viewport: egui::ViewportBuilder::default(),
+
+            #[cfg(feature = "wgpu")]
+            renderer: eframe::Renderer::Wgpu,
+
+            ..Default::default()
+        },
         Box::new(|_cc| Box::<MyApp>::default()),
     )
 }
 
 struct MyApp {
     keep_repainting: bool,
+
+    // It is useful to be able t oinspect how eframe acts with multiple viewport
+    // so we have two viewports here that we can toggle on/off.
+    show_immediate_viewport: bool,
+    show_deferred_viewport: Arc<AtomicBool>,
 }
 
 impl Default for MyApp {
     fn default() -> Self {
         Self {
             keep_repainting: true,
+            show_immediate_viewport: Default::default(),
+            show_deferred_viewport: Default::default(),
         }
     }
 }
@@ -63,12 +82,86 @@ impl eframe::App for MyApp {
                 std::thread::sleep(std::time::Duration::from_millis(50));
             }
 
-            {
-                // Sleep a bit to emulate some work:
-                puffin::profile_scope!("small_sleep");
-                std::thread::sleep(std::time::Duration::from_millis(10));
-            }
+            ui.checkbox(
+                &mut self.show_immediate_viewport,
+                "Show immediate child viewport",
+            );
+
+            let mut show_deferred_viewport = self.show_deferred_viewport.load(Ordering::Relaxed);
+            ui.checkbox(&mut show_deferred_viewport, "Show deferred child viewport");
+            self.show_deferred_viewport
+                .store(show_deferred_viewport, Ordering::Relaxed);
         });
+
+        {
+            // Sleep a bit to emulate some work:
+            puffin::profile_scope!("small_sleep");
+            std::thread::sleep(std::time::Duration::from_millis(10));
+        }
+
+        if self.show_immediate_viewport {
+            ctx.show_viewport_immediate(
+                egui::ViewportId::from_hash_of("immediate_viewport"),
+                egui::ViewportBuilder::default()
+                    .with_title("Immediate Viewport")
+                    .with_inner_size([200.0, 100.0]),
+                |ctx, class| {
+                    puffin::profile_scope!("immediate_viewport");
+
+                    assert!(
+                        class == egui::ViewportClass::Immediate,
+                        "This egui backend doesn't support multiple viewports"
+                    );
+
+                    egui::CentralPanel::default().show(ctx, |ui| {
+                        ui.label("Hello from immediate viewport");
+                    });
+
+                    if ctx.input(|i| i.viewport().close_requested()) {
+                        // Tell parent viewport that we should not show next frame:
+                        self.show_immediate_viewport = false;
+                    }
+
+                    {
+                        // Sleep a bit to emulate some work:
+                        puffin::profile_scope!("small_sleep");
+                        std::thread::sleep(std::time::Duration::from_millis(10));
+                    }
+                },
+            );
+        }
+
+        if self.show_deferred_viewport.load(Ordering::Relaxed) {
+            let show_deferred_viewport = self.show_deferred_viewport.clone();
+            ctx.show_viewport_deferred(
+                egui::ViewportId::from_hash_of("deferred_viewport"),
+                egui::ViewportBuilder::default()
+                    .with_title("Deferred Viewport")
+                    .with_inner_size([200.0, 100.0]),
+                move |ctx, class| {
+                    puffin::profile_scope!("deferred_viewport");
+
+                    assert!(
+                        class == egui::ViewportClass::Deferred,
+                        "This egui backend doesn't support multiple viewports"
+                    );
+
+                    egui::CentralPanel::default().show(ctx, |ui| {
+                        ui.label("Hello from deferred viewport");
+                    });
+                    if ctx.input(|i| i.viewport().close_requested()) {
+                        // Tell parent to close us.
+                        show_deferred_viewport.store(false, Ordering::Relaxed);
+                    }
+
+                    {
+                        // Sleep a bit to emulate some work:
+                        puffin::profile_scope!("small_sleep");
+                        std::thread::sleep(std::time::Duration::from_millis(10));
+                    }
+                },
+            );
+        }
     }
 }