Introduce ViewConstraint and layoutWithConstraint - #45
Conversation
StackView::layoutSubviews sized and positioned siblings from each
subview's leftover frame.w/h from the previous layout pass, so a
Contain-sized child's real intrinsic size was only known after it had
already been used (stale) to size and position its siblings within
the same pass -- no fixed point was guaranteed, and a Fill child
inside a Contain ancestor was flatly unresolvable, since the ancestor
can't hand a real bound to a child it's still sizing itself from.
Split layout into a bottom-up View::sizeThatSatisfies pass (what size
do you want, given this ViewConstraint) that runs to completion for a
dirty subtree before any frame is committed, followed by the existing
top-down View::layoutSubviews arrange pass. A Fill child measured
while its Contain ancestor is itself unresolved receives
ViewConstraintUnspecified and degrades to its own intrinsic size,
rather than reading a stale number; it only actually fills once an
ancestor hands it ViewConstraintEqual, during arrange.
View::sizeToSatisfy joins sizeToContain/sizeToFit/sizeToFill as the
resize-and-mark-dirty sibling of sizeThatSatisfies. layoutIfNeeded,
base layoutSubviews's arrange loop, StackView's arrange loop, and
Panel's contentView bypass each used to hand-duplicate a "resize (or
sizeToSatisfy), clearWarnings, layoutSubviews, needsLayout = false"
sequence, with a comment at each explaining why layoutIfNeeded itself
couldn't be called instead (it would re-derive its own, weaker guess
at the applicable constraint instead of using the one the caller
already resolved). That shared tail is now View::layoutWithConstraint
("what size do you want, given this ViewConstraint" -- resolved via
sizeToSatisfy, honoring ViewAutoresizingWidth/Height per axis) and
View::layoutWithSize ("this is your size" -- applied verbatim, no
negotiation, for StackView's and CollectionView's own distribution
math, which overrides a subview's size regardless of its own
autoresizing bits). needsLayout = false is now written in exactly one
place in the codebase instead of four, and layoutWithConstraint always
resolves regardless of isContainer -- gating it on isContainer (an
earlier version of this same change) skipped resizing entirely for a
Fill-only, non-container View, such as Slider's `bar`.
The `constrainedSize` field this went through along the way -- cached
by sizeThatSatisfies, consumed by StackView -- is gone; sizeThatSatisfies
is a pure function of a View's own state, and StackView holds its
subviews' measured sizes in a plain local array scoped to its own
layoutSubviews call, not on the View instances themselves.
Fixes found only by running Examples/Hello against real widgets, not
caught by the synthetic fixture tests: sizeThatSatisfies must skip a
Contain view whose own sizeThatFits override (Text, TableView, Select)
is meaningful only when that view opted into Contain/Fit itself
(TableView.c), a StackView subview without a matching autoresizing bit
must not have its distribution-computed size re-derived through a
constraint it never agreed to honor, and a View re-laid-out standalone
(e.g. a selected TableRowView, whose style rebind marks only itself
dirty) must trust its own established frame as exact rather than
merely an upper bound, or it shrinks back to its unconstrained content
size. Slider and TextView needed an explicit `min-width` in CSS for
the same reason a Contain view's authored size can't otherwise survive
being summed from children that have none of their own.
Adds Tests/ObjectivelyMVC/View.c (StackView and plain-View fixture
graphs asserting exact post-layout frame values) as a fourth
check_PROGRAMS entry alongside Selector/Style/Stylesheet -- there was
previously no ObjectivelyMVC-View test target at all.Mirrors ObjectivelyMVC-Style: a command-line-tool target building Tests/ObjectivelyMVC/View.c against the same frameworks, plus a shared scheme, so the new View check_PROGRAMS test can run from Xcode like Selector/Style/Stylesheet already could.
There was a problem hiding this comment.
🟡 Changes recommended
The new constraint pipeline introduces at least two confirmed behavior bugs (standalone relayout can force an axis to 0, and Panel’s contentView height constraint is ignored), which can cause incorrect layout at runtime.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Introduces a constraint-based sizing pass (ViewConstraint, sizeThatSatisfies, layoutWithConstraint) to make layout idempotent and to resolve historical ambiguity between Contain and Fill, while also fixing StackView measurement staleness by measuring subviews fresh per layout pass.
Changes:
- Add
ViewConstraint+ new sizing/layout APIs (sizeThatSatisfies,sizeToSatisfy,layoutWithConstraint,layoutWithSize) and wire them into the defaultViewlayout pipeline. - Update key containers (e.g., StackView, Panel, CollectionView) to use constraint/size-driven layout entrypoints instead of
resize+layoutIfNeeded. - Add a new
Viewtest suite covering the previously ambiguous and stale measurement scenarios; update CSS to add explicitmin-widthfloors for widgets relying on contain/fill behavior.
File summaries
| File | Description |
|---|---|
| Tests/ObjectivelyMVC/View.c | Adds layout regression tests for contain/fill ambiguity and StackView staleness/idempotency. |
| Tests/ObjectivelyMVC/Makefile.am | Registers the new View test binary in autotools test runner. |
| Tests/ObjectivelyMVC/.gitignore | Ignores the new View test executable. |
| Sources/ObjectivelyMVC/View.h | Defines ViewConstraint and documents/declares new sizing/layout APIs. |
| Sources/ObjectivelyMVC/View.c | Implements constraint-based sizing/layout and updates default layout behavior to use it. |
| Sources/ObjectivelyMVC/TableView.c | Adjusts sizeThatFits to avoid returning expensive natural size when not acting as a container. |
| Sources/ObjectivelyMVC/StackView.c | Fixes stale measurement by measuring subviews fresh and laying them out via layoutWithSize. |
| Sources/ObjectivelyMVC/Panel.c | Switches content view layout from direct resize to constraint-based layout. |
| Sources/ObjectivelyMVC/CollectionView.c | Uses layoutWithSize for dictated item sizes during layout. |
| ObjectivelyMVC.xcodeproj/xcshareddata/xcschemes/ObjectivelyMVC-View.xcscheme | Adds an Xcode scheme for the new View-focused test/target. |
| ObjectivelyMVC.xcodeproj/project.pbxproj | Adds the new ObjectivelyMVC-View target and related build settings/dependencies. |
| Assets/stylesheet.css.h | Updates embedded stylesheet bytes/length for CSS changes. |
| Assets/stylesheet.css | Adds min-width rules (e.g., Slider/TextView) to ensure contain sizing has a non-zero floor. |
Review details
- Files reviewed: 13/13 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| View *contentView = (View *) this->contentView; | ||
| const ViewConstraint w = MakeConstraint(ViewConstraintEqual, size.w); | ||
| const ViewConstraint h = MakeConstraint(ViewConstraintEqual, size.h); | ||
| $(contentView, layoutWithConstraint, w, h); |
| ViewConstraint w, h; | ||
| if (self->frame.w || self->frame.h) { | ||
| w = MakeConstraint(ViewConstraintEqual, self->frame.w); | ||
| h = MakeConstraint(ViewConstraintEqual, self->frame.h); | ||
| } else { | ||
| w = MakeConstraint(ViewConstraintUnspecified, 0); | ||
| h = MakeConstraint(ViewConstraintUnspecified, 0); | ||
| } |
| * @remarks This is the shared tail of View::layoutIfNeeded: resolve self's size via | ||
| * View::sizeToSatisfy if self is a container, then View::layoutSubviews, then clear | ||
| * `needsLayout`. It exists so a caller that already knows the correct ViewConstraint for a |
| * @remarks The default implementation resolves each subview's size via View::layoutWithConstraint, | ||
| * offering `Exact` for a `ViewAutoresizingWidth`/`Height` subview (since this View's bounds are | ||
| * already final) or `Unspecified` otherwise (so the subview sizes itself from its own content). |
ObjectivelyMVC's layout engine has had some fundamental issues for years. Namely, there are two ViewAutoresizing bits that pull in opposite directions:
ContainandFill. Scenarios where a child specifiedFilland a parent specifiedContainare ambiguous.There was also a staleness bug in StackView that did not re-compute remaining size while laying out subviews. These sorts of bugs led to layoutSubviews not being idempotent.
This branch aims to fix these problems by introducing the concept of constraints, where a parent negotiates each child's size before proceeding to layout. It allows bottom-up size requests to propagate upward before top-down layout clobbers them.