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

* Closes #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,19 +697,24 @@ 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()"),
             )
         })
     }
 
     /// 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>,