Fix framebuffer console: scroll drift causes progressive line overlap in TTF mode
fb_scroll_rows() hardcoded the pixel distance it physically shifts the framebuffer by as char_rows * 16 * scale -- the bitmap-font (font_8x16.c) cell height -- regardless of which glyph mode vt100.c actually had active. In TTF mode (the REPL's default, cell height 24px via VT100_TTF_CELL_H_PX) this meant every scroll_up(1) call physically shifted the framebuffer by only 16px while the text model (g_vt.rows, py_of()) placed each row 24px apart. That 8px-per-scroll shortfall compounds with every subsequent scroll: a few scrolls barely show it, but enough scrolls -- or scrolling quickly, which is just many scrolls in a short span -- accumulates into visible pixel overlap between rows, with newer lines drawn on top of the tail end of older ones. fb_scroll_rect() (the box-confined scroll added later for 4.4t) already carried a doc comment calling this out explicitly, describing its own explicit pixel_rows parameter as the fix for fb_scroll_rows()'s "fixed 16px-row assumption" -- fb_scroll_rows() itself was just never updated to match. Fixed by changing fb_scroll_rows()'s parameter from an implicit char_rows count to an explicit pixel_rows count (matching fb_scroll_rect()'s existing convention), and having its one caller (vt100.c's scroll_up()) pass lines * cell_h() -- the real active cell height -- instead of a raw line count for the callee to guess at. Verified: booted amd64 to the REPL (TTF mode active per sk_repl()'s own console_fb_enable_ttf() call), let boot chatter + WORDS output scroll the screen through thousands of accumulated scroll_up() calls, then measured every visible line's y-position via a QMP screendump. Spacing held at a perfectly consistent 24px (TTF cell height) top to bottom with zero drift -- the old hardcoded-16px bug could not have produced that after this many scrolls. Re-verified boot to ok> on all three architectures (amd64/aarch64/riscv64) per repo acceptance policy. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
c14324f498
commit
f81e9c92bc
@@ -1,5 +1,5 @@
|
|||||||
# Capsule Block Manifest — Auto-generated
|
# Capsule Block Manifest — Auto-generated
|
||||||
<!-- Generated by mkcapsule --manifest 2026-09-02T21:10:42Z -->
|
<!-- Generated by mkcapsule --manifest 2026-09-02T21:23:27Z -->
|
||||||
<!-- DO NOT EDIT — re-run mkcapsule --manifest to refresh. -->
|
<!-- DO NOT EDIT — re-run mkcapsule --manifest to refresh. -->
|
||||||
<!-- Hand-written justifications and immutability notes live -->
|
<!-- Hand-written justifications and immutability notes live -->
|
||||||
<!-- in MANIFEST.md alongside this auto-generated index. -->
|
<!-- in MANIFEST.md alongside this auto-generated index. -->
|
||||||
|
|||||||
Binary file not shown.
@@ -100,16 +100,22 @@ void fb_draw_orientation_test(void);
|
|||||||
* --------------------------------------------------------------------- */
|
* --------------------------------------------------------------------- */
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Scroll the framebuffer up by `char_rows` character rows (each 16 px).
|
* Scroll the whole framebuffer up by `pixel_rows` pixel rows. The vacated
|
||||||
* The vacated rows at the bottom are filled with bg.
|
* rows at the bottom are filled with bg. Takes an explicit pixel-row count
|
||||||
|
* (not a hardcoded 8x16-cell assumption) so callers with a non-8x16 cell
|
||||||
|
* height (e.g. TTF mode, 24px) pass their own cell height directly --
|
||||||
|
* same convention fb_scroll_rect() below already uses, for the same reason
|
||||||
|
* (a caller-computed char_rows * fixed-16px assumption drifts out of sync
|
||||||
|
* with the text model's own row height in TTF mode, and that drift
|
||||||
|
* compounds with every scroll).
|
||||||
*/
|
*/
|
||||||
void fb_scroll_rows(uint32_t char_rows, uint32_t bg);
|
void fb_scroll_rows(uint32_t pixel_rows, uint32_t bg);
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Scroll a sub-rectangle of the framebuffer up by `pixel_rows` pixel rows
|
* Scroll a sub-rectangle of the framebuffer up by `pixel_rows` pixel rows
|
||||||
* (FABRIC.md item 4.4t: box-confined REPL scrolling). Unlike fb_scroll_rows()
|
* (FABRIC.md item 4.4t: box-confined REPL scrolling). Unlike fb_scroll_rows()
|
||||||
* (whole-framebuffer, fixed 16px-row assumption), this is bounded to
|
* (whole-framebuffer), this is bounded to
|
||||||
* [x, x+w) x [y, y+h) and takes an explicit pixel-row count so callers with
|
* [x, x+w) x [y, y+h). Both take an explicit pixel-row count so callers with
|
||||||
* a non-8x16 cell height (e.g. TTF mode) pass their own cell height directly.
|
* a non-8x16 cell height (e.g. TTF mode) pass their own cell height directly.
|
||||||
* Pixels outside the rect are untouched. The vacated rows at the bottom of
|
* Pixels outside the rect are untouched. The vacated rows at the bottom of
|
||||||
* the rect are filled with bg.
|
* the rect are filled with bg.
|
||||||
|
|||||||
File diff suppressed because it is too large
Load Diff
File diff suppressed because it is too large
Load Diff
File diff suppressed because it is too large
Load Diff
@@ -251,10 +251,18 @@ void fb_draw_orientation_test(void)
|
|||||||
* --------------------------------------------------------------------- */
|
* --------------------------------------------------------------------- */
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Scroll the framebuffer up by `char_rows` character rows.
|
* Scroll the framebuffer up by `pixel_rows` pixel rows.
|
||||||
* Each character row is (16 × scale) pixels tall.
|
|
||||||
* The vacated rows at the bottom are filled with bg.
|
* The vacated rows at the bottom are filled with bg.
|
||||||
*
|
*
|
||||||
|
* Takes a pixel-row count directly rather than a character-row count
|
||||||
|
* scaled by a hardcoded 16px cell height: the caller (vt100.c's
|
||||||
|
* scroll_up()) knows the actual active glyph cell height (16 × scale for
|
||||||
|
* bitmap mode, 24 for TTF mode), and a mismatch between the height this
|
||||||
|
* function scrolls by and the height the text model actually draws rows
|
||||||
|
* at compounds by the gap on every single scroll -- a few scrolls barely
|
||||||
|
* show it, but enough scrolls (or scrolling quickly) accumulates visible
|
||||||
|
* pixel overlap between rows.
|
||||||
|
*
|
||||||
* Copies through non-volatile pointers (FABRIC.md item 4.4g performance
|
* Copies through non-volatile pointers (FABRIC.md item 4.4g performance
|
||||||
* fix, 2026-08-11): the GOP framebuffer is mapped write-back, not
|
* fix, 2026-08-11): the GOP framebuffer is mapped write-back, not
|
||||||
* cache-disabled MMIO (vmm.c:350-363 -- "QEMU's VGA emulation is coherent
|
* cache-disabled MMIO (vmm.c:350-363 -- "QEMU's VGA emulation is coherent
|
||||||
@@ -268,7 +276,7 @@ void fb_draw_orientation_test(void)
|
|||||||
* under TCG). Casting away volatile here for the bulk copy/fill lets the
|
* under TCG). Casting away volatile here for the bulk copy/fill lets the
|
||||||
* compiler batch these loops normally.
|
* compiler batch these loops normally.
|
||||||
*/
|
*/
|
||||||
void fb_scroll_rows(uint32_t char_rows, uint32_t bg)
|
void fb_scroll_rows(uint32_t pixel_rows, uint32_t bg)
|
||||||
{
|
{
|
||||||
uint32_t pixel_rows_to_scroll;
|
uint32_t pixel_rows_to_scroll;
|
||||||
uint32_t src_y, dst_y;
|
uint32_t src_y, dst_y;
|
||||||
@@ -276,9 +284,9 @@ void fb_scroll_rows(uint32_t char_rows, uint32_t bg)
|
|||||||
uint32_t packed_bg;
|
uint32_t packed_bg;
|
||||||
uint32_t *fb = (uint32_t *)(uintptr_t)g_fb.base;
|
uint32_t *fb = (uint32_t *)(uintptr_t)g_fb.base;
|
||||||
|
|
||||||
if (!g_fb.ready || char_rows == 0) return;
|
if (!g_fb.ready || pixel_rows == 0) return;
|
||||||
|
|
||||||
pixel_rows_to_scroll = char_rows * 16u * g_fb.scale;
|
pixel_rows_to_scroll = pixel_rows;
|
||||||
if (pixel_rows_to_scroll >= g_fb.height) {
|
if (pixel_rows_to_scroll >= g_fb.height) {
|
||||||
fb_fill_rect(0, 0, g_fb.width, g_fb.height, bg);
|
fb_fill_rect(0, 0, g_fb.width, g_fb.height, bg);
|
||||||
return;
|
return;
|
||||||
|
|||||||
@@ -726,9 +726,12 @@ static void scroll_up(uint32_t lines)
|
|||||||
}
|
}
|
||||||
|
|
||||||
/* Full-framebuffer scroll -- both glyph modes cover the whole screen
|
/* Full-framebuffer scroll -- both glyph modes cover the whole screen
|
||||||
* now, so there is no box to scope this to. */
|
* now, so there is no box to scope this to. fb_scroll_rows() takes an
|
||||||
|
* explicit pixel-row count (not a char-row count it would otherwise
|
||||||
|
* have to assume a cell height for), so pass the real active cell
|
||||||
|
* height here rather than letting it guess. */
|
||||||
if (g_terminal_visible) {
|
if (g_terminal_visible) {
|
||||||
fb_scroll_rows(lines, g_vt.def_bg);
|
fb_scroll_rows(lines * cell_h(), g_vt.def_bg);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user