Merge pull request #3553 from Tibsfox/fix/cmux-image-rerender-loop
fix(tui): break infinite re-render loop for images in cmux
This commit is contained in:
commit
53cd0bf892
3 changed files with 61 additions and 4 deletions
|
|
@ -3,6 +3,7 @@ import {
|
|||
Container,
|
||||
getCapabilities,
|
||||
Image,
|
||||
type ImageDimensions,
|
||||
imageFallback,
|
||||
Spacer,
|
||||
Text,
|
||||
|
|
@ -88,6 +89,9 @@ export class ToolExecutionComponent extends Container {
|
|||
private editDiffArgsKey?: string; // Track which args the preview is for
|
||||
// Cached converted images for Kitty protocol (which requires PNG), keyed by index
|
||||
private convertedImages: Map<number, { data: string; mimeType: string }> = new Map();
|
||||
// Cached resolved image dimensions to avoid re-triggering async parsing
|
||||
// when updateDisplay() recreates Image components (#3455).
|
||||
private resolvedImageDimensions: Map<number, ImageDimensions> = new Map();
|
||||
// Incremental syntax highlighting cache for write tool call args
|
||||
private writeHighlightCache?: WriteHighlightCache;
|
||||
// When true, this component intentionally renders no lines
|
||||
|
|
@ -481,16 +485,28 @@ export class ToolExecutionComponent extends Container {
|
|||
const spacer = new Spacer(1);
|
||||
this.addChild(spacer);
|
||||
this.imageSpacers.push(spacer);
|
||||
// Pass cached dimensions to avoid re-triggering async parsing
|
||||
// when updateDisplay() recreates Image components (#3455).
|
||||
const cachedDims = this.resolvedImageDimensions.get(i);
|
||||
const imageComponent = new Image(
|
||||
imageData,
|
||||
imageMimeType,
|
||||
{ fallbackColor: (s: string) => theme.fg("toolOutput", s) },
|
||||
{ maxWidthCells: 60 },
|
||||
cachedDims,
|
||||
);
|
||||
imageComponent.setOnDimensionsResolved(() => {
|
||||
this.updateDisplay();
|
||||
this.ui.requestRender();
|
||||
});
|
||||
if (!cachedDims) {
|
||||
const imgIdx = i;
|
||||
imageComponent.setOnDimensionsResolved(() => {
|
||||
// Cache resolved dimensions so future updateDisplay() calls
|
||||
// don't re-trigger async parsing → infinite loop (#3455).
|
||||
const dims = imageComponent.getDimensions?.();
|
||||
if (dims) this.resolvedImageDimensions.set(imgIdx, dims);
|
||||
// Just re-render — don't call updateDisplay() which would
|
||||
// destroy and recreate all Image components.
|
||||
this.ui.requestRender();
|
||||
});
|
||||
}
|
||||
this.imageComponents.push(imageComponent);
|
||||
this.addChild(imageComponent);
|
||||
}
|
||||
|
|
|
|||
36
packages/pi-tui/src/components/image.test.ts
Normal file
36
packages/pi-tui/src/components/image.test.ts
Normal file
|
|
@ -0,0 +1,36 @@
|
|||
/**
|
||||
* Regression test for #3455: Image component must not trigger infinite
|
||||
* re-render loop when dimensions resolve in cmux sessions.
|
||||
*/
|
||||
|
||||
import { describe, test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { Image } from "./image.js";
|
||||
|
||||
describe("Image component (#3455)", () => {
|
||||
const theme = { fallbackColor: (s: string) => s };
|
||||
|
||||
test("getDimensions returns undefined before resolution", () => {
|
||||
// Pass explicit dimensions to avoid async parsing
|
||||
const img = new Image("base64data", "image/png", theme, {});
|
||||
// Without explicit dims, getDimensions should be undefined until async resolve
|
||||
// But we can't easily test async here, so verify the method exists
|
||||
assert.equal(typeof img.getDimensions, "function");
|
||||
});
|
||||
|
||||
test("getDimensions returns dimensions when provided at construction", () => {
|
||||
const dims = { widthPx: 100, heightPx: 200 };
|
||||
const img = new Image("base64data", "image/png", theme, {}, dims);
|
||||
const result = img.getDimensions();
|
||||
assert.deepEqual(result, dims, "Should return provided dimensions");
|
||||
});
|
||||
|
||||
test("onDimensionsResolved callback is not called when dimensions provided", () => {
|
||||
let callCount = 0;
|
||||
const dims = { widthPx: 100, heightPx: 200 };
|
||||
const img = new Image("base64data", "image/png", theme, {}, dims);
|
||||
img.setOnDimensionsResolved(() => { callCount++; });
|
||||
// With pre-resolved dims, the async path is skipped entirely
|
||||
assert.equal(callCount, 0, "Callback should not fire for pre-resolved dimensions");
|
||||
});
|
||||
});
|
||||
|
|
@ -72,6 +72,11 @@ export class Image implements Component {
|
|||
return this.imageId;
|
||||
}
|
||||
|
||||
/** Get the resolved image dimensions (for caching across recreations). */
|
||||
getDimensions(): ImageDimensions | undefined {
|
||||
return this.dimensionsResolved ? this.dimensions : undefined;
|
||||
}
|
||||
|
||||
invalidate(): void {
|
||||
this.cachedLines = undefined;
|
||||
this.cachedWidth = undefined;
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue