Skip to content

Conversation

@joeleaver
Copy link
Contributor

Summary

This PR fixes a bug where DOM mutations via set_attribute, set_node_text, and clear_attribute would not result in visual updates during continuous operations (like slider dragging), even though the correct damage/hint flags were being set.

The Problem

The Stylo style traversal uses the dirty_descendants flag on ancestor nodes to determine which subtrees need restyling. When a node needs restyling, the traversal algorithm walks down from the root but only descends into subtrees where an ancestor has dirty_descendants = true.

The mutation methods (set_attribute, set_node_text, clear_attribute) were correctly setting:

  • RestyleHint::restyle_subtree() on the node
  • ALL_DAMAGE on the node

However, they were not calling mark_ancestors_dirty(), which sets the dirty_descendants flag on all ancestor nodes up to the root.

Without this flag, the style traversal would skip the modified nodes entirely, leaving their damage unprocessed until something else (like a hover state change) triggered a proper traversal of that subtree.

Why hover() works correctly

The hover() and unhover() methods on Node call set_restyle_hint(), which internally calls mark_ancestors_dirty():

pub fn set_restyle_hint(&self, hint: RestyleHint) {
    if let Some(element_data) = self.stylo_element_data.borrow_mut().as_mut() {
        element_data.hint.insert(hint);
    }
    // Mark all ancestors as having dirty descendants so the style traversal
    // will visit this node's subtree
    self.mark_ancestors_dirty();  // <-- This is the key!
}

This is why hover state changes would "fix" the visual state - they would trigger a proper traversal that would also pick up pending damage from earlier mutations.

The Fix

Added mark_ancestors_dirty() calls to:

  • set_node_text() - after insert_damage()
  • set_attribute() - after setting hint/damage on node and parent
  • clear_attribute() - after setting hint/damage

Testing

Tested with a slider widget that updates inline styles during drag. Before this fix, the slider bar/thumb would not move visually during drag (only on mouse release or when cursor moved over a different element). After this fix, the slider updates in real-time during drag.

Impact

This is a correctness fix with minimal performance impact. The mark_ancestors_dirty() function is O(tree depth) and early-exits when it finds an ancestor that already has the flag set, so repeated mutations to the same subtree have minimal overhead.

joeleaver and others added 2 commits January 22, 2026 16:09
Add proper dirty_descendants flag tracking to enable Stylo's incremental
style traversal to skip unchanged subtrees. Previously, has_dirty_descendants()
always returned true, causing the style traversal to visit every node in the
DOM on every resolve() call.

Changes:
- Add dirty_descendants: AtomicBool field to Node struct
- Implement has_dirty_descendants(), set_dirty_descendants(), unset_dirty_descendants()
  methods on Node
- Add mark_ancestors_dirty() to propagate dirty flags up the tree
- Update TElement trait implementation to use real dirty flag values
- Call mark_ancestors_dirty() when set_restyle_hint() is called
- Clear dirty_descendants flags after style resolution (with incremental feature)

This optimization allows the style traversal to skip entire subtrees that
haven't changed, significantly reducing style resolution time for incremental
updates. For example, hovering over a single element no longer requires
restyling the entire DOM.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The style traversal uses the `dirty_descendants` flag on ancestor nodes
to determine which subtrees need restyling. Without marking ancestors
dirty, nodes with pending RestyleHint or damage may be skipped during
the traversal, even though their hint/damage was correctly set.

This was causing a bug where DOM mutations via `set_attribute`,
`set_node_text`, and `clear_attribute` would set the correct damage
flags but the visual update wouldn't appear until something else
(like a hover state change) triggered a full restyle.

The `hover()` and `unhover()` methods already call `set_restyle_hint()`
which internally calls `mark_ancestors_dirty()`. This change adds the
same call to the mutation methods for consistency.

Methods fixed:
- `set_node_text`: Added mark_ancestors_dirty after insert_damage
- `set_attribute`: Added mark_ancestors_dirty after setting hint/damage
- `clear_attribute`: Added mark_ancestors_dirty after setting hint/damage
@nicoburns
Copy link
Collaborator

@joeleaver This looks like the right fix where it's fixed, but I think there are a few more places where the damage needs to be set. Namely when adding and removing nodes.

Specifically I thinkremove_node, remove_and_drop_node, remove_and_drop_all_children, and add_children_to_parent. And perhaps process_added_subtree (but it doesn't need to recurse up the hierarchy, it can just set it on the specific node as it's already doing a downward recursive traversal, and the parent is already covered).

But you may want to use your own judgement, because it's definitely tricky stuff this.


Btw, is there a reason why there are now 2 PRs? Could this and #332 be combined into one?

@joeleaver joeleaver closed this Jan 24, 2026
@joeleaver
Copy link
Contributor Author

joeleaver commented Jan 24, 2026

Sorry, #332 covers this, this was made in error.

@nicoburns
Copy link
Collaborator

(#332 appears not to contain the second commit from this PR atm)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants