Skip to content

Reconciler skip-path omits gesture/drag slots → stale gesture/drag closure after a skipped update #721

Description

@azchohfi

Summary

The reconciler's element skip-path (Element.ShallowEqualsModifiersEqualModifierCallbacksEqual, in src/Reactor/Core/Element.cs) deliberately does not compare the gesture (Pan/Pinch/Rotate/LongPress) and drag-drop (DragSource/DropTarget) slots. Gesture/drag handlers dispatch through cached per-element state (state.Pan, state.Source/state.Target in Reconciler.Gestures.cs / Reconciler.DragDrop.cs), which is refreshed only on the non-skip Update path (ApplyGestureHandlers / ApplyDragDropHandlers).

Consequence: if an element is skipped while its gesture/drag closure changed, dispatch keeps firing the stale (previous-render) closure — so a gesture/drag callback can run against stale captured state.

New vs. pre-existing

  • Partly pre-existing: on main before the skip-path change, a gesture-only element (no routed handlers) was already skip-eligible and already exhibited this staleness — ModifiersEqual never compared gesture/drag slots.
  • Widened by the skip-path restore (perf: restore diff skip-path + cut DSL/element per-render allocations #665): the old rule only skipped when a fixed set of routed handlers (OnPointerPressed/OnTapped/…) were null on both sides. The new ModifierCallbacksEqual skips when all 21 routed handlers are reference-equal. So an element carrying a reference-stable routed handler + a changing gesture/drag closure is now skipped where it previously was not → newly stale.

The change documents this as intentional and adds a test (tests/Reactor.Tests/PerfDiffSkipPathTests.cs) asserting ModifiersEqual(one, changed) == true for changed gesture configs. The accompanying code comment states it "preserves observable behavior exactly" — accurate for gesture-only elements, but slightly overstated for the widened (gesture + stable-routed) case above.

Trigger conditions (narrow)

All of: a declarative gesture/drag handler (.OnPan(...) etc.) whose closure changes per render (capturing closure), on an element whose routed handlers are reference-stable, and the gesture interaction spans the render where the skip occurs. Not exercised by the grid / keyed-list perf workloads (those use routed handlers, not declarative gestures), which is why it does not affect the measured perf wins.

Design rationale (from the author's comment — to weigh against any fix)

Comparing gesture/drag slots would force an Update where the framework previously skipped, re-arming an in-flight gesture mid-interaction (e.g. re-registering a long-press handler between its Began and Ended phases → double-dispatch). So a naive "just compare the gesture/drag delegates too" is itself risky.

Suggested fix options

  1. A narrow gesture/drag state refresh on the skip path — update state.Pan / state.Source / state.Target to the latest config without re-subscribing the manipulation/drag trampolines (so no mid-gesture re-arm).
  2. Exclude elements that carry gesture/drag slots from the skip predicate (simpler, small perf cost; safe since those elements are rare).

Either way, add a regression test that mounts a gesture element, changes only the gesture closure across a skipped render, drives the gesture, and asserts the latest closure fires.

Provenance

Found during the multi-dimension pr-review of #665 (DSL alloc-cut perf change); confirmed independently by a second model and by base-vs-head reading of ModifiersEqual/ModifierCallbacksEqual. Filed as an accepted follow-up so #665 can land on its (separate) alloc-win merits.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions