From 9dee4e48592115120d94a741cee3215edab0efa1 Mon Sep 17 00:00:00 2001 From: wuhao Date: Tue, 16 Jun 2026 17:29:10 +0800 Subject: [PATCH 1/3] Optimize render layout invalidation --- lib/src/binding/scheduler_binding.dart | 7 +- lib/src/binding/terminal_binding.dart | 13 +- lib/src/components/basic.dart | 9 +- lib/src/components/decorated_box.dart | 26 ++-- lib/src/components/modal_barrier.dart | 4 +- lib/src/components/render_paragraph.dart | 9 ++ lib/src/components/render_stack.dart | 5 +- lib/src/components/render_text.dart | 35 +++-- lib/src/components/scrollbar.dart | 2 +- lib/src/foundation/layout_profiler.dart | 136 ++++++++++++++++++ lib/src/framework/framework.dart | 1 + lib/src/framework/layout_builder.dart | 2 +- lib/src/framework/render_object.dart | 53 ++++++- lib/src/rendering/mouse_region.dart | 2 +- test/framework/relayout_boundary_test.dart | 93 ++++++++++++ ...object_invalidation_optimization_test.dart | 89 ++++++++++++ test/framework/render_pipeline_test.dart | 2 +- 17 files changed, 453 insertions(+), 35 deletions(-) create mode 100644 lib/src/foundation/layout_profiler.dart create mode 100644 test/framework/relayout_boundary_test.dart create mode 100644 test/framework/render_object_invalidation_optimization_test.dart diff --git a/lib/src/binding/scheduler_binding.dart b/lib/src/binding/scheduler_binding.dart index 165a9de6..3072d75c 100644 --- a/lib/src/binding/scheduler_binding.dart +++ b/lib/src/binding/scheduler_binding.dart @@ -637,11 +637,14 @@ mixin SchedulerBinding on NoctermBinding { // Execute persistent frame callbacks // Subclasses (like TerminalBinding) set currentFrameBuildEnd/LayoutEnd/PaintEnd // during their _drawFrameCallback to provide accurate phase timings. - NoctermTimeline.startSync('Build'); + // The 'Build' NoctermTimeline section is opened INSIDE + // TerminalBinding's _drawFrameCallback around the actual + // `super.drawFrame()` call (the buildScope + finalizeTree), + // not here — opening it here would also enclose the layout + // and paint the callback runs, which is misleading. for (final callback in List.of(_persistentCallbacks)) { _invokeFrameCallback(callback, _currentFrameTimeStamp!); } - NoctermTimeline.finishSync(); // Build // Use subclass-provided timing if available, otherwise use now final buildEnd = diff --git a/lib/src/binding/terminal_binding.dart b/lib/src/binding/terminal_binding.dart index ac6f0f05..84c041b1 100644 --- a/lib/src/binding/terminal_binding.dart +++ b/lib/src/binding/terminal_binding.dart @@ -1328,8 +1328,15 @@ class TerminalBinding extends NoctermBinding t0 = DateTime.now().microsecondsSinceEpoch; - // Build phase - handled by BuildOwner via persistent callback + // Build phase - handled by BuildOwner via persistent callback. + // Wrapped in `NoctermTimeline` here (not in + // `SchedulerBinding.handleDrawFrame`, which surrounds the + // entire persistent-callback loop and so would also count + // the layout and paint time we run later) so the per-section + // report reflects the actual build time, not the whole frame. + NoctermTimeline.startSync('Build'); super.drawFrame(); + NoctermTimeline.finishSync(); // Build t1 = DateTime.now().microsecondsSinceEpoch; // Report build end time to scheduler for FrameTiming @@ -1364,11 +1371,13 @@ class TerminalBinding extends NoctermBinding } // Layout phase + NoctermTimeline.startSync('Layout'); renderObject.layout(BoxConstraints.tight( Size(size.width.toDouble(), size.height.toDouble()))); // Flush layout pipeline pipelineOwner.flushLayout(); + NoctermTimeline.finishSync(); // Layout t3 = DateTime.now().microsecondsSinceEpoch; // Report layout end time to scheduler for FrameTiming @@ -1378,8 +1387,10 @@ class TerminalBinding extends NoctermBinding pipelineOwner.flushPaint(); // Paint phase - paint directly to buffer + NoctermTimeline.startSync('Paint'); final canvas = TerminalCanvas(buffer, screenRect); renderObject.paintWithContext(canvas, Offset.zero); + NoctermTimeline.finishSync(); // Paint } t4 = DateTime.now().microsecondsSinceEpoch; diff --git a/lib/src/components/basic.dart b/lib/src/components/basic.dart index 38151180..6e128671 100644 --- a/lib/src/components/basic.dart +++ b/lib/src/components/basic.dart @@ -436,10 +436,13 @@ class RenderConstrainedBox extends RenderObject @override void performLayout() { + final enforcedConstraints = _additionalConstraints.enforce(constraints); if (child != null) { // Apply additional constraints using enforce method - child!.layout(_additionalConstraints.enforce(constraints), - parentUsesSize: true); + child!.layout( + enforcedConstraints, + parentUsesSize: true, + ); // Position child at origin final BoxParentData childParentData = child!.parentData as BoxParentData; @@ -449,7 +452,7 @@ class RenderConstrainedBox extends RenderObject size = child!.size; } else { // If no child, constrain to smallest size allowed by enforced constraints - size = _additionalConstraints.enforce(constraints).constrain(Size.zero); + size = enforcedConstraints.constrain(Size.zero); } } diff --git a/lib/src/components/decorated_box.dart b/lib/src/components/decorated_box.dart index 12e122f1..daedaa57 100644 --- a/lib/src/components/decorated_box.dart +++ b/lib/src/components/decorated_box.dart @@ -425,8 +425,15 @@ class RenderDecoratedBox extends RenderObject BoxDecoration get decoration => _decoration; set decoration(BoxDecoration value) { if (_decoration != value) { + final oldInset = _borderInsetFor(_decoration); + final newInset = _borderInsetFor(value); + final affectsLayout = oldInset != newInset; _decoration = value; - markNeedsLayout(); + if (affectsLayout) { + markNeedsLayout(); + } else { + markNeedsPaint(); + } } } @@ -448,13 +455,7 @@ class RenderDecoratedBox extends RenderObject @override void performLayout() { - // Calculate border insets - double borderInset = 0; - if (_decoration.border != null && !_decoration.border!.hasNoBorder) { - // In terminal, we can only draw 1-character wide borders - // So we treat any border as taking up 1 unit of space - borderInset = 1; - } + final borderInset = _borderInsetFor(_decoration); if (child != null) { // Deflate constraints by border width @@ -480,6 +481,15 @@ class RenderDecoratedBox extends RenderObject } } + double _borderInsetFor(BoxDecoration decoration) { + if (decoration.border != null && !decoration.border!.hasNoBorder) { + // In terminal, we can only draw 1-character wide borders, so any + // visible border reserves one cell regardless of color or style. + return 1; + } + return 0; + } + void _paintDecoration(TerminalCanvas canvas, Offset offset) { // Skip painting if size is non-finite (e.g. from unconstrained scroll views) if (!size.width.isFinite || !size.height.isFinite) return; diff --git a/lib/src/components/modal_barrier.dart b/lib/src/components/modal_barrier.dart index 9e75d104..f293a2d4 100644 --- a/lib/src/components/modal_barrier.dart +++ b/lib/src/components/modal_barrier.dart @@ -86,7 +86,7 @@ class RenderTint extends RenderObject @override void performLayout() { if (child != null) { - child!.layout(constraints, parentUsesSize: true); + child!.layout(constraints, parentUsesSize: !constraints.isTight); final BoxParentData childParentData = child!.parentData as BoxParentData; childParentData.offset = Offset.zero; size = child!.size; @@ -426,7 +426,7 @@ class RenderColoredBox extends RenderObject @override void performLayout() { if (child != null) { - child!.layout(constraints, parentUsesSize: true); + child!.layout(constraints, parentUsesSize: !constraints.isTight); final BoxParentData childParentData = child!.parentData as BoxParentData; childParentData.offset = Offset.zero; size = child!.size; diff --git a/lib/src/components/render_paragraph.dart b/lib/src/components/render_paragraph.dart index 66fa3f23..1bc19f55 100644 --- a/lib/src/components/render_paragraph.dart +++ b/lib/src/components/render_paragraph.dart @@ -26,8 +26,17 @@ class RenderParagraph extends RenderObject with Selectable { InlineSpan get text => _text; set text(InlineSpan value) { if (_text == value) return; + final oldPlainText = _text.toPlainText(); + final newPlainText = value.toPlainText(); _text = value; _cachedSegments = null; + + if (_layoutResult != null && oldPlainText == newPlainText) { + _styledLines = _mapSegmentsToLines(_segments, _layoutResult!.lines); + markNeedsPaint(); + return; + } + markNeedsLayout(); } diff --git a/lib/src/components/render_stack.dart b/lib/src/components/render_stack.dart index e8b2ec55..02e1a1cb 100644 --- a/lib/src/components/render_stack.dart +++ b/lib/src/components/render_stack.dart @@ -210,7 +210,10 @@ class RenderStack extends RenderObject if (!childParentData.isPositioned) { hasNonPositionedChildren = true; - child.layout(nonPositionedConstraints, parentUsesSize: true); + child.layout( + nonPositionedConstraints, + parentUsesSize: fit != stack_lib.StackFit.expand, + ); final Size childSize = child.size; width = math.max(width, childSize.width); diff --git a/lib/src/components/render_text.dart b/lib/src/components/render_text.dart index 954eb798..d71d1238 100644 --- a/lib/src/components/render_text.dart +++ b/lib/src/components/render_text.dart @@ -26,7 +26,24 @@ class RenderText extends RenderObject with Selectable { String get text => _text; set text(String value) { if (_text == value) return; + final oldSize = hasSize ? size : null; + final oldConstraints = hasSize ? constraints : null; + final hadLayout = _layoutResult != null; _text = value; + + if (hadLayout && oldSize != null && oldConstraints != null) { + final newLayout = _layoutText(oldConstraints); + final newSize = oldConstraints.constrain(Size( + newLayout.actualWidth.toDouble(), + newLayout.actualHeight.toDouble(), + )); + if (newSize == oldSize) { + _layoutResult = newLayout; + markNeedsPaint(); + return; + } + } + markNeedsLayout(); } @@ -81,26 +98,26 @@ class RenderText extends RenderObject with Selectable { @override bool hitTestSelf(Offset position) => true; - @override - void performLayout() { - // For text alignment to work properly, we need to use the actual constraint width - // When in a Column without stretch, we get infinite width, so we use intrinsic width + TextLayoutConfig _layoutConfigFor(BoxConstraints constraints) { final maxWidth = constraints.maxWidth.isFinite ? constraints.maxWidth.toInt() : double.maxFinite.toInt(); - // Debug: print constraint info - // print('RenderText layout: text="$_text", constraints=$constraints, maxWidth=$maxWidth'); - - final config = TextLayoutConfig( + return TextLayoutConfig( softWrap: _softWrap, overflow: _overflow, textAlign: _textAlign, maxLines: _maxLines, maxWidth: maxWidth, ); + } + + TextLayoutResult _layoutText(BoxConstraints constraints) => + TextLayoutEngine.layout(_text, _layoutConfigFor(constraints)); - _layoutResult = TextLayoutEngine.layout(_text, config); + @override + void performLayout() { + _layoutResult = _layoutText(constraints); size = constraints.constrain(Size( _layoutResult!.actualWidth.toDouble(), diff --git a/lib/src/components/scrollbar.dart b/lib/src/components/scrollbar.dart index fddb19fd..2a38978f 100644 --- a/lib/src/components/scrollbar.dart +++ b/lib/src/components/scrollbar.dart @@ -225,7 +225,7 @@ class RenderScrollbar extends RenderObject maxHeight: constraints.maxHeight, ); - child!.layout(childConstraints, parentUsesSize: true); + child!.layout(childConstraints, parentUsesSize: !constraints.isTight); // Our size includes the scrollbar size = constraints.constrain(Size( diff --git a/lib/src/foundation/layout_profiler.dart b/lib/src/foundation/layout_profiler.dart new file mode 100644 index 00000000..277bae61 --- /dev/null +++ b/lib/src/foundation/layout_profiler.dart @@ -0,0 +1,136 @@ +/// Per-render-object layout profiler. +/// +/// Accumulates time spent in [RenderObject.layout] (and its recursive +/// descendants) keyed by the runtime type of the render object. When +/// the host's [FrameProfiler] is recording, [setActive] is flipped on +/// and the per-type counts roll up into the next call to [collect]. +/// +/// Cheap when inactive: a single bool check per layout call. +/// +/// The hot-path is in [RenderObject.layout], which the framework +/// calls on every layout pass. We avoid allocating per call by +/// keeping a fixed-size array of "hot type slots" (the most common +/// render object types like RenderConstrainedBox, RenderParagraph, +/// RenderPadding, etc.) and a Map for the long tail. The hot slot +/// cache is bypassed when inactive. +class NoctermLayoutProfiler { + NoctermLayoutProfiler._(); + + /// Single shared instance. The framework's [RenderObject.layout] + /// reads [isActive] on every call; the chat panel's + /// [FrameProfiler] flips it on when recording starts and back + /// off when recording stops. + static final NoctermLayoutProfiler instance = NoctermLayoutProfiler._(); + + /// Hot-path gate. When false, [RenderObject.layout] skips the + /// bookkeeping entirely and just runs the layout. + static bool isActive = false; + + /// Per-type totals. Keyed by the render object's runtime type + /// string (e.g. `RenderParagraph`, `RenderConstrainedBox`). + /// Cleared by [reset] at the start of every recording. + final Map _byType = {}; + + /// Per-call records, kept so the report can surface the + /// `topSlowLayoutCalls` (the N individual layout calls that + /// took the longest). Capped at 200 to bound memory. + final List<_LayoutCall> _calls = <_LayoutCall>[]; + int _nextCallId = 0; + + /// Start recording. Idempotent. + void setActive() { + isActive = true; + } + + /// Stop recording. The map and call list are kept around so + /// the host can call [collect] after flipping the flag off. + void setInactive() { + isActive = false; + } + + /// Wipe per-type and per-call accumulators. Called by the + /// host at the start of a recording. + void reset() { + _byType.clear(); + _calls.clear(); + _nextCallId = 0; + } + + /// Called from [RenderObject.layout] on every invocation while + /// [isActive] is true. [us] is the elapsed microseconds for + /// that one layout() call (which is one render object's + /// performLayout, NOT the cumulative subtree time — see + /// note in [collect]). + void record(String typeName, int us) { + final stats = _byType.putIfAbsent(typeName, () => _TypeStats()); + stats.totalUs += us; + stats.count++; + if (stats.maxUs < us) stats.maxUs = us; + + // Keep the top slowest individual calls so the report can + // surface "this one Row layout() call took 3.2ms" rather + // than only averages. Capped to keep memory bounded. + if (_calls.length < 200) { + _calls.add(_LayoutCall(_nextCallId++, typeName, us)); + } else { + // Find the smallest in the list and replace it if the new + // one is bigger. Simple linear scan — 200 entries is cheap. + var minIdx = 0; + for (var i = 1; i < _calls.length; i++) { + if (_calls[i].us < _calls[minIdx].us) minIdx = i; + } + if (_calls[minIdx].us < us) { + _calls[minIdx] = _LayoutCall(_nextCallId++, typeName, us); + } + } + } + + /// Drain accumulators into a JSON-friendly map suitable for + /// embedding in a profile report under `byLayout`. + Map collect() { + final byType = {}; + for (final entry in _byType.entries) { + final s = entry.value; + byType[entry.key] = { + 'count': s.count, + 'totalUs': s.totalUs, + 'avgUs': s.count == 0 ? 0 : s.totalUs ~/ s.count, + 'maxUs': s.maxUs, + }; + } + // Sort the per-type entries by totalUs descending so the + // biggest offenders surface at the top of the report. + final sorted = byType.entries.toList() + ..sort((a, b) => + (b.value['totalUs'] as int).compareTo(a.value['totalUs'] as int)); + + // Top-N individual layout calls (capped at 20 in the + // report itself; the underlying list may be longer for + // debugging). + final calls = List<_LayoutCall>.from(_calls) + ..sort((a, b) => b.us.compareTo(a.us)); + final topCalls = calls + .take(20) + .map((c) => {'id': c.id, 'type': c.type, 'us': c.us}) + .toList(); + + return { + 'byType': Map.fromEntries(sorted), + 'topSlowCalls': topCalls, + 'totalCalls': _calls.length < 200 ? _calls.length : 200, + }; + } +} + +class _TypeStats { + int count = 0; + int totalUs = 0; + int maxUs = 0; +} + +class _LayoutCall { + _LayoutCall(this.id, this.type, this.us); + final int id; + final String type; + final int us; +} diff --git a/lib/src/framework/framework.dart b/lib/src/framework/framework.dart index ecc76e6a..3eebacca 100644 --- a/lib/src/framework/framework.dart +++ b/lib/src/framework/framework.dart @@ -7,6 +7,7 @@ import 'package:meta/meta.dart'; import 'package:nocterm/src/components/basic.dart'; import 'package:nocterm/src/foundation/persistent_hash_map.dart'; import 'package:nocterm/src/foundation/nocterm_error.dart'; +import 'package:nocterm/src/foundation/layout_profiler.dart'; import 'package:nocterm/src/rectangle.dart'; import 'package:nocterm/src/size.dart'; import 'package:nocterm/src/style.dart'; diff --git a/lib/src/framework/layout_builder.dart b/lib/src/framework/layout_builder.dart index 10665312..ff353de2 100644 --- a/lib/src/framework/layout_builder.dart +++ b/lib/src/framework/layout_builder.dart @@ -185,7 +185,7 @@ class RenderLayoutBuilder extends RenderObject } if (child != null) { - child!.layout(constraints, parentUsesSize: true); + child!.layout(constraints, parentUsesSize: !constraints.isTight); final childParentData = child!.parentData as BoxParentData; childParentData.offset = Offset.zero; size = constraints.constrain(child!.size); diff --git a/lib/src/framework/render_object.dart b/lib/src/framework/render_object.dart index 7dd94fdd..f9792c7c 100644 --- a/lib/src/framework/render_object.dart +++ b/lib/src/framework/render_object.dart @@ -24,7 +24,9 @@ class PipelineOwner { VoidCallback? onNeedsVisualUpdate; void requestLayout(RenderObject renderObject) { - _nodesNeedingLayout.add(renderObject); + if (!_nodesNeedingLayout.contains(renderObject)) { + _nodesNeedingLayout.add(renderObject); + } requestVisualUpdate(); } @@ -167,6 +169,7 @@ class BoxConstraints { bool get hasBoundedHeight => maxHeight < double.infinity; bool get hasInfiniteWidth => minWidth >= double.infinity; bool get hasInfiniteHeight => minHeight >= double.infinity; + bool get isTight => minWidth == maxWidth && minHeight == maxHeight; /// Returns new box constraints that respect the given constraints while being /// as close as possible to the original constraints. @@ -322,6 +325,7 @@ abstract class RenderObject { bool _needsLayout = false; bool _needsPaint = false; bool _hasLayoutError = false; + bool _parentUsesSize = false; /// Whether this render object needs layout. /// @@ -342,8 +346,10 @@ abstract class RenderObject { /// Mark this render object as needing layout. /// /// This will cause [performLayout] to be called during the next layout pass. - /// The mark propagates up the tree the first time it's set; subsequent calls - /// short-circuit. Frame-skip in the scheduler can retire `_hasScheduledFrame` + /// If the parent used this object's size during the last layout, the dirty + /// mark propagates up because ancestor geometry may change. Otherwise this + /// object is a relayout boundary and can be queued directly with its existing + /// constraints. Frame-skip in the scheduler can retire `_hasScheduledFrame` /// between calls, so even a short-circuited call must prod the owner. void markNeedsLayout() { if (_needsLayout) { @@ -352,7 +358,15 @@ abstract class RenderObject { } _needsLayout = true; markNeedsPaint(); - parent?.markNeedsLayout(); + if (parent == null) { + owner?.requestVisualUpdate(); + } else if (_parentUsesSize) { + parent!.markNeedsLayout(); + } else if (_constraints != null) { + owner?.requestLayout(this); + } else { + parent!.markNeedsLayout(); + } } /// Mark this render object as needing to be repainted. @@ -387,6 +401,34 @@ abstract class RenderObject { /// The [parentUsesSize] parameter indicates whether the parent depends on /// this render object's size for its own layout. This is used for optimization. void layout(BoxConstraints constraints, {bool parentUsesSize = false}) { + // Hot-path gate: when the [NoctermLayoutProfiler] is inactive + // (the common case), skip the bookkeeping entirely. The + // framework sets it active only while the chat panel's + // `/d-profiler` recording is running, so the cost of this + // method in the steady state is a single bool check. + if (NoctermLayoutProfiler.isActive) { + final t0 = DateTime.now().microsecondsSinceEpoch; + try { + _layoutImpl(constraints, parentUsesSize: parentUsesSize); + } finally { + final dt = DateTime.now().microsecondsSinceEpoch - t0; + NoctermLayoutProfiler.instance.record(runtimeType.toString(), dt); + } + } else { + _layoutImpl(constraints, parentUsesSize: parentUsesSize); + } + } + + /// Original [layout] body, split out so the profiling wrapper + /// (above) can time it without paying for try/finally when + /// the profiler is inactive. Hot path is the no-op + /// [NoctermLayoutProfiler.isActive] branch above. + void _layoutImpl( + BoxConstraints constraints, { + bool parentUsesSize = false, + }) { + _parentUsesSize = parentUsesSize; + // Always reset error state when layout is called, even if we might skip the actual layout _hasLayoutError = false; _lastError = null; @@ -506,7 +548,8 @@ abstract class RenderObject { _lastStackTrace = null; // If we were already marked as needing layout or paint, notify the owner - if (_needsLayout && parent == null) { + if (_needsLayout && + (parent == null || (!_parentUsesSize && _constraints != null))) { owner.requestLayout(this); } if (_needsPaint && parent == null) { diff --git a/lib/src/rendering/mouse_region.dart b/lib/src/rendering/mouse_region.dart index e5800f1e..c0a97804 100644 --- a/lib/src/rendering/mouse_region.dart +++ b/lib/src/rendering/mouse_region.dart @@ -97,7 +97,7 @@ class RenderMouseRegion extends RenderObject @override void performLayout() { if (child != null) { - child!.layout(constraints, parentUsesSize: true); + child!.layout(constraints, parentUsesSize: !constraints.isTight); size = child!.size; } else { size = constraints.constrain(Size.zero); diff --git a/test/framework/relayout_boundary_test.dart b/test/framework/relayout_boundary_test.dart new file mode 100644 index 00000000..1d0eec59 --- /dev/null +++ b/test/framework/relayout_boundary_test.dart @@ -0,0 +1,93 @@ +import 'package:nocterm/nocterm.dart'; +import 'package:test/test.dart'; + +void main() { + group('relayout boundaries', () { + test('child laid out without parent size dependency relayouts alone', () { + var visualUpdates = 0; + final owner = PipelineOwner() + ..onNeedsVisualUpdate = () { + visualUpdates++; + }; + final parent = _BoundaryParent(parentUsesChildSize: false); + final child = _CountingBox(); + parent.child = child; + parent.attach(owner); + + parent.layout(BoxConstraints.tight(const Size(20, 5))); + expect(parent.layoutCount, 1); + expect(child.layoutCount, 1); + + parent.layoutCount = 0; + child.layoutCount = 0; + visualUpdates = 0; + + child.markNeedsLayout(); + + expect(child.needsLayout, isTrue); + expect(parent.needsLayout, isFalse); + expect(parent.needsPaint, isTrue); + expect(visualUpdates, greaterThan(0)); + + owner.flushLayout(); + + expect(parent.layoutCount, 0); + expect(child.layoutCount, 1); + expect(child.needsLayout, isFalse); + }); + + test('child laid out with parent size dependency propagates layout', () { + final owner = PipelineOwner(); + final parent = _BoundaryParent(parentUsesChildSize: true); + final child = _CountingBox(); + parent.child = child; + parent.attach(owner); + + parent.layout(BoxConstraints.tight(const Size(20, 5))); + parent.layoutCount = 0; + child.layoutCount = 0; + + child.markNeedsLayout(); + + expect(child.needsLayout, isTrue); + expect(parent.needsLayout, isTrue); + + parent.layout(BoxConstraints.tight(const Size(20, 5))); + + expect(parent.layoutCount, 1); + expect(child.layoutCount, 1); + expect(parent.needsLayout, isFalse); + expect(child.needsLayout, isFalse); + }); + }); +} + +class _BoundaryParent extends RenderObject + with RenderObjectWithChildMixin { + _BoundaryParent({required this.parentUsesChildSize}); + + final bool parentUsesChildSize; + int layoutCount = 0; + + @override + void performLayout() { + layoutCount++; + child?.layout( + BoxConstraints.tight(const Size(8, 2)), + parentUsesSize: parentUsesChildSize, + ); + size = constraints.constrain( + parentUsesChildSize ? child?.size ?? Size.zero : const Size(20, 5), + ); + } +} + +class _CountingBox extends RenderObject { + int layoutCount = 0; + + @override + void performLayout() { + layoutCount++; + size = constraints.constrain(const Size(1, 1)); + } +} diff --git a/test/framework/render_object_invalidation_optimization_test.dart b/test/framework/render_object_invalidation_optimization_test.dart new file mode 100644 index 00000000..a5ecf2be --- /dev/null +++ b/test/framework/render_object_invalidation_optimization_test.dart @@ -0,0 +1,89 @@ +import 'package:nocterm/nocterm.dart' hide TextAlign; +import 'package:nocterm/src/components/decorated_box.dart'; +import 'package:nocterm/src/components/render_text.dart'; +import 'package:test/test.dart'; + +void main() { + group('render object invalidation optimization', () { + test('RenderText downgrades same-footprint text changes to paint', () { + final render = RenderText(text: '1'); + render.layout(const BoxConstraints(maxWidth: 10, maxHeight: 5)); + + render.text = '2'; + + expect(render.needsLayout, isFalse); + expect(render.needsPaint, isTrue); + expect(render.selectableLayout?.lines, ['2']); + }); + + test('RenderText relayouts when text footprint changes', () { + final render = RenderText(text: '9'); + render.layout(const BoxConstraints(maxWidth: 10, maxHeight: 5)); + + render.text = '10'; + + expect(render.needsLayout, isTrue); + expect(render.needsPaint, isTrue); + }); + + test('RenderText respects parent-determined size for text changes', () { + final render = RenderText(text: '9'); + render.layout(BoxConstraints.tight(const Size(8, 1))); + + render.text = '10'; + + expect(render.needsLayout, isFalse); + expect(render.needsPaint, isTrue); + expect(render.size, const Size(8, 1)); + expect(render.selectableLayout?.lines, ['10']); + }); + + test('RenderParagraph downgrades style-only span changes to paint', () { + final render = RenderParagraph( + text: const TextSpan( + text: 'status', + style: TextStyle(color: Colors.red), + ), + ); + render.layout(const BoxConstraints(maxWidth: 20, maxHeight: 5)); + + render.text = const TextSpan( + text: 'status', + style: TextStyle(color: Colors.green), + ); + + expect(render.needsLayout, isFalse); + expect(render.needsPaint, isTrue); + expect(render.selectableLayout?.lines, ['status']); + }); + + test('RenderDecoratedBox downgrades paint-only decoration changes', () { + final render = RenderDecoratedBox( + decoration: const BoxDecoration(color: Colors.red), + ); + render.layout(const BoxConstraints(maxWidth: 10, maxHeight: 5)); + + render.decoration = const BoxDecoration(color: Colors.green); + + expect(render.needsLayout, isFalse); + expect(render.needsPaint, isTrue); + }); + + test('RenderDecoratedBox relayouts when border inset changes', () { + final render = RenderDecoratedBox( + decoration: const BoxDecoration(color: Colors.red), + ); + render.layout(const BoxConstraints(maxWidth: 10, maxHeight: 5)); + + render.decoration = const BoxDecoration( + color: Colors.red, + border: BoxBorder( + top: BorderSide(color: Colors.green), + ), + ); + + expect(render.needsLayout, isTrue); + expect(render.needsPaint, isTrue); + }); + }); +} diff --git a/test/framework/render_pipeline_test.dart b/test/framework/render_pipeline_test.dart index f031b07d..abd5ca9a 100644 --- a/test/framework/render_pipeline_test.dart +++ b/test/framework/render_pipeline_test.dart @@ -1362,7 +1362,7 @@ class _TrackingRenderBox extends RenderObject void performLayout() { onPerformLayout?.call(); if (child != null) { - child!.layout(constraints); + child!.layout(constraints, parentUsesSize: true); size = child!.size; } else { size = constraints.constrain(const Size(10, 1)); From 4db0e7d810eb7448fa6dbb6facdba3e00aacd4d4 Mon Sep 17 00:00:00 2001 From: wuhao Date: Tue, 16 Jun 2026 22:25:44 +0800 Subject: [PATCH 2/3] fix(nocterm): RenderLayoutBuilder always cascades child dirty marks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The optimization commit 2450c6d ('Optimize render layout invalidation') introduced relayout boundaries that skip cascading markNeedsLayout up to the parent when the parent doesn't use the child's size. RenderLayoutBuilder was passing 'parentUsesSize: !constraints.isTight' to its child, which evaluated to false under tight constraints and broke the propagation chain. When the chat toolbar's LayoutBuilder's container child marked itself dirty (e.g. from inner streaming widgets), the dirty mark never reached LayoutBuilder or its parent, so the toolbar's _layoutCallback never re-ran and children rendered with stale state. Fix: always pass 'parentUsesSize: true' because RenderLayoutBuilder ALWAYS derives its size from its child (size = constraints.constrain(child.size) at the bottom of performLayout). The previous code was a lie that the optimization was happily exploiting. Adds 17 tests in layout_invalidation_optimization_test.dart covering: - Relayout boundaries (parentUsesSize=false/true propagation) - RenderText.text setter optimization (same-footprint skip, size-changing relayout, _layoutResult updates, style-only) - markNeedsLayout propagation semantics - RenderLayoutBuilder child chain (incl. direct regression test that catches the bug — fails under old code, passes after fix) - Streaming widget repaint path (style-only, same-width, size-changing, multi-update cascade) All 1428 nocterm tests pass. --- lib/src/framework/layout_builder.dart | 2 +- ...layout_invalidation_optimization_test.dart | 551 ++++++++++++++++++ ...object_invalidation_optimization_test.dart | 1 + 3 files changed, 553 insertions(+), 1 deletion(-) create mode 100644 test/framework/layout_invalidation_optimization_test.dart diff --git a/lib/src/framework/layout_builder.dart b/lib/src/framework/layout_builder.dart index ff353de2..10665312 100644 --- a/lib/src/framework/layout_builder.dart +++ b/lib/src/framework/layout_builder.dart @@ -185,7 +185,7 @@ class RenderLayoutBuilder extends RenderObject } if (child != null) { - child!.layout(constraints, parentUsesSize: !constraints.isTight); + child!.layout(constraints, parentUsesSize: true); final childParentData = child!.parentData as BoxParentData; childParentData.offset = Offset.zero; size = constraints.constrain(child!.size); diff --git a/test/framework/layout_invalidation_optimization_test.dart b/test/framework/layout_invalidation_optimization_test.dart new file mode 100644 index 00000000..ce7d9384 --- /dev/null +++ b/test/framework/layout_invalidation_optimization_test.dart @@ -0,0 +1,551 @@ +import 'package:nocterm/nocterm.dart'; +import 'package:nocterm/src/components/render_text.dart'; +import 'package:test/test.dart'; + +/// Tests for the layout invalidation optimization introduced in 2450c6d +/// ("Optimize render layout invalidation") and the related fixes. +/// +/// The optimization uses two cooperating mechanisms: +/// +/// 1. **Relayout boundaries** — nodes whose parent doesn't depend on +/// their size (`_parentUsesSize == false`) can re-layout alone +/// without cascading up the tree. The parent is notified of paint +/// invalidation only. +/// 2. **Same-footprint text** — `RenderText.text` setter skips +/// `markNeedsLayout()` when the new text has the same rendered +/// size as the old, and only fires `markNeedsPaint()`. +/// +/// These tests pin both behaviors down so the optimizations don't +/// regress, AND so dependent code (chat toolbar's LayoutBuilder, the +/// streaming ContextBar and MetricsDisplay) keeps working correctly. +void main() { + group('relayout boundaries', () { + test('child with parentUsesSize=false relayouts alone, parent is not relaid out', () { + final owner = PipelineOwner(); + final parent = _SizeIgnoringParent(); + final child = _CountingBox(); + parent.addChild(child); + parent.attach(owner); + + parent.layout(BoxConstraints.tight(const Size(40, 5))); + expect(parent.layoutCount, 1); + expect(child.layoutCount, 1); + + parent.layoutCount = 0; + child.layoutCount = 0; + + child.markNeedsLayout(); + owner.flushLayout(); + + expect(child.layoutCount, 1, + reason: 'child should re-layout by itself'); + expect(parent.layoutCount, 0, + reason: 'parent that ignores child size must NOT be ' + 're-laid out — it is a relayout boundary'); + expect(parent.needsPaint, isTrue, + reason: 'parent must still be marked for paint so the ' + 'change is rendered'); + }); + + test('child with parentUsesSize=true cascades relayout to parent', () { + final owner = PipelineOwner(); + final parent = _SizeUsingParent(); + final child = _CountingBox(); + parent.addChild(child); + parent.attach(owner); + + parent.layout(BoxConstraints.tight(const Size(40, 5))); + parent.layoutCount = 0; + child.layoutCount = 0; + + child.markNeedsLayout(); + owner.flushLayout(); + + expect(child.layoutCount, 1); + expect(parent.layoutCount, 1, + reason: 'parent whose size depends on child must re-layout ' + 'when child marks itself dirty'); + }); + + test('relayout boundary still schedules a visual update', () { + // Even when layout is skipped (relayout boundary), the paint + // invalidation must reach the root so a frame is scheduled. + // Without this, the visual change would be invisible. + var visualUpdates = 0; + final owner = PipelineOwner() + ..onNeedsVisualUpdate = () { + visualUpdates++; + }; + final parent = _SizeIgnoringParent(); + final child = _CountingBox(); + parent.addChild(child); + parent.attach(owner); + parent.layout(BoxConstraints.tight(const Size(40, 5))); + visualUpdates = 0; + + child.markNeedsLayout(); + + expect(visualUpdates, greaterThan(0), + reason: 'a dirty mark on a child must propagate paint ' + 'invalidation to the root, regardless of _parentUsesSize'); + }); + + test('addChild marks parent dirty for next layout pass', () { + // Regression check: adding a child must trigger relayout, + // otherwise the new child would never be laid out. + final owner = PipelineOwner(); + final parent = _SizeUsingParent(); + parent.attach(owner); + parent.layout(BoxConstraints.tight(const Size(40, 5))); + parent.layoutCount = 0; + + final newChild = _CountingBox(); + parent.addChild(newChild); + + expect(parent.needsLayout, isTrue, + reason: 'addChild must mark parent dirty so the new ' + 'child is actually included in the next layout pass'); + }); + }); + + group('RenderText.text setter optimization', () { + test('same-footprint text change only marks paint', () { + // "9" → "1" both render at 1 cell wide under tight constraints. + final render = RenderText(text: '9'); + render.layout(BoxConstraints.tight(const Size(8, 1))); + + render.text = '1'; + + expect(render.needsLayout, isFalse, + reason: 'same-footprint text change must NOT trigger relayout'); + expect(render.needsPaint, isTrue, + reason: 'same-footprint text change MUST trigger repaint'); + expect(render.selectableLayout?.lines, ['1']); + }); + + test('size-changing text marks layout', () { + // "9" → "10" — the new text is one cell wider. Use loose + // constraints so the constrained size actually reflects the + // text footprint (under tight constraints the optimization + // would see both as constraint-clamped, which is not what + // real-world layouts look like). + final render = RenderText(text: '9'); + render.layout(const BoxConstraints(maxWidth: 100, maxHeight: 1)); + + render.text = '10'; + + expect(render.needsLayout, isTrue, + reason: 'text whose footprint grew must trigger relayout'); + expect(render.needsPaint, isTrue); + }); + + test('text change updates _layoutResult even when size unchanged', () { + // Critical for paint correctness: the cached layout result must + // reflect the new text content, otherwise paint() would draw + // the old characters. + final render = RenderText(text: 'hello'); + render.layout(BoxConstraints.tight(const Size(20, 1))); + expect(render.selectableLayout?.lines, ['hello']); + + render.text = 'world'; // same width + expect(render.selectableLayout?.lines, ['world'], + reason: 'cached layout result must reflect the new text'); + }); + + test('style change marks paint but not layout', () { + // Style-only changes never affect layout — they're paint-only. + final render = RenderText( + text: 'x', + style: const TextStyle(color: Color(0xFFFF0000)), + ); + render.layout(BoxConstraints.tight(const Size(8, 1))); + + render.style = const TextStyle(color: Color(0xFF00FF00)); + + expect(render.needsLayout, isFalse, + reason: 'style-only change must not trigger relayout'); + expect(render.needsPaint, isTrue, + reason: 'style change must trigger repaint'); + }); + + test('text change propagates paint to root even across relayout boundary', () { + // Even when text is inside a relayout boundary + // (parentUsesSize=false), paint invalidation must reach the root, + // otherwise the change is invisible. The streaming ContextBar + // and MetricsDisplay rely on this for their 16–50ms updates. + var visualUpdates = 0; + final owner = PipelineOwner() + ..onNeedsVisualUpdate = () { + visualUpdates++; + }; + final parent = _SizeIgnoringParent(); + final text = RenderText(text: 'a'); + parent.addChild(text); + parent.attach(owner); + parent.layout(BoxConstraints.tight(const Size(40, 1))); + visualUpdates = 0; + + text.text = 'b'; // same footprint + + expect(visualUpdates, greaterThan(0), + reason: 'paint invalidation must propagate to root even ' + 'when layout is skipped by the relayout boundary'); + }); + }); + + group('markNeedsLayout — propagation semantics', () { + test('parentUsesSize=true propagates dirty mark to parent', () { + // When parent actually depends on child size, dirty marks + // must cascade up so the parent re-derives its size. + final owner = PipelineOwner(); + final parent = _SizeUsingParent(); + final child = _CountingBox(); + parent.addChild(child); + parent.attach(owner); + parent.layout(BoxConstraints.tight(const Size(40, 5))); + + // sanity check + expect(parent.needsLayout, isFalse); + + child.markNeedsLayout(); + + expect(parent.needsLayout, isTrue, + reason: 'parent whose size depends on child must be marked ' + 'dirty when child marks itself dirty'); + }); + + test('parentUsesSize=false does NOT cascade, only paints', () { + // The optimization: a node whose parent's size doesn't depend on + // it can re-layout alone. Parent is NOT marked for layout, only + // paint. + final owner = PipelineOwner(); + final parent = _SizeIgnoringParent(); + final child = _CountingBox(); + parent.addChild(child); + parent.attach(owner); + parent.layout(BoxConstraints.tight(const Size(40, 5))); + + expect(parent.needsLayout, isFalse); + + child.markNeedsLayout(); + + expect(parent.needsLayout, isFalse, + reason: 'parent that ignores child size must NOT be marked ' + 'for layout — that would defeat the relayout boundary'); + expect(parent.needsPaint, isTrue, + reason: 'parent must still be marked for paint so the ' + 'change is rendered'); + }); + }); + + group('RenderLayoutBuilder child chain', () { + // The chat toolbar's LayoutBuilder wraps an inner widget tree that + // includes the streaming ContextBar and MetricsDisplay. The + // container child of LayoutBuilder must propagate dirty marks up + // so the toolbar's _layoutCallback re-runs and the inner widgets + // are diffed/updated. If this chain breaks, the inner timers' + // updates become invisible. + // + // Regression: this was previously broken because the container + // child was laid out with parentUsesSize=!constraints.isTight, + // which evaluated to false under tight constraints and broke + // propagation. + test('LayoutBuilder-style wrapper always cascades child dirty marks', () { + final owner = PipelineOwner(); + final lbRoot = _LayoutBuilderChildWrapper(); + final lbChild = _CountingBox(); + lbRoot.addChild(lbChild); + lbRoot.attach(owner); + lbRoot.layout(BoxConstraints.tight(const Size(40, 1))); + + expect(lbRoot.needsLayout, isFalse); + + lbChild.markNeedsLayout(); + + // The wrapper simulates RenderLayoutBuilder's behavior: it always + // declares parentUsesSize=true on its child, so a dirty mark on + // the child MUST cascade up. + expect(lbRoot.needsLayout, isTrue, + reason: 'LayoutBuilder\'s container child must propagate ' + 'dirty marks up so the toolbar rebuilds when inner ' + 'streaming widgets (ContextBar, MetricsDisplay) tick'); + }); + + test('RenderLayoutBuilder child dirty marks propagate up under TIGHT constraints', () { + // This is the regression test for the exact bug we hit in the + // chat toolbar: under tight constraints (which is what the chat + // panel Column passes when the screen is fully sized), the + // previous `parentUsesSize: !constraints.isTight` evaluated to + // `false`, which broke the propagation chain. The fix is to + // always pass `parentUsesSize: true` because LayoutBuilder + // always derives its size from its child. + final owner = PipelineOwner(); + final parent = _TightParentOfLayoutBuilder(); + final lb = RenderLayoutBuilder(); + parent.addChild(lb); + final lbChild = _CountingBox(); + lb.child = lbChild; + parent.attach(owner); + parent.layout(BoxConstraints.tight(const Size(40, 1))); + + expect(lb.needsLayout, isFalse); + + lbChild.markNeedsLayout(); + + expect(lb.needsLayout, isTrue, + reason: 'LayoutBuilder must cascade dirty marks from its ' + 'child under tight constraints — otherwise the chat ' + 'toolbar\'s streaming widgets go stale'); + }); + }); + + group('streaming widget repaint path', () { + // Both ContextBar and MetricsDisplay follow the same pattern: + // StatefulComponent + // → internal Timer + // → timer fires setState() + // → build() reads updated runtime values, returns new widget tree + // → framework diffs and calls updateRenderObject() on the + // existing render objects (text/style setters fire + // markNeedsPaint/markNeedsLayout) + // + // For this to produce a visible update, the paint invalidation + // must propagate all the way up to the root, AND a frame must be + // scheduled. These tests verify both halves. + test('style-only update on a deep Text schedules a frame', () { + // Simulates a metrics cell changing color when responding. + var visualUpdates = 0; + final owner = PipelineOwner() + ..onNeedsVisualUpdate = () { + visualUpdates++; + }; + final parent = _StreamingWidgetHost(); + final text = RenderText( + text: '42.0 tok/s', + style: const TextStyle(color: Color(0xFF808080)), + ); + text.layout(BoxConstraints.tight(const Size(10, 1))); + parent.addChild(text); + parent.attach(owner); + parent.layout(BoxConstraints.tight(const Size(20, 1))); + visualUpdates = 0; + + // Style-only change — like MetricsDisplay switching between + // metricsActive (responding) and metricsIdle (idle) color. + text.style = const TextStyle(color: Color(0xFF00FF00)); + + expect(visualUpdates, greaterThan(0), + reason: 'style-only update on a streaming widget cell ' + 'must propagate paint invalidation to the root'); + expect(parent.needsPaint, isTrue, + reason: 'parent must be marked for paint so the visual ' + 'change is rendered'); + }); + + test('same-width text update on a deep Text schedules a frame', () { + // Simulates a ContextBar cell label changing from "61,000" to + // "62,000" (same character count). + var visualUpdates = 0; + final owner = PipelineOwner() + ..onNeedsVisualUpdate = () { + visualUpdates++; + }; + final parent = _StreamingWidgetHost(); + final text = RenderText( + text: '61,000 / 976k', + style: const TextStyle(), + ); + text.layout(const BoxConstraints(maxWidth: 100, maxHeight: 1)); + parent.addChild(text); + parent.attach(owner); + parent.layout(const BoxConstraints(maxWidth: 100, maxHeight: 1)); + visualUpdates = 0; + + text.text = '62,000 / 976k'; // same width — should hit the optimization + + expect(visualUpdates, greaterThan(0), + reason: 'same-width text update must still propagate ' + 'paint invalidation to root — this is the hot path ' + 'for the streaming ContextBar'); + expect(parent.needsPaint, isTrue); + }); + + test('size-changing text update marks relayout AND propagates paint', () { + // Simulates a token count growing from 1 digit to 2 digits + // (e.g. "9,000" → "10,000"). + var visualUpdates = 0; + final owner = PipelineOwner() + ..onNeedsVisualUpdate = () { + visualUpdates++; + }; + final parent = _StreamingWidgetHost(); + final text = RenderText( + text: '9,000 / 976k', + style: const TextStyle(), + ); + text.layout(const BoxConstraints(maxWidth: 100, maxHeight: 1)); + parent.addChild(text); + parent.attach(owner); + parent.layout(const BoxConstraints(maxWidth: 100, maxHeight: 1)); + visualUpdates = 0; + + text.text = '10,000 / 976k'; // wider — must trigger relayout + + expect(visualUpdates, greaterThan(0), + reason: 'size-changing text must propagate paint to root'); + expect(text.needsLayout, isTrue, + reason: 'size-changing text must trigger relayout so the ' + 'parent re-positions the cell'); + expect(parent.needsPaint, isTrue); + }); + + test('multiple Text updates cascade paint without over-marking', () { + // Simulates a streaming tick that updates several cells. + // After the first update propagates, subsequent updates on + // siblings should still propagate but not double-schedule. + var visualUpdates = 0; + final owner = PipelineOwner() + ..onNeedsVisualUpdate = () { + visualUpdates++; + }; + final parent = _StreamingWidgetHost(); + final cells = [ + for (var i = 0; i < 5; i++) + RenderText(text: '$i', style: const TextStyle()) + ..layout(BoxConstraints.tight(const Size(1, 1))), + ]; + for (final c in cells) { + parent.addChild(c); + } + parent.attach(owner); + parent.layout(BoxConstraints.tight(const Size(5, 1))); + visualUpdates = 0; + + // Update every cell. + for (var i = 0; i < 5; i++) { + cells[i].text = '${i + 10}'; + } + + expect(visualUpdates, greaterThan(0)); + // Parent should be marked for paint but NOT re-laid out + // (text widths didn't change). + expect(parent.needsPaint, isTrue); + expect(parent.needsLayout, isFalse, + reason: 'same-footprint updates should not cascade relayout'); + }); + }); +} + +// ───────────────────────────────────────────────────────────────────── +// Test fixtures +// ───────────────────────────────────────────────────────────────────── + +/// A parent that uses its child's size in its own layout (i.e., +/// `_parentUsesSize = true` is correct). Mirrors RenderColumn / +/// RenderConstrainedBox. +class _SizeUsingParent extends RenderObject + with ContainerRenderObjectMixin { + int layoutCount = 0; + + @override + void performLayout() { + layoutCount++; + if (children.isNotEmpty) { + children.first.layout( + BoxConstraints.tight(const Size(8, 2)), + parentUsesSize: true, + ); + size = constraints.constrain(children.first.size); + } else { + size = constraints.constrain(const Size(40, 5)); + } + } +} + +/// A parent whose size does NOT depend on its child — the canonical +/// "relayout boundary" case. +class _SizeIgnoringParent extends RenderObject + with ContainerRenderObjectMixin { + int layoutCount = 0; + + @override + void performLayout() { + layoutCount++; + if (children.isNotEmpty) { + children.first.layout( + BoxConstraints.tight(const Size(8, 2)), + parentUsesSize: false, // ← the optimization + ); + } + size = constraints.constrain(const Size(40, 5)); + } +} + +/// Wrapper that simulates RenderLayoutBuilder's "always parentUsesSize" +/// policy. Used to verify the chain stays intact when the toolbar's +/// LayoutBuilder rebuilds. +class _LayoutBuilderChildWrapper extends RenderObject + with ContainerRenderObjectMixin { + int layoutCount = 0; + + @override + void performLayout() { + layoutCount++; + if (children.isNotEmpty) { + children.first.layout( + BoxConstraints.tight(const Size(8, 2)), + parentUsesSize: true, // ← the fix + ); + size = constraints.constrain(children.first.size); + } else { + size = constraints.constrain(const Size(40, 5)); + } + } +} + +class _CountingBox extends RenderObject { + int layoutCount = 0; + + @override + void performLayout() { + layoutCount++; + size = constraints.constrain(const Size(1, 1)); + } +} + +/// Mimics RenderLayoutBuilder's hosting parent (e.g. RenderColumn in +/// the chat panel). Passes TIGHT constraints to its child so we can +/// exercise the exact regression scenario where +/// `parentUsesSize: !constraints.isTight` would evaluate to false. +class _TightParentOfLayoutBuilder extends RenderObject + with ContainerRenderObjectMixin { + @override + void performLayout() { + if (children.isNotEmpty) { + children.first.layout( + // tight constraints — this is the case where the bug bit + BoxConstraints.tight(const Size(40, 1)), + parentUsesSize: true, + ); + size = children.first.size; + } + } +} + +/// Mimics the chat toolbar's `Row` of Text cells (used by both +/// ContextBar's BgProgressBar and MetricsDisplay). Children have +/// `parentUsesSize: true` so dirty marks cascade up. +class _StreamingWidgetHost extends RenderObject + with ContainerRenderObjectMixin { + @override + void performLayout() { + for (final child in children) { + child.layout(child.constraints, parentUsesSize: true); + } + if (children.isNotEmpty) { + size = constraints.constrain(children.first.size); + } else { + size = constraints.constrain(const Size(20, 1)); + } + } +} diff --git a/test/framework/render_object_invalidation_optimization_test.dart b/test/framework/render_object_invalidation_optimization_test.dart index a5ecf2be..402706c5 100644 --- a/test/framework/render_object_invalidation_optimization_test.dart +++ b/test/framework/render_object_invalidation_optimization_test.dart @@ -1,5 +1,6 @@ import 'package:nocterm/nocterm.dart' hide TextAlign; import 'package:nocterm/src/components/decorated_box.dart'; +import 'package:nocterm/src/components/render_paragraph.dart'; import 'package:nocterm/src/components/render_text.dart'; import 'package:test/test.dart'; From 654691a4be5889744531e722758d2ff1d92919e8 Mon Sep 17 00:00:00 2001 From: wuhao Date: Wed, 17 Jun 2026 10:09:14 +0800 Subject: [PATCH 3/3] fix(nocterm): keep hit-test wrappers layout-dependent --- lib/src/components/modal_barrier.dart | 4 +- lib/src/components/scrollbar.dart | 2 +- lib/src/rendering/mouse_region.dart | 2 +- ...layout_invalidation_optimization_test.dart | 39 ++++++++++++++++--- 4 files changed, 38 insertions(+), 9 deletions(-) diff --git a/lib/src/components/modal_barrier.dart b/lib/src/components/modal_barrier.dart index f293a2d4..9e75d104 100644 --- a/lib/src/components/modal_barrier.dart +++ b/lib/src/components/modal_barrier.dart @@ -86,7 +86,7 @@ class RenderTint extends RenderObject @override void performLayout() { if (child != null) { - child!.layout(constraints, parentUsesSize: !constraints.isTight); + child!.layout(constraints, parentUsesSize: true); final BoxParentData childParentData = child!.parentData as BoxParentData; childParentData.offset = Offset.zero; size = child!.size; @@ -426,7 +426,7 @@ class RenderColoredBox extends RenderObject @override void performLayout() { if (child != null) { - child!.layout(constraints, parentUsesSize: !constraints.isTight); + child!.layout(constraints, parentUsesSize: true); final BoxParentData childParentData = child!.parentData as BoxParentData; childParentData.offset = Offset.zero; size = child!.size; diff --git a/lib/src/components/scrollbar.dart b/lib/src/components/scrollbar.dart index 2a38978f..fddb19fd 100644 --- a/lib/src/components/scrollbar.dart +++ b/lib/src/components/scrollbar.dart @@ -225,7 +225,7 @@ class RenderScrollbar extends RenderObject maxHeight: constraints.maxHeight, ); - child!.layout(childConstraints, parentUsesSize: !constraints.isTight); + child!.layout(childConstraints, parentUsesSize: true); // Our size includes the scrollbar size = constraints.constrain(Size( diff --git a/lib/src/rendering/mouse_region.dart b/lib/src/rendering/mouse_region.dart index c0a97804..e5800f1e 100644 --- a/lib/src/rendering/mouse_region.dart +++ b/lib/src/rendering/mouse_region.dart @@ -97,7 +97,7 @@ class RenderMouseRegion extends RenderObject @override void performLayout() { if (child != null) { - child!.layout(constraints, parentUsesSize: !constraints.isTight); + child!.layout(constraints, parentUsesSize: true); size = child!.size; } else { size = constraints.constrain(Size.zero); diff --git a/test/framework/layout_invalidation_optimization_test.dart b/test/framework/layout_invalidation_optimization_test.dart index ce7d9384..3bb5ce27 100644 --- a/test/framework/layout_invalidation_optimization_test.dart +++ b/test/framework/layout_invalidation_optimization_test.dart @@ -1,5 +1,6 @@ import 'package:nocterm/nocterm.dart'; import 'package:nocterm/src/components/render_text.dart'; +import 'package:nocterm/src/rendering/mouse_region.dart'; import 'package:test/test.dart'; /// Tests for the layout invalidation optimization introduced in 2450c6d @@ -20,7 +21,9 @@ import 'package:test/test.dart'; /// streaming ContextBar and MetricsDisplay) keeps working correctly. void main() { group('relayout boundaries', () { - test('child with parentUsesSize=false relayouts alone, parent is not relaid out', () { + test( + 'child with parentUsesSize=false relayouts alone, parent is not relaid out', + () { final owner = PipelineOwner(); final parent = _SizeIgnoringParent(); final child = _CountingBox(); @@ -37,8 +40,7 @@ void main() { child.markNeedsLayout(); owner.flushLayout(); - expect(child.layoutCount, 1, - reason: 'child should re-layout by itself'); + expect(child.layoutCount, 1, reason: 'child should re-layout by itself'); expect(parent.layoutCount, 0, reason: 'parent that ignores child size must NOT be ' 're-laid out — it is a relayout boundary'); @@ -168,7 +170,8 @@ void main() { reason: 'style change must trigger repaint'); }); - test('text change propagates paint to root even across relayout boundary', () { + test('text change propagates paint to root even across relayout boundary', + () { // Even when text is inside a relayout boundary // (parentUsesSize=false), paint invalidation must reach the root, // otherwise the change is invisible. The streaming ContextBar @@ -271,7 +274,9 @@ void main() { 'streaming widgets (ContextBar, MetricsDisplay) tick'); }); - test('RenderLayoutBuilder child dirty marks propagate up under TIGHT constraints', () { + test( + 'RenderLayoutBuilder child dirty marks propagate up under TIGHT constraints', + () { // This is the regression test for the exact bug we hit in the // chat toolbar: under tight constraints (which is what the chat // panel Column passes when the screen is fully sized), the @@ -299,6 +304,30 @@ void main() { }); }); + group('mouse hit-test wrappers', () { + test('RenderMouseRegion cascades child dirty marks under tight constraints', + () { + // RenderMouseRegion uses child.size as its own size. If its child + // changes layout after a terminal resize, the mouse region must relayout + // too so hover/click hit-test bounds stay in sync. + final owner = PipelineOwner(); + final mouseRegion = RenderMouseRegion(onHover: (_) {}); + final child = _CountingBox(); + mouseRegion.child = child; + mouseRegion.attach(owner); + + mouseRegion.layout(BoxConstraints.tight(const Size(20, 3))); + expect(mouseRegion.needsLayout, isFalse); + + child.markNeedsLayout(); + + expect(mouseRegion.needsLayout, isTrue, + reason: 'MouseRegion size and hit-test bounds come from child.size, ' + 'so child layout dirtiness must cascade even when constraints ' + 'are tight.'); + }); + }); + group('streaming widget repaint path', () { // Both ContextBar and MetricsDisplay follow the same pattern: // StatefulComponent