Skip to content

perf!: skip widget rebuilds during divider drag#134

Open
andyhorn wants to merge 6 commits into
mainfrom
optimize-animated-builder
Open

perf!: skip widget rebuilds during divider drag#134
andyhorn wants to merge 6 commits into
mainfrom
optimize-animated-builder

Conversation

@andyhorn
Copy link
Copy Markdown
Owner

@andyhorn andyhorn commented May 27, 2026

Summary

  • Splits ResizableController's notification surface: drag/post-layout pixel updates flow through a new pixelsListenable (ValueListenable<List<double>>); the main controller listener fires only on structural changes.
  • ResizableLayoutRenderObject subscribes to the pixel listenable and markNeedsLayouts on change. The widget tree above is never rebuilt during a drag — only performLayout runs.
  • ResizableContainer replaces AnimatedBuilder with ListenableBuilder, drops _flexFromFullSizes for the idle phase (still used for hide-animation phases), and wraps each child in RepaintBoundary.
  • RTL offset reversal moves into the render object since the live path no longer goes through Flex.

Closes #120.

Breaking change

ResizableController.addListener(...) no longer fires on divider drag — it now fires only on structural changes (children list, declared sizes, hidden set, layout invalidation). Callers that watched the controller for live size changes should subscribe to controller.pixelsListenable instead.

- controller.addListener(() => print(controller.pixels));
+ controller.pixelsListenable.addListener(() => print(controller.pixelsListenable.value));

Test plan

  • All 158 unit/widget tests pass.
  • New rebuild-count test asserts zero child rebuilds and zero main-controller-listener firings during a drag.
  • New render-object tests cover the live-pixel path, the cold ↔ live transition, the listenable swap, RTL reversal, and hidden-divider collapse.
  • New controller tests cover the four notification sites (adjustChildSize, setRenderedSizes, setSizes, setHidden, setChildren) and disposal semantics.
  • Smoke-test in the example app.

🤖 Generated with Claude Code

andyhorn added a commit that referenced this pull request May 27, 2026
Address review findings on PR #134:

- Replace `_PixelsListenable` wrapper with a `ValueNotifier<List<double>>`
  that publishes a fresh `UnmodifiableListView` snapshot per notification,
  restoring proper `ValueListenable` old/new comparison semantics.
- Push a fresh snapshot from `_setAvailableSpace` so the live-pixel render
  path sees redistributed pixels after a LayoutBuilder-driven rebuild.
- Rewrite the `pixelsListenable` dartdoc: drop private-symbol references
  and the negative contract, add a migration note for callers that watched
  the controller for size changes.
- Acknowledge in the controller class doc that the main listener also fires
  on the post-layout rendered-size update.
- Update the README size-tracking snippet to use `pixelsListenable`.
- Assert `constraints.maxWidth.isFinite` in the RTL horizontal reversal
  path now that the assumption is no longer enforced by a `Flex` parent.
- Add tests: `show()` notification, render-object detach/reattach lifecycle,
  vertical-axis rebuild and live-pixel coverage, and the `_canUseLivePixels`
  length-mismatch cold-path fallback.
andyhorn and others added 5 commits May 27, 2026 21:24
Replaces the per-tick ChangeNotifier-driven rebuild of the entire
container subtree with a render-object listener on a dedicated pixel
channel. Drag updates now relayout the children directly without
rebuilding any widgets.

* `ResizableController` exposes `pixelsListenable` — a
  `ValueListenable<List<double>>` that fires on every pixel change
  (drag, post-layout, structural). The main controller listener no
  longer fires during a drag.
* `ResizableLayoutRenderObject` accepts the listenable, subscribes
  on attach, and `markNeedsLayout`s on change. `performLayout`
  branches: when the listenable is present, lay out children
  directly from its value (fast path); otherwise resolve from the
  declared sizes (cold path) and report the resolved values via
  `onComplete`.
* `ResizableContainer` replaces its outer `AnimatedBuilder` with
  `ListenableBuilder` and drops `_flexFromFullSizes` for the idle
  phase; hide-animation phases continue to use the flex path. Each
  child is wrapped in `RepaintBoundary` so paint dirtiness doesn't
  propagate to siblings during a drag.
* RTL offset reversal moves into the render object, since the live
  path no longer goes through `Flex`.

BREAKING CHANGE: `ResizableController`'s `addListener` no longer
fires on divider drag updates — it now fires only on structural
changes (children list, declared sizes, hidden set, layout
invalidation). Callers that watched the controller for live size
changes should subscribe to `controller.pixelsListenable` instead.

Closes #120

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Address review findings on PR #134:

- Replace `_PixelsListenable` wrapper with a `ValueNotifier<List<double>>`
  that publishes a fresh `UnmodifiableListView` snapshot per notification,
  restoring proper `ValueListenable` old/new comparison semantics.
- Push a fresh snapshot from `_setAvailableSpace` so the live-pixel render
  path sees redistributed pixels after a LayoutBuilder-driven rebuild.
- Rewrite the `pixelsListenable` dartdoc: drop private-symbol references
  and the negative contract, add a migration note for callers that watched
  the controller for size changes.
- Acknowledge in the controller class doc that the main listener also fires
  on the post-layout rendered-size update.
- Update the README size-tracking snippet to use `pixelsListenable`.
- Assert `constraints.maxWidth.isFinite` in the RTL horizontal reversal
  path now that the assumption is no longer enforced by a `Flex` parent.
- Add tests: `show()` notification, render-object detach/reattach lifecycle,
  vertical-axis rebuild and live-pixel coverage, and the `_canUseLivePixels`
  length-mismatch cold-path fallback.
Expose `ResizableController.needsLayoutListenable: ValueListenable<bool>`,
which fires when the controller's `needsLayout` flag transitions. The
container now subscribes to this listenable alongside the controller via
`Listenable.merge`, decoupling the cold↔live build-path swap from the
main listener.

With this in place, `_setRenderedSizes` no longer calls `notifyListeners`.
The main controller listener is now reserved strictly for structural
changes (children list, declared sizes, hidden set); post-layout updates
flow through `needsLayoutListenable` (for the build-path swap) and
`pixelsListenable` (for the rendered pixel values).

Breaking: callers that watched the controller via `addListener` to react
to *any* post-layout state change now see fires only on structural events.
Subscribe to `needsLayoutListenable` for layout-state transitions or to
`pixelsListenable` for live pixel values.

Update the `notify count` tests in `resizable_container_test.dart` to
assert the new contract on both listenables.
Cover the three-listenable surface from a consumer's POV: main
ChangeNotifier fires only on structural changes, pixelsListenable carries
drag updates and snapshot-stable values, needsLayoutListenable tracks
build-path swap. Also pins the Listenable.merge coalescing the container
relies on and the README migration pattern.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Pin that drag clamping happens in the controller before publishing to
  pixelsListenable, not in the render-object live layout. A documenting
  test asserts the live layout renders constraint-violating pixels
  verbatim, so any future re-clamping there fails loudly.
- Pin the hide/show animation contract under the new dual-listenable
  surface: pixelsListenable fires a bounded number of times per
  hide/show (the animation lives in HideAnimationCoordinator, not the
  controller, so it is not per-frame), main listener fires exactly once
  per call, and a concurrent drag does not bump main.
- Pin controller swap via didUpdateWidget (old detached, new attached,
  consumer-owned), late subscriber delivery, dispose ordering, and the
  documented multi-container-sharing limitation (each initChildren wipes
  hidden indices, so containers cannot share a controller).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@andyhorn andyhorn force-pushed the optimize-animated-builder branch from c84543f to c1c2316 Compare May 28, 2026 04:24
The perf live path built dividers without the enabled flag, so locked
dividers (resizable: false or ResizableDivider.enabled: false) still
responded to drag, tap, and hover. Pass enabled through to match the
hide-animation path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

AnimatedBuilder rebuilds the whole subtree on every drag tick

1 participant