Skip to content

Commit 4cedad6

Browse files
stkclaude
authored andcommitted
fix(cell): draw spaces into a freshly cleared screen
After Init or Resume, the first frame skips every cell that the application fills with a space in the default style. The terminal keeps showing the blank cells left by clearing the screen. Most terminals draw a blank cell and a space the same way, but kitty does not. It draws a symbol from the private use area, such as a Nerd Font icon, over two cells only if an actual space follows it, and scales it down to one cell otherwise. File icons in lazygit come out too small for this reason until something redraws the cells after them. See kovidgoyal/kitty#10790 for an analysis. Marking a cell dirty changes lastStr so that it differs from currStr. It becomes an empty string, or a space if the cell has never been drawn. If the application then puts that same value into the cell without changing its style, Dirty sees no difference and the cell is not drawn. At startup and after Resume, CellBuffer.Resize marks all cells dirty while they are still empty, so the first frame skips every space in the default style. This is a regression. The space marker for cells that have never been drawn came with gdamore#1108 (v3.4.0), and it was meant to force a redraw of such cells. Startup has skipped the spaces since then. Resume has skipped them since gdamore#1176 (v3.6.0) made Resize mark new cells dirty. Record a redraw in a flag of its own, so that nothing put into the cell can make it clean before it is drawn. The first frame after Init or Resume now writes every blank cell again, as v3.3.0 did. This is about as much output as a redraw after resizing the window. For lazygit in a 250x70 terminal, the output at startup grows from 10 KB to 25 KB, and a single resize redraw is 23 KB. Later frames are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent d3a071c commit 4cedad6

3 files changed

Lines changed: 76 additions & 12 deletions

File tree

‎cell.go‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,15 @@ type cell struct {
2323
lastStyle Style
2424
width int
2525
lock bool
26-
lastWidth int8 // width of lastStr, or 0 when lastStr marks a redraw
26+
redraw bool // draw the cell even if its content and style are unchanged
27+
lastWidth int8 // width of lastStr, or 0 while redraw is set
2728
}
2829

2930
func (c *cell) setDirty(dirty bool) {
31+
c.redraw = dirty
3032
if dirty {
31-
// Empty cells use currStr == "" until they are first drawn, at which
32-
// point SetDirty(false) normalizes them to a space. Using "" as the
33-
// dirty marker for an untouched empty cell would therefore leave
34-
// lastStr == currStr and fail to force a redraw.
35-
if c.currStr == "" {
36-
c.lastStr = " "
37-
} else {
38-
c.lastStr = ""
39-
}
33+
// What the terminal shows is unknown until the cell is drawn.
34+
c.lastStr = ""
4035
c.lastWidth = 0
4136
} else {
4237
if c.currStr == "" {
@@ -159,14 +154,18 @@ func (cb *CellBuffer) Invalidate() {
159154
}
160155

161156
// Dirty checks if a character at the given location needs to be
162-
// refreshed on the physical display. This returns true if the cell
163-
// content is different since the last time it was marked clean.
157+
// refreshed on the physical display. This returns true if the cell was
158+
// marked dirty, or if its content is different since the last time it
159+
// was marked clean.
164160
func (cb *CellBuffer) Dirty(x, y int) bool {
165161
if x >= 0 && y >= 0 && x < cb.w && y < cb.h {
166162
c := &cb.cells[(y*cb.w)+x]
167163
if c.lock {
168164
return false
169165
}
166+
if c.redraw {
167+
return true
168+
}
170169
if c.lastStyle != c.currStyle {
171170
return true
172171
}

‎cell_test.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,26 @@ func TestCellBufferDirtyState(t *testing.T) {
113113
}
114114
}
115115

116+
// A cell that is marked dirty must be drawn even if a blank is put into it
117+
// before the next draw.
118+
func TestCellBufferBlankKeepsDirtyCellDirty(t *testing.T) {
119+
for _, drawn := range []bool{false, true} {
120+
for _, blank := range []string{"", " "} {
121+
cb := &CellBuffer{}
122+
cb.Resize(1, 1) // marks the cell dirty
123+
if drawn {
124+
cb.Put(0, 0, "x", StyleDefault)
125+
cb.SetDirty(0, 0, false)
126+
cb.Invalidate()
127+
}
128+
cb.Put(0, 0, blank, StyleDefault)
129+
if !cb.Dirty(0, 0) {
130+
t.Errorf("drawn=%v, put %q: the cell is no longer dirty", drawn, blank)
131+
}
132+
}
133+
}
134+
}
135+
116136
func TestCellBufferLockResizeFill(t *testing.T) {
117137
cb := &CellBuffer{w: 2, h: 2, cells: make([]cell, 4)}
118138
cb.Fill('x', StyleDefault)

‎tscreen_test.go‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -775,6 +775,51 @@ func TestUnlockRegionRedrawsUntouchedBlankCell(t *testing.T) {
775775
}
776776
}
777777

778+
// After the screen is cleared, the terminal shows blank cells, not spaces.
779+
// Some terminals treat the two differently. kitty, for example, draws a
780+
// private use symbol such as a Nerd Font icon over two cells only if an
781+
// actual space follows it. So the first frame after clearing the screen
782+
// must write the spaces it puts, both at startup and after resuming.
783+
func TestSpacesAreDrawnAfterClearingTheScreen(t *testing.T) {
784+
t.Setenv("LC_ALL", "en_US.UTF-8")
785+
frame := func(scr Screen) {
786+
scr.Clear()
787+
scr.Put(0, 0, "\uf313", StyleDefault)
788+
scr.Put(1, 0, " ", StyleDefault)
789+
scr.Put(2, 0, "a", StyleDefault)
790+
scr.Show()
791+
}
792+
expectSpaceAfterSymbol := func(t *testing.T, mt vt.MockTerm) {
793+
t.Helper()
794+
if got := mt.GetCell(vt.Coord{X: 1}); got.C != " " {
795+
t.Fatalf("terminal shows %q after the symbol, want a space", got.C)
796+
}
797+
}
798+
799+
t.Run("startup", func(t *testing.T) {
800+
mt, scr := NewMockScreen(t, vt.MockOptSize{X: 3, Y: 1})
801+
defer scr.Fini()
802+
803+
frame(scr)
804+
expectSpaceAfterSymbol(t, mt)
805+
})
806+
807+
t.Run("resume", func(t *testing.T) {
808+
mt, scr := NewMockScreen(t, vt.MockOptSize{X: 3, Y: 1})
809+
defer scr.Fini()
810+
811+
frame(scr)
812+
if err := scr.Suspend(); err != nil {
813+
t.Fatalf("failed to suspend screen: %v", err)
814+
}
815+
if err := scr.Resume(); err != nil {
816+
t.Fatalf("failed to resume screen: %v", err)
817+
}
818+
frame(scr)
819+
expectSpaceAfterSymbol(t, mt)
820+
})
821+
}
822+
778823
// Redrawing an unchanged wide character after Clear, as immediate-mode
779824
// apps do every frame, must not send it again; a changed one must be sent.
780825
func TestWideCharAfterClear(t *testing.T) {

0 commit comments

Comments
 (0)