fix(daemon): align floating popup coordinates
- commit
- 37d52d38026dd460f84351f0b5fac82bd72739cc
- author
- brice · · 2026-07-11 05:17 UTC
- parent
- 4005fb8
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +67 -41
M internal/usecase/daemon/floating_render.go +6 -6
| @@ -86,7 +86,7 @@ func calculateFloatingGeometry(content domain.Rect, cfg domain.FloatingConfig) f | |||
| 86 | 86 | // composeFloatingFrame applies the popup to a copy of the normal composed | |
| 87 | 87 | // destination. In particular it never writes the VT screen frame: that frame | |
| 88 | 88 | // is shared with the PTY reader and is the source for future renders. | |
| 89 | - | func composeFloatingFrame(base renderer.Frame, baseDamage []renderer.Damage, p *pane, generation uint64, content domain.Rect, cfg domain.FloatingConfig, layoutSnap tabLayoutSnapshot, theme themeui.Theme, cache *composedFrameCache, full bool) (renderer.Frame, []renderer.Damage) { | |
| 89 | + | func composeFloatingFrame(base renderer.Frame, baseDamage []renderer.Damage, p *pane, generation uint64, content domain.Rect, desired floatingGeometry, layoutSnap tabLayoutSnapshot, theme themeui.Theme, cache *composedFrameCache, full bool) (renderer.Frame, []renderer.Damage, floatingGeometry) { | |
| 90 | 90 | var frame renderer.Frame | |
| 91 | 91 | if cache != nil && cache.floatingFrame.Width == base.Width && cache.floatingFrame.Height == base.Height { | |
| 92 | 92 | frame = cache.floatingFrame | |
| @@ -100,13 +100,13 @@ func composeFloatingFrame(base renderer.Frame, baseDamage []renderer.Damage, p * | |||
| 100 | 100 | } | |
| 101 | 101 | } | |
| 102 | 102 | (overlayBackdrop{DimPaneContents: true}).apply(frame, content, layoutSnap, theme) | |
| 103 | - | desiredGeometry := calculateFloatingGeometry(content, cfg) | |
| 104 | - | ||
| 105 | 103 | // The PTY reader mutates both the frame and its damage under p.mu. Keep | |
| 106 | 104 | // every read of them, including the blit, in the same critical section. | |
| 107 | 105 | // In particular, Frame is not safe to retain as an alias after unlocking. | |
| 108 | 106 | p.mu.Lock() | |
| 109 | - | geometry := p.committedFloatingGeometryLocked(desiredGeometry) | |
| 107 | + | // Committed popup geometry is content-relative; translate it exactly once | |
| 108 | + | // while holding the same lock that protects the committed state and frame. | |
| 109 | + | geometry := p.committedFloatingGeometryLocked(desired).translate(content.X, content.Y) | |
| 110 | 110 | title := p.displayTitleLocked() | |
| 111 | 111 | titleGeneration := p.title.generation | |
| 112 | 112 | screenDamage := p.screen.Damage() | |
| @@ -131,12 +131,12 @@ func composeFloatingFrame(base renderer.Frame, baseDamage []renderer.Damage, p * | |||
| 131 | 131 | cache.floatingTitleGeneration = titleGeneration | |
| 132 | 132 | } | |
| 133 | 133 | if full || popupChanged { | |
| 134 | - | return frame, []renderer.Damage{renderer.FullRedraw()} | |
| 134 | + | return frame, []renderer.Damage{renderer.FullRedraw()}, geometry | |
| 135 | 135 | } | |
| 136 | 136 | if titleChanged && geometry.Bounds.Height >= 3 { | |
| 137 | 137 | damage = append(damage, renderer.Damage{Kind: renderer.DamageText, X: geometry.Bounds.X, Y: geometry.Bounds.Y, Width: geometry.Bounds.Width, Height: 1}) | |
| 138 | 138 | } | |
| 139 | - | return frame, damage | |
| 139 | + | return frame, damage, geometry | |
| 140 | 140 | } | |
| 141 | 141 | ||
| 142 | 142 | func drawFloatingBorder(frame renderer.Frame, bounds domain.Rect, title string, style renderer.Style) { | |
| @@ -86,7 +86,7 @@ func calculateFloatingGeometry(content domain.Rect, cfg domain.FloatingConfig) f | |||||
| 86 | // composeFloatingFrame applies the popup to a copy of the normal composed | 86 | // composeFloatingFrame applies the popup to a copy of the normal composed | ||
| 87 | // destination. In particular it never writes the VT screen frame: that frame | 87 | // destination. In particular it never writes the VT screen frame: that frame | ||
| 88 | // is shared with the PTY reader and is the source for future renders. | 88 | // is shared with the PTY reader and is the source for future renders. | ||
| 89 | - | func composeFloatingFrame(base renderer.Frame, baseDamage []renderer.Damage, p *pane, generation uint64, content domain.Rect, cfg domain.FloatingConfig, layoutSnap tabLayoutSnapshot, theme themeui.Theme, cache *composedFrameCache, full bool) (renderer.Frame, []renderer.Damage) { | 89 | + | func composeFloatingFrame(base renderer.Frame, baseDamage []renderer.Damage, p *pane, generation uint64, content domain.Rect, desired floatingGeometry, layoutSnap tabLayoutSnapshot, theme themeui.Theme, cache *composedFrameCache, full bool) (renderer.Frame, []renderer.Damage, floatingGeometry) { |
| 90 | var frame renderer.Frame | 90 | var frame renderer.Frame | ||
| 91 | if cache != nil && cache.floatingFrame.Width == base.Width && cache.floatingFrame.Height == base.Height { | 91 | if cache != nil && cache.floatingFrame.Width == base.Width && cache.floatingFrame.Height == base.Height { | ||
| 92 | frame = cache.floatingFrame | 92 | frame = cache.floatingFrame | ||
| @@ -100,13 +100,13 @@ func composeFloatingFrame(base renderer.Frame, baseDamage []renderer.Damage, p * | |||||
| 100 | } | 100 | } | ||
| 101 | } | 101 | } | ||
| 102 | (overlayBackdrop{DimPaneContents: true}).apply(frame, content, layoutSnap, theme) | 102 | (overlayBackdrop{DimPaneContents: true}).apply(frame, content, layoutSnap, theme) | ||
| 103 | - | desiredGeometry := calculateFloatingGeometry(content, cfg) | |||
| 104 | - | ||||
| 105 | // The PTY reader mutates both the frame and its damage under p.mu. Keep | 103 | // The PTY reader mutates both the frame and its damage under p.mu. Keep | ||
| 106 | // every read of them, including the blit, in the same critical section. | 104 | // every read of them, including the blit, in the same critical section. | ||
| 107 | // In particular, Frame is not safe to retain as an alias after unlocking. | 105 | // In particular, Frame is not safe to retain as an alias after unlocking. | ||
| 108 | p.mu.Lock() | 106 | p.mu.Lock() | ||
| 109 | - | geometry := p.committedFloatingGeometryLocked(desiredGeometry) | 107 | + | // Committed popup geometry is content-relative; translate it exactly once |
| 108 | + | // while holding the same lock that protects the committed state and frame. | |||
| 109 | + | geometry := p.committedFloatingGeometryLocked(desired).translate(content.X, content.Y) | |||
| 110 | title := p.displayTitleLocked() | 110 | title := p.displayTitleLocked() | ||
| 111 | titleGeneration := p.title.generation | 111 | titleGeneration := p.title.generation | ||
| 112 | screenDamage := p.screen.Damage() | 112 | screenDamage := p.screen.Damage() | ||
| @@ -131,12 +131,12 @@ func composeFloatingFrame(base renderer.Frame, baseDamage []renderer.Damage, p * | |||||
| 131 | cache.floatingTitleGeneration = titleGeneration | 131 | cache.floatingTitleGeneration = titleGeneration | ||
| 132 | } | 132 | } | ||
| 133 | if full || popupChanged { | 133 | if full || popupChanged { | ||
| 134 | - | return frame, []renderer.Damage{renderer.FullRedraw()} | 134 | + | return frame, []renderer.Damage{renderer.FullRedraw()}, geometry |
| 135 | } | 135 | } | ||
| 136 | if titleChanged && geometry.Bounds.Height >= 3 { | 136 | if titleChanged && geometry.Bounds.Height >= 3 { | ||
| 137 | damage = append(damage, renderer.Damage{Kind: renderer.DamageText, X: geometry.Bounds.X, Y: geometry.Bounds.Y, Width: geometry.Bounds.Width, Height: 1}) | 137 | damage = append(damage, renderer.Damage{Kind: renderer.DamageText, X: geometry.Bounds.X, Y: geometry.Bounds.Y, Width: geometry.Bounds.Width, Height: 1}) | ||
| 138 | } | 138 | } | ||
| 139 | - | return frame, damage | 139 | + | return frame, damage, geometry |
| 140 | } | 140 | } | ||
| 141 | 141 | ||||
| 142 | func drawFloatingBorder(frame renderer.Frame, bounds domain.Rect, title string, style renderer.Style) { | 142 | func drawFloatingBorder(frame renderer.Frame, bounds domain.Rect, title string, style renderer.Style) { | ||
M internal/usecase/daemon/floating_render_test.go +21 -19
| @@ -78,7 +78,7 @@ func TestComposeFloatingFrameDoesNotMutateSourceAndDamagesTitle(t *testing.T) { | |||
| 78 | 78 | base.Set(2, 2, renderer.Cell{Rune: 'B'}) | |
| 79 | 79 | content := domain.Rect{Y: 1, Width: 40, Height: 10} | |
| 80 | 80 | cache := &composedFrameCache{} | |
| 81 | - | frame, _ := composeFloatingFrame(base, nil, p, 1, content, domain.FloatingConfig{Width: 80, Height: 80}, tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | |
| 81 | + | frame, _, _ := composeFloatingFrame(base, nil, p, 1, content, calculateFloatingGeometry(domain.Rect{Width: content.Width, Height: content.Height}, domain.FloatingConfig{Width: 80, Height: 80}), tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | |
| 82 | 82 | require.Equal(t, 'B', base.At(2, 2).Rune, "backdrop must only touch the composed destination") | |
| 83 | 83 | geometry := calculateFloatingGeometry(content, domain.FloatingConfig{Width: 80, Height: 80}) | |
| 84 | 84 | require.Equal(t, '┌', frame.At(geometry.Bounds.X, geometry.Bounds.Y).Rune) | |
| @@ -92,7 +92,7 @@ func TestComposeFloatingFrameDoesNotMutateSourceAndDamagesTitle(t *testing.T) { | |||
| 92 | 92 | p.mu.Lock() | |
| 93 | 93 | p.title.generation++ | |
| 94 | 94 | p.mu.Unlock() | |
| 95 | - | _, damage := composeFloatingFrame(base, nil, p, 1, content, domain.FloatingConfig{Width: 80, Height: 80}, tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | |
| 95 | + | _, damage, _ := composeFloatingFrame(base, nil, p, 1, content, calculateFloatingGeometry(domain.Rect{Width: content.Width, Height: content.Height}, domain.FloatingConfig{Width: 80, Height: 80}), tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | |
| 96 | 96 | require.Len(t, damage, 1) | |
| 97 | 97 | require.Equal(t, geometry.Bounds.Y, damage[0].Y) | |
| 98 | 98 | require.Equal(t, 1, damage[0].Height) | |
| @@ -123,15 +123,15 @@ func TestComposeFloatingFrameGeometryChangesInvalidateCache(t *testing.T) { | |||
| 123 | 123 | cache := &composedFrameCache{} | |
| 124 | 124 | base := renderer.NewFrame(30, 12) | |
| 125 | 125 | content := domain.Rect{Y: 1, Width: 30, Height: 10} | |
| 126 | - | _, damage := composeFloatingFrame(base, nil, p, 1, content, domain.FloatingConfig{}, tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | |
| 126 | + | _, damage, _ := composeFloatingFrame(base, nil, p, 1, content, floatingGeometry{}, tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | |
| 127 | 127 | require.Equal(t, []renderer.Damage{renderer.FullRedraw()}, damage) | |
| 128 | 128 | ||
| 129 | 129 | p.mu.Lock() | |
| 130 | 130 | p.rect, p.popupGeometry = tt.next.Inner, tt.next | |
| 131 | 131 | p.mu.Unlock() | |
| 132 | - | _, damage = composeFloatingFrame(base, nil, p, 1, content, domain.FloatingConfig{}, tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | |
| 132 | + | _, damage, _ = composeFloatingFrame(base, nil, p, 1, content, floatingGeometry{}, tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | |
| 133 | 133 | require.Equal(t, []renderer.Damage{renderer.FullRedraw()}, damage) | |
| 134 | - | require.Equal(t, tt.next, cache.floatingGeometry) | |
| 134 | + | require.Equal(t, tt.next.translate(content.X, content.Y), cache.floatingGeometry) | |
| 135 | 135 | }) | |
| 136 | 136 | } | |
| 137 | 137 | } | |
| @@ -181,7 +181,7 @@ func TestComposeFloatingFrameRendersPaneOwnedCommandFallback(t *testing.T) { | |||
| 181 | 181 | base := renderer.NewFrame(40, 12) | |
| 182 | 182 | content := domain.Rect{Y: 1, Width: 40, Height: 10} | |
| 183 | 183 | cfg := domain.FloatingConfig{Width: 80, Height: 80} | |
| 184 | - | frame, _ := composeFloatingFrame(base, nil, p, 1, content, cfg, tabLayoutSnapshot{}, themeui.Theme{}, &composedFrameCache{}, false) | |
| 184 | + | frame, _, _ := composeFloatingFrame(base, nil, p, 1, content, calculateFloatingGeometry(domain.Rect{Width: content.Width, Height: content.Height}, cfg), tabLayoutSnapshot{}, themeui.Theme{}, &composedFrameCache{}, false) | |
| 185 | 185 | geometry := calculateFloatingGeometry(content, cfg) | |
| 186 | 186 | var gotTitle strings.Builder | |
| 187 | 187 | for x := geometry.Bounds.X + 2; x < geometry.Bounds.X+geometry.Bounds.Width-2; x++ { | |
| @@ -214,7 +214,7 @@ func TestComposeFloatingFrameSynchronizesWithPTYReader(t *testing.T) { | |||
| 214 | 214 | <-start | |
| 215 | 215 | cache := &composedFrameCache{} | |
| 216 | 216 | for range 500 { | |
| 217 | - | composeFloatingFrame(base, nil, p, 1, content, cfg, tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | |
| 217 | + | composeFloatingFrame(base, nil, p, 1, content, calculateFloatingGeometry(domain.Rect{Width: content.Width, Height: content.Height}, cfg), tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | |
| 218 | 218 | } | |
| 219 | 219 | }() | |
| 220 | 220 | close(start) | |
| @@ -271,13 +271,13 @@ func BenchmarkComposeFloatingFrameCached(b *testing.B) { | |||
| 271 | 271 | content := domain.Rect{Y: 1, Width: 80, Height: 22} | |
| 272 | 272 | cfg := domain.FloatingConfig{Width: 80, Height: 80} | |
| 273 | 273 | cache := &composedFrameCache{} | |
| 274 | - | composeFloatingFrame(base, nil, p, 1, content, cfg, tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | |
| 274 | + | composeFloatingFrame(base, nil, p, 1, content, calculateFloatingGeometry(domain.Rect{Width: content.Width, Height: content.Height}, cfg), tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | |
| 275 | 275 | p.screen.ClearDamage() | |
| 276 | 276 | ||
| 277 | 277 | b.ReportAllocs() | |
| 278 | 278 | b.ResetTimer() | |
| 279 | 279 | for b.Loop() { | |
| 280 | - | composeFloatingFrame(base, nil, p, 1, content, cfg, tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | |
| 280 | + | composeFloatingFrame(base, nil, p, 1, content, calculateFloatingGeometry(domain.Rect{Width: content.Width, Height: content.Height}, cfg), tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | |
| 281 | 281 | } | |
| 282 | 282 | } | |
| 283 | 283 | ||
| @@ -288,11 +288,11 @@ func TestComposeFloatingFrameCachedDoesNotAllocate(t *testing.T) { | |||
| 288 | 288 | content := domain.Rect{Y: 1, Width: 80, Height: 22} | |
| 289 | 289 | cfg := domain.FloatingConfig{Width: 80, Height: 80} | |
| 290 | 290 | cache := &composedFrameCache{} | |
| 291 | - | composeFloatingFrame(base, nil, p, 1, content, cfg, tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | |
| 291 | + | composeFloatingFrame(base, nil, p, 1, content, calculateFloatingGeometry(domain.Rect{Width: content.Width, Height: content.Height}, cfg), tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | |
| 292 | 292 | p.screen.ClearDamage() | |
| 293 | 293 | ||
| 294 | 294 | allocs := testing.AllocsPerRun(100, func() { | |
| 295 | - | composeFloatingFrame(base, nil, p, 1, content, cfg, tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | |
| 295 | + | composeFloatingFrame(base, nil, p, 1, content, calculateFloatingGeometry(domain.Rect{Width: content.Width, Height: content.Height}, cfg), tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | |
| 296 | 296 | }) | |
| 297 | 297 | require.Zero(t, allocs, "cached floating composition must not allocate without damage or title changes") | |
| 298 | 298 | } | |
| @@ -345,8 +345,8 @@ func TestFailedFloatingResizeKeepsCommittedRenderAndInputGeometry(t *testing.T) | |||
| 345 | 345 | cfg := domain.FloatingConfig{Width: 50, Height: 50} | |
| 346 | 346 | oldContent := domain.Rect{Y: 1, Width: 80, Height: 24} | |
| 347 | 347 | newContent := domain.Rect{Y: 1, Width: 100, Height: 40} | |
| 348 | - | oldGeometry := calculateFloatingGeometry(oldContent, cfg) | |
| 349 | - | newGeometry := calculateFloatingGeometry(newContent, cfg) | |
| 348 | + | oldGeometry := calculateFloatingGeometry(domain.Rect{Width: oldContent.Width, Height: oldContent.Height}, cfg) | |
| 349 | + | newGeometry := calculateFloatingGeometry(domain.Rect{Width: newContent.Width, Height: newContent.Height}, cfg) | |
| 350 | 350 | pty := &resizePTY{err: errors.New("resize failed")} | |
| 351 | 351 | p := newPane("floating", pty, rectSize(oldGeometry.Inner)) | |
| 352 | 352 | p.rect = oldGeometry.Inner | |
| @@ -359,17 +359,19 @@ func TestFailedFloatingResizeKeepsCommittedRenderAndInputGeometry(t *testing.T) | |||
| 359 | 359 | require.False(t, d.resizeFloatingPane(p, newGeometry)) | |
| 360 | 360 | tb.mu.Lock() | |
| 361 | 361 | _, inputGeometry, visible := tb.visibleFloatingSnapshotLocked(cfg) | |
| 362 | - | copyRect := copyTargetRectLocked(tabLayoutSnapshot{}, newContent, p, p, true, cfg) | |
| 362 | + | copyRect := copyTargetRectLocked(tabLayoutSnapshot{}, newContent, p, p, true, oldGeometry.translate(newContent.X, newContent.Y)) | |
| 363 | 363 | tb.mu.Unlock() | |
| 364 | 364 | require.True(t, visible) | |
| 365 | 365 | require.Equal(t, oldGeometry, inputGeometry) | |
| 366 | - | require.Equal(t, oldGeometry.Inner, copyRect) | |
| 366 | + | require.Equal(t, oldGeometry.translate(newContent.X, newContent.Y).Inner, copyRect) | |
| 367 | 367 | ||
| 368 | 368 | base := renderer.NewFrame(newContent.Width, newContent.Height+2) | |
| 369 | - | frame, _ := composeFloatingFrame(base, nil, p, 1, newContent, cfg, tabLayoutSnapshot{}, themeui.Theme{}, &composedFrameCache{}, true) | |
| 370 | - | require.Equal(t, '┌', frame.At(oldGeometry.Bounds.X, oldGeometry.Bounds.Y).Rune) | |
| 371 | - | if newGeometry.Bounds.X != oldGeometry.Bounds.X || newGeometry.Bounds.Y != oldGeometry.Bounds.Y { | |
| 372 | - | require.NotEqual(t, '┌', frame.At(newGeometry.Bounds.X, newGeometry.Bounds.Y).Rune) | |
| 369 | + | frame, _, _ := composeFloatingFrame(base, nil, p, 1, newContent, calculateFloatingGeometry(domain.Rect{Width: newContent.Width, Height: newContent.Height}, cfg), tabLayoutSnapshot{}, themeui.Theme{}, &composedFrameCache{}, true) | |
| 370 | + | oldFrameGeometry := oldGeometry.translate(newContent.X, newContent.Y) | |
| 371 | + | newFrameGeometry := newGeometry.translate(newContent.X, newContent.Y) | |
| 372 | + | require.Equal(t, '┌', frame.At(oldFrameGeometry.Bounds.X, oldFrameGeometry.Bounds.Y).Rune) | |
| 373 | + | if newFrameGeometry.Bounds.X != oldFrameGeometry.Bounds.X || newFrameGeometry.Bounds.Y != oldFrameGeometry.Bounds.Y { | |
| 374 | + | require.NotEqual(t, '┌', frame.At(newFrameGeometry.Bounds.X, newFrameGeometry.Bounds.Y).Rune) | |
| 373 | 375 | } | |
| 374 | 376 | } | |
| 375 | 377 | ||
| @@ -78,7 +78,7 @@ func TestComposeFloatingFrameDoesNotMutateSourceAndDamagesTitle(t *testing.T) { | |||||
| 78 | base.Set(2, 2, renderer.Cell{Rune: 'B'}) | 78 | base.Set(2, 2, renderer.Cell{Rune: 'B'}) | ||
| 79 | content := domain.Rect{Y: 1, Width: 40, Height: 10} | 79 | content := domain.Rect{Y: 1, Width: 40, Height: 10} | ||
| 80 | cache := &composedFrameCache{} | 80 | cache := &composedFrameCache{} | ||
| 81 | - | frame, _ := composeFloatingFrame(base, nil, p, 1, content, domain.FloatingConfig{Width: 80, Height: 80}, tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | 81 | + | frame, _, _ := composeFloatingFrame(base, nil, p, 1, content, calculateFloatingGeometry(domain.Rect{Width: content.Width, Height: content.Height}, domain.FloatingConfig{Width: 80, Height: 80}), tabLayoutSnapshot{}, themeui.Theme{}, cache, false) |
| 82 | require.Equal(t, 'B', base.At(2, 2).Rune, "backdrop must only touch the composed destination") | 82 | require.Equal(t, 'B', base.At(2, 2).Rune, "backdrop must only touch the composed destination") | ||
| 83 | geometry := calculateFloatingGeometry(content, domain.FloatingConfig{Width: 80, Height: 80}) | 83 | geometry := calculateFloatingGeometry(content, domain.FloatingConfig{Width: 80, Height: 80}) | ||
| 84 | require.Equal(t, '┌', frame.At(geometry.Bounds.X, geometry.Bounds.Y).Rune) | 84 | require.Equal(t, '┌', frame.At(geometry.Bounds.X, geometry.Bounds.Y).Rune) | ||
| @@ -92,7 +92,7 @@ func TestComposeFloatingFrameDoesNotMutateSourceAndDamagesTitle(t *testing.T) { | |||||
| 92 | p.mu.Lock() | 92 | p.mu.Lock() | ||
| 93 | p.title.generation++ | 93 | p.title.generation++ | ||
| 94 | p.mu.Unlock() | 94 | p.mu.Unlock() | ||
| 95 | - | _, damage := composeFloatingFrame(base, nil, p, 1, content, domain.FloatingConfig{Width: 80, Height: 80}, tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | 95 | + | _, damage, _ := composeFloatingFrame(base, nil, p, 1, content, calculateFloatingGeometry(domain.Rect{Width: content.Width, Height: content.Height}, domain.FloatingConfig{Width: 80, Height: 80}), tabLayoutSnapshot{}, themeui.Theme{}, cache, false) |
| 96 | require.Len(t, damage, 1) | 96 | require.Len(t, damage, 1) | ||
| 97 | require.Equal(t, geometry.Bounds.Y, damage[0].Y) | 97 | require.Equal(t, geometry.Bounds.Y, damage[0].Y) | ||
| 98 | require.Equal(t, 1, damage[0].Height) | 98 | require.Equal(t, 1, damage[0].Height) | ||
| @@ -123,15 +123,15 @@ func TestComposeFloatingFrameGeometryChangesInvalidateCache(t *testing.T) { | |||||
| 123 | cache := &composedFrameCache{} | 123 | cache := &composedFrameCache{} | ||
| 124 | base := renderer.NewFrame(30, 12) | 124 | base := renderer.NewFrame(30, 12) | ||
| 125 | content := domain.Rect{Y: 1, Width: 30, Height: 10} | 125 | content := domain.Rect{Y: 1, Width: 30, Height: 10} | ||
| 126 | - | _, damage := composeFloatingFrame(base, nil, p, 1, content, domain.FloatingConfig{}, tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | 126 | + | _, damage, _ := composeFloatingFrame(base, nil, p, 1, content, floatingGeometry{}, tabLayoutSnapshot{}, themeui.Theme{}, cache, false) |
| 127 | require.Equal(t, []renderer.Damage{renderer.FullRedraw()}, damage) | 127 | require.Equal(t, []renderer.Damage{renderer.FullRedraw()}, damage) | ||
| 128 | 128 | ||||
| 129 | p.mu.Lock() | 129 | p.mu.Lock() | ||
| 130 | p.rect, p.popupGeometry = tt.next.Inner, tt.next | 130 | p.rect, p.popupGeometry = tt.next.Inner, tt.next | ||
| 131 | p.mu.Unlock() | 131 | p.mu.Unlock() | ||
| 132 | - | _, damage = composeFloatingFrame(base, nil, p, 1, content, domain.FloatingConfig{}, tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | 132 | + | _, damage, _ = composeFloatingFrame(base, nil, p, 1, content, floatingGeometry{}, tabLayoutSnapshot{}, themeui.Theme{}, cache, false) |
| 133 | require.Equal(t, []renderer.Damage{renderer.FullRedraw()}, damage) | 133 | require.Equal(t, []renderer.Damage{renderer.FullRedraw()}, damage) | ||
| 134 | - | require.Equal(t, tt.next, cache.floatingGeometry) | 134 | + | require.Equal(t, tt.next.translate(content.X, content.Y), cache.floatingGeometry) |
| 135 | }) | 135 | }) | ||
| 136 | } | 136 | } | ||
| 137 | } | 137 | } | ||
| @@ -181,7 +181,7 @@ func TestComposeFloatingFrameRendersPaneOwnedCommandFallback(t *testing.T) { | |||||
| 181 | base := renderer.NewFrame(40, 12) | 181 | base := renderer.NewFrame(40, 12) | ||
| 182 | content := domain.Rect{Y: 1, Width: 40, Height: 10} | 182 | content := domain.Rect{Y: 1, Width: 40, Height: 10} | ||
| 183 | cfg := domain.FloatingConfig{Width: 80, Height: 80} | 183 | cfg := domain.FloatingConfig{Width: 80, Height: 80} | ||
| 184 | - | frame, _ := composeFloatingFrame(base, nil, p, 1, content, cfg, tabLayoutSnapshot{}, themeui.Theme{}, &composedFrameCache{}, false) | 184 | + | frame, _, _ := composeFloatingFrame(base, nil, p, 1, content, calculateFloatingGeometry(domain.Rect{Width: content.Width, Height: content.Height}, cfg), tabLayoutSnapshot{}, themeui.Theme{}, &composedFrameCache{}, false) |
| 185 | geometry := calculateFloatingGeometry(content, cfg) | 185 | geometry := calculateFloatingGeometry(content, cfg) | ||
| 186 | var gotTitle strings.Builder | 186 | var gotTitle strings.Builder | ||
| 187 | for x := geometry.Bounds.X + 2; x < geometry.Bounds.X+geometry.Bounds.Width-2; x++ { | 187 | for x := geometry.Bounds.X + 2; x < geometry.Bounds.X+geometry.Bounds.Width-2; x++ { | ||
| @@ -214,7 +214,7 @@ func TestComposeFloatingFrameSynchronizesWithPTYReader(t *testing.T) { | |||||
| 214 | <-start | 214 | <-start | ||
| 215 | cache := &composedFrameCache{} | 215 | cache := &composedFrameCache{} | ||
| 216 | for range 500 { | 216 | for range 500 { | ||
| 217 | - | composeFloatingFrame(base, nil, p, 1, content, cfg, tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | 217 | + | composeFloatingFrame(base, nil, p, 1, content, calculateFloatingGeometry(domain.Rect{Width: content.Width, Height: content.Height}, cfg), tabLayoutSnapshot{}, themeui.Theme{}, cache, false) |
| 218 | } | 218 | } | ||
| 219 | }() | 219 | }() | ||
| 220 | close(start) | 220 | close(start) | ||
| @@ -271,13 +271,13 @@ func BenchmarkComposeFloatingFrameCached(b *testing.B) { | |||||
| 271 | content := domain.Rect{Y: 1, Width: 80, Height: 22} | 271 | content := domain.Rect{Y: 1, Width: 80, Height: 22} | ||
| 272 | cfg := domain.FloatingConfig{Width: 80, Height: 80} | 272 | cfg := domain.FloatingConfig{Width: 80, Height: 80} | ||
| 273 | cache := &composedFrameCache{} | 273 | cache := &composedFrameCache{} | ||
| 274 | - | composeFloatingFrame(base, nil, p, 1, content, cfg, tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | 274 | + | composeFloatingFrame(base, nil, p, 1, content, calculateFloatingGeometry(domain.Rect{Width: content.Width, Height: content.Height}, cfg), tabLayoutSnapshot{}, themeui.Theme{}, cache, false) |
| 275 | p.screen.ClearDamage() | 275 | p.screen.ClearDamage() | ||
| 276 | 276 | ||||
| 277 | b.ReportAllocs() | 277 | b.ReportAllocs() | ||
| 278 | b.ResetTimer() | 278 | b.ResetTimer() | ||
| 279 | for b.Loop() { | 279 | for b.Loop() { | ||
| 280 | - | composeFloatingFrame(base, nil, p, 1, content, cfg, tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | 280 | + | composeFloatingFrame(base, nil, p, 1, content, calculateFloatingGeometry(domain.Rect{Width: content.Width, Height: content.Height}, cfg), tabLayoutSnapshot{}, themeui.Theme{}, cache, false) |
| 281 | } | 281 | } | ||
| 282 | } | 282 | } | ||
| 283 | 283 | ||||
| @@ -288,11 +288,11 @@ func TestComposeFloatingFrameCachedDoesNotAllocate(t *testing.T) { | |||||
| 288 | content := domain.Rect{Y: 1, Width: 80, Height: 22} | 288 | content := domain.Rect{Y: 1, Width: 80, Height: 22} | ||
| 289 | cfg := domain.FloatingConfig{Width: 80, Height: 80} | 289 | cfg := domain.FloatingConfig{Width: 80, Height: 80} | ||
| 290 | cache := &composedFrameCache{} | 290 | cache := &composedFrameCache{} | ||
| 291 | - | composeFloatingFrame(base, nil, p, 1, content, cfg, tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | 291 | + | composeFloatingFrame(base, nil, p, 1, content, calculateFloatingGeometry(domain.Rect{Width: content.Width, Height: content.Height}, cfg), tabLayoutSnapshot{}, themeui.Theme{}, cache, false) |
| 292 | p.screen.ClearDamage() | 292 | p.screen.ClearDamage() | ||
| 293 | 293 | ||||
| 294 | allocs := testing.AllocsPerRun(100, func() { | 294 | allocs := testing.AllocsPerRun(100, func() { | ||
| 295 | - | composeFloatingFrame(base, nil, p, 1, content, cfg, tabLayoutSnapshot{}, themeui.Theme{}, cache, false) | 295 | + | composeFloatingFrame(base, nil, p, 1, content, calculateFloatingGeometry(domain.Rect{Width: content.Width, Height: content.Height}, cfg), tabLayoutSnapshot{}, themeui.Theme{}, cache, false) |
| 296 | }) | 296 | }) | ||
| 297 | require.Zero(t, allocs, "cached floating composition must not allocate without damage or title changes") | 297 | require.Zero(t, allocs, "cached floating composition must not allocate without damage or title changes") | ||
| 298 | } | 298 | } | ||
| @@ -345,8 +345,8 @@ func TestFailedFloatingResizeKeepsCommittedRenderAndInputGeometry(t *testing.T) | |||||
| 345 | cfg := domain.FloatingConfig{Width: 50, Height: 50} | 345 | cfg := domain.FloatingConfig{Width: 50, Height: 50} | ||
| 346 | oldContent := domain.Rect{Y: 1, Width: 80, Height: 24} | 346 | oldContent := domain.Rect{Y: 1, Width: 80, Height: 24} | ||
| 347 | newContent := domain.Rect{Y: 1, Width: 100, Height: 40} | 347 | newContent := domain.Rect{Y: 1, Width: 100, Height: 40} | ||
| 348 | - | oldGeometry := calculateFloatingGeometry(oldContent, cfg) | 348 | + | oldGeometry := calculateFloatingGeometry(domain.Rect{Width: oldContent.Width, Height: oldContent.Height}, cfg) |
| 349 | - | newGeometry := calculateFloatingGeometry(newContent, cfg) | 349 | + | newGeometry := calculateFloatingGeometry(domain.Rect{Width: newContent.Width, Height: newContent.Height}, cfg) |
| 350 | pty := &resizePTY{err: errors.New("resize failed")} | 350 | pty := &resizePTY{err: errors.New("resize failed")} | ||
| 351 | p := newPane("floating", pty, rectSize(oldGeometry.Inner)) | 351 | p := newPane("floating", pty, rectSize(oldGeometry.Inner)) | ||
| 352 | p.rect = oldGeometry.Inner | 352 | p.rect = oldGeometry.Inner | ||
| @@ -359,17 +359,19 @@ func TestFailedFloatingResizeKeepsCommittedRenderAndInputGeometry(t *testing.T) | |||||
| 359 | require.False(t, d.resizeFloatingPane(p, newGeometry)) | 359 | require.False(t, d.resizeFloatingPane(p, newGeometry)) | ||
| 360 | tb.mu.Lock() | 360 | tb.mu.Lock() | ||
| 361 | _, inputGeometry, visible := tb.visibleFloatingSnapshotLocked(cfg) | 361 | _, inputGeometry, visible := tb.visibleFloatingSnapshotLocked(cfg) | ||
| 362 | - | copyRect := copyTargetRectLocked(tabLayoutSnapshot{}, newContent, p, p, true, cfg) | 362 | + | copyRect := copyTargetRectLocked(tabLayoutSnapshot{}, newContent, p, p, true, oldGeometry.translate(newContent.X, newContent.Y)) |
| 363 | tb.mu.Unlock() | 363 | tb.mu.Unlock() | ||
| 364 | require.True(t, visible) | 364 | require.True(t, visible) | ||
| 365 | require.Equal(t, oldGeometry, inputGeometry) | 365 | require.Equal(t, oldGeometry, inputGeometry) | ||
| 366 | - | require.Equal(t, oldGeometry.Inner, copyRect) | 366 | + | require.Equal(t, oldGeometry.translate(newContent.X, newContent.Y).Inner, copyRect) |
| 367 | 367 | ||||
| 368 | base := renderer.NewFrame(newContent.Width, newContent.Height+2) | 368 | base := renderer.NewFrame(newContent.Width, newContent.Height+2) | ||
| 369 | - | frame, _ := composeFloatingFrame(base, nil, p, 1, newContent, cfg, tabLayoutSnapshot{}, themeui.Theme{}, &composedFrameCache{}, true) | 369 | + | frame, _, _ := composeFloatingFrame(base, nil, p, 1, newContent, calculateFloatingGeometry(domain.Rect{Width: newContent.Width, Height: newContent.Height}, cfg), tabLayoutSnapshot{}, themeui.Theme{}, &composedFrameCache{}, true) |
| 370 | - | require.Equal(t, '┌', frame.At(oldGeometry.Bounds.X, oldGeometry.Bounds.Y).Rune) | 370 | + | oldFrameGeometry := oldGeometry.translate(newContent.X, newContent.Y) |
| 371 | - | if newGeometry.Bounds.X != oldGeometry.Bounds.X || newGeometry.Bounds.Y != oldGeometry.Bounds.Y { | 371 | + | newFrameGeometry := newGeometry.translate(newContent.X, newContent.Y) |
| 372 | - | require.NotEqual(t, '┌', frame.At(newGeometry.Bounds.X, newGeometry.Bounds.Y).Rune) | 372 | + | require.Equal(t, '┌', frame.At(oldFrameGeometry.Bounds.X, oldFrameGeometry.Bounds.Y).Rune) |
| 373 | + | if newFrameGeometry.Bounds.X != oldFrameGeometry.Bounds.X || newFrameGeometry.Bounds.Y != oldFrameGeometry.Bounds.Y { | |||
| 374 | + | require.NotEqual(t, '┌', frame.At(newFrameGeometry.Bounds.X, newFrameGeometry.Bounds.Y).Rune) | |||
| 373 | } | 375 | } | ||
| 374 | } | 376 | } | ||
| 375 | 377 | ||||
M internal/usecase/daemon/render.go +10 -14
| @@ -278,16 +278,12 @@ func (d *Daemon) render(sess *session, tb *tab, p *pane) { | |||
| 278 | 278 | // copyTargetRectLocked maps a captured copy source into the already-composed | |
| 279 | 279 | // client frame. The caller holds tb.mu, preserving the layout/floating | |
| 280 | 280 | // snapshot while the rectangle is chosen. | |
| 281 | - | func copyTargetRectLocked(layoutSnap tabLayoutSnapshot, contentArea domain.Rect, p, floating *pane, hasFloating bool, cfg domain.FloatingConfig) domain.Rect { | |
| 281 | + | func copyTargetRectLocked(layoutSnap tabLayoutSnapshot, contentArea domain.Rect, p, floating *pane, hasFloating bool, floatingFrameGeometry floatingGeometry) domain.Rect { | |
| 282 | 282 | if p == nil { | |
| 283 | 283 | return domain.Rect{} | |
| 284 | 284 | } | |
| 285 | 285 | if hasFloating && p == floating { | |
| 286 | - | desired := calculateFloatingGeometry(contentArea, cfg) | |
| 287 | - | p.mu.Lock() | |
| 288 | - | geometry := p.committedFloatingGeometryLocked(desired) | |
| 289 | - | p.mu.Unlock() | |
| 290 | - | return geometry.Inner | |
| 286 | + | return floatingFrameGeometry.Inner | |
| 291 | 287 | } | |
| 292 | 288 | for _, placement := range layoutSnap.placements { | |
| 293 | 289 | if placement.ID == p.id && !placement.Collapsed { | |
| @@ -399,15 +395,17 @@ func (d *Daemon) paint(sess *session, ac *attachedClient, reset bool) { | |||
| 399 | 395 | frame, damage = composeClientFrameWithLayoutCachedConsumeDamage(bars, tb, reset, layoutSnap, &ac.bars, &ac.composed) | |
| 400 | 396 | } | |
| 401 | 397 | contentArea := domain.Rect{Y: 1, Width: frame.Width, Height: max(0, frame.Height-2)} | |
| 398 | + | floatingFrameGeometry := floatingGeometry{} | |
| 402 | 399 | if hasFloating { | |
| 403 | - | frame, damage = composeFloatingFrame(frame, damage, floating, tb.floating.generation, contentArea, floatingCfg, layoutSnap, bars.theme, &ac.composed, reset || overlayActive) | |
| 400 | + | desiredFloatingGeometry := calculateFloatingGeometry(domain.Rect{Width: contentArea.Width, Height: contentArea.Height}, floatingCfg) | |
| 401 | + | frame, damage, floatingFrameGeometry = composeFloatingFrame(frame, damage, floating, tb.floating.generation, contentArea, desiredFloatingGeometry, layoutSnap, bars.theme, &ac.composed, reset || overlayActive) | |
| 404 | 402 | } | |
| 405 | 403 | if overlays.copyActive { | |
| 406 | 404 | copyPane := overlays.copyPane | |
| 407 | 405 | if copyPane == nil { | |
| 408 | 406 | copyPane = p | |
| 409 | 407 | } | |
| 410 | - | copyTarget := copyTargetRectLocked(layoutSnap, contentArea, copyPane, floating, hasFloating, floatingCfg) | |
| 408 | + | copyTarget := copyTargetRectLocked(layoutSnap, contentArea, copyPane, floating, hasFloating, floatingFrameGeometry) | |
| 411 | 409 | frame, damage = composeCopyClientFrame(overlays.copyMode, copyPane, copyTarget, frame, bars) | |
| 412 | 410 | } | |
| 413 | 411 | // A palette above normal/copy content dims that composed content. When a | |
| @@ -439,15 +437,13 @@ func (d *Daemon) paint(sess *session, ac *attachedClient, reset bool) { | |||
| 439 | 437 | overlays.Unlock() | |
| 440 | 438 | cursorPane := p | |
| 441 | 439 | cursorContent, cursorVisible := focusedPaneContentRect(layoutSnap, p.id) | |
| 440 | + | cursorContent.X += contentArea.X | |
| 441 | + | cursorContent.Y += contentArea.Y | |
| 442 | 442 | if hasFloating { | |
| 443 | 443 | cursorPane = floating | |
| 444 | + | cursorContent, cursorVisible = floatingFrameGeometry.Inner, floatingFrameGeometry.Inner.Width > 0 && floatingFrameGeometry.Inner.Height > 0 | |
| 444 | 445 | } | |
| 445 | 446 | cursorPane.mu.Lock() | |
| 446 | - | if hasFloating { | |
| 447 | - | desiredGeometry := calculateFloatingGeometry(contentArea, floatingCfg) | |
| 448 | - | geometry := floating.committedFloatingGeometryLocked(desiredGeometry) | |
| 449 | - | cursorContent, cursorVisible = geometry.Inner, geometry.Inner.Width > 0 && geometry.Inner.Height > 0 | |
| 450 | - | } | |
| 451 | 447 | desiredCursor := desiredCursorOut(cursorPane.screen, cursorContent, !cursorVisible || overlays.copyActive || overlays.copySearchModel != nil || overlays.pickerActive || overlays.paletteActive || overlays.promptActive) | |
| 452 | 448 | cursorPane.mu.Unlock() | |
| 453 | 449 | ac.sess.mu.Lock() | |
| @@ -519,7 +515,7 @@ func desiredCursorOut(s *vt.Screen, content domain.Rect, hide bool) cursorOut { | |||
| 519 | 515 | if !ok { | |
| 520 | 516 | style = 1 | |
| 521 | 517 | } | |
| 522 | - | return cursorOut{row: content.Y + s.CursorRow() + 1, col: content.X + s.CursorCol(), style: style, hasStyle: true} | |
| 518 | + | return cursorOut{row: content.Y + s.CursorRow(), col: content.X + s.CursorCol(), style: style, hasStyle: true} | |
| 523 | 519 | } | |
| 524 | 520 | ||
| 525 | 521 | func (ac *attachedClient) encodeCursorTail(desired cursorOut, force bool) []byte { | |
| @@ -278,16 +278,12 @@ func (d *Daemon) render(sess *session, tb *tab, p *pane) { | |||||
| 278 | // copyTargetRectLocked maps a captured copy source into the already-composed | 278 | // copyTargetRectLocked maps a captured copy source into the already-composed | ||
| 279 | // client frame. The caller holds tb.mu, preserving the layout/floating | 279 | // client frame. The caller holds tb.mu, preserving the layout/floating | ||
| 280 | // snapshot while the rectangle is chosen. | 280 | // snapshot while the rectangle is chosen. | ||
| 281 | - | func copyTargetRectLocked(layoutSnap tabLayoutSnapshot, contentArea domain.Rect, p, floating *pane, hasFloating bool, cfg domain.FloatingConfig) domain.Rect { | 281 | + | func copyTargetRectLocked(layoutSnap tabLayoutSnapshot, contentArea domain.Rect, p, floating *pane, hasFloating bool, floatingFrameGeometry floatingGeometry) domain.Rect { |
| 282 | if p == nil { | 282 | if p == nil { | ||
| 283 | return domain.Rect{} | 283 | return domain.Rect{} | ||
| 284 | } | 284 | } | ||
| 285 | if hasFloating && p == floating { | 285 | if hasFloating && p == floating { | ||
| 286 | - | desired := calculateFloatingGeometry(contentArea, cfg) | 286 | + | return floatingFrameGeometry.Inner |
| 287 | - | p.mu.Lock() | |||
| 288 | - | geometry := p.committedFloatingGeometryLocked(desired) | |||
| 289 | - | p.mu.Unlock() | |||
| 290 | - | return geometry.Inner | |||
| 291 | } | 287 | } | ||
| 292 | for _, placement := range layoutSnap.placements { | 288 | for _, placement := range layoutSnap.placements { | ||
| 293 | if placement.ID == p.id && !placement.Collapsed { | 289 | if placement.ID == p.id && !placement.Collapsed { | ||
| @@ -399,15 +395,17 @@ func (d *Daemon) paint(sess *session, ac *attachedClient, reset bool) { | |||||
| 399 | frame, damage = composeClientFrameWithLayoutCachedConsumeDamage(bars, tb, reset, layoutSnap, &ac.bars, &ac.composed) | 395 | frame, damage = composeClientFrameWithLayoutCachedConsumeDamage(bars, tb, reset, layoutSnap, &ac.bars, &ac.composed) | ||
| 400 | } | 396 | } | ||
| 401 | contentArea := domain.Rect{Y: 1, Width: frame.Width, Height: max(0, frame.Height-2)} | 397 | contentArea := domain.Rect{Y: 1, Width: frame.Width, Height: max(0, frame.Height-2)} | ||
| 398 | + | floatingFrameGeometry := floatingGeometry{} | |||
| 402 | if hasFloating { | 399 | if hasFloating { | ||
| 403 | - | frame, damage = composeFloatingFrame(frame, damage, floating, tb.floating.generation, contentArea, floatingCfg, layoutSnap, bars.theme, &ac.composed, reset || overlayActive) | 400 | + | desiredFloatingGeometry := calculateFloatingGeometry(domain.Rect{Width: contentArea.Width, Height: contentArea.Height}, floatingCfg) |
| 401 | + | frame, damage, floatingFrameGeometry = composeFloatingFrame(frame, damage, floating, tb.floating.generation, contentArea, desiredFloatingGeometry, layoutSnap, bars.theme, &ac.composed, reset || overlayActive) | |||
| 404 | } | 402 | } | ||
| 405 | if overlays.copyActive { | 403 | if overlays.copyActive { | ||
| 406 | copyPane := overlays.copyPane | 404 | copyPane := overlays.copyPane | ||
| 407 | if copyPane == nil { | 405 | if copyPane == nil { | ||
| 408 | copyPane = p | 406 | copyPane = p | ||
| 409 | } | 407 | } | ||
| 410 | - | copyTarget := copyTargetRectLocked(layoutSnap, contentArea, copyPane, floating, hasFloating, floatingCfg) | 408 | + | copyTarget := copyTargetRectLocked(layoutSnap, contentArea, copyPane, floating, hasFloating, floatingFrameGeometry) |
| 411 | frame, damage = composeCopyClientFrame(overlays.copyMode, copyPane, copyTarget, frame, bars) | 409 | frame, damage = composeCopyClientFrame(overlays.copyMode, copyPane, copyTarget, frame, bars) | ||
| 412 | } | 410 | } | ||
| 413 | // A palette above normal/copy content dims that composed content. When a | 411 | // A palette above normal/copy content dims that composed content. When a | ||
| @@ -439,15 +437,13 @@ func (d *Daemon) paint(sess *session, ac *attachedClient, reset bool) { | |||||
| 439 | overlays.Unlock() | 437 | overlays.Unlock() | ||
| 440 | cursorPane := p | 438 | cursorPane := p | ||
| 441 | cursorContent, cursorVisible := focusedPaneContentRect(layoutSnap, p.id) | 439 | cursorContent, cursorVisible := focusedPaneContentRect(layoutSnap, p.id) | ||
| 440 | + | cursorContent.X += contentArea.X | |||
| 441 | + | cursorContent.Y += contentArea.Y | |||
| 442 | if hasFloating { | 442 | if hasFloating { | ||
| 443 | cursorPane = floating | 443 | cursorPane = floating | ||
| 444 | + | cursorContent, cursorVisible = floatingFrameGeometry.Inner, floatingFrameGeometry.Inner.Width > 0 && floatingFrameGeometry.Inner.Height > 0 | |||
| 444 | } | 445 | } | ||
| 445 | cursorPane.mu.Lock() | 446 | cursorPane.mu.Lock() | ||
| 446 | - | if hasFloating { | |||
| 447 | - | desiredGeometry := calculateFloatingGeometry(contentArea, floatingCfg) | |||
| 448 | - | geometry := floating.committedFloatingGeometryLocked(desiredGeometry) | |||
| 449 | - | cursorContent, cursorVisible = geometry.Inner, geometry.Inner.Width > 0 && geometry.Inner.Height > 0 | |||
| 450 | - | } | |||
| 451 | desiredCursor := desiredCursorOut(cursorPane.screen, cursorContent, !cursorVisible || overlays.copyActive || overlays.copySearchModel != nil || overlays.pickerActive || overlays.paletteActive || overlays.promptActive) | 447 | desiredCursor := desiredCursorOut(cursorPane.screen, cursorContent, !cursorVisible || overlays.copyActive || overlays.copySearchModel != nil || overlays.pickerActive || overlays.paletteActive || overlays.promptActive) | ||
| 452 | cursorPane.mu.Unlock() | 448 | cursorPane.mu.Unlock() | ||
| 453 | ac.sess.mu.Lock() | 449 | ac.sess.mu.Lock() | ||
| @@ -519,7 +515,7 @@ func desiredCursorOut(s *vt.Screen, content domain.Rect, hide bool) cursorOut { | |||||
| 519 | if !ok { | 515 | if !ok { | ||
| 520 | style = 1 | 516 | style = 1 | ||
| 521 | } | 517 | } | ||
| 522 | - | return cursorOut{row: content.Y + s.CursorRow() + 1, col: content.X + s.CursorCol(), style: style, hasStyle: true} | 518 | + | return cursorOut{row: content.Y + s.CursorRow(), col: content.X + s.CursorCol(), style: style, hasStyle: true} |
| 523 | } | 519 | } | ||
| 524 | 520 | ||||
| 525 | func (ac *attachedClient) encodeCursorTail(desired cursorOut, force bool) []byte { | 521 | func (ac *attachedClient) encodeCursorTail(desired cursorOut, force bool) []byte { | ||
M internal/usecase/daemon/render_test.go +30 -2
| @@ -1305,6 +1305,28 @@ func TestCursorTailVisibleHideAndMoveOnly(t *testing.T) { | |||
| 1305 | 1305 | require.Contains(t, string(data), "\x1b[?25l") | |
| 1306 | 1306 | } | |
| 1307 | 1307 | ||
| 1308 | + | func TestPaintAlignsFloatingCursorWithCommittedGeometry(t *testing.T) { | |
| 1309 | + | p, release := newBlockingPTY(t) | |
| 1310 | + | defer release() | |
| 1311 | + | d, sess, ac, sends := newManualSessionWithPTYs(t, p) | |
| 1312 | + | contentArea := domain.Rect{Y: 1, Width: 80, Height: 23} | |
| 1313 | + | cfg := d.currentFloatingConfig() | |
| 1314 | + | committed := calculateFloatingGeometry(domain.Rect{Width: contentArea.Width, Height: contentArea.Height}, cfg) | |
| 1315 | + | floating := newPane("floating", nil, rectSize(committed.Inner)) | |
| 1316 | + | floating.screen.Frame.Set(0, 0, renderer.Cell{Rune: 'F'}) | |
| 1317 | + | floating.popupGeometry = committed | |
| 1318 | + | installTestFloating(sess.activeTab(), floating, true) | |
| 1319 | + | ||
| 1320 | + | d.paint(sess, ac, true) | |
| 1321 | + | data := mustOutputData(t, sends) | |
| 1322 | + | want := committed.translate(contentArea.X, contentArea.Y) | |
| 1323 | + | require.Contains(t, string(data), cursorCSI(want.Inner.Y+1, 1)) | |
| 1324 | + | require.Contains(t, string(data), "F") | |
| 1325 | + | require.Equal(t, want.Inner.Y, ac.lastCursor.row) | |
| 1326 | + | require.Equal(t, want.Inner.X, ac.lastCursor.col) | |
| 1327 | + | require.Contains(t, string(data), cursorCSI(want.Inner.Y+1, want.Inner.X+1)) | |
| 1328 | + | } | |
| 1329 | + | ||
| 1308 | 1330 | func TestCursorTailUsesFocusedPanePlacement(t *testing.T) { | |
| 1309 | 1331 | d, sess, ac, sends := newManualSessionWithPTYs(t, nil) | |
| 1310 | 1332 | win := sess.tabs[0] | |
| @@ -1460,6 +1482,8 @@ func TestCopyTargetRectLocked(t *testing.T) { | |||
| 1460 | 1482 | defer tb.mu.Unlock() | |
| 1461 | 1483 | layoutSnap := solveTabLayoutLocked(tb) | |
| 1462 | 1484 | main := tb.focusedPane() | |
| 1485 | + | fp.popupGeometry = calculateFloatingGeometry(domain.Rect{Width: contentArea.Width, Height: contentArea.Height}, cfg) | |
| 1486 | + | floatingFrameGeometry := fp.popupGeometry.translate(contentArea.X, contentArea.Y) | |
| 1463 | 1487 | ||
| 1464 | 1488 | cases := []struct { | |
| 1465 | 1489 | name string | |
| @@ -1468,13 +1492,17 @@ func TestCopyTargetRectLocked(t *testing.T) { | |||
| 1468 | 1492 | hasFloating bool | |
| 1469 | 1493 | want domain.Rect | |
| 1470 | 1494 | }{ | |
| 1471 | - | {name: "floating source targets popup inner", pane: fp, floating: fp, hasFloating: true, want: calculateFloatingGeometry(contentArea, cfg).Inner}, | |
| 1495 | + | {name: "floating source targets committed frame-absolute popup inner", pane: fp, floating: fp, hasFloating: true, want: calculateFloatingGeometry(domain.Rect{Width: contentArea.Width, Height: contentArea.Height}, cfg).translate(contentArea.X, contentArea.Y).Inner}, | |
| 1472 | 1496 | {name: "normal source targets solved placement", pane: main, want: domain.Rect{X: 0, Y: 1, Width: 80, Height: 23}}, | |
| 1473 | 1497 | {name: "unplaced source falls back to pane frame", pane: stray, want: domain.Rect{X: 0, Y: 1, Width: 10, Height: 4}}, | |
| 1474 | 1498 | } | |
| 1475 | 1499 | for _, tc := range cases { | |
| 1476 | 1500 | t.Run(tc.name, func(t *testing.T) { | |
| 1477 | - | require.Equal(t, tc.want, copyTargetRectLocked(layoutSnap, contentArea, tc.pane, tc.floating, tc.hasFloating, cfg)) | |
| 1501 | + | got := copyTargetRectLocked(layoutSnap, contentArea, tc.pane, tc.floating, tc.hasFloating, floatingFrameGeometry) | |
| 1502 | + | require.Equal(t, tc.want, got) | |
| 1503 | + | if tc.hasFloating { | |
| 1504 | + | require.NotEqual(t, fp.popupGeometry.Inner, got, "content-relative geometry must not target the client frame directly") | |
| 1505 | + | } | |
| 1478 | 1506 | }) | |
| 1479 | 1507 | } | |
| 1480 | 1508 | } | |
| @@ -1305,6 +1305,28 @@ func TestCursorTailVisibleHideAndMoveOnly(t *testing.T) { | |||||
| 1305 | require.Contains(t, string(data), "\x1b[?25l") | 1305 | require.Contains(t, string(data), "\x1b[?25l") | ||
| 1306 | } | 1306 | } | ||
| 1307 | 1307 | ||||
| 1308 | + | func TestPaintAlignsFloatingCursorWithCommittedGeometry(t *testing.T) { | |||
| 1309 | + | p, release := newBlockingPTY(t) | |||
| 1310 | + | defer release() | |||
| 1311 | + | d, sess, ac, sends := newManualSessionWithPTYs(t, p) | |||
| 1312 | + | contentArea := domain.Rect{Y: 1, Width: 80, Height: 23} | |||
| 1313 | + | cfg := d.currentFloatingConfig() | |||
| 1314 | + | committed := calculateFloatingGeometry(domain.Rect{Width: contentArea.Width, Height: contentArea.Height}, cfg) | |||
| 1315 | + | floating := newPane("floating", nil, rectSize(committed.Inner)) | |||
| 1316 | + | floating.screen.Frame.Set(0, 0, renderer.Cell{Rune: 'F'}) | |||
| 1317 | + | floating.popupGeometry = committed | |||
| 1318 | + | installTestFloating(sess.activeTab(), floating, true) | |||
| 1319 | + | ||||
| 1320 | + | d.paint(sess, ac, true) | |||
| 1321 | + | data := mustOutputData(t, sends) | |||
| 1322 | + | want := committed.translate(contentArea.X, contentArea.Y) | |||
| 1323 | + | require.Contains(t, string(data), cursorCSI(want.Inner.Y+1, 1)) | |||
| 1324 | + | require.Contains(t, string(data), "F") | |||
| 1325 | + | require.Equal(t, want.Inner.Y, ac.lastCursor.row) | |||
| 1326 | + | require.Equal(t, want.Inner.X, ac.lastCursor.col) | |||
| 1327 | + | require.Contains(t, string(data), cursorCSI(want.Inner.Y+1, want.Inner.X+1)) | |||
| 1328 | + | } | |||
| 1329 | + | ||||
| 1308 | func TestCursorTailUsesFocusedPanePlacement(t *testing.T) { | 1330 | func TestCursorTailUsesFocusedPanePlacement(t *testing.T) { | ||
| 1309 | d, sess, ac, sends := newManualSessionWithPTYs(t, nil) | 1331 | d, sess, ac, sends := newManualSessionWithPTYs(t, nil) | ||
| 1310 | win := sess.tabs[0] | 1332 | win := sess.tabs[0] | ||
| @@ -1460,6 +1482,8 @@ func TestCopyTargetRectLocked(t *testing.T) { | |||||
| 1460 | defer tb.mu.Unlock() | 1482 | defer tb.mu.Unlock() | ||
| 1461 | layoutSnap := solveTabLayoutLocked(tb) | 1483 | layoutSnap := solveTabLayoutLocked(tb) | ||
| 1462 | main := tb.focusedPane() | 1484 | main := tb.focusedPane() | ||
| 1485 | + | fp.popupGeometry = calculateFloatingGeometry(domain.Rect{Width: contentArea.Width, Height: contentArea.Height}, cfg) | |||
| 1486 | + | floatingFrameGeometry := fp.popupGeometry.translate(contentArea.X, contentArea.Y) | |||
| 1463 | 1487 | ||||
| 1464 | cases := []struct { | 1488 | cases := []struct { | ||
| 1465 | name string | 1489 | name string | ||
| @@ -1468,13 +1492,17 @@ func TestCopyTargetRectLocked(t *testing.T) { | |||||
| 1468 | hasFloating bool | 1492 | hasFloating bool | ||
| 1469 | want domain.Rect | 1493 | want domain.Rect | ||
| 1470 | }{ | 1494 | }{ | ||
| 1471 | - | {name: "floating source targets popup inner", pane: fp, floating: fp, hasFloating: true, want: calculateFloatingGeometry(contentArea, cfg).Inner}, | 1495 | + | {name: "floating source targets committed frame-absolute popup inner", pane: fp, floating: fp, hasFloating: true, want: calculateFloatingGeometry(domain.Rect{Width: contentArea.Width, Height: contentArea.Height}, cfg).translate(contentArea.X, contentArea.Y).Inner}, |
| 1472 | {name: "normal source targets solved placement", pane: main, want: domain.Rect{X: 0, Y: 1, Width: 80, Height: 23}}, | 1496 | {name: "normal source targets solved placement", pane: main, want: domain.Rect{X: 0, Y: 1, Width: 80, Height: 23}}, | ||
| 1473 | {name: "unplaced source falls back to pane frame", pane: stray, want: domain.Rect{X: 0, Y: 1, Width: 10, Height: 4}}, | 1497 | {name: "unplaced source falls back to pane frame", pane: stray, want: domain.Rect{X: 0, Y: 1, Width: 10, Height: 4}}, | ||
| 1474 | } | 1498 | } | ||
| 1475 | for _, tc := range cases { | 1499 | for _, tc := range cases { | ||
| 1476 | t.Run(tc.name, func(t *testing.T) { | 1500 | t.Run(tc.name, func(t *testing.T) { | ||
| 1477 | - | require.Equal(t, tc.want, copyTargetRectLocked(layoutSnap, contentArea, tc.pane, tc.floating, tc.hasFloating, cfg)) | 1501 | + | got := copyTargetRectLocked(layoutSnap, contentArea, tc.pane, tc.floating, tc.hasFloating, floatingFrameGeometry) |
| 1502 | + | require.Equal(t, tc.want, got) | |||
| 1503 | + | if tc.hasFloating { | |||
| 1504 | + | require.NotEqual(t, fp.popupGeometry.Inner, got, "content-relative geometry must not target the client frame directly") | |||
| 1505 | + | } | |||
| 1478 | }) | 1506 | }) | ||
| 1479 | } | 1507 | } | ||
| 1480 | } | 1508 | } | ||