From de04af8fb71b6978a452b7c34b8ec3b19b6719cc Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 23 Jun 2026 17:14:16 +0200 Subject: [PATCH] Fix "drunk text" bug (#8250) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Symptom Fix this long-standing, occasional bug, that can cause text to look compressed and "drunk": Screenshot 2026-06-22 at 13 12 56 ## Root cause `epaint::TextureAtlas::take_delta` is fire-and-forget: it resets the dirty region as soon as it hands out a delta, assuming the delta will be uploaded. Atlas growth always emits a **full** `ImageDelta` (`pos: None`) which recreates the GPU texture at the new size — *as long as it is applied*. But both native integrations applied `textures_delta` inside skippable code paths: - **wgpu** (`egui-wgpu/src/winit.rs`): textures were uploaded only *after* surface-dependent early-returns (`render_state` / `surfaces.get_mut(viewport_id)` missing). Texture uploads are device-level and don't need a surface. - **glow** (`eframe/src/native/glow_integration.rs`): textures were uploaded only inside `if is_visible { … }` (and after a viewport-missing early-return), while `integration.update` still ran and grew the atlas. The root window even starts hidden on purpose (`with_visible(false)`, to avoid a startup white flash), so the very first frames hit this. When the delta was dropped, the GPU font texture stayed smaller than the CPU-side atlas; every glyph UV (normalized by the CPU atlas size) then sampled the wrong rows until the next full atlas recreation. wgpu/Metal can't detect this — the read is in-bounds, just the wrong row. ## Fixes - **wgpu**: apply `textures_delta.set` right after `render_state` is obtained, **before** any surface-dependent early-return. `free` still runs after submit (unchanged). - **glow**: apply `textures_delta.set` (and `free`) regardless of `is_visible`, making the GL context current when there's anything to upload; only tessellation/paint/swap stay gated on visibility. - **debug assert** in `egui-wgpu`'s `Renderer::update_texture`: a full delta must (re)create the GPU texture at exactly the delta size — catches any future CPU/GPU size desync at the source. ## wgpu ruled out Confirmed the desync is **not** inside wgpu: Metal `create_texture` uses the exact descriptor size, and `queue.write_texture` validates against the texture's own live `desc` — a single texture can't have CPU/GPU sizes disagree. The mismatch is born at the egui boundary (atlas size for UVs vs. last-applied upload), which wgpu cannot see. ## Testing note A headless regression test of `paint_and_update_textures` isn't practical (it needs a real winit window; `render_state` is private with no surface-less setter). I verified the failure *mechanism* separately on macOS/Metal (texture lagging the atlas → silent wrong-row sampling, no wgpu error), but that demo did not exercise the fixed code path, so it's not included. The fixes rest on the reasoning above. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- crates/eframe/src/native/glow_integration.rs | 33 ++++++++++++-------- crates/egui-wgpu/src/renderer.rs | 10 ++++++ crates/egui-wgpu/src/winit.rs | 24 ++++++++------ 3 files changed, 45 insertions(+), 22 deletions(-) diff --git a/crates/eframe/src/native/glow_integration.rs b/crates/eframe/src/native/glow_integration.rs index c42b0aa68..c44a70153 100644 --- a/crates/eframe/src/native/glow_integration.rs +++ b/crates/eframe/src/native/glow_integration.rs @@ -688,28 +688,30 @@ impl GlowWinitRunning<'_> { egui_winit.handle_platform_output_with_event_loop(&window, event_loop, platform_output); + // Upload textures even when not visible: the atlas dirty region is already + // consumed, so dropping the delta would desync the font texture. + let has_texture_updates = !textures_delta.set.is_empty() || !textures_delta.free.is_empty(); + if is_visible || has_texture_updates { + // We may need to switch contexts again, because of immediate viewports: + frame_timer.pause(); + change_gl_context(current_gl_context, not_current_gl_context, gl_surface); + frame_timer.resume(); + } + + for (id, image_delta) in &textures_delta.set { + painter.set_texture(*id, image_delta); + } + if is_visible { let clipped_primitives = integration.egui_ctx.tessellate(shapes, pixels_per_point); - { - // We may need to switch contexts again, because of immediate viewports: - frame_timer.pause(); - change_gl_context(current_gl_context, not_current_gl_context, gl_surface); - frame_timer.resume(); - } - let screen_size_in_pixels: [u32; 2] = window.inner_size().into(); if !clear_before_update { painter.clear(screen_size_in_pixels, clear_color); } - painter.paint_and_update_textures( - screen_size_in_pixels, - pixels_per_point, - &clipped_primitives, - &textures_delta, - ); + painter.paint_primitives(screen_size_in_pixels, pixels_per_point, &clipped_primitives); { for action in viewport.actions_requested.drain(..) { @@ -771,6 +773,11 @@ impl GlowWinitRunning<'_> { } } + // Free textures *after* painting, since they may still be used in the frame we just drew. + for id in &textures_delta.free { + painter.free_texture(*id); + } + glutin.handle_viewport_output(event_loop, &integration.egui_ctx, &viewport_output); integration.report_frame_time(frame_timer.total_time_sec()); // don't count auto-save time as part of regular frame time diff --git a/crates/egui-wgpu/src/renderer.rs b/crates/egui-wgpu/src/renderer.rs index e55f7581a..ff850af33 100644 --- a/crates/egui-wgpu/src/renderer.rs +++ b/crates/egui-wgpu/src/renderer.rs @@ -729,6 +729,16 @@ impl Renderer { }); queue_write_data_to_texture(&texture, origin); + + // A full update must (re)create the texture at exactly the delta's size, + // or glyph UVs (normalized by the CPU atlas size) will sample the wrong rows. + debug_assert!( + image_delta.pos.is_some() || [texture.width(), texture.height()] == [width, height], + "egui texture {id:?}: GPU texture is {}x{} but full delta is {width}x{height}", + texture.width(), + texture.height(), + ); + self.textures.insert( id, Texture { diff --git a/crates/egui-wgpu/src/winit.rs b/crates/egui-wgpu/src/winit.rs index 78817e879..2da393e49 100644 --- a/crates/egui-wgpu/src/winit.rs +++ b/crates/egui-wgpu/src/winit.rs @@ -543,6 +543,21 @@ impl Painter { commands_submitted: false, }; + { + // Upload textures before the surface-dependent early-returns below: + // uploads only need the device + queue, and the atlas dirty region is + // already consumed, so dropping the delta would desync the font texture. + let mut renderer = render_state.renderer.write(); + for (id, image_delta) in &textures_delta.set { + renderer.update_texture( + &render_state.device, + &render_state.queue, + *id, + image_delta, + ); + } + } + let Some(surface_state) = self.surfaces.get_mut(&viewport_id) else { return vsync_sec; }; @@ -562,15 +577,6 @@ impl Painter { let user_cmd_bufs = { let mut renderer = render_state.renderer.write(); - for (id, image_delta) in &textures_delta.set { - renderer.update_texture( - &render_state.device, - &render_state.queue, - *id, - image_delta, - ); - } - renderer.update_buffers( &render_state.device, &render_state.queue,