Conversation
Compute child layout for Size items to update cross axis, and after resolving child min/max and flex clamping, re-run relative-positioned children using their final resolved main/cross constraints so descendant layouts use the same cached dimensions. Adds a test (auto_min_width_propagates_to_nested_children_after_flex_clamp) to verify min-width propagation to nested children after flex clamp.
Avoid repeated tree traversals and full-sum recomputations in layout. Compute and store prior values (for grid cols/rows and child main extents) and update width_sum/height_sum/main_sum incrementally instead of calling .iter().sum(). Classify visible children once into relative and absolute SmallVecs, use their lengths for child counts, reverse relative order for RTL when needed, and iterate absolute_children directly. These changes are intended as performance optimizations without changing layout semantics.
Add a `measured` field to StretchItem and initialize it in `new`. Set `item.measured` when computing the clamped/actual main size during the flex iteration. In the child layout phase, skip relayout for Size items when `item.computed` equals `item.measured` (comparison uses `f32::EPSILON`), avoiding redundant child layout calls when the size hasn't actually changed.
Add per-child caching of the last parent constraints to skip redundant layout calls. Introduce fields on ChildNode (last_layout_main, last_layout_cross, has_layout_constraints) and a same_f32 helper for bitwise float comparison. Update layout logic in multiple phases to set and check these cached constraint values (initialization, stretch handling, size recomputation, final child layout) so repeated layout(...) calls are avoided when parent constraints haven't changed. This reduces unnecessary work and improves layout performance.
Introduce a cross_is_stretch flag on item records and use index-range loops instead of cloning iterator views to avoid repeated method calls and iterator allocations. Compute stretch_sum and fixed_sum with simple loops, compute per-line max/main sums via ranges, and resolve cross-stretch children using the cached flag. Also simplify recomputation of line cross extents and adjust RTL/LTR positioning loops to avoid clone().enumerate(). These changes improve performance and reduce allocations in the flex layout algorithm.
Introduce pass-scoped layout memoization to avoid recomputing node sizes when parent constraints haven't changed. Added LayoutMemo and a layout_memo SecondaryMap plus a layout_pass counter to NodeCache, along with same_f32 for exact f32 comparison. Extended the Cache trait with begin_layout_pass, get_layout_result and set_layout_result (default no-ops) and implemented them in NodeCache. Updated layout logic to consult and store memoized results, and ensure begin_layout_pass is called at the start of node layout. Also added a "Steady State Tree" benchmark that builds a tree and repeatedly runs layout to measure the effect.
Introduce cross-pass memoization for layout caches by adding revision tracking and control APIs. NodeCache gains cross_pass_memo_enabled and layout_revision fields and LayoutMemo now stores a revision; cache lookups accept entries that match either the current pass or the layout revision when cross-pass memoization is enabled. New NodeCache methods (enable_cross_pass_memoization, set_layout_revision, bump_layout_revision) are provided and layout entries record the revision when stored. World.bump_layout_revision is added and invoked on structural changes and on most property setters (add/remove/clear and mutators) to ensure cached layouts are invalidated when relevant state changes. Benchmarks updated to compare a baseline world vs a cached world with cross-pass memoization enabled.
Replace the previous pass/revision cross-pass memoization with a single generation-based invalidation scheme. Renamed fields (layout_revision -> layout_generation, cross_pass_memo_enabled -> memoization_enabled) and LayoutMemo.pass/revision -> generation; added enable_layout_memoization and a Default impl for NodeCache. Simplified enable_cross_pass_memoization to delegate to the new toggle, removed per-layout-pass tracking and begin_layout_pass, and updated cache get/set logic to validate against the current generation. Also removed the begin_layout_pass call in node layout code.
Treat both Row and Column as inline layouts when applying RTL behavior: replace equality checks with matches!(..., Row | Column), rename is_row_rtl to is_inline_rtl, and flip horizontal alignment/positioning accordingly. This fixes RTL handling for column-based inline layouts. Added a unit test (rtl_reverses_column_alignment_horizontally) to verify column alignment is flipped in RTL.
Treat absolute children as sized without parent padding. Adjust abs_avail calculations to subtract only borders, use abs_avail for positioning, and compute absolute child sizing/constraints against abs_size_main/abs_size_cross. Update related tests to reflect the corrected bounding box expectations.
There was a problem hiding this comment.
Pull request overview
This PR extends Morphorm’s layout engine with inline RTL support, introduces Row/Column wrapping behavior, and adds cross-pass layout memoization to improve steady-state performance.
Changes:
- Add
Direction(LTR/RTL) andLayoutWrap(Wrap/NoWrap) and wire them through theNodeAPI and ECS demo world. - Implement wrapped layout (
layout_wrap) for Row/Column containers, including gaps, padding, alignment, and RTL placement. - Add layout-result memoization hooks to
Cache, implement them in the ECSNodeCache, and add tests/examples/benchmarks.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
src/types.rs |
Adds Direction and LayoutWrap public types. |
src/node.rs |
Extends Node with direction()/wrap(); changes root layout axis selection. |
src/layout.rs |
Adds wrap layout algorithm, RTL alignment flipping, and layout-result caching/optimizations. |
src/cache.rs |
Adds optional memoization hooks (begin_layout_pass, get_layout_result, set_layout_result). |
ecs/src/world.rs |
Adds setters for direction/wrap and bumps layout revision on mutations. |
ecs/src/store.rs |
Stores direction/wrap components. |
ecs/src/implementations.rs |
Implements direction()/wrap() and layout memoization in NodeCache. |
tests/wrap.rs |
Adds comprehensive wrapping tests (including RTL + auto-height behavior). |
tests/direction.rs |
Adds RTL behavioral tests (order, padding swap, alignment flipping). |
tests/auto.rs |
Adds regression test for nested auto min-width propagation after flex clamping. |
tests/size_constraints.rs |
Updates expectations for absolute + auto-min sizing behavior. |
examples/wrap.rs |
Adds a runnable wrap/RTL demo. |
benches/stack.rs |
Adds steady-state benchmark group intended to compare cached vs baseline layout. |
Comments suppressed due to low confidence (1)
src/layout.rs:1244
- In the main-axis flex loop,
StretchItem.measuredis never set (unlike the grid flex loops), but later it’s used in the freeze phase to decide whether to re-layout ((item.computed - item.measured).abs() > EPSILON). Sincemeasuredstays at its default (0.0), this condition will be true for most items, causing unnecessary extra layouts and undermining the intended optimization. Setitem.measuredto the unclampedactual_mainbefore computingitem.computed.
for item in main_axis.iter_mut().filter(|item| !item.frozen) {
let mut actual_main = (item.factor * free_main_space / main_flex_sum).round();
let child = &mut children[item.index];
if item.item_type == ItemType::Size {
let target_cross = if child.node.cross(store, layout_type).is_stretch() {
child.cross
} else {
parent_cross
};
if !child.has_layout_constraints
|| !same_f32(child.last_layout_main, actual_main)
|| !same_f32(child.last_layout_cross, target_cross)
{
let child_size = layout(
child.node,
layout_type,
actual_main,
target_cross,
cache,
tree,
store,
sublayout,
);
child.cross = child_size.cross;
actual_main = child_size.main;
child.last_layout_main = actual_main;
child.last_layout_cross = target_cross;
child.has_layout_constraints = true;
} else {
actual_main = child.main;
}
if child.node.min_main(store, layout_type).is_auto() {
item.min = child.main;
}
}
let clamped = actual_main.min(item.max).max(item.min);
item.violation = clamped - actual_main;
total_violation += item.violation;
item.computed = clamped;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Several fixes and refactors across the layout, ECS, examples and tests: - Disable NodeCache memoization by default and add explicit begin_layout_pass() to Node layout entry. Benchmarks now explicitly toggle cross-pass memoization. - Fix wrapping/line-assignment and auto-size computation (use per-line sizing, treat stretch contributions correctly) and correct absolute-child min/max main computation. - Iterate and mutate child/row/col vectors using idiomatic iterators, simplify computations, and clean up formatting for readability. - Update Store::clear to reset wrap, and adjust world/node/type docstrings to clarify horizontal/inline semantics. - Update example code to match changed canvas and drawing APIs (set_size/clear_rect/fill_path/fill_text signatures), adjust draw_node signature, and small test comment improvements. These changes both correct layout behavior and align examples with updated APIs while improving code clarity.
- Remove get_layout_result() and set_layout_result() from Cache trait - Remove memoization logic from layout computation - Remove LayoutMemo struct and memoization fields from NodeCache - Remove enable_layout_memoization() and related methods - Remove bump_layout_revision() calls from World API - Simplify benchmark to remove cross-pass memo comparison - All tests passing
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Apply horizontal alignment flip whenever a node is RTL (remove parent-type guard), and restrict RTL wrap reversal to row layouts. Change absolute-child sizing to use the padding box (content + padding, excluding border) rather than ignoring parent padding. Refactor flex item sizing: separate the rounded allocated size (input/computed) from the actual measured main size, store measured values on items, and use the input allocation for cache comparisons. These changes fix RTL alignment edge-cases, ensure absolute children are sized relative to the padding box, and stabilize flex layout/caching against rounding-induced relayouts.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/node.rs:301
- RTL padding swapping is only applied when
parent_layout_type == LayoutType::Row. ForLayoutType::Column, the horizontal axis is the cross axis, sopadding_cross_before/after(and likely other horizontal semantics likecross_before/afterandborder_cross_before/after) should also be direction-aware; otherwiseDirection::RightToLeftwill flip alignment but still treat left/right padding as physical, leading to inconsistent RTL behavior in column layouts and wrapped columns.
fn padding_main_before(&self, store: &Self::Store, parent_layout_type: LayoutType) -> Units {
if parent_layout_type == LayoutType::Row && self.direction(store).unwrap_or_default() == Direction::RightToLeft
{
self.padding_right(store).unwrap_or_default()
} else {
parent_layout_type.select_unwrap(store, |store| self.padding_left(store), |store| self.padding_top(store))
}
}
fn padding_main_after(&self, store: &Self::Store, parent_layout_type: LayoutType) -> Units {
if parent_layout_type == LayoutType::Row && self.direction(store).unwrap_or_default() == Direction::RightToLeft
{
self.padding_left(store).unwrap_or_default()
} else {
parent_layout_type.select_unwrap(
store,
|store| self.padding_right(store),
|store| self.padding_bottom(store),
)
}
}
fn padding_cross_before(&self, store: &Self::Store, parent_layout_type: LayoutType) -> Units {
parent_layout_type.select_unwrap(store, |store| self.padding_top(store), |store| self.padding_left(store))
}
fn padding_cross_after(&self, store: &Self::Store, parent_layout_type: LayoutType) -> Units {
parent_layout_type.select_unwrap(store, |store| self.padding_bottom(store), |store| self.padding_right(store))
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fix positioning for right-to-left layouts by swapping main_before/main_after when layout direction is RTL. Introduces an is_rtl flag and uses it to invert main-axis offsets for absolute and flow children (and preserves cross-axis lookups). This ensures correct child placement in RTL row/column layouts rather than relying only on reversing child order.
No description provided.