fix(renderer): synchronize floating pane composition

commit: 0ae8e2a5cd28eb1064ae74b5faa90ea855f5fb59
author: brice · 3mo ago · 2026-07-10 06:10 UTC
parent: 250429c
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

2 files · +41 -7

M internal/usecase/daemon/floating_render.go +10 -7


@@ -62,18 +62,25 @@ func composeFloatingFrame(base renderer.Frame, baseDamage []renderer.Damage, p *
62	62		(overlayBackdrop{DimPaneContents: true}).apply(frame, content, layoutSnap, theme)
63	63		geometry := calculateFloatingGeometry(content, cfg)
64	64
	65	+	popupChanged := cache == nil \|\| cache.floating != p \|\| cache.floatingGeneration != generation
	66	+
	67	+	// The PTY reader mutates both the frame and its damage under p.mu. Keep
	68	+	// every read of them, including the blit, in the same critical section.
	69	+	// In particular, Frame is not safe to retain as an alias after unlocking.
65	70		p.mu.Lock()
66	71		title := p.displayTitleLocked()
67	72		titleGeneration := p.title.generation
68		-	screen := p.screen.Frame
69	73		screenDamage := p.screen.Damage()
	74	+	blitPaneFrame(frame, geometry.Inner, p.screen.Frame, false, theme)
	75	+	damage := append([]renderer.Damage(nil), baseDamage...)
	76	+	for _, d := range screenDamage {
	77	+	damage = append(damage, translatePaneDamage(d, geometry.Inner, content)...)
	78	+	}
70	79		p.screen.ClearDamage()
71	80		p.mu.Unlock()
72	81
73		-	popupChanged := cache == nil \|\| cache.floating != p \|\| cache.floatingGeneration != generation
74	82		titleChanged := popupChanged \|\| cache == nil \|\| cache.floatingTitleGeneration != titleGeneration
75	83		drawFloatingBorder(frame, geometry.Bounds, title, newThemeStyles(theme).border)
76		-	blitPaneFrame(frame, geometry.Inner, screen, false, theme)
77	84		if cache != nil {
78	85		cache.floating = p
79	86		cache.floatingGeneration = generation

@@ -82,10 +89,6 @@ func composeFloatingFrame(base renderer.Frame, baseDamage []renderer.Damage, p *
82	89		if full \|\| popupChanged {
83	90		return frame, []renderer.Damage{renderer.FullRedraw()}
84	91		}
85		-	damage := append([]renderer.Damage(nil), baseDamage...)
86		-	for _, d := range screenDamage {
87		-	damage = append(damage, translatePaneDamage(d, geometry.Inner, content)...)
88		-	}
89	92		if titleChanged && geometry.Bounds.Height >= 3 {
90	93		damage = append(damage, renderer.Damage{Kind: renderer.DamageText, X: geometry.Bounds.X, Y: geometry.Bounds.Y, Width: geometry.Bounds.Width, Height: 1})
91	94		}

 	(overlayBackdrop{DimPaneContents: true}).apply(frame, content, layoutSnap, theme)
 	geometry := calculateFloatingGeometry(content, cfg)
+	popupChanged := cache == nil || cache.floating != p || cache.floatingGeneration != generation
+	// The PTY reader mutates both the frame and its damage under p.mu. Keep
+	// every read of them, including the blit, in the same critical section.
+	// In particular, Frame is not safe to retain as an alias after unlocking.
 	p.mu.Lock()
 	title := p.displayTitleLocked()
 	titleGeneration := p.title.generation
-	screen := p.screen.Frame
 	screenDamage := p.screen.Damage()
+	blitPaneFrame(frame, geometry.Inner, p.screen.Frame, false, theme)
+	damage := append([]renderer.Damage(nil), baseDamage...)
+	for _, d := range screenDamage {
+		damage = append(damage, translatePaneDamage(d, geometry.Inner, content)...)
+	}
 	p.screen.ClearDamage()
 	p.mu.Unlock()
-	popupChanged := cache == nil || cache.floating != p || cache.floatingGeneration != generation
 	titleChanged := popupChanged || cache == nil || cache.floatingTitleGeneration != titleGeneration
 	drawFloatingBorder(frame, geometry.Bounds, title, newThemeStyles(theme).border)
-	blitPaneFrame(frame, geometry.Inner, screen, false, theme)
 	if cache != nil {
 		cache.floating = p
 		cache.floatingGeneration = generation
 	if full || popupChanged {
 		return frame, []renderer.Damage{renderer.FullRedraw()}
 	}
-	damage := append([]renderer.Damage(nil), baseDamage...)
-	for _, d := range screenDamage {
-		damage = append(damage, translatePaneDamage(d, geometry.Inner, content)...)
-	}
 	if titleChanged && geometry.Bounds.Height >= 3 {
 		damage = append(damage, renderer.Damage{Kind: renderer.DamageText, X: geometry.Bounds.X, Y: geometry.Bounds.Y, Width: geometry.Bounds.Width, Height: 1})
 	}

M internal/usecase/daemon/floating_render_test.go +31 -0


@@ -63,6 +63,37 @@ func TestComposeFloatingFrameDoesNotMutateSourceAndDamagesTitle(t *testing.T) {
63	63		require.Equal(t, 1, damage[0].Height)
64	64		}
65	65
	66	+	func TestComposeFloatingFrameSynchronizesWithPTYReader(t *testing.T) {
	67	+	p := newPane("floating", nil, domain.Size{Cols: 80, Rows: 24})
	68	+	base := renderer.NewFrame(80, 24)
	69	+	content := domain.Rect{Width: 80, Height: 24}
	70	+	cfg := domain.FloatingConfig{Width: 100, Height: 100}
	71	+
	72	+	start := make(chan struct{})
	73	+	var wg sync.WaitGroup
	74	+	wg.Add(2)
	75	+	go func() {
	76	+	defer wg.Done()
	77	+	<-start
	78	+	for range 500 {
	79	+	// This is the same pane lock used by ptyReader around Screen.Write.
	80	+	p.mu.Lock()
	81	+	p.screen.Write([]byte("\x1b[Hreader"))
	82	+	p.mu.Unlock()
	83	+	}
	84	+	}()
	85	+	go func() {
	86	+	defer wg.Done()
	87	+	<-start
	88	+	cache := &composedFrameCache{}
	89	+	for range 500 {
	90	+	composeFloatingFrame(base, nil, p, 1, content, cfg, tabLayoutSnapshot{}, themeui.Theme{}, cache, false)
	91	+	}
	92	+	}()
	93	+	close(start)
	94	+	wg.Wait()
	95	+	}
	96	+
66	97		func TestDrawFloatingBorderOmitsTinyAxes(t *testing.T) {
67	98		frame := renderer.NewFrame(2, 1)
68	99		drawFloatingBorder(frame, domain.Rect{Width: 2, Height: 1}, "title", renderer.Style{})


@@ -63,6 +63,37 @@ func TestComposeFloatingFrameDoesNotMutateSourceAndDamagesTitle(t *testing.T) {
63	require.Equal(t, 1, damage[0].Height)	63		require.Equal(t, 1, damage[0].Height)
64	}	64		}
65		65
		66	+	func TestComposeFloatingFrameSynchronizesWithPTYReader(t *testing.T) {
		67	+	p := newPane("floating", nil, domain.Size{Cols: 80, Rows: 24})
		68	+	base := renderer.NewFrame(80, 24)
		69	+	content := domain.Rect{Width: 80, Height: 24}
		70	+	cfg := domain.FloatingConfig{Width: 100, Height: 100}
		71	+
		72	+	start := make(chan struct{})
		73	+	var wg sync.WaitGroup
		74	+	wg.Add(2)
		75	+	go func() {
		76	+	defer wg.Done()
		77	+	<-start
		78	+	for range 500 {
		79	+	// This is the same pane lock used by ptyReader around Screen.Write.
		80	+	p.mu.Lock()
		81	+	p.screen.Write([]byte("\x1b[Hreader"))
		82	+	p.mu.Unlock()
		83	+	}
		84	+	}()
		85	+	go func() {
		86	+	defer wg.Done()
		87	+	<-start
		88	+	cache := &composedFrameCache{}
		89	+	for range 500 {
		90	+	composeFloatingFrame(base, nil, p, 1, content, cfg, tabLayoutSnapshot{}, themeui.Theme{}, cache, false)
		91	+	}
		92	+	}()
		93	+	close(start)
		94	+	wg.Wait()
		95	+	}
		96	+
66	func TestDrawFloatingBorderOmitsTinyAxes(t *testing.T) {	97		func TestDrawFloatingBorderOmitsTinyAxes(t *testing.T) {
67	frame := renderer.NewFrame(2, 1)	98		frame := renderer.NewFrame(2, 1)
68	drawFloatingBorder(frame, domain.Rect{Width: 2, Height: 1}, "title", renderer.Style{})	99		drawFloatingBorder(frame, domain.Rect{Width: 2, Height: 1}, "title", renderer.Style{})