mirror of
https://github.com/emilk/egui.git
synced 2026-06-26 22:53:14 -04:00
Fix "drunk text" bug (#8250)
## Symptom Fix this long-standing, occasional bug, that can cause text to look compressed and "drunk": <img width="552" height="226" alt="Screenshot 2026-06-22 at 13 12 56" src="https://github.com/user-attachments/assets/9b1abad4-5ef6-4771-8168-f201afc341ab" /> ## 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) <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user