From 75fc459dbd6a60a3f9b7b31ca031088e13dba210 Mon Sep 17 00:00:00 2001 From: Andrey Kolkov Date: Sun, 5 Jul 2026 13:55:50 +0300 Subject: [PATCH] feat(widget): animation tick before layout (ADR-032 GAP-3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Flutter pattern: handleBeginFrame (animate) → handleDrawFrame (layout). Layout must be a pure function of (constraints + widget state) for RelayoutBoundary correctness (Phase 5). Changes: - Add AnimationTicker interface (widget/animation_ticker.go) - Add tickAnimationsInTree walk in Window.Frame() after BeginFrame - Collapsible: TickAnimation() called by framework, Layout() read-only - Transition: TickAnimation() called by framework, Draw() read-only - Fade/Slide: paint-only, keep updateAnimation in Draw, remove ctx.InvalidateRect - Replace ctx.Invalidate() with MarkNeedsLayout() in animation ticks - Update all animation tests to call TickAnimation before Layout/Draw - Update RENDER-PIPELINE.md with Frame() lifecycle (ADR-032 GAP-3) - Update CHANGELOG --- CHANGELOG.md | 1 + app/window.go | 23 +++++++++++++++++++++++ core/collapsible/collapsible.go | 16 ++++++---------- core/collapsible/collapsible_test.go | 12 ++++++++++++ core/collapsible/internal_test.go | 2 +- docs/RENDER-PIPELINE.md | 8 ++++++-- transition/fade.go | 2 -- transition/slide.go | 2 -- transition/transition.go | 25 ++++++++++++++----------- transition/transition_test.go | 23 +++++++++++++++++++---- widget/animation_ticker.go | 16 ++++++++++++++++ 11 files changed, 98 insertions(+), 32 deletions(-) create mode 100644 widget/animation_ticker.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 741b8fb..22160b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - **Layout cache activation** (ADR-032 Phase 2b, [#160](https://github.com/gogpu/ui/pull/160), @TimLai666) — convert 32 parent→child Layout calls to `widget.LayoutChild` across 23 files, activating per-widget layout caching. Layout cost: O(total nodes) → O(affected subtree). Removes `MarkLayoutCleanRecursive` shim. Adds `InvalidateLayoutTree` for downward cache propagation on signal fires. `IsLayoutVerifying` sentinel for debug verifier (Flutter `debugCheckingIntrinsics` pattern). +- **Animation tick before layout** (ADR-032 GAP-3) — `AnimationTicker` interface + `tickAnimationsInTree` walk before layout pass. Collapsible and Transition animations now tick BEFORE layout (Flutter `handleBeginFrame` → `handleDrawFrame` pattern). Layout is a pure function of constraints + widget state, required for RelayoutBoundary (Phase 5). ### Fixed diff --git a/app/window.go b/app/window.go index 994bcb6..2cdf106 100644 --- a/app/window.go +++ b/app/window.go @@ -552,6 +552,13 @@ func (w *Window) Frame() { } } + // ADR-032 GAP-3: Tick layout-affecting animations BEFORE layout. + // Flutter pattern: handleBeginFrame (Animate) → handleDrawFrame (Layout). + // Layout must be a pure function of (constraints + widget state) for + // RelayoutBoundary correctness (Phase 5). Animation ticks mutate state + // (e.g., Collapsible.progress) so they must complete before layout reads it. + tickAnimationsInTree(w.root, w.ctx) + // Update scale factor (may change between frames on multi-monitor setups). w.updateScale() @@ -771,6 +778,22 @@ func (w *Window) syncContextFocusToManager() { } } +// tickAnimationsInTree walks the widget tree and calls TickAnimation on +// widgets that implement [widget.AnimationTicker]. Called BEFORE layout +// so that animation state is final when Layout reads it (Flutter pattern: +// handleBeginFrame → Animate → handleDrawFrame → Layout). +func tickAnimationsInTree(w widget.Widget, ctx widget.Context) { + if w == nil { + return + } + if ticker, ok := w.(widget.AnimationTicker); ok { + ticker.TickAnimation(ctx) + } + for _, child := range w.Children() { + tickAnimationsInTree(child, ctx) + } +} + // layout performs the layout pass on the widget tree and overlays. func (w *Window) layout() { if w.root == nil { diff --git a/core/collapsible/collapsible.go b/core/collapsible/collapsible.go index 4857b8f..175eb06 100644 --- a/core/collapsible/collapsible.go +++ b/core/collapsible/collapsible.go @@ -146,9 +146,6 @@ func (w *Widget) IsAnimating() bool { // - Expanded: headerHeight + content height // - Animating: headerHeight + content height * progress func (w *Widget) Layout(ctx widget.Context, constraints geometry.Constraints) geometry.Size { - // Tick animation if active. - w.tickAnimation(ctx) - headerH := w.cfg.headerHeight // Layout content to determine its natural height. @@ -359,8 +356,10 @@ func (w *Widget) startAnimation(expanding bool) { Start(w.animCtrl) } -// tickAnimation advances animation by delta time from context. -func (w *Widget) tickAnimation(ctx widget.Context) { +// TickAnimation advances the expand/collapse animation by delta time. +// Called by the framework BEFORE layout (ADR-032 GAP-3, Flutter pattern). +// Layout reads w.progress without mutation. +func (w *Widget) TickAnimation(ctx widget.Context) { if w.animCtrl == nil || !w.animCtrl.HasActive() { return } @@ -375,12 +374,9 @@ func (w *Widget) tickAnimation(ctx widget.Context) { wasActive := w.animCtrl.HasActive() w.animCtrl.Tick(dt) - if w.animCtrl.HasActive() { - w.SetNeedsRedraw(true) - ctx.Invalidate() - } else if wasActive { + if w.animCtrl.HasActive() || wasActive { + w.MarkNeedsLayout() w.SetNeedsRedraw(true) - ctx.Invalidate() } } diff --git a/core/collapsible/collapsible_test.go b/core/collapsible/collapsible_test.go index 5304b24..0dedba9 100644 --- a/core/collapsible/collapsible_test.go +++ b/core/collapsible/collapsible_test.go @@ -365,6 +365,7 @@ func TestLayout_Collapsed(t *testing.T) { ctx := widget.NewContext() constraints := geometry.Loose(geometry.Sz(300, 500)) + w.TickAnimation(ctx) size := w.Layout(ctx, constraints) if size.Height != 40 { @@ -382,6 +383,7 @@ func TestLayout_Expanded(t *testing.T) { ctx := widget.NewContext() constraints := geometry.Loose(geometry.Sz(300, 500)) + w.TickAnimation(ctx) size := w.Layout(ctx, constraints) if size.Height != 140 { @@ -398,6 +400,7 @@ func TestLayout_NoContent(t *testing.T) { ctx := widget.NewContext() constraints := geometry.Loose(geometry.Sz(300, 500)) + w.TickAnimation(ctx) size := w.Layout(ctx, constraints) if size.Height != 36 { @@ -410,6 +413,7 @@ func TestLayout_UsesMaxWidth(t *testing.T) { ctx := widget.NewContext() constraints := geometry.Loose(geometry.Sz(400, 500)) + w.TickAnimation(ctx) size := w.Layout(ctx, constraints) if size.Width != 400 { @@ -537,6 +541,7 @@ func TestAnimation_Expand(t *testing.T) { // Advance 16ms ticks until animation completes (simulates ~60fps). ctx.BeginFrame(ctx.Now().Add(16 * time.Millisecond)) + w.TickAnimation(ctx) w.Layout(ctx, constraints) if w.Progress() <= 0.0 || w.Progress() >= 1.0 { @@ -546,6 +551,7 @@ func TestAnimation_Expand(t *testing.T) { // Run remaining ticks until animation is done. for i := 0; i < 20 && w.IsAnimating(); i++ { ctx.BeginFrame(ctx.Now().Add(16 * time.Millisecond)) + w.TickAnimation(ctx) w.Layout(ctx, constraints) } @@ -573,6 +579,7 @@ func TestAnimation_Collapse(t *testing.T) { // Run ticks until animation completes. for i := 0; i < 20 && w.IsAnimating(); i++ { ctx.BeginFrame(ctx.Now().Add(16 * time.Millisecond)) + w.TickAnimation(ctx) w.Layout(ctx, constraints) } @@ -753,6 +760,7 @@ func TestAnimation_ProgressAdapter_InvalidatesScene(t *testing.T) { ctx := widget.NewContext() constraints := geometry.Loose(geometry.Sz(200, 500)) ctx.BeginFrame(ctx.Now().Add(16 * time.Millisecond)) + w.TickAnimation(ctx) w.Layout(ctx, constraints) // progressAdapter.Set should have called InvalidateScene. @@ -940,6 +948,7 @@ func TestAnimation_ProgressesWithoutMouseEvents(t *testing.T) { for range 10 { now = now.Add(16 * time.Millisecond) ctx.BeginFrame(now) + w.TickAnimation(ctx) w.Layout(ctx, constraints) progresses = append(progresses, w.Progress()) } @@ -979,6 +988,7 @@ func TestAnimation_DeltaTimeClamping_MinimumOneMilli(t *testing.T) { now := ctx.Now() ctx.BeginFrame(now) ctx.BeginFrame(now) // dt=0 + w.TickAnimation(ctx) w.Layout(ctx, constraints) if w.Progress() <= initialProgress { @@ -1007,6 +1017,7 @@ func TestAnimation_DeltaTimeClamping_MaximumThirtyTwoMilli(t *testing.T) { ctx.BeginFrame(now) bigDelta := now.Add(100 * time.Millisecond) ctx.BeginFrame(bigDelta) + w.TickAnimation(ctx) w.Layout(ctx, constraints) // With 200ms duration and 32ms max tick, progress should be at most ~16%. @@ -1239,6 +1250,7 @@ func TestTitleSignal_ResolvesTitle(t *testing.T) { // Signal > Fn > Static ctx := widget.NewContext() constraints := geometry.Tight(geometry.Sz(400, 40)) + w.TickAnimation(ctx) w.Layout(ctx, constraints) w.SetBounds(geometry.NewRect(0, 0, 400, 40)) diff --git a/core/collapsible/internal_test.go b/core/collapsible/internal_test.go index cc303c1..8d1375f 100644 --- a/core/collapsible/internal_test.go +++ b/core/collapsible/internal_test.go @@ -546,7 +546,7 @@ func TestTickAnimation_NilCtrl(t *testing.T) { w.animCtrl = nil ctx := widget.NewContext() // Should not panic. - w.tickAnimation(ctx) + w.TickAnimation(ctx) } // --- Draw Content With Non-Settable Bounds Widget --- diff --git a/docs/RENDER-PIPELINE.md b/docs/RENDER-PIPELINE.md index a3a800e..1fe8c99 100644 --- a/docs/RENDER-PIPELINE.md +++ b/docs/RENDER-PIPELINE.md @@ -42,13 +42,17 @@ Each frame executes these steps in order: ### Step 1: Frame Setup ``` -Frame() // flush signals, layout, animations +Frame() // signals, animation ticks, layout (see below) BeginAcceleratorFrame() // reset GPU frame state BeginGPUFrame() // prepare gg render context ResetFrameDamage() // clear damage tracking ``` -`Frame()` runs the signal scheduler (up to 2 re-flushes for cascading changes), layout pass if needed, and animation tick. +`Frame()` runs in Flutter-correct order (ADR-032 GAP-3): +1. **BeginFrame** — set DeltaTime +2. **Signal flush** — process pending signal changes (up to 2 re-flushes) +3. **Animation tick** — `tickAnimationsInTree()` walks the tree, calls `AnimationTicker.TickAnimation()` on widgets with layout-affecting animations (Collapsible, Transition). All animation values are final BEFORE layout runs. +4. **Layout** — pure function of (constraints + widget state). No mutation. ### Step 2: Root Invalidation diff --git a/transition/fade.go b/transition/fade.go index 557d2e2..ad7cc11 100644 --- a/transition/fade.go +++ b/transition/fade.go @@ -252,9 +252,7 @@ func (f *Fade) updateAnimation(ctx widget.Context) { f.progress = 1.0 f.animating = false } else { - // Request another frame while animating. f.SetNeedsRedraw(true) - ctx.InvalidateRect(f.Bounds()) } // Compute opacity from eased progress. diff --git a/transition/slide.go b/transition/slide.go index 68f2734..f7ff059 100644 --- a/transition/slide.go +++ b/transition/slide.go @@ -244,9 +244,7 @@ func (s *Slide) updateAnimation(ctx widget.Context) { s.progress = 1.0 s.animating = false } else { - // Request another frame while animating. s.SetNeedsRedraw(true) - ctx.InvalidateRect(s.Bounds()) } } diff --git a/transition/transition.go b/transition/transition.go index 0038275..070a78d 100644 --- a/transition/transition.go +++ b/transition/transition.go @@ -207,10 +207,8 @@ func (t *Transition) Draw(ctx widget.Context, canvas widget.Canvas) { return } - // Update animation progress. - if t.animating { - t.updateAnimation(ctx) - } + // Animation progress is updated by TickAnimation (called before layout). + // Draw only reads the current progress — no mutation here. // Determine the active effect and eased progress. eff, easedProgress := t.currentEffect() @@ -219,11 +217,16 @@ func (t *Transition) Draw(ctx widget.Context, canvas widget.Canvas) { t.drawWithEffects(ctx, canvas, eff, easedProgress) } -// updateAnimation advances the animation based on elapsed time. -func (t *Transition) updateAnimation(ctx widget.Context) { +// TickAnimation advances the transition animation based on elapsed time. +// Called by the framework BEFORE layout (ADR-032 GAP-3, Flutter pattern). +// Draw reads t.progress without mutation. +func (t *Transition) TickAnimation(ctx widget.Context) { + if !t.animating { + return + } + now := ctx.Now() - // Initialize start time on first tick. if t.startTime.IsZero() { t.startTime = now } @@ -239,13 +242,13 @@ func (t *Transition) updateAnimation(ctx widget.Context) { t.progress = 1.0 t.animating = false if !t.entering { - t.shown = false // exit complete: hide + t.shown = false } + t.MarkNeedsLayout() + t.SetNeedsRedraw(true) } else { - // Request another frame while animating. - // ADR-028: layout-dependent — animation tick may change widget size. + t.MarkNeedsLayout() t.SetNeedsRedraw(true) - ctx.Invalidate() } } diff --git a/transition/transition_test.go b/transition/transition_test.go index fc05390..f1614ea 100644 --- a/transition/transition_test.go +++ b/transition/transition_test.go @@ -304,6 +304,7 @@ func TestWrapNilChild(t *testing.T) { ctx := newMockContext(time.Now()) canvas := &mockCanvas{} _ = tr.Layout(ctx, geometry.Constraints{MaxWidth: 100, MaxHeight: 100}) + tr.TickAnimation(ctx) tr.Draw(ctx, canvas) if tr.Event(ctx, &event.MouseEvent{}) { t.Error("Event should return false for nil child") @@ -428,6 +429,7 @@ func TestDrawHiddenDoesNothing(t *testing.T) { ctx := newMockContext(time.Now()) canvas := &mockCanvas{} + tr.TickAnimation(ctx) tr.Draw(ctx, canvas) if child.drawn != 0 { @@ -441,6 +443,7 @@ func TestDrawVisibleDrawsChild(t *testing.T) { ctx := newMockContext(time.Now()) canvas := &mockCanvas{} + tr.TickAnimation(ctx) tr.Draw(ctx, canvas) if child.drawn != 1 { @@ -462,6 +465,7 @@ func TestDrawFadeEnterWithOpacityCanvas(t *testing.T) { tr.Show() // First draw: initializes start time, progress = 0. + tr.TickAnimation(ctx) tr.Draw(ctx, canvas) if child.drawn != 1 { t.Errorf("draw count: got %d, want 1", child.drawn) @@ -469,6 +473,7 @@ func TestDrawFadeEnterWithOpacityCanvas(t *testing.T) { // Advance halfway. ctx.now = now.Add(100 * time.Millisecond) + tr.TickAnimation(ctx) tr.Draw(ctx, canvas) if child.drawn != 2 { t.Errorf("draw count: got %d, want 2", child.drawn) @@ -479,6 +484,7 @@ func TestDrawFadeEnterWithOpacityCanvas(t *testing.T) { // Advance past duration. ctx.now = now.Add(300 * time.Millisecond) + tr.TickAnimation(ctx) tr.Draw(ctx, canvas) if tr.IsAnimating() { t.Error("should stop animating after duration") @@ -501,10 +507,12 @@ func TestDrawFadeExitHidesAfterComplete(t *testing.T) { // Still "shown" during exit animation; we check after draw completes below. // First draw. + tr.TickAnimation(ctx) tr.Draw(ctx, canvas) // Advance past duration. ctx.now = now.Add(200 * time.Millisecond) + tr.TickAnimation(ctx) tr.Draw(ctx, canvas) if tr.IsAnimating() { t.Error("should stop animating after exit duration") @@ -515,6 +523,7 @@ func TestDrawFadeExitHidesAfterComplete(t *testing.T) { // Further draws should not draw child. child.drawn = 0 + tr.TickAnimation(ctx) tr.Draw(ctx, canvas) if child.drawn != 0 { t.Error("hidden transition should not draw child") @@ -538,6 +547,7 @@ func TestDrawSlideEnter(t *testing.T) { tr.Show() // Draw at t=0: full offset. + tr.TickAnimation(ctx) tr.Draw(ctx, canvas) if child.drawn != 1 { t.Errorf("draw count: got %d, want 1", child.drawn) @@ -562,6 +572,7 @@ func TestDrawScaleEnter(t *testing.T) { tr.Layout(ctx, geometry.Constraints{MaxWidth: 200, MaxHeight: 200}) tr.Show() + tr.TickAnimation(ctx) tr.Draw(ctx, canvas) if child.drawn != 1 { @@ -580,6 +591,7 @@ func TestDrawWithCanvasWithoutOpacity(t *testing.T) { canvas := &mockCanvas{} // no OpacityPusher tr.Show() + tr.TickAnimation(ctx) tr.Draw(ctx, canvas) // Should still draw child even without opacity support. @@ -654,14 +666,16 @@ func TestAnimationInvalidatesOnProgress(t *testing.T) { canvas := &mockCanvas{} tr.Show() + tr.TickAnimation(ctx) tr.Draw(ctx, canvas) - // Mid-animation should have invalidated. + // Mid-animation: TickAnimation should mark widget for redraw. ctx.now = now.Add(50 * time.Millisecond) - ctx.invalidated = false + tr.SetNeedsRedraw(false) + tr.TickAnimation(ctx) tr.Draw(ctx, canvas) - if !ctx.invalidated { - t.Error("should invalidate during animation") + if !tr.NeedsRedraw() { + t.Error("should set NeedsRedraw during animation") } } @@ -677,6 +691,7 @@ func TestAnimationSetsNeedsRedraw(t *testing.T) { tr.Show() tr.SetNeedsRedraw(false) + tr.TickAnimation(ctx) tr.Draw(ctx, canvas) // After first draw (still animating), should set needs redraw. diff --git a/widget/animation_ticker.go b/widget/animation_ticker.go new file mode 100644 index 0000000..ef402b0 --- /dev/null +++ b/widget/animation_ticker.go @@ -0,0 +1,16 @@ +package widget + +// AnimationTicker is implemented by widgets whose animations affect layout +// (e.g., Collapsible height, Transition size). The framework calls +// TickAnimation on every frame BEFORE the layout pass, following the +// Flutter pattern: handleBeginFrame (animate) → handleDrawFrame (layout). +// +// Layout-affecting animations must tick here, not inside Layout() or Draw(), +// so that Layout remains a pure function of (constraints + widget state). +// This invariant is required for RelayoutBoundary (ADR-032 Phase 5). +// +// Paint-only animations (spinner rotation, cursor blink) do NOT implement +// this interface — they use ScheduleAnimationFrame + SetNeedsRedraw instead. +type AnimationTicker interface { + TickAnimation(ctx Context) +}