Add Display::Contents support#944
Open
rlch wants to merge 6 commits into
Open
Conversation
Adds Display::Contents to the Display enum. Contents nodes generate no box — they are invisible to layout, and their children should be laid out as if they were direct children of the node's parent (CSS `display: contents` semantics). The layout dispatch treats Contents like hidden (zero layout, clear cache) but does NOT recursively hide children. Child flattening is the responsibility of the TraversePartialTree implementation.
Tests verify: - Contents nodes get zero layout (no box generated) - Contents children are not recursively hidden like Display::None - Style roundtrip (set/get Display::Contents) Note: child flattening tests belong in the consumer's TraversePartialTree implementation, not here.
When a node's direct children include Display::Contents nodes, their children are recursively promoted into the parent's child list. This is resolved once before each layout pass (not on every child_ids call). Performance: O(0) when no contents nodes exist (tracked by counter). When contents nodes are present, O(n) walk to pre-compute flattened children stored in a SecondaryMap. No RefCell, no per-call allocation, no cache thrashing — addresses the performance concerns from PR DioxusLabs#534. Tests verify: basic promotion, nested contents, contents vs none behavior, style roundtrip, and zero-overhead fast path.
Criterion benchmarks comparing layout performance with and without Display::Contents nodes across flat trees (10/100/1000), deep trees (4^6 = 4096 nodes), and relayout scenarios. Results: zero overhead when no contents nodes exist (counter skip). With 20% contents nodes at 1000 items: ~186µs relayout vs ~123µs baseline — well within frame budget for typical UI trees.
Simpler and less error-prone than maintaining a counter across every mutation method. The flag is set whenever a Display::Contents-related mutation occurs, and cleared after resolve if no contents nodes exist. Benchmarks show equal or slightly better performance vs the counter.
6 browser-verified test cases: - basic (5 items, 2 via contents wrapper) - nested (contents within contents) - deeply nested (3 levels of contents) - single child (GestureDetector use case) - flex-grow through contents - contents vs none side-by-side All expected values verified against Chrome's computed layout. Also adds "contents" to the FromStr parser for Display.
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.
Implements CSS
display: contentsfor TaffyTree. A contents node generates no box — its children get promoted to the parent's child list during layout.Picks up where #534 left off, but avoids the RefCell cache thrashing that caused the perf regression.
Changes
Display::Contentsvariant on the enumchild_ids()/child_count()/get_child_id()flatten through contents nodes recursivelySecondaryMapcontents_dirtyflag skips the resolve walk when no contents nodes existPerformance
Zero overhead when no contents nodes exist. ~40% when they're present (resolve walk + extra nodes).
Criterion benchmarks in
benches/benches/display_contents.rs.Tests
Closes #533