[layout] correct handling of cursor position w.r.t. reversed layouts

egui/src/demos/app.rs

@@ -390,7 +390,7 @@ fn show_menu_bar(ui: &mut Ui, windows: &mut OpenWindows, env: &DemoEnvironment)
                 (time.rem_euclid(1.0) * 100.0).floor()
             );
 
-            ui.with_layout(Layout::horizontal(Align::Max).reverse(), |ui| {
+            ui.with_layout(Layout::horizontal(Align::Center).reverse(), |ui| {
                 if ui
                     .add(Button::new(time).text_style(TextStyle::Monospace))
                     .clicked

egui/src/layout.rs

@@ -129,22 +129,110 @@ impl Layout {
         }
     }
 
+    #[must_use]
+    pub fn with_reversed(self, reversed: bool) -> Self {
+        if reversed {
+            self.reverse()
+        } else {
+            self
+        }
+    }
+
     pub fn dir(self) -> Direction {
         self.dir
     }
 
+    pub fn align(self) -> Option<Align> {
+        self.align
+    }
+
     pub fn is_reversed(self) -> bool {
         self.reversed
     }
 
+    pub fn initial_cursor(self, max_rect: Rect) -> Pos2 {
+        match self.dir {
+            Direction::Horizontal => {
+                if self.reversed {
+                    max_rect.right_top()
+                } else {
+                    max_rect.left_top()
+                }
+            }
+            Direction::Vertical => {
+                if self.reversed {
+                    max_rect.left_bottom()
+                } else {
+                    max_rect.left_top()
+                }
+            }
+        }
+    }
+
     /// Given the cursor in the region, how much space is available
     /// for the next widget?
-    pub fn available(self, cursor: Pos2, rect: Rect) -> Rect {
+    pub fn available(self, cursor: Pos2, max_rect: Rect) -> Rect {
-        if self.reversed {
+        let mut rect = max_rect;
-            Rect::from_min_max(rect.min, cursor)
+        match self.dir {
-        } else {
+            Direction::Horizontal => {
-            Rect::from_min_max(cursor, rect.max)
+                rect.min.y = cursor.y;
+                if self.reversed {
+                    rect.max.x = cursor.x;
+                } else {
+                    rect.min.x = cursor.x;
+                }
+            }
+            Direction::Vertical => {
+                rect.min.x = cursor.x;
+                if self.reversed {
+                    rect.max.y = cursor.y;
+                } else {
+                    rect.min.y = cursor.y;
+                }
+            }
         }
+        rect
+    }
+
+    /// Advance the cursor by this many points.
+    pub fn advance_cursor(self, cursor: &mut Pos2, amount: f32) {
+        match self.dir() {
+            Direction::Horizontal => {
+                if self.is_reversed() {
+                    cursor.x -= amount;
+                } else {
+                    cursor.x += amount;
+                }
+            }
+            Direction::Vertical => {
+                if self.is_reversed() {
+                    cursor.y -= amount;
+                } else {
+                    cursor.y += amount;
+                }
+            }
+        }
+    }
+
+    pub fn rect_from_cursor_size(self, cursor: Pos2, size: Vec2) -> Rect {
+        let mut rect = Rect::from_min_size(cursor, size);
+
+        match self.dir {
+            Direction::Horizontal => {
+                if self.reversed {
+                    rect.min.x = cursor.x - size.x;
+                    rect.max.x = rect.min.x - size.x
+                }
+            }
+            Direction::Vertical => {
+                if self.reversed {
+                    rect.min.y = cursor.y - size.y;
+                    rect.max.y = rect.min.y - size.y
+                }
+            }
+        }
+
+        rect
     }
 
     /// Reserve this much space and move the cursor.
@@ -204,19 +292,11 @@ impl Layout {
         }
 
         if self.is_reversed() {
-            // reverse: cursor starts at bottom right corner of new widget.
+            let child_pos = *cursor + child_move;
-
             let child_pos = match self.dir {
-                Direction::Horizontal => pos2(
+                Direction::Horizontal => child_pos + vec2(-child_size.x, 0.0),
-                    cursor.x - child_size.x,
+                Direction::Vertical => child_pos + vec2(0.0, -child_size.y),
-                    cursor.y - available_size.y + child_move.y,
-                ),
-                Direction::Vertical => pos2(
-                    cursor.x - available_size.x + child_move.x,
-                    cursor.y - child_size.y,
-                ),
             };
-            // let child_pos = *cursor - child_move - child_size;
             *cursor -= cursor_change;
             Rect::from_min_size(child_pos, child_size)
         } else {
@@ -226,3 +306,40 @@ impl Layout {
         }
     }
 }
+
+// ----------------------------------------------------------------------------
+
+/// ## Debug stuff
+impl Layout {
+    /// Shows where the next widget is going to be placed
+    pub fn debug_paint_cursor(&self, cursor: Pos2, painter: &crate::Painter) {
+        use crate::paint::*;
+        let color = color::GREEN;
+        let stroke = Stroke::new(2.0, color);
+
+        let align;
+
+        match self.dir {
+            Direction::Horizontal => {
+                if self.reversed {
+                    painter.debug_arrow(cursor, vec2(-1.0, 0.0), stroke);
+                    align = (Align::Max, Align::Min);
+                } else {
+                    painter.debug_arrow(cursor, vec2(1.0, 0.0), stroke);
+                    align = (Align::Min, Align::Min);
+                }
+            }
+            Direction::Vertical => {
+                if self.reversed {
+                    painter.debug_arrow(cursor, vec2(0.0, -1.0), stroke);
+                    align = (Align::Min, Align::Max);
+                } else {
+                    painter.debug_arrow(cursor, vec2(0.0, 1.0), stroke);
+                    align = (Align::Min, Align::Min);
+                }
+            }
+        }
+
+        painter.text(cursor, align, "cursor", TextStyle::Monospace, color);
+    }
+}

egui/src/painter.rs

@@ -136,6 +136,30 @@ impl Painter {
         });
         self.galley(rect.min, galley, text_style, color::RED);
     }
+
+    pub fn debug_arrow(&self, origin: Pos2, dir: Vec2, stroke: Stroke) {
+        use crate::math::*;
+        let full_length = dir.length().at_least(64.0);
+        let tip_length = full_length / 3.0;
+        let dir = dir.normalized();
+        let tip = origin + dir * full_length;
+        let angle = TAU / 10.0;
+        self.line_segment([origin, tip], stroke);
+        self.line_segment(
+            [
+                tip,
+                tip - tip_length * Vec2::angled(angle).rotate_other(dir),
+            ],
+            stroke,
+        );
+        self.line_segment(
+            [
+                tip,
+                tip - tip_length * Vec2::angled(-angle).rotate_other(dir),
+            ],
+            stroke,
+        );
+    }
 }
 
 /// # Paint different primitives

egui/src/ui.rs

@@ -71,11 +71,7 @@ impl Ui {
         let id = self.make_position_id(); // TODO: is this a good idea?
         self.child_count += 1;
 
-        let cursor = if layout.is_reversed() {
+        let cursor = layout.initial_cursor(child_rect);
-            child_rect.max
-        } else {
-            child_rect.min
-        };
 
         Ui {
             id,
@@ -363,22 +359,7 @@ impl Ui {
     /// The direction is dependent on the layout.
     /// This is useful for creating some extra space between widgets.
     pub fn advance_cursor(&mut self, amount: f32) {
-        match self.layout.dir() {
+        self.layout.advance_cursor(&mut self.cursor, amount);
-            Direction::Horizontal => {
-                if self.layout.is_reversed() {
-                    self.cursor.x -= amount;
-                } else {
-                    self.cursor.x += amount;
-                }
-            }
-            Direction::Vertical => {
-                if self.layout.is_reversed() {
-                    self.cursor.y -= amount;
-                } else {
-                    self.cursor.y += amount;
-                }
-            }
-        }
     }
 
     /// Reserve this much space and move the cursor.
@@ -609,7 +590,7 @@ impl Ui {
     /// will decide how much space will be used in the parent ui.
     pub fn add_custom_contents(&mut self, size: Vec2, add_contents: impl FnOnce(&mut Ui)) -> Rect {
         let size = size.at_most(self.available().size());
-        let child_rect = Rect::from_min_size(self.cursor, size);
+        let child_rect = self.layout.rect_from_cursor_size(self.cursor, size);
         let mut child_ui = self.child_ui(child_rect, self.layout);
         add_contents(&mut child_ui);
         self.allocate_space(child_ui.bounding_size())
@@ -635,7 +616,7 @@ impl Ui {
             "You can only indent vertical layouts"
         );
         let indent = vec2(self.style().spacing.indent, 0.0);
-        let child_rect = Rect::from_min_max(self.cursor + indent, self.bottom_right());
+        let child_rect = Rect::from_min_max(self.cursor + indent, self.bottom_right()); // TODO: wrong for reversed layouts
         let mut child_ui = Ui {
             id: self.id.with(id_source),
             ..self.child_ui(child_rect, self.layout)
@@ -689,19 +670,23 @@ impl Ui {
     pub fn horizontal<R>(&mut self, add_contents: impl FnOnce(&mut Ui) -> R) -> (R, Rect) {
         let initial_size = vec2(
             self.available().width(),
-            self.style().spacing.interact_size.y,
+            self.style().spacing.interact_size.y, // Assume there will be something interactive on the horizontal layout
         );
+
+        let right_to_left =
+            (self.layout.dir(), self.layout.align()) == (Direction::Vertical, Some(Align::Max));
+
         self.inner_layout(
-            Layout::horizontal(Align::Center),
+            Layout::horizontal(Align::Center).with_reversed(right_to_left),
             initial_size,
             add_contents,
         )
     }
 
-    /// Start a ui with vertical layout
+    /// Start a ui with vertical layout.
+    /// Widgets will be left-justified.
     pub fn vertical<R>(&mut self, add_contents: impl FnOnce(&mut Ui) -> R) -> (R, Rect) {
-        let initial_size = vec2(0.0, self.available().height());
+        self.with_layout(Layout::vertical(Align::Min), add_contents)
-        self.inner_layout(Layout::vertical(Align::Min), initial_size, add_contents)
     }
 
     pub fn inner_layout<R>(
@@ -710,7 +695,7 @@ impl Ui {
         initial_size: Vec2,
         add_contents: impl FnOnce(&mut Self) -> R,
     ) -> (R, Rect) {
-        let child_rect = Rect::from_min_size(self.cursor, initial_size);
+        let child_rect = self.layout.rect_from_cursor_size(self.cursor, initial_size);
         let mut child_ui = self.child_ui(child_rect, layout);
         let ret = add_contents(&mut child_ui);
         let size = child_ui.bounding_size();
@@ -723,7 +708,7 @@ impl Ui {
         layout: Layout,
         add_contents: impl FnOnce(&mut Self) -> R,
     ) -> (R, Rect) {
-        let mut child_ui = self.child_ui(self.rect(), layout);
+        let mut child_ui = self.child_ui(self.available(), layout);
         let ret = add_contents(&mut child_ui);
         let size = child_ui.bounding_size();
         let rect = self.allocate_space(size);
@@ -778,3 +763,13 @@ impl Ui {
         result
     }
 }
+
+// ----------------------------------------------------------------------------
+
+/// ## Debug stuff
+impl Ui {
+    /// Shows where the next widget is going to be placed
+    pub fn debug_paint_cursor(&self) {
+        self.layout.debug_paint_cursor(self.cursor, &self.painter);
+    }
+}

egui/src/widgets/mod.rs

@@ -287,16 +287,22 @@ impl Widget for Button {
             sense,
         } = self;
 
+        let button_padding = ui.style().spacing.button_padding;
+
         let id = ui.make_position_id();
         let font = &ui.fonts()[text_style];
         let galley = font.layout_multiline(text, ui.available().width());
-        let mut desired_size = galley.size + 2.0 * ui.style().spacing.button_padding;
+        let mut desired_size = galley.size + 2.0 * button_padding;
         desired_size = desired_size.at_least(ui.style().spacing.interact_size);
         let rect = ui.allocate_space(desired_size);
 
         let response = ui.interact(rect, id, sense);
         let style = ui.style().interact(&response);
-        let text_cursor = response.rect.center() - 0.5 * galley.size;
+        // let text_cursor = response.rect.center() - 0.5 * galley.size; // centered-centered (looks bad for justified drop-down menus
+        let text_cursor = pos2(
+            response.rect.left() + button_padding.x,
+            response.rect.center().y - 0.5 * galley.size.y,
+        ); // left-centered
         let fill = fill.unwrap_or(style.bg_fill);
         ui.painter().add(PaintCmd::Rect {
             rect: response.rect,