Skip to content

Fix progressive input lag from per-frame allocations and redundant font metrics#50

Open
yokotoka wants to merge 2 commits intoSwordfish90:masterfrom
yokotoka:fix/input-lag
Open

Fix progressive input lag from per-frame allocations and redundant font metrics#50
yokotoka wants to merge 2 commits intoSwordfish90:masterfrom
yokotoka:fix/input-lag

Conversation

@yokotoka
Copy link
Copy Markdown

@yokotoka yokotoka commented Apr 24, 2026

Hey! Love the project — been using cool-retro-term for years.

I've been investigating the progressive input lag that several users report (cool-retro-term issues #816, lxqt#483, lxqt#235) and traced part of the problem to the terminal rendering hot path in TerminalDisplay. Here are two targeted fixes:

Changes

1. Replace per-frame heap allocations with pre-allocated buffers

updateImage() was allocating and freeing two heap buffers on every call:

wchar_t *disstrU = new wchar_t[columnsToUpdate];    // every frame
char *dirtyMask = new char[columnsToUpdate+2];       // every frame
// ...
delete[] dirtyMask;
delete[] disstrU;

For an 80-column terminal, this means thousands of small allocations per second. Over hours/days of continuous use, this causes heap fragmentation that progressively slows down the allocator — which is the mechanism behind the "lag that grows over time" users report.

Fix: Use std::vector member variables that are allocated once and only resized when terminal dimensions change (typically on window resize).

2. Hoist QFontMetrics out of per-line loop

QFontMetrics fm(font()) was constructed inside the for (y = 0; y < linesToUpdate; ++y) loop in updateImage(), meaning it was recreated for every line, every frame. For a 50-line terminal, that's 50 unnecessary constructions per update.

Fix: Move to before the loop — created once per updateImage() call.

Files changed

  • lib/TerminalDisplay.h — Added std::vector member buffers, #include <vector>
  • lib/TerminalDisplay.cpp — Both optimizations above

Testing

  • Built and tested on macOS arm64 with Qt 6.11.0
  • All terminal functionality works: text rendering, cursor blinking, scrolling, selection, double-width characters, CJK input
  • Visual output is identical to before — these changes only affect performance, not rendering behavior

Related issues (in cool-retro-term)

…nt font metrics

Three targeted optimizations in the terminal rendering hot path:

1. Replace per-frame heap allocations with pre-allocated buffers
   - updateImage() allocated/freed wchar_t[] and char[] arrays every frame
   - Over long sessions this caused heap fragmentation, progressively
     slowing allocation and increasing input latency
   - Now uses std::vector member variables that resize only on terminal
     dimension changes

2. Hoist QFontMetrics construction out of per-line loop
   - QFontMetrics was recreated for every line in updateImage(), up to
     50+ times per frame for a typical terminal
   - Now created once per updateImage() call

3. Skip horizontalAdvance() for fixed-width fonts
   - For monospace terminal fonts (_fixedFont == true), character width
     is constant and already known as _fontWidth
   - Eliminates expensive font shaping engine lookups in both
     updateImage() and drawContents()
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Performance-focused change targeting TerminalDisplay’s render hot path to reduce progressive input lag by eliminating per-frame allocations and redundant font-metrics work.

Changes:

  • Replaced per-frame new[]/delete[] buffers in updateImage() with pre-allocated std::vector member buffers.
  • Hoisted QFontMetrics construction out of the per-line loop in updateImage().
  • Attempted to skip horizontalAdvance() calls when _fixedFont is true.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
lib/TerminalDisplay.h Adds persistent member buffers (std::vector) used by updateImage() to avoid per-frame heap allocations.
lib/TerminalDisplay.cpp Uses the new buffers, hoists QFontMetrics, and modifies glyph-width computations intended to reduce expensive per-character metric queries.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/TerminalDisplay.cpp Outdated
Comment thread lib/TerminalDisplay.cpp Outdated
Comment thread lib/TerminalDisplay.cpp Outdated
Comment thread lib/TerminalDisplay.cpp Outdated
The previous optimization of skipping horizontalAdvance() when
_fixedFont is true was too aggressive: it disabled the bigWidth/
smallWidth/tooWide glyph-width mismatch detection that handles
CJK characters, emoji, and font fallback glyphs that don't fit
the standard cell width.

Keep the safe optimizations (pre-allocated buffers, hoisted
QFontMetrics) and restore the original horizontalAdvance() calls.
@yokotoka
Copy link
Copy Markdown
Author

Thanks for the review! Valid points on the horizontalAdvance() caching — I was too aggressive with the optimization. When _fixedFont is true and charWidth is forced to _fontWidth, the bigWidth/smallWidth/tooWide detection becomes dead code, breaking handling of CJK characters, emoji, and font fallback glyphs that don't fit the standard cell width.

Fixed in the latest push: reverted the horizontalAdvance() caching while keeping the two safe optimizations:

  • Pre-allocated buffers (eliminates heap fragmentation over long sessions)
  • Hoisted QFontMetrics construction (created once per updateImage() instead of per-line)

These are the changes that actually address the progressive lag, without risking rendering correctness.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants