fix(tui): stopped loader spinner re-wrapping text every tick
The Loader baked the advancing braille glyph into the underlying Text via setText, so Text.render's wrap cache (keyed on text) missed every 80ms tick and re-ran wrapTextWithAnsi plus per-line width measurement over the whole message even though only a 1-cell glyph changed. On a single-subagent hub wait the direct-write render loop consumed ~48% of a saturated core. The wrapped text now carries a stable sentinel glyph (frames[0]); the spinner advance only bumps the frame index and requests a repaint, and render() swaps the visible glyph over the cached layout. wrapTextWithAnsi and stringWidth now run only when the message actually changes. Fixes #6940 (cherry picked from commit 6ec38053ebc1e33859358df3b66f71fea27532c7)
This commit is contained in:
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed the `Loader` spinner pegging a CPU core during idle waits: advancing the braille glyph baked it into the underlying `Text` via `setText`, invalidating the wrap cache every 80 ms tick so `wrapTextWithAnsi` and per-line width measurement re-ran over the whole message. The wrapped text now carries a stable sentinel glyph and only the visible glyph is swapped at render time, so the wrap/width pipeline runs only when the message changes ([#6940](https://github.com/can1357/oh-my-pi/issues/6940)).
|
||||
|
||||
## [17.1.8] - 2026-07-28
|
||||
|
||||
### Fixed
|
||||
|
||||
@@ -58,12 +58,17 @@ export class Loader extends Text {
|
||||
}
|
||||
|
||||
const frame = this.#frames[this.#currentFrame];
|
||||
// The wrapped text carries a fixed sentinel glyph (frames[0]); advancing
|
||||
// the spinner swaps only the visible glyph here, so the wrap/width cache
|
||||
// stays valid across ticks. Assumes every frame shares one display width
|
||||
// (true for the default braille set and all in-repo callers).
|
||||
const sentinel = this.#frames[0];
|
||||
const lines = [""];
|
||||
const layout = this.#layout ?? [];
|
||||
for (let i = 0; i < layout.length; i++) {
|
||||
const { leading, content, trailing } = layout[i];
|
||||
if (i === 0 && content.startsWith(frame)) {
|
||||
const remainder = content.slice(frame.length);
|
||||
if (i === 0 && content.startsWith(sentinel)) {
|
||||
const remainder = content.slice(sentinel.length);
|
||||
const separator = remainder.startsWith(" ") ? " " : "";
|
||||
const message = remainder.slice(separator.length);
|
||||
lines.push(
|
||||
@@ -78,7 +83,8 @@ export class Loader extends Text {
|
||||
|
||||
start() {
|
||||
this.#lastSpinnerTick = performance.now();
|
||||
this.#updateDisplay();
|
||||
this.#syncText();
|
||||
this.#requestPaint();
|
||||
const intervalMs = this.messageColorFn.animated === true ? RENDER_INTERVAL_MS : SPINNER_ADVANCE_MS;
|
||||
this.#intervalId = setInterval(() => {
|
||||
const now = performance.now();
|
||||
@@ -90,7 +96,7 @@ export class Loader extends Text {
|
||||
this.#lastSpinnerTick += steps * SPINNER_ADVANCE_MS;
|
||||
}
|
||||
if (shouldAdvanceSpinner || this.#ui?.synchronizedOutput === true) {
|
||||
this.#updateDisplay();
|
||||
this.#requestPaint();
|
||||
}
|
||||
}, intervalMs);
|
||||
}
|
||||
@@ -112,22 +118,27 @@ export class Loader extends Text {
|
||||
return;
|
||||
}
|
||||
this.message = message;
|
||||
this.#updateDisplay();
|
||||
this.#syncText();
|
||||
this.#requestPaint();
|
||||
}
|
||||
|
||||
#updateDisplay() {
|
||||
const frame = this.#frames[this.#currentFrame];
|
||||
const textChanged = this.setText(`${frame} ${this.message}`);
|
||||
if ((textChanged || this.messageColorFn.animated === true) && this.#ui) {
|
||||
// Direct write: a loader tick changes only this component, so the TUI
|
||||
// can update the already-positioned rows without driving the full
|
||||
// compose/prepare/diff pipeline. Lightweight test stubs may not carry
|
||||
// the newer API; keep their legacy component-scoped path working.
|
||||
if (typeof this.#ui.requestDirectWrite === "function") {
|
||||
this.#ui.requestDirectWrite(this);
|
||||
} else {
|
||||
this.#ui.requestComponentRender(this);
|
||||
}
|
||||
/** Re-wrap the underlying Text with the stable sentinel glyph plus message. */
|
||||
#syncText(): boolean {
|
||||
return this.setText(`${this.#frames[0]} ${this.message}`);
|
||||
}
|
||||
|
||||
#requestPaint() {
|
||||
if (!this.#ui) {
|
||||
return;
|
||||
}
|
||||
// Direct write: a loader tick changes only this component, so the TUI can
|
||||
// update the already-positioned rows without driving the full
|
||||
// compose/prepare/diff pipeline. Lightweight test stubs may not carry the
|
||||
// newer API; keep their legacy component-scoped path working.
|
||||
if (typeof this.#ui.requestDirectWrite === "function") {
|
||||
this.#ui.requestDirectWrite(this);
|
||||
} else {
|
||||
this.#ui.requestComponentRender(this);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -157,6 +157,32 @@ describe("Loader component", () => {
|
||||
loader.stop();
|
||||
});
|
||||
|
||||
it("reuses the wrapped layout across static spinner frames without re-measuring", () => {
|
||||
vi.useFakeTimers();
|
||||
const ui = { synchronizedOutput: true, requestDirectWrite: vi.fn(), requestComponentRender: vi.fn() };
|
||||
const loader = new Loader(
|
||||
ui as unknown as TUI,
|
||||
s => s,
|
||||
m => m,
|
||||
"Checking",
|
||||
["⠋", "⠙", "⠹"],
|
||||
);
|
||||
const stringWidth = spyOn(Bun, "stringWidth");
|
||||
|
||||
const initial = loader.render(40);
|
||||
stringWidth.mockClear();
|
||||
vi.advanceTimersByTime(80);
|
||||
const advanced = loader.render(40);
|
||||
|
||||
// Advancing the spinner glyph must not re-run the wrap/width pipeline:
|
||||
// only the leading 1-cell glyph changed, so the cached layout stands.
|
||||
expect(stringWidth).not.toHaveBeenCalled();
|
||||
expect(advanced[1]).not.toBe(initial[1]);
|
||||
expect(advanced[1]).toContain("⠙ Checking");
|
||||
expect(visibleWidth(initial[1])).toBe(visibleWidth(advanced[1]));
|
||||
loader.stop();
|
||||
});
|
||||
|
||||
it("holds animated message-only frames when synchronized output is unavailable", () => {
|
||||
vi.useFakeTimers();
|
||||
setSystemTime(new Date(1_000));
|
||||
|
||||
Reference in New Issue
Block a user