feat(atlas): integrate Taffy layout with state-machine lifecycle#14
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds the initial Atlas layout implementation by wrapping Taffy behind a LayoutAtlas state-machine API and introducing shared frame geometry primitives.
Changes:
- Adds
RectandViewportshared frame types. - Adds the
atlas::layoutmodule withLayoutAtlas, node IDs, styles, errors, and tests. - Adds the
taffydependency tobyard-core.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 12 comments.
| File | Description |
|---|---|
crates/byard-core/src/frame.rs |
Adds shared rectangle and viewport primitives plus rectangle tests. |
crates/byard-core/src/atlas/mod.rs |
Updates Atlas module docs and re-exports layout API types. |
crates/byard-core/src/atlas/layout.rs |
Implements the Taffy-backed layout atlas and lifecycle tests. |
crates/byard-core/Cargo.toml |
Adds the taffy dependency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 13 comments.
Comments suppressed due to low confidence (4)
crates/byard-core/src/lib.rs:39
- These public variants and the public fields on
PipelineCompilationlost their doc comments. The workspace enablesmissing_docs = "warn"and CI runs clippy with-D warnings, and CONTRIBUTING.md requires public API docs, so this will fail the documented checks until the variant and field docs are restored.
/// A render pipeline failed to compile during initialisation.
PipelineCompilation {
crates/byard-core/src/atlas/layout.rs:153
- This intra-doc link points to
AtlasError::Taffy, but the enum only definesBackend. The public error documentation foradd_containeris currently wrong and will produce a broken link when building docs.
/// Returns [`AtlasError::Taffy`] if the underlying engine refuses the
crates/byard-core/src/atlas/layout.rs:205
- This intra-doc link points to
AtlasError::Taffy, but the enum only definesBackend. The public error documentation forcomputeis currently wrong and will produce a broken link when building docs.
pub fn compute(&mut self, viewport: Viewport) -> Result<(), AtlasError> {
crates/byard-core/src/lib.rs:41
- Wrapping
AtlasErrorhere without updating thestd::error::Errorimpl meansByardError::source()still returnsNonefor layout failures. That breaks normal Rust error-chain reporting for the new nested error even though the underlyingAtlasErroris stored directly.
/// Name of the pipeline that failed (e.g. `"SolidBox"`).
pipeline: String,
…t order doc for RenderFrame rects
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Implements the Atlas subsystem's Taffy integration as defined in
RFC-0001 4.1 and 9. Wraps
taffy::TaffyTreebehind a state-machine APIthat enforces strict separation between tree-building and layout-result
phases.
The Atlas exposes a minimal high-level API (
add_leaf,add_container,set_root,compute,resolved_rect,populate_frame) and neverleaks Taffy types to the rest of the engine —
AtlasNodeIdwrapstaffy::NodeIdas an opaque newtype so future layout backend changesstay contained.
Resolved geometry crosses to the Encoder exclusively through
RenderFrame::push_rect, respecting the dependency graph inRFC-0001 9.
Linked issue
Closes #3
Refs #1
Tasks
LayoutAtlas::new)(
computes_layout_for_container_with_one_child)frame.rsprimitives(
Rect,RenderFrame::push_rect,LayoutAtlas::populate_frame)Acceptance criteria
—
computes_layout_for_container_with_one_child(a "text child" ismodelled as a sized leaf since glyphon integration is a later
sub-issue; this matches Phase 1 scope per RFC-0001 §1).
RenderFramewithout crossingsubsystem boundaries directly —
populate_frame_writes_resolved_geometryasserts the full path, and the atlas module imports nothing from
encoderor any other subsystem.Design decisions
These were discussed before implementation and recorded here for review:
Zero-allocation reuse via
clear(). BothLayoutAtlasandRenderFrameexposeclear()that retains internal capacity. Afterthe first frame, subsequent layouts pay zero allocation cost as long
as node counts stay within the high-water mark.
Dirty tracking lives in the Evaluator. The Atlas does not
maintain its own dirty set. The
EvaluatorTick::collect_dirty()implemented in feat(evaluator): implement dirty-flag tick collection loop #13 already produces
TargetIds the Atlas can consumein a future sub-issue (
recompute_dirty). This avoids stateduplication and respects the dependency graph.
State-machine lifecycle.
LayoutAtlashas two states:Building(nodes can be added/modified) andComputed(geometryaccessible, mutations panic). This prevents accidental partial
recomputation mid-frame. A type-state encoding was considered but
rejected for MVP — the enum + panic approach is less invasive on
callers and can be tightened to type-state later if it proves fragile.
Opaque
AtlasNodeId. Wrapstaffy::NodeIdso the rest of theengine never imports from Taffy.
Errors via
AtlasError. Wrapstaffy::TaffyErrorand composeswith
ByardError. Never panics on Taffy failures — returnsResult.Performance notes
No benchmarks added in this PR — Phase 1 deliverable is correctness and
the lifecycle contract. Performance benchmarks for typical and worst-case
layouts will land alongside the dirty-recompute sub-issue, where they
can compare full vs incremental recomputation meaningfully.
Checklist
cargo fmt --allpassescargo clippy --workspace --all-targets -- -D warningspassescargo test --workspacepasses (55 unit tests, +16 in this PR)CHANGELOG.mdhas an entry under[Unreleased]Follow-up sub-issues
Will be opened as sub-issues of #1 after merge:
feat(atlas): builder API for ergonomic tree construction— fluent chainable API on top of
add_leaf/add_containerfor thebylangtranspiler to target.feat(atlas): incremental recompute_dirty consuming EvaluatorTick output— wires
TargetIds from the Evaluator into Taffy'smark_dirtyAPIfor selective recomputation.
feat(atlas): implement spatial hash grid for O(1) hit-testing— the other half of the Atlas subsystem per RFC-0001 4.1.
Notes for reviewers
The state-machine assertions use
panic!rather thanResultbecausethey protect against caller bugs (calling
add_leafaftercompute),not user input. This matches the convention in
wgpu::CommandEncoderwhere
finish()consumes the encoder.populate_framewalks the tree in pre-order. This will become Z-binorder once the Encoder lands and primitives are sorted by pipeline +
local Z-index. The current order is documented as provisional.
CHANGELOG checkbox N/A — pre-alpha; changelog will be introduced at v0.1.0.