Add Display::Contents support#943
Closed
rlch wants to merge 5 commits into
Closed
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.
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
1 participant
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 are promoted to the parent's child list during layout.Builds on #534 but redesigned to avoid the RefCell cache thrashing that caused the perf regression there.
Changes
Display::Contentsvariant added to the enumchild_ids()/child_count()/get_child_id()flatten through contents nodes recursivelySecondaryMapcontents_dirtyflag — zero overhead when no contents nodes existPerformance
When no contents nodes exist: zero overhead (one
boolcheck per layout).When contents nodes are present: ~40% overhead from the resolve walk + extra tree nodes.
Criterion benchmarks included in
benches/benches/display_contents.rs.Tests
Closes #533