Persist Area positions again (emilk#4749)

* Reverts emilk#4577

We again persist area positions, but not sizes.

Author: emilk

crates/eframe/src/web/web_logger.rs

@@ -38,6 +38,8 @@ impl log::Log for WebLogger {
     }
 
     fn log(&self, record: &log::Record<'_>) {
+        #![allow(clippy::match_same_arms)]
+
         if !self.enabled(record.metadata()) {
             return;
         }

crates/egui/src/containers/area.rs

@@ -8,29 +8,44 @@ use crate::*;
 ///
 /// Areas back [`crate::Window`]s and other floating containers,
 /// like tooltips and the popups of [`crate::ComboBox`].
-///
-/// Area state is intentionally NOT persisted between sessions,
-/// so that a bad tooltip or menu size won't be remembered forever.
-/// A resizable [`Window`] remembers the size the user picked using
-/// the state in the [`Resize`] container.
 #[derive(Clone, Copy, Debug)]
+#[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
 pub struct AreaState {
     /// Last known position of the pivot.
-    pub pivot_pos: Pos2,
+    pub pivot_pos: Option<Pos2>,
 
     /// The anchor point of the area, i.e. where on the area the [`Self::pivot_pos`] refers to.
     pub pivot: Align2,
 
     /// Last known size.
-    pub size: Vec2,
+    ///
+    /// Area size is intentionally NOT persisted between sessions,
+    /// so that a bad tooltip or menu size won't be remembered forever.
+    /// A resizable [`Window`] remembers the size the user picked using
+    /// the state in the [`Resize`] container.
+    #[cfg_attr(feature = "serde", serde(skip))]
+    pub size: Option<Vec2>,
 
     /// If false, clicks goes straight through to what is behind us. Useful for tooltips etc.
     pub interactable: bool,
 
     /// At what time was this area first shown?
     ///
     /// Used to fade in the area.
-    pub last_became_visible_at: f64,
+    #[cfg_attr(feature = "serde", serde(skip))]
+    pub last_became_visible_at: Option<f64>,
+}
+
+impl Default for AreaState {
+    fn default() -> Self {
+        Self {
+            pivot_pos: None,
+            pivot: Align2::LEFT_TOP,
+            size: None,
+            interactable: true,
+            last_became_visible_at: None,
+        }
+    }
 }
 
 impl AreaState {
@@ -42,23 +57,27 @@ impl AreaState {
 
     /// The left top positions of the area.
     pub fn left_top_pos(&self) -> Pos2 {
+        let pivot_pos = self.pivot_pos.unwrap_or_default();
+        let size = self.size.unwrap_or_default();
         pos2(
-            self.pivot_pos.x - self.pivot.x().to_factor() * self.size.x,
-            self.pivot_pos.y - self.pivot.y().to_factor() * self.size.y,
+            pivot_pos.x - self.pivot.x().to_factor() * size.x,
+            pivot_pos.y - self.pivot.y().to_factor() * size.y,
         )
     }
 
     /// Move the left top positions of the area.
     pub fn set_left_top_pos(&mut self, pos: Pos2) {
-        self.pivot_pos = pos2(
-            pos.x + self.pivot.x().to_factor() * self.size.x,
-            pos.y + self.pivot.y().to_factor() * self.size.y,
-        );
+        let size = self.size.unwrap_or_default();
+        self.pivot_pos = Some(pos2(
+            pos.x + self.pivot.x().to_factor() * size.x,
+            pos.y + self.pivot.y().to_factor() * size.y,
+        ));
     }
 
     /// Where the area is on screen.
     pub fn rect(&self) -> Rect {
-        Rect::from_min_size(self.left_top_pos(), self.size)
+        let size = self.size.unwrap_or_default();
+        Rect::from_min_size(self.left_top_pos(), size)
     }
 }
 
@@ -371,16 +390,28 @@ impl Area {
 
         let layer_id = LayerId::new(order, id);
 
-        let state = AreaState::load(ctx, id).map(|mut state| {
-            // override the saved state with the correct value
-            state.pivot = pivot;
-            state
+        let state = AreaState::load(ctx, id);
+        let mut sizing_pass = state.is_none();
+        let mut state = state.unwrap_or(AreaState {
+            pivot_pos: None,
+            pivot,
+            size: None,
+            interactable,
+            last_became_visible_at: None,
         });
-        let is_new = state.is_none();
-        if is_new {
-            ctx.request_repaint(); // if we don't know the previous size we are likely drawing the area in the wrong place
+        state.pivot = pivot;
+        state.interactable = interactable;
+        if let Some(new_pos) = new_pos {
+            state.pivot_pos = Some(new_pos);
         }
-        let mut state = state.unwrap_or_else(|| {
+        state.pivot_pos.get_or_insert_with(|| {
+            default_pos.unwrap_or_else(|| automatic_area_position(ctx, layer_id))
+        });
+        state.interactable = interactable;
+
+        let size = *state.size.get_or_insert_with(|| {
+            sizing_pass = true;
+
             // during the sizing pass we will use this as the max size
             let mut size = default_size;
 
@@ -396,28 +427,20 @@ impl Area {
                 size = size.at_most(constrain_rect.size());
             }
 
-            AreaState {
-                pivot_pos: default_pos.unwrap_or_else(|| automatic_area_position(ctx)),
-                pivot,
-                size,
-                interactable,
-                last_became_visible_at: ctx.input(|i| i.time),
-            }
+            size
         });
-        state.pivot_pos = new_pos.unwrap_or(state.pivot_pos);
-        state.interactable = interactable;
 
-        // TODO(emilk): if last frame was sizing pass, it should be considered invisible for smmother fade-in
+        // TODO(emilk): if last frame was sizing pass, it should be considered invisible for smoother fade-in
         let visible_last_frame = ctx.memory(|mem| mem.areas().visible_last_frame(&layer_id));
 
-        if !visible_last_frame {
-            state.last_became_visible_at = ctx.input(|i| i.time);
+        if !visible_last_frame || state.last_became_visible_at.is_none() {
+            state.last_became_visible_at = Some(ctx.input(|i| i.time));
         }
 
         if let Some((anchor, offset)) = anchor {
             state.set_left_top_pos(
                 anchor
-                    .align_size_within_rect(state.size, constrain_rect)
+                    .align_size_within_rect(size, constrain_rect)
                     .left_top()
                     + offset,
             );
@@ -446,7 +469,9 @@ impl Area {
             });
 
             if movable && move_response.dragged() {
-                state.pivot_pos += move_response.drag_delta();
+                if let Some(pivot_pos) = &mut state.pivot_pos {
+                    *pivot_pos += move_response.drag_delta();
+                }
             }
 
             if (move_response.dragged() || move_response.clicked())
@@ -481,7 +506,7 @@ impl Area {
             enabled,
             constrain,
             constrain_rect,
-            sizing_pass: is_new,
+            sizing_pass,
             fade_in,
         }
     }
@@ -505,7 +530,7 @@ impl Prepared {
     }
 
     pub(crate) fn content_ui(&self, ctx: &Context) -> Ui {
-        let max_rect = Rect::from_min_size(self.state.left_top_pos(), self.state.size);
+        let max_rect = self.state.rect();
 
         let clip_rect = self.constrain_rect; // Don't paint outside our bounds
 
@@ -519,13 +544,14 @@ impl Prepared {
         );
 
         if self.fade_in {
-            let age =
-                ctx.input(|i| (i.time - self.state.last_became_visible_at) as f32 + i.predicted_dt);
-            let opacity = crate::remap_clamp(age, 0.0..=ctx.style().animation_time, 0.0..=1.0);
-            let opacity = emath::easing::cubic_out(opacity); // slow fade-out = quick fade-in
-            ui.multiply_opacity(opacity);
-            if opacity < 1.0 {
-                ctx.request_repaint();
+            if let Some(last_became_visible_at) = self.state.last_became_visible_at {
+                let age = ctx.input(|i| (i.time - last_became_visible_at) as f32 + i.predicted_dt);
+                let opacity = crate::remap_clamp(age, 0.0..=ctx.style().animation_time, 0.0..=1.0);
+                let opacity = emath::easing::cubic_out(opacity); // slow fade-out = quick fade-in
+                ui.multiply_opacity(opacity);
+                if opacity < 1.0 {
+                    ctx.request_repaint();
+                }
             }
         }
 
@@ -545,10 +571,11 @@ impl Prepared {
             layer_id,
             mut state,
             move_response: mut response,
+            sizing_pass,
             ..
         } = self;
 
-        state.size = content_ui.min_size();
+        state.size = Some(content_ui.min_size());
 
         // Make sure we report back the correct size.
         // Very important after the initial sizing pass, when the initial estimate of the size is way off.
@@ -558,6 +585,11 @@ impl Prepared {
 
         ctx.memory_mut(|m| m.areas_mut().set_state(layer_id, state));
 
+        if sizing_pass {
+            // If we didn't know the size, we were likely drawing the area in the wrong place.
+            ctx.request_repaint();
+        }
+
         response
     }
 }
@@ -571,12 +603,13 @@ fn pointer_pressed_on_area(ctx: &Context, layer_id: LayerId) -> bool {
     }
 }
 
-fn automatic_area_position(ctx: &Context) -> Pos2 {
+fn automatic_area_position(ctx: &Context, layer_id: LayerId) -> Pos2 {
     let mut existing: Vec<Rect> = ctx.memory(|mem| {
         mem.areas()
             .visible_windows()
-            .into_iter()
-            .map(AreaState::rect)
+            .filter(|(id, _)| id != &layer_id) // ignore ourselves
+            .filter(|(_, state)| state.pivot_pos.is_some() && state.size.is_some())
+            .map(|(_, state)| state.rect())
             .collect()
     });
     existing.sort_by_key(|r| r.left().round() as i32);

crates/egui/src/containers/combo_box.rs

@@ -296,7 +296,12 @@ fn combo_box_dyn<'c, R>(
 
     let is_popup_open = ui.memory(|m| m.is_popup_open(popup_id));
 
-    let popup_height = ui.memory(|m| m.areas().get(popup_id).map_or(100.0, |state| state.size.y));
+    let popup_height = ui.memory(|m| {
+        m.areas()
+            .get(popup_id)
+            .and_then(|state| state.size)
+            .map_or(100.0, |size| size.y)
+    });
 
     let above_or_below =
         if ui.next_widget_position().y + ui.spacing().interact_size.y + popup_height

crates/egui/src/containers/popup.rs

@@ -118,8 +118,9 @@ fn show_tooltip_at_avoid_dyn<'c, R>(
     });
 
     let tooltip_area_id = tooltip_id(widget_id, state.tooltip_count);
-    let expected_tooltip_size =
-        AreaState::load(ctx, tooltip_area_id).map_or(vec2(64.0, 32.0), |area| area.size);
+    let expected_tooltip_size = AreaState::load(ctx, tooltip_area_id)
+        .and_then(|area| area.size)
+        .unwrap_or(vec2(64.0, 32.0));
 
     let screen_rect = ctx.screen_rect();
 

crates/egui/src/context.rs

@@ -495,11 +495,11 @@ impl ContextImpl {
         self.memory.areas_mut().set_state(
             LayerId::background(),
             AreaState {
-                pivot_pos: screen_rect.left_top(),
+                pivot_pos: Some(screen_rect.left_top()),
                 pivot: Align2::LEFT_TOP,
-                size: screen_rect.size(),
+                size: Some(screen_rect.size()),
                 interactable: true,
-                last_became_visible_at: f64::NEG_INFINITY,
+                last_became_visible_at: None,
             },
         );
 
@@ -2209,7 +2209,7 @@ impl Context {
     pub fn used_rect(&self) -> Rect {
         self.write(|ctx| {
             let mut used = ctx.viewport().frame_state.used_by_panels;
-            for window in ctx.memory.areas().visible_windows() {
+            for (_id, window) in ctx.memory.areas().visible_windows() {
                 used = used.union(window.rect());
             }
             used

crates/egui/src/memory.rs

@@ -934,9 +934,6 @@ impl Memory {
 #[cfg_attr(feature = "serde", derive(serde::Deserialize, serde::Serialize))]
 #[cfg_attr(feature = "serde", serde(default))]
 pub struct Areas {
-    /// Area state is intentionally NOT persisted between sessions,
-    /// so that a bad tooltip or menu size won't be remembered forever.
-    #[cfg_attr(feature = "serde", serde(skip))]
     areas: IdMap<area::AreaState>,
 
     /// Back-to-front. Top is last.
@@ -1030,12 +1027,12 @@ impl Areas {
             .collect()
     }
 
-    pub(crate) fn visible_windows(&self) -> Vec<&area::AreaState> {
+    pub(crate) fn visible_windows(&self) -> impl Iterator<Item = (LayerId, &area::AreaState)> {
         self.visible_layer_ids()
-            .iter()
+            .into_iter()
             .filter(|layer| layer.order == crate::Order::Middle)
-            .filter_map(|layer| self.get(layer.id))
-            .collect()
+            .filter(|&layer| !self.is_sublayer(&layer))
+            .filter_map(|layer| Some((layer, self.get(layer.id)?)))
     }
 
     pub fn move_to_top(&mut self, layer_id: LayerId) {