Runtime check on constraint order, fix explicit constraint combination

examples/egui-case-study/src/main.rs

@@ -55,7 +55,7 @@ impl Default for MyApp {
           ]
         })
         .collect(),
-      show_backer: false,
+      show_backer: true,
     }
   }
 }

examples/macroquad-example/src/main.rs

@@ -54,7 +54,11 @@ fn layout_for_highlight(ctx: &mut State) -> Node<State> {
     row_spaced(
         20.,
         vec![
-            scope::<_, _, HighlightScoper>(|highlight| rel_abs_seq(*highlight)),
+            if highlight == HighlightedCase::RelAbsSequence || highlight == HighlightedCase::None {
+                scope::<_, _, HighlightScoper>(|highlight| rel_abs_seq(*highlight))
+            } else {
+                empty()
+            },
             if highlight == HighlightedCase::AlignmentOffset || highlight == HighlightedCase::None {
                 column_spaced(
                     10.,
@@ -105,30 +109,27 @@ fn layout_for_highlight(ctx: &mut State) -> Node<State> {
     )
 }
 
-fn rel_abs_seq(highlight: HighlightedCase) -> Node<HighlightedCase> {
-    if highlight == HighlightedCase::RelAbsSequence || highlight == HighlightedCase::None {
-        return column_spaced(
-            10.,
-            vec![
-                text("Mixed (rel/abs) Sequence Constraints", 15., WHITE),
-                stack(vec![
-                    rect(BLUE),
-                    column_spaced(10., vec![rect(WHITE), rect(WHITE).height(30.), rect(WHITE)])
-                        .pad(10.),
-                ]),
-                button("Fullscreen", |highlight: &mut HighlightedCase| {
-                    if *highlight == HighlightedCase::RelAbsSequence {
-                        *highlight = HighlightedCase::None;
-                    } else {
-                        *highlight = HighlightedCase::RelAbsSequence;
-                    }
-                })
-                .height(BTN_SIZE)
-                .align(Align::Bottom),
-            ],
-        );
-    }
-    empty()
+fn rel_abs_seq(_highlight: HighlightedCase) -> Node<HighlightedCase> {
+    column_spaced(
+        10.,
+        vec![
+            text("Mixed (rel/abs) Sequence Constraints", 15., WHITE),
+            stack(vec![
+                rect(BLUE),
+                column_spaced(10., vec![rect(WHITE), rect(WHITE).height(30.), rect(WHITE)])
+                    .pad(10.),
+            ]),
+            button("Fullscreen", |highlight: &mut HighlightedCase| {
+                if *highlight == HighlightedCase::RelAbsSequence {
+                    *highlight = HighlightedCase::None;
+                } else {
+                    *highlight = HighlightedCase::RelAbsSequence;
+                }
+            })
+            .height(BTN_SIZE)
+            .align(Align::Bottom),
+        ],
+    )
 }
 
 fn text<U>(string: &'static str, font_size: f32, color: Color) -> Node<U> {

src/constraints.rs

@@ -12,11 +12,29 @@ pub(crate) struct SizeConstraints {
 
 #[derive(Debug, Clone, Copy, PartialEq)]
 pub(crate) struct Constraint {
-    pub(crate) lower: Option<f32>,
-    pub(crate) upper: Option<f32>,
+    lower: Option<f32>,
+    upper: Option<f32>,
 }
 
 impl Constraint {
+    pub(crate) fn new(lower: Option<f32>, upper: Option<f32>) -> Self {
+        assert!(Self::check_constraints(lower, upper));
+        Self { lower, upper }
+    }
+    pub(crate) fn get_lower(&self) -> Option<f32> {
+        self.lower
+    }
+    pub(crate) fn set_lower(&mut self, value: Option<f32>) {
+        assert!(Self::check_constraints(value, self.upper));
+        self.lower = value;
+    }
+    pub(crate) fn get_upper(&self) -> Option<f32> {
+        self.upper
+    }
+    pub(crate) fn set_upper(&mut self, value: Option<f32>) {
+        assert!(Self::check_constraints(self.lower, value));
+        self.upper = value;
+    }
     pub(crate) fn clamp(&self, value: f32) -> f32 {
         match (self.lower, self.upper) {
             (None, None) => value,
@@ -25,6 +43,13 @@ impl Constraint {
             (Some(lower), Some(upper)) => value.clamp(lower, upper),
         }
     }
+    fn check_constraints(lower: Option<f32>, upper: Option<f32>) -> bool {
+        if let (Some(lower_unwrapped), Some(upper_unwrapped)) = (lower, upper) {
+            lower_unwrapped <= upper_unwrapped
+        } else {
+            true
+        }
+    }
 }
 
 impl<State, Ctx> NodeValue<State, Ctx> {
@@ -46,24 +71,26 @@ impl<State, Ctx> NodeValue<State, Ctx> {
             NodeValue::Padding { amounts, element } => {
                 let child = element.constraints(allocations[0], state, ctx);
                 SizeConstraints {
-                    width: Constraint {
-                        lower: Some(
-                            amounts.leading + child.width.lower.unwrap_or(0.) + amounts.trailing,
-                        ),
-                        upper: child
+                    width: Constraint::new(
+                        child
                             .width
-                            .upper
+                            .get_lower()
+                            .map(|lower| lower + amounts.leading + amounts.trailing),
+                        child
+                            .width
+                            .get_upper()
                             .map(|upper| upper + amounts.leading + amounts.trailing),
-                    },
-                    height: Constraint {
-                        lower: Some(
-                            amounts.top + child.height.lower.unwrap_or(0.) + amounts.bottom,
-                        ),
-                        upper: child
+                    ),
+                    height: Constraint::new(
+                        child
+                            .height
+                            .get_lower()
+                            .map(|lower| lower + amounts.top + amounts.bottom),
+                        child
                             .height
-                            .upper
+                            .get_upper()
                             .map(|upper| upper + amounts.top + amounts.bottom),
-                    },
+                    ),
                     aspect: None,
                 }
             }
@@ -147,14 +174,10 @@ impl<State, Ctx> NodeValue<State, Ctx> {
                     height: Constraint::none(),
                     aspect: None,
                 }),
-            NodeValue::Explicit { options, element } => element
-                .constraints(allocations[0], state, ctx)
-                .combine_equal_priority(SizeConstraints::from_size(
-                    options.clone(),
-                    allocations[0],
-                    state,
-                    ctx,
-                )),
+            NodeValue::Explicit { options, element } => {
+                SizeConstraints::from_size(options.clone(), allocations[0], state, ctx)
+                    .combine_explicit_with_child(element.constraints(allocations[0], state, ctx))
+            }
             NodeValue::Offset { element, .. } => element.constraints(allocations[0], state, ctx),
             NodeValue::Scope { scoped } => scoped.constraints(allocations[0], state, ctx),
             NodeValue::Draw(_) | NodeValue::Space | NodeValue::AreaReader { .. } => {
@@ -172,10 +195,7 @@ impl<State, Ctx> NodeValue<State, Ctx> {
 
 impl Constraint {
     pub(crate) fn none() -> Self {
-        Self {
-            lower: None,
-            upper: None,
-        }
+        Self::new(None, None)
     }
 }
 
@@ -187,18 +207,18 @@ impl SizeConstraints {
             aspect: None,
         }
     }
-    pub(crate) fn combine_equal_priority(self, other: Self) -> Self {
+    pub(crate) fn combine_explicit_with_child(self, child: Self) -> Self {
         SizeConstraints {
-            width: self.width.combine_equal_priority(other.width),
-            height: self.height.combine_equal_priority(other.height),
-            aspect: self.aspect.or(other.aspect),
+            width: self.width.combine_explicit_with_child(child.width),
+            height: self.height.combine_explicit_with_child(child.height),
+            aspect: self.aspect.or(child.aspect),
         }
     }
 }
 
 impl Constraint {
     pub(crate) fn clamping(&self, value: f32) -> f32 {
-        match (self.lower, self.upper) {
+        match (self.get_lower(), self.get_upper()) {
             (None, None) => value,
             (None, Some(upper)) => value.min(upper),
             (Some(lower), None) => value.max(lower),
@@ -210,83 +230,74 @@ impl Constraint {
 impl Constraint {
     pub(crate) fn combine_adjacent_priority(self, other: Self) -> Self {
         // This always takes the bigger bound
-        let lower = match (self.lower, other.lower) {
+        let lower = match (self.get_lower(), other.get_lower()) {
             (None, None) => None,
             (None, Some(a)) | (Some(a), None) => Some(a),
             (Some(bound_a), Some(bound_b)) => Some(bound_a.max(bound_b)),
         };
         // In terms of upper constraints - no constraint is the biggest constraint
-        let upper = match (self.upper, other.upper) {
+        let upper = match (self.get_upper(), other.get_upper()) {
             (None, None) => None,
             (None, Some(_)) | (Some(_), None) => None,
             (Some(bound_a), Some(bound_b)) => Some(bound_a.max(bound_b)),
         };
-        Constraint { lower, upper }
+        Constraint::new(lower, upper)
     }
-    pub(crate) fn combine_equal_priority(self, other: Self) -> Self {
-        let lower = match (self.lower, other.lower) {
-            (None, None) => None,
-            (None, Some(a)) | (Some(a), None) => Some(a),
-            (Some(bound_a), Some(bound_b)) => Some(bound_a.max(bound_b)),
-        };
-        let upper = match (self.upper, other.upper) {
-            (None, None) => None,
-            (None, Some(a)) | (Some(a), None) => Some(a),
-            (Some(bound_a), Some(bound_b)) => Some(bound_a.max(bound_b)),
-        };
-        Constraint { lower, upper }
+    pub(crate) fn combine_explicit_with_child(self, child: Self) -> Self {
+        // Child constraint is limited by parent constraint as it propogates up
+        // The parent can be thought of as a wrapper which hides the constraints of it's child
+        //
+        // For example: if there is no lower bound on the parent
+        // & the lower bound on the child is higher than the parent's upper bound
+        // we should limit the lower bound to the parent's upper bound
+        //
+        // The child can't override the parent
+        Constraint::new(
+            self.lower
+                .or(child.lower.map(|cl| cl.min(self.upper.unwrap_or(cl)))),
+            self.upper
+                .or(child.upper.map(|cl| cl.max(self.lower.unwrap_or(cl)))),
+        )
     }
     pub(crate) fn combine_sum(self, other: Self, spacing: f32) -> Self {
-        let lower = match (self.lower, other.lower) {
+        let lower = match (self.get_lower(), other.get_lower()) {
             (None, None) => None,
             (None, Some(bound)) | (Some(bound), None) => Some(bound + spacing),
             (Some(bound_a), Some(bound_b)) => Some(bound_a + bound_b + spacing),
         };
-        let upper = match (self.upper, other.upper) {
+        let upper = match (self.get_upper(), other.get_upper()) {
             (None, None) => None,
             (None, Some(_)) | (Some(_), None) => None,
             (Some(bound_a), Some(bound_b)) => Some(bound_a + bound_b + spacing),
         };
-        Constraint { lower, upper }
+        Constraint::new(lower, upper)
     }
 }
 
 impl SizeConstraints {
     pub(crate) fn from_size<A, B>(value: Size<A, B>, area: Area, a: &mut A, b: &mut B) -> Self {
         let mut initial = SizeConstraints {
             width: if value.width_min.is_some() || value.width_max.is_some() {
-                Constraint {
-                    lower: value.width_min,
-                    upper: value.width_max,
-                }
+                Constraint::new(value.width_min, value.width_max)
             } else {
-                Constraint {
-                    lower: None,
-                    upper: None,
-                }
+                Constraint::none()
             },
             height: if value.height_min.is_some() || value.height_max.is_some() {
-                Constraint {
-                    lower: value.height_min,
-                    upper: value.height_max,
-                }
+                Constraint::new(value.height_min, value.height_max)
             } else {
-                Constraint {
-                    lower: None,
-                    upper: None,
-                }
+                Constraint::none()
             },
             aspect: value.aspect,
         };
         if let Some(dynamic) = value.dynamic_height {
             let result = Some(initial.height.clamp(dynamic(area.width, a, b)));
-            initial.height.lower = result;
-            initial.height.upper = result;
+            initial.height.set_lower(result);
+            initial.height.set_upper(result);
         }
         if let Some(dynamic) = value.dynamic_width {
             let result = Some(initial.width.clamp(dynamic(area.height, a, b)));
-            initial.width.lower = result;
-            initial.width.upper = result;
+            initial.width.set_lower(result);
+            initial.width.set_upper(result);
         }
         initial
     }