Merge pull request #656 from mitchmindtree/parent_inference_fix

[Breaking Change] Fix bug in parent widget inference. Simplify Widget::default_*_dimension methods as a result.

Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 
 name = "conrod"
-version = "0.27.0"
+version = "0.28.0"
 authors = [
     "Mitchell Nordine <mitchell.nordine@gmail.com>",
     "Sven Nilsen <bvssvni@gmail.com>"

examples/custom_widget.rs

@@ -24,14 +24,11 @@ extern crate vecmath;
 /// The module in which we'll implement our own custom circular button.
 mod circular_button {
     use conrod::{
-        default_x_dimension,
-        default_y_dimension,
         CharacterCache,
         Circle,
         Color,
         Colorable,
         CommonBuilder,
-        Dimension,
         Dimensions,
         FontSize,
         IndexSlot,
@@ -45,7 +42,6 @@ mod circular_button {
         UpdateArgs,
         Widget,
         WidgetKind,
-        Ui,
     };
 
 
@@ -229,34 +225,6 @@ mod circular_button {
             self.style.clone()
         }
 
-        /// Default width of the widget.
-        ///
-        /// This method is optional.
-        ///
-        /// The default implementation is the same as below, but unwraps to an absolute scalar of
-        /// `0.0` instead of `64.0`.
-        fn default_x_dimension<C: CharacterCache>(&self, ui: &Ui<C>) -> Dimension {
-            // If no width was given via the `Sizeable` (a trait implemented for all widgets)
-            // methods, some default width must be chosen.
-            //
-            // Defaults can come from several places. Here, we define how certain defaults take
-            // precedence over others.
-            //
-            // Most commonly, defaults are to be retrieved from the `Theme`, however in some cases
-            // some other logic may need to be considered.
-            default_x_dimension(self, ui).unwrap_or(Dimension::Absolute(64.0))
-        }
-
-        /// Default height of the widget.
-        ///
-        /// This method is optional.
-        ///
-        /// The default implementation is the same as below, but unwraps to an absolute scalar of
-        /// `0.0` instead of `64.0`.
-        fn default_y_dimension<C: CharacterCache>(&self, ui: &Ui<C>) -> Dimension {
-            default_y_dimension(self, ui).unwrap_or(Dimension::Absolute(64.0))
-        }
-
         /// Update the state of the button. The state may or may not have changed since
         /// the last update. (E.g. it may have changed because the user moused over the
         /// button.) If the state has changed, return the new state. Else, return None.

src/position/matrix.rs

@@ -60,9 +60,8 @@ impl Matrix {
 
         // If we can infer some new current parent from the position, set that as the current
         // parent within the given `Ui`.
-        if let Some(idx) = ui::parent_from_position(ui, x_pos, y_pos) {
-            ui::set_current_parent_idx(ui, idx);
-        }
+        let parent_idx = ui::infer_parent_unchecked(ui, x_pos, y_pos);
+        ui::set_current_parent_idx(ui, parent_idx);
 
         let xy = ui.calc_xy(None, x_pos, y_pos, dim, true);
         let (half_w, half_h) = (dim[0] / 2.0, dim[1] / 2.0);

src/ui.rs

@@ -602,12 +602,14 @@ pub fn widget_graph_mut<C>(ui: &mut Ui<C>) -> &mut Graph {
 }
 
 
-/// Check the given position for an attached parent widget.
-pub fn parent_from_position<C>(ui: &Ui<C>, x_pos: Position, y_pos: Position)
+/// Infer a widget's `Depth` parent by examining it's *x* and *y* `Position`s.
+///
+/// When a different parent may be inferred from either `Position`, the *x* `Position` is favoured.
+pub fn infer_parent_from_position<C>(ui: &Ui<C>, x_pos: Position, y_pos: Position)
     -> Option<widget::Index>
 {
     use Position::{Place, Relative, Direction, Align};
-    let maybe_parent = match (x_pos, y_pos) {
+    match (x_pos, y_pos) {
         (Place(_, maybe_parent_idx), _) | (_, Place(_, maybe_parent_idx)) =>
             maybe_parent_idx,
         (Direction(_, _, maybe_idx), _) | (_, Direction(_, _, maybe_idx)) |
@@ -616,8 +618,23 @@ pub fn parent_from_position<C>(ui: &Ui<C>, x_pos: Position, y_pos: Position)
             maybe_idx.or(ui.maybe_prev_widget_idx)
                 .and_then(|idx| ui.widget_graph.depth_parent(idx)),
         _ => None,
-    };
-    maybe_parent.or(ui.maybe_current_parent_idx)
+    }
+}
+
+
+/// Attempts to infer the parent of a widget from its *x*/*y* `Position`s and the current state of
+/// the `Ui`.
+/// 
+/// If no parent can be inferred via the `Position`s, the `maybe_current_parent_idx` will be used.
+///
+/// If `maybe_current_parent_idx` is `None`, the `Ui`'s `window` widget will be used.
+///
+/// **Note:** This function does not check whether or not using the `window` widget would cause a
+/// cycle.
+pub fn infer_parent_unchecked<C>(ui: &Ui<C>, x_pos: Position, y_pos: Position) -> widget::Index {
+    infer_parent_from_position(ui, x_pos, y_pos)
+        .or(ui.maybe_prev_widget_idx)
+        .unwrap_or(ui.window.into())
 }
 
 

src/widget/button.rs

@@ -3,7 +3,6 @@ use {
     CharacterCache,
     Color,
     Colorable,
-    Dimension,
     FontSize,
     Frameable,
     FramedRectangle,
@@ -13,7 +12,6 @@ use {
     Positionable,
     Text,
     Theme,
-    Ui,
     Widget,
 };
 use widget;
@@ -148,14 +146,6 @@ impl<'a, F> Widget for Button<'a, F>
         self.style.clone()
     }
 
-    fn default_x_dimension<C: CharacterCache>(&self, ui: &Ui<C>) -> Dimension {
-        widget::default_x_dimension(self, ui).unwrap_or(Dimension::Absolute(64.0))
-    }
-
-    fn default_y_dimension<C: CharacterCache>(&self, ui: &Ui<C>) -> Dimension {
-        widget::default_y_dimension(self, ui).unwrap_or(Dimension::Absolute(64.0))
-    }
-
     /// Update the state of the Button.
     fn update<C: CharacterCache>(self, args: widget::UpdateArgs<Self, C>) {
         let widget::UpdateArgs { idx, state, style, rect, mut ui, .. } = args;

src/widget/canvas.rs

@@ -2,7 +2,6 @@ use {
     CharacterCache,
     Color,
     Colorable,
-    Dimension,
     Dimensions,
     FontSize,
     Frameable,
@@ -266,18 +265,6 @@ impl<'a> Widget for Canvas<'a> {
         Position::Place(Place::Middle, None)
     }
 
-    fn default_x_dimension<C: CharacterCache>(&self, ui: &Ui<C>) -> Dimension {
-        widget::default_x_dimension(self, ui)
-            .or_else(|| ui.w_of(ui.window).map(|w| Dimension::Absolute(w)))
-            .expect("`Ui.window` should always have some width")
-    }
-
-    fn default_y_dimension<C: CharacterCache>(&self, ui: &Ui<C>) -> Dimension {
-        widget::default_y_dimension(self, ui)
-            .or_else(|| ui.h_of(ui.window).map(|h| Dimension::Absolute(h)))
-            .expect("`Ui.window` should always have some height")
-    }
-
     /// The title bar area at which the Canvas can be clicked and dragged.
     ///
     /// Note: the position of the returned **Rect** should be relative to the center of the widget.

src/widget/drop_down_list.rs

@@ -5,7 +5,6 @@ use ::{
     CharacterCache,
     Color,
     Colorable,
-    Dimension,
     FontSize,
     Frameable,
     IndexSlot,
@@ -17,7 +16,6 @@ use ::{
     Scalar,
     Sizeable,
     Theme,
-    Ui,
 };
 use widget::{self, Widget};
 
@@ -158,14 +156,6 @@ impl<'a, F> Widget for DropDownList<'a, F> where
         self.style.clone()
     }
 
-    fn default_x_dimension<C: CharacterCache>(&self, ui: &Ui<C>) -> Dimension {
-        widget::default_x_dimension(self, ui).unwrap_or(Dimension::Absolute(128.0))
-    }
-
-    fn default_y_dimension<C: CharacterCache>(&self, ui: &Ui<C>) -> Dimension {
-        widget::default_y_dimension(self, ui).unwrap_or(Dimension::Absolute(32.0))
-    }
-
     /// Update the state of the DropDownList.
     fn update<C: CharacterCache>(mut self, args: widget::UpdateArgs<Self, C>) {
         let widget::UpdateArgs { idx, state, rect, style, mut ui, .. } = args;

src/widget/envelope_editor.rs

@@ -4,7 +4,6 @@ use {
     Circle,
     Color,
     Colorable,
-    Dimension,
     Direction,
     Edge,
     Frameable,
@@ -21,7 +20,6 @@ use {
     Sizeable,
     Text,
     Theme,
-    Ui,
     Widget,
 };
 use num::Float;
@@ -317,14 +315,6 @@ impl<'a, E, F> Widget for EnvelopeEditor<'a, E, F>
         self.style.clone()
     }
 
-    fn default_x_dimension<C: CharacterCache>(&self, ui: &Ui<C>) -> Dimension {
-        widget::default_x_dimension(self, ui).unwrap_or(Dimension::Absolute(256.0))
-    }
-
-    fn default_y_dimension<C: CharacterCache>(&self, ui: &Ui<C>) -> Dimension {
-        widget::default_y_dimension(self, ui).unwrap_or(Dimension::Absolute(128.0))
-    }
-
     /// Update the state of the EnvelopeEditor's cached state.
     fn update<C: CharacterCache>(self, args: widget::UpdateArgs<Self, C>) {
         use self::Interaction::{Clicked, Highlighted, Normal};

src/widget/matrix.rs

@@ -1,6 +1,6 @@
 
-use ::{CharacterCache, Dimension, NodeIndex, Scalar, Theme, Ui};
-use widget::{self, Widget};
+use {CharacterCache, NodeIndex, Scalar, Theme, Widget};
+use widget;
 
 
 /// Reaction params.
@@ -105,14 +105,6 @@ impl<'a, F, W> Widget for Matrix<F> where
         self.style.clone()
     }
 
-    fn default_x_dimension<C: CharacterCache>(&self, ui: &Ui<C>) -> Dimension {
-        widget::default_x_dimension(self, ui).unwrap_or(Dimension::Absolute(128.0))
-    }
-
-    fn default_y_dimension<C: CharacterCache>(&self, ui: &Ui<C>) -> Dimension {
-        widget::default_y_dimension(self, ui).unwrap_or(Dimension::Absolute(64.0))
-    }
-
     /// Update the state of the Matrix.
     fn update<C: CharacterCache>(self, args: widget::UpdateArgs<Self, C>) {
         let widget::UpdateArgs { idx, state, rect, style, mut ui, .. } = args;