feat: enhance Unicode placeholder handling and protocol detection for Kitty graphics

Author: blacktop

cmd/gallery/main.go

@@ -355,7 +355,8 @@ func (m *model) renderImageForTUI(filename string) (string, int) {
 
 	// Apply virtual mode if enabled
 	if m.virtualMode && termimg.KittySupported() {
-		img = img.Protocol(termimg.Kitty).Virtual(true)
+		// Virtual mode rendering in gallery relies on Unicode placeholders for in-band display.
+		img = img.Protocol(termimg.Kitty).UseUnicode(true)
 	}
 
 	// Render the image to get the escape codes

detect.go

@@ -312,6 +312,9 @@ func ParallelProtocolDetection() (kitty, sixel, iterm2 bool) {
 		results.kitty = true
 	}
 
+	// Populate Sixel from environment before early-returning for Kitty/iTerm hints.
+	results.sixel = DetectSixelFromEnvironment()
+
 	// If we've already detected a graphics protocol from environment variables,
 	// skip all terminal queries to avoid leaving garbage in the input buffer.
 	// This prevents issues with TUI frameworks like bubbletea that read from stdin.

detect_test.go

@@ -163,6 +163,29 @@ func TestParallelProtocolDetection(t *testing.T) {
 	// (but this might fail in CI environments, so just check types)
 }
 
+func TestParallelProtocolDetectionPreservesSixelWhenKittyDetectedFromEnv(t *testing.T) {
+	wasTmuxForced := IsTmuxForced()
+	defer ForceTmux(wasTmuxForced)
+	ForceTmux(false)
+
+	// Normalize all relevant env hints so this assertion is deterministic.
+	t.Setenv("TERM", "xterm-256color")
+	t.Setenv("TERM_PROGRAM", "WezTerm")
+	t.Setenv("KITTY_WINDOW_ID", "")
+	t.Setenv("XTERM_VERSION", "")
+	t.Setenv("TMUX", "")
+	t.Setenv("GHOSTTY_RESOURCES_DIR", "")
+	t.Setenv("WEZTERM_EXECUTABLE", "")
+	t.Setenv("ITERM_SESSION_ID", "")
+	t.Setenv("LC_TERMINAL", "")
+	t.Setenv("TERM_SESSION_ID", "")
+
+	kitty, sixel, iterm2 := ParallelProtocolDetection()
+	assert.True(t, kitty, "WezTerm should be detected as kitty-capable from env hints")
+	assert.True(t, sixel, "Sixel env detection should still run before early return")
+	assert.False(t, iterm2, "WezTerm env detection path should not force iTerm2 support")
+}
+
 func TestProtocolStrings(t *testing.T) {
 	tests := []struct {
 		protocol Protocol

kitty.go

@@ -84,8 +84,9 @@ type KittyOptions struct {
 // Initialized with process ID + timestamp to prevent conflicts between program runs
 var globalKittyImageID = uint32(os.Getpid()<<16) + uint32(time.Now().UnixMicro()&0xFFFF)
 
-// Global image number counter for Unicode placeholders - must fit in 24 bits for RGB encoding
-// Starts at 1 to avoid 0 (which would be invisible in some color modes)
+// Global image number counter for Unicode placeholders.
+// IDs are 32-bit and encoded as: high byte via optional id_extra diacritic, low 24 bits via RGB.
+// Starts at 1 to avoid 0 (which can be ambiguous in some color mappings).
 var globalKittyImageNum uint32 = 1
 
 // KittyRenderer implements the Renderer interface for Kitty graphics protocol
@@ -355,12 +356,6 @@ func (r *KittyRenderer) Render(img image.Image, opts RenderOptions) (string, err
 
 	// Handle non-unicode Kitty options
 	if kittyOpts != nil {
-		if opts.Virtual && !kittyOpts.UseUnicode {
-			// Non-unicode virtual placement - just generate simple placeholders
-			placeholders := r.generateUnicodePlaceholders(imageID, cols, rows)
-			output.WriteString(placeholders)
-		}
-
 		// Handle animation after image transfer
 		if kittyOpts.Animation != nil && len(kittyOpts.Animation.ImageIDs) > 0 {
 			// TODO: Animation is handled separately after all images are transferred
@@ -497,11 +492,6 @@ func (r *KittyRenderer) PlaceImageWithSize(imageID string, xCells, yCells, zInde
 	// specific newline behavior from shifting rows back to column 1.
 	output.WriteString(renderAnchoredPlaceholderArea(imgID, xCells, yCells, widthCells, heightCells))
 
-	if inTmux() {
-		result := wrapTmuxPassthrough(output.String())
-		_, err := io.WriteString(os.Stdout, result)
-		return err
-	}
 	_, err := io.WriteString(os.Stdout, output.String())
 	return err
 }
@@ -514,20 +504,14 @@ func renderAnchoredPlaceholderArea(imageID uint32, xCells, yCells, widthCells, h
 		return ""
 	}
 
-	area := CreatePlaceholderArea(imageID, uint16(heightCells), uint16(widthCells))
-
-	r := (imageID >> 16) & 0xFF
-	g := (imageID >> 8) & 0xFF
-	b := imageID & 0xFF
-	colorStart := fmt.Sprintf("\x1b[38;2;%d;%d;%dm", r, g, b)
+	idExtra := byte(imageID >> 24)
+	colorStart := placeholderColorStart(imageID)
 
 	var builder strings.Builder
-	for rowIdx, row := range area {
+	for rowIdx := range heightCells {
 		builder.WriteString(fmt.Sprintf("\x1b[%d;%dH", yCells+rowIdx+1, xCells+1))
 		builder.WriteString(colorStart)
-		for _, placeholder := range row {
-			builder.WriteString(placeholder)
-		}
+		writeInheritedPlaceholderRow(&builder, CreatePlaceholder(uint16(rowIdx), 0, idExtra), widthCells)
 		builder.WriteString("\x1b[39m")
 	}
 	return builder.String()
@@ -676,9 +660,9 @@ func diacritic(pos uint16) rune {
 	return kittyDiacritics[pos]
 }
 
-// CreatePlaceholder generates a Unicode placeholder for the given image position
-// placeholder_char + row_diacritic + column_diacritic + id_extra_diacritic
-func CreatePlaceholder(row, column uint16, id_extra byte) string {
+// CreatePlaceholder generates a Unicode placeholder for the given image position.
+// Kitty placeholder encoding uses row, column, and extra-id diacritics.
+func CreatePlaceholder(row, column uint16, idExtra byte) string {
 	var builder strings.Builder
 
 	// Add the placeholder character
@@ -687,26 +671,42 @@ func CreatePlaceholder(row, column uint16, id_extra byte) string {
 	// Add diacritical marks for row and column
 	builder.WriteRune(diacritic(row))
 	builder.WriteRune(diacritic(column))
-	// Add diacritic for the extra ID byte
-	builder.WriteRune(diacritic(uint16(id_extra)))
+	builder.WriteRune(diacritic(uint16(idExtra)))
 
 	return builder.String()
 }
 
 // CreatePlaceholderArea generates a grid of placeholders for an image
 func CreatePlaceholderArea(imageID uint32, rows, columns uint16) [][]string {
-	id_extra := byte(imageID >> 24)
+	idExtra := byte(imageID >> 24)
 	area := make([][]string, rows)
-	for r := range rows {
-		area[r] = make([]string, columns)
-		for c := range columns {
-			// Use 0-based indexing for row and column positions
-			area[r][c] = CreatePlaceholder(r, c, id_extra)
+	for row := range rows {
+		area[row] = make([]string, columns)
+		for column := range columns {
+			area[row][column] = CreatePlaceholder(row, column, idExtra)
 		}
 	}
 	return area
 }
 
+func placeholderColorStart(imageID uint32) string {
+	r := (imageID >> 16) & 0xFF
+	g := (imageID >> 8) & 0xFF
+	b := imageID & 0xFF
+	return fmt.Sprintf("\x1b[38;2;%d;%d;%dm", r, g, b)
+}
+
+func writeInheritedPlaceholderRow(builder *strings.Builder, firstPlaceholder string, width int) {
+	if width <= 0 {
+		return
+	}
+
+	builder.WriteString(firstPlaceholder)
+	for colIdx := 1; colIdx < width; colIdx++ {
+		builder.WriteString(PLACEHOLDER_CHAR)
+	}
+}
+
 // RenderPlaceholderAreaWithImageID converts a placeholder area to a string with proper positioning.
 // The placeholders are rendered inline at the current cursor position and will scroll with content.
 // NOTE: We intentionally do NOT save/restore cursor position - the placeholders ARE the content
@@ -716,18 +716,17 @@ func RenderPlaceholderAreaWithImageID(area [][]string, imageID uint32) string {
 
 	// Encode ID in truecolor RGB: (R<<16 | G<<8 | B) == imageID&0xFFFFFF.
 	// Using 38;2 avoids palette-index ambiguity from 38;5 in terminals.
-	r := (imageID >> 16) & 0xFF
-	g := (imageID >> 8) & 0xFF
-	b := imageID & 0xFF
-	colorStart := fmt.Sprintf("\x1b[38;2;%d;%d;%dm", r, g, b)
+	colorStart := placeholderColorStart(imageID)
 
 	// Set foreground color to encode image ID
 	builder.WriteString(colorStart)
 
 	// Render each row
 	for rowIdx, row := range area {
-		for _, placeholder := range row {
-			builder.WriteString(placeholder)
+		if len(row) > 0 {
+			// Use inherited row/column/id diacritics after the first placeholder
+			// to match Kitty Unicode rendering behavior across terminals.
+			writeInheritedPlaceholderRow(&builder, row[0], len(row))
 		}
 		// After each row (except last), reset color, newline, then restore color
 		// This ensures proper color handling across line boundaries

kitty_test.go

@@ -6,9 +6,12 @@ import (
 	"encoding/base64"
 	"fmt"
 	"image"
+	"io"
+	"os"
 	"strconv"
 	"strings"
 	"testing"
+	"unicode/utf8"
 
 	"github.com/stretchr/testify/assert"
 )
@@ -36,6 +39,51 @@ func extractFirstKittyImageID(output string) (uint32, error) {
 	return uint32(id), nil
 }
 
+func captureStdout(t *testing.T, fn func() error) (string, error) {
+	t.Helper()
+
+	oldStdout := os.Stdout
+	readPipe, writePipe, err := os.Pipe()
+	if err != nil {
+		return "", fmt.Errorf("failed to create stdout pipe: %w", err)
+	}
+
+	os.Stdout = writePipe
+	var runErr error
+	var panicValue any
+	func() {
+		defer func() {
+			panicValue = recover()
+		}()
+		runErr = fn()
+	}()
+
+	os.Stdout = oldStdout
+	closeErr := writePipe.Close()
+	if closeErr != nil {
+		_ = readPipe.Close()
+		if panicValue != nil {
+			panic(panicValue)
+		}
+		return "", fmt.Errorf("failed to close write pipe: %w", closeErr)
+	}
+
+	data, readErr := io.ReadAll(readPipe)
+	_ = readPipe.Close()
+	if readErr != nil {
+		if panicValue != nil {
+			panic(panicValue)
+		}
+		return "", fmt.Errorf("failed to read stdout pipe: %w", readErr)
+	}
+
+	if panicValue != nil {
+		panic(panicValue)
+	}
+
+	return string(data), runErr
+}
+
 func TestKittyZlibCompression(t *testing.T) {
 	img := image.NewRGBA(image.Rect(0, 0, 1, 1))
 	opts := RenderOptions{
@@ -170,7 +218,7 @@ func TestKittyUnicodeHonorsImageNumber(t *testing.T) {
 	assert.Equal(t, uint32(42), renderer.GetLastImageID(), "last image ID should match caller-provided Unicode image number")
 }
 
-func TestKittyUnicodeUsesPngTransferWithPlacementCommand(t *testing.T) {
+func TestKittyUnicodeUsesRawTransferWithPlacementCommandByDefault(t *testing.T) {
 	img := image.NewRGBA(image.Rect(0, 0, 16, 16))
 	opts := RenderOptions{
 		KittyOpts: &KittyOptions{
@@ -186,10 +234,51 @@ func TestKittyUnicodeUsesPngTransferWithPlacementCommand(t *testing.T) {
 	renderer := &KittyRenderer{}
 	output, err := renderer.Render(img, opts)
 	assert.NoError(t, err)
-	assert.Contains(t, output, "f=100,t=d,i=42", "Unicode path should transmit image data using PNG transfer")
+	assert.Contains(t, output, "f=32,s=16,v=16,t=d,i=42", "Unicode path should transmit image data using raw RGBA by default")
 	assert.Contains(t, output, "a=p,U=1,i=42", "Unicode path should emit explicit virtual placement command")
 }
 
+func TestKittyUnicodeUsesPngTransferWhenRequested(t *testing.T) {
+	img := image.NewRGBA(image.Rect(0, 0, 16, 16))
+	opts := RenderOptions{
+		KittyOpts: &KittyOptions{
+			UseUnicode: true,
+			ImageNum:   42,
+			PNG:        true,
+		},
+		features: &TerminalFeatures{
+			FontWidth:  8,
+			FontHeight: 16,
+		},
+	}
+
+	renderer := &KittyRenderer{}
+	output, err := renderer.Render(img, opts)
+	assert.NoError(t, err)
+	assert.Contains(t, output, "f=100,t=d,i=42", "Unicode path should transmit PNG when explicitly requested")
+	assert.Contains(t, output, "a=p,U=1,i=42", "Unicode path should emit explicit virtual placement command")
+}
+
+func TestKittyVirtualWithoutUnicodeDoesNotEmitPlaceholders(t *testing.T) {
+	img := image.NewRGBA(image.Rect(0, 0, 16, 16))
+	opts := RenderOptions{
+		Virtual: true,
+		KittyOpts: &KittyOptions{
+			UseUnicode: false,
+		},
+		features: &TerminalFeatures{
+			FontWidth:  8,
+			FontHeight: 16,
+		},
+	}
+
+	renderer := &KittyRenderer{}
+	output, err := renderer.Render(img, opts)
+	assert.NoError(t, err)
+	assert.Contains(t, output, "U=1", "virtual transfer should still create a virtual placement")
+	assert.NotContains(t, output, PLACEHOLDER_CHAR, "non-unicode virtual transfer should not append placeholder text")
+}
+
 func TestProcessImageUnicodeHonorsExplicitResize(t *testing.T) {
 	img := image.NewRGBA(image.Rect(0, 0, 20, 20))
 	opts := RenderOptions{
@@ -278,6 +367,15 @@ func TestRenderPlaceholderAreaWithImageIDUsesTruecolorForLowIDs(t *testing.T) {
 	assert.NotContains(t, rendered, "\x1b[38;5;1m", "palette mode should not be used for ID encoding")
 }
 
+func TestRenderPlaceholderAreaWithImageIDUsesInheritedPlaceholders(t *testing.T) {
+	area := CreatePlaceholderArea(1, 1, 3)
+	rendered := RenderPlaceholderAreaWithImageID(area, 1)
+
+	assert.Contains(t, rendered, CreatePlaceholder(0, 0, 0)+PLACEHOLDER_CHAR+PLACEHOLDER_CHAR)
+	assert.NotContains(t, rendered, CreatePlaceholder(0, 1, 0))
+	assert.NotContains(t, rendered, CreatePlaceholder(0, 2, 0))
+}
+
 func TestRenderAnchoredPlaceholderAreaPositionsEveryRow(t *testing.T) {
 	rendered := renderAnchoredPlaceholderArea(1, 5, 5, 2, 3)
 	assert.Contains(t, rendered, "\x1b[6;6H")
@@ -287,7 +385,38 @@ func TestRenderAnchoredPlaceholderAreaPositionsEveryRow(t *testing.T) {
 	assert.Equal(t, 3, strings.Count(rendered, "\x1b[39m"))
 }
 
+func TestRenderAnchoredPlaceholderAreaUsesInheritedPlaceholders(t *testing.T) {
+	rendered := renderAnchoredPlaceholderArea(1, 0, 0, 3, 1)
+
+	assert.Contains(t, rendered, CreatePlaceholder(0, 0, 0)+PLACEHOLDER_CHAR+PLACEHOLDER_CHAR)
+	assert.NotContains(t, rendered, CreatePlaceholder(0, 1, 0))
+	assert.NotContains(t, rendered, CreatePlaceholder(0, 2, 0))
+}
+
 func TestRenderAnchoredPlaceholderAreaEmptyWhenInvalidDimensions(t *testing.T) {
 	assert.Equal(t, "", renderAnchoredPlaceholderArea(1, 0, 0, 0, 3))
 	assert.Equal(t, "", renderAnchoredPlaceholderArea(1, 0, 0, 3, 0))
 }
+
+func TestPlaceImageWithSizeDoesNotWrapPlaceholderTextInTmuxPassthrough(t *testing.T) {
+	ForceTmux(true)
+	defer ForceTmux(false)
+
+	renderer := &KittyRenderer{}
+	output, err := captureStdout(t, func() error {
+		return renderer.PlaceImageWithSize("42", 1, 2, 0, 2, 1)
+	})
+	assert.NoError(t, err)
+	assert.Contains(t, output, "\x1b[3;2H", "place command should move cursor to requested absolute coordinates")
+	assert.NotContains(t, output, "\x1bPtmux;\x1b", "placeholder text should remain in-band and not be tmux passthrough wrapped")
+}
+
+func TestCreatePlaceholderIncludesExtraDiacriticFor24BitIDs(t *testing.T) {
+	placeholder := CreatePlaceholder(1, 2, 0)
+	assert.Equal(t, 4, utf8.RuneCountInString(placeholder), "24-bit IDs should still include the extra-id diacritic")
+}
+
+func TestCreatePlaceholderIncludesExtraDiacriticFor32BitIDs(t *testing.T) {
+	placeholder := CreatePlaceholder(1, 2, 1)
+	assert.Equal(t, 4, utf8.RuneCountInString(placeholder), "32-bit IDs should include the high-byte diacritic")
+}