sovereign-rf/OpenRTX
1
0
Fork 0
mirror of https://github.com/OpenRTX/OpenRTX synced 2026-08-08 12:29:06 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

gfx: add gfx_printBufferClipped

Browse source 
Adds gfx_printBufferClipped: a variant of gfx_printBuffer that accepts
an explicit right-edge limit (max_x) and a vertical clip window
[clip_top_y, clip_bot_y].  Word-wrap fires when the next glyph would
exceed max_x.  Pixels outside the clip window are suppressed without
affecting layout, so a negative start.y can be used to implement smooth
text scrolling.

gfx_printBuffer now delegates entirely to gfx_printBufferClipped with
clip_top_y=0 and clip_bot_y=CONFIG_SCREEN_HEIGHT-1.  The old pixel guard
used strict > 0 comparisons, which excluded pixels at row 0 and column 0.
The new implementation renders those pixels; callers that relied on the
old exclusion will see a behaviour change.

Compared to a naive port of gfx_printBuffer, this implementation
addresses several correctness issues:

- Control characters '\n' and '\r' are checked before the glyph array
  is indexed, avoiding undefined behaviour on fonts whose first codepoint
  is at or above 0x20 (e.g. TomThumb, first=0x20), since '\n' (0x0A)
  would produce a negative index.

- get_line_size uses <= instead of < when accumulating glyph widths so
  that a glyph whose xAdvance exactly equals max_width is counted.  The
  render loop uses strict > to trigger wrap, so the old < mismatch caused
  text_size.x to be under-reported by one glyph whenever a line was an
  exact fit, and misplaced CENTER/RIGHT lines in that case.

- For TEXT_ALIGN_LEFT, line_size is recomputed on every '\n' before the
  max_line_size comparison.  Without this, max_line_size was stuck at the
  first-line width for the entire call, so text_size.x was wrong whenever
  a subsequent line was wider.

- saved_start_y is int16_t to match point_t.y, allowing correct
  text_size.y computation when start.y is negative (scroll offset).

- text_size.y is computed as (start.y - saved_start_y) + line_h,
  matching the sign fix introduced for gfx_printBuffer.

- Pixels above row 0 (py < 0) are suppressed before the uint16_t cast
  passed to gfx_setPixel, preventing a silent wrap to a large row index.

- max_line_size tracks the widest line across all line breaks and wraps;
  text_size.x reflects the widest line, not just the last.

Extend tests/unit/gfx_text.cpp with return-value tests covering:
single-line height, two-line height delta equals yAdvance, negative
start.y scroll offset, negative clip_top_y, exact-fit line width
(get_line_size <= boundary), max line width from a wider first line,
max line width from a wider second line (TEXT_ALIGN_LEFT regression),
glyph at clip_bot_y not skipped, and TEXT_ALIGN_RIGHT per-line reset.

Co-authored-by: GitHub Copilot <copilot@github.com>
Signed-off-by: Ryan Turner <ryan@turnrye.com>
This commit is contained in:
Ryan Turner 2026-05-19 20:13:27 -05:00 • committed by Silvano Seva
parent 9e472454a1
commit 64ec7a1d70
3 changed files with 345 additions and 101 deletions
Download patch file Download diff file Expand all files Collapse all files

33
openrtx/include/core/graphics.h
View file

@ -220,6 +220,11 @@ point_t gfx_printBuffer(point_t start, fontSize_t size, textAlign_t alignment,
*
* Simulates the same word-wrap line-break decisions (wrap at max_x).
* Alignment is not simulated; only the vertical extent is computed.
* Wrap decisions always use start_x as the left margin of every line,
* which matches left-aligned layout. For TEXT_ALIGN_CENTER or
* TEXT_ALIGN_RIGHT the actual rendered start.x may be larger than
* start_x, so the wrap point may differ and the returned height may
* not exactly match what gfx_printBufferClipped produces.
* Passing char_count < strlen(buf) lets callers find the y-coordinate
* of an arbitrary cursor position for scroll-offset calculations.
*
@ -237,6 +242,34 @@ point_t gfx_printBuffer(point_t start, fontSize_t size, textAlign_t alignment,
uint16_t gfx_measureText(fontSize_t size, const char *buf, uint16_t start_x,
uint16_t max_x, size_t char_count);
/**
* Prints text from a char buffer into a clipped rectangular region.
*
* Like gfx_printBuffer but with an explicit right-edge limit and a
* vertical clip window. Word-wrap fires when the next glyph would
* exceed max_x rather than CONFIG_SCREEN_WIDTH. Pixels outside the
* range [clip_top_y, clip_bot_y] are suppressed without affecting
* layout, so a negative start.y can be used to implement scrolling.
*
* The returned text_size.y reflects only the lines processed before
* clip_bot_y is reached, not the full height of the text block. Use
* gfx_measureText to obtain the total height for scroll calculations.
*
* @param start: top-left origin for the text block, in pixel coordinates.
* @param size: text font size.
* @param alignment: text alignment.
* @param color: text colour.
* @param buf: NUL-terminated string to render.
* @param max_x: right-edge pixel limit (exclusive) for word-wrap.
* @param clip_top_y: topmost pixel row to draw (inclusive).
* @param clip_bot_y: bottommost pixel row to draw (inclusive).
* @return text width and height of the processed portion as point_t.
*/
point_t gfx_printBufferClipped(point_t start, fontSize_t size,
textAlign_t alignment, color_t color,
const char *buf, uint16_t max_x,
int16_t clip_top_y, int16_t clip_bot_y);
/**
* Prints text on the screen at the specified coordinates.
* @param start: text line start point, in pixel coordinates.

208
openrtx/src/core/graphics.c
View file

@ -403,7 +403,7 @@ static inline uint16_t get_line_size(GFXfont f, const char *text,
uint16_t line_size = 0;
for(unsigned i = 0; i < length && text[i] != '\n' && text[i] != '\r'; i++)
{
if(text[i] < f.first || text[i] > f.last)
if (text[i] < f.first || text[i] > f.last)
continue;
GFXglyph glyph = f.glyph[text[i] - f.first];
if (line_size + glyph.xAdvance <= max_width)
@ -417,20 +417,22 @@ static inline uint16_t get_line_size(GFXfont f, const char *text,
/**
* Compute the start x coordinate of a new line of given pixel size
* @param alinment: enum representing the text alignment
* @param alignment: enum representing the text alignment
* @param line_size: the size of the current text line in pixels
* @param startx: left-edge x position of the text block
* @param max_width: right-edge pixel limit (width of the text region)
*/
static inline uint16_t get_reset_x(textAlign_t alignment, uint16_t line_size,
uint16_t startx)
uint16_t startx, uint16_t max_width)
{
switch(alignment)
{
case TEXT_ALIGN_LEFT:
return startx;
case TEXT_ALIGN_CENTER:
return (CONFIG_SCREEN_WIDTH - line_size)/2;
return (max_width - line_size)/2;
case TEXT_ALIGN_RIGHT:
return CONFIG_SCREEN_WIDTH - line_size - startx;
return max_width - line_size - startx;
}
return 0;
@ -446,102 +448,9 @@ uint8_t gfx_getFontHeight(fontSize_t size)
point_t gfx_printBuffer(point_t start, fontSize_t size, textAlign_t alignment,
color_t color, const char *buf)
{
GFXfont f = fonts[size];
size_t len = strlen(buf);
// Compute size of the first row in pixels
uint16_t line_size = get_line_size(f, buf, len, CONFIG_SCREEN_WIDTH);
uint16_t reset_x = get_reset_x(alignment, line_size, start.x);
start.x = reset_x;
// Save initial start.y value to calculate vertical size
int16_t saved_start_y = start.y;
uint16_t line_h = 0;
/* For each char in the string */
for(unsigned i = 0; i < len; i++)
{
char c = buf[i];
// Handle newline and carriage return before accessing glyph table
if (c == '\n')
{
if(alignment != TEXT_ALIGN_CENTER)
{
start.x = reset_x;
}
else
{
line_size = get_line_size(f, &buf[i+1], len-(i+1),
CONFIG_SCREEN_WIDTH);
start.x = reset_x = get_reset_x(alignment, line_size, start.x);
}
start.y += f.yAdvance;
continue;
}
else if (c == '\r')
{
start.x = reset_x;
continue;
}
GFXglyph glyph = f.glyph[c - f.first];
uint8_t *bitmap = f.bitmap;
uint16_t bo = glyph.bitmapOffset;
uint8_t w = glyph.width, h = glyph.height;
int8_t xo = glyph.xOffset,
yo = glyph.yOffset;
uint8_t xx, yy, bits = 0, bit = 0;
line_h = h;
// Handle wrap around
if (start.x + glyph.xAdvance > CONFIG_SCREEN_WIDTH)
{
// Compute size of the remaining text for this new line
line_size = get_line_size(f, &buf[i], len - i,
CONFIG_SCREEN_WIDTH);
// Pass reset_x (the original start x) not start.x (current pos)
start.x = reset_x = get_reset_x(alignment, line_size, reset_x);
start.y += f.yAdvance;
}
// Draw bitmap
for (yy = 0; yy < h; yy++)
{
for (xx = 0; xx < w; xx++)
{
if (!(bit++ & 7))
{
bits = bitmap[bo++];
}
if (bits & 0x80)
{
if (start.y + yo + yy < CONFIG_SCREEN_HEIGHT &&
start.x + xo + xx < CONFIG_SCREEN_WIDTH &&
start.y + yo + yy > 0 &&
start.x + xo + xx > 0)
{
point_t pos;
pos.x = start.x + xo + xx;
pos.y = start.y + yo + yy;
gfx_setPixel(pos, color);
}
}
bits <<= 1;
}
}
start.x += glyph.xAdvance;
}
// Calculate text size
point_t text_size = {0, 0};
text_size.x = line_size;
text_size.y = (start.y - saved_start_y) + line_h;
return text_size;
return gfx_printBufferClipped(start, size, alignment, color, buf,
CONFIG_SCREEN_WIDTH, 0,
CONFIG_SCREEN_HEIGHT - 1);
}
uint16_t gfx_measureText(fontSize_t size, const char *buf, uint16_t start_x,
@ -582,6 +491,103 @@ uint16_t gfx_measureText(fontSize_t size, const char *buf, uint16_t start_x,
return cur_y;
}
point_t gfx_printBufferClipped(point_t start, fontSize_t size,
textAlign_t alignment, color_t color,
const char *buf, uint16_t max_x,
int16_t clip_top_y, int16_t clip_bot_y)
{
GFXfont f = fonts[size];
size_t len = strlen(buf);
uint16_t line_size = get_line_size(f, buf, (uint16_t)len, max_x);
uint16_t max_line_size = line_size;
uint16_t origin_x = start.x;
uint16_t reset_x = get_reset_x(alignment, line_size, origin_x, max_x);
start.x = reset_x;
int16_t saved_start_y = start.y;
uint16_t line_h = 0;
for (unsigned i = 0; i < len; i++) {
/* Even the topmost pixel of the next glyph falls below clip_bot_y. */
if ((int16_t)(start.y - (int16_t)f.yAdvance) > clip_bot_y)
break;
char c = buf[i];
/* Handle control characters before accessing the glyph table. */
if (c == '\n') {
if (line_size > max_line_size)
max_line_size = line_size;
line_size = get_line_size(f, &buf[i + 1],
(uint16_t)(len - (i + 1)), max_x);
if (alignment == TEXT_ALIGN_CENTER
|| alignment == TEXT_ALIGN_RIGHT) {
start.x = reset_x = get_reset_x(alignment, line_size, origin_x,
max_x);
} else {
start.x = reset_x;
}
start.y += f.yAdvance;
continue;
} else if (c == '\r') {
start.x = reset_x;
continue;
}
if (c < f.first || c > f.last)
continue;
GFXglyph glyph = f.glyph[c - f.first];
uint8_t *bitmap = f.bitmap;
uint16_t bo = glyph.bitmapOffset;
uint8_t w = glyph.width, h = glyph.height;
int8_t xo = glyph.xOffset, yo = glyph.yOffset;
uint8_t xx, yy, bits = 0, bit = 0;
line_h = h;
/* Wrap when the glyph would exceed the right-edge limit. */
if (start.x + glyph.xAdvance > max_x) {
if (line_size > max_line_size)
max_line_size = line_size;
line_size = get_line_size(f, &buf[i], (uint16_t)(len - i), max_x);
start.x = reset_x = get_reset_x(alignment, line_size, origin_x,
max_x);
start.y += f.yAdvance;
}
/* Draw pixels, suppressing those outside the clip window. */
for (yy = 0; yy < h; yy++) {
for (xx = 0; xx < w; xx++) {
if (!(bit++ & 7))
bits = bitmap[bo++];
if (bits & 0x80) {
int16_t px = (int16_t)(start.x + xo + xx);
int16_t py = (int16_t)(start.y + yo + yy);
if (py >= clip_top_y && py <= clip_bot_y && py >= 0
&& px >= 0 && px < (int16_t)max_x) {
point_t pos = {(uint16_t)px, (uint16_t)py};
gfx_setPixel(pos, color);
}
}
bits <<= 1;
}
}
start.x += glyph.xAdvance;
}
if (line_size > max_line_size)
max_line_size = line_size;
point_t text_size = {0, 0};
text_size.x = max_line_size;
text_size.y = (start.y - saved_start_y) + line_h;
return text_size;
}
point_t gfx_print(point_t start, fontSize_t size, textAlign_t alignment,
color_t color, const char *fmt, ... )
{

205
tests/unit/gfx_text.cpp
View file

@ -24,6 +24,50 @@ static constexpr uint16_t WIDE = 4096;
/* A max_x so narrow (1 px) that every printable glyph triggers a wrap. */
static constexpr uint16_t NARROW = 1;
/* -----------------------------------------------------------------------
* gfx_printBuffer regression tests.
*
* These pin the behaviour fixed in the commit that corrected wrap logic,
* UB glyph access order, and the sign of text_size.y.
* ----------------------------------------------------------------------- */
TEST_CASE("gfx_printBuffer text_size.y is positive", "[gfx][text]")
{
color_t white = { 255, 255, 255, 255 };
point_t start = { 0, 10 };
point_t sz = gfx_printBuffer(start, FONT_SIZE_5PT, TEXT_ALIGN_LEFT, white,
"Hello");
REQUIRE(sz.y > 0);
}
TEST_CASE("gfx_printBuffer text_size.y grows with newlines", "[gfx][text]")
{
color_t white = { 255, 255, 255, 255 };
point_t start = { 0, 10 };
point_t sz1 = gfx_printBuffer(start, FONT_SIZE_5PT, TEXT_ALIGN_LEFT, white,
"Hello");
point_t sz2 = gfx_printBuffer(start, FONT_SIZE_5PT, TEXT_ALIGN_LEFT, white,
"Hello\nHello");
REQUIRE(sz2.y > sz1.y);
REQUIRE((sz2.y - sz1.y) == TT_Y_ADVANCE);
}
TEST_CASE("gfx_printBuffer does not access glyph for newline", "[gfx][text]")
{
/* If the UB glyph-access-before-newline-check bug were present, this
* would either crash or corrupt memory when '\\n' (0x0A) is below
* f.first. Reaching REQUIRE without fault is the assertion. */
color_t white = { 255, 255, 255, 255 };
point_t start = { 0, 10 };
point_t sz = gfx_printBuffer(start, FONT_SIZE_5PT, TEXT_ALIGN_LEFT, white,
"a\nb\nc");
REQUIRE(sz.y > 0);
}
TEST_CASE("gfx_measureText single-line strings", "[gfx][text]")
{
SECTION("empty string occupies one line")
@ -117,3 +161,164 @@ TEST_CASE("gfx_measureText word-wrap at max_x", "[gfx][text]")
REQUIRE(h_narrow > h_wide);
}
}
/* -----------------------------------------------------------------------
* gfx_printBufferClipped return-value tests.
*
* gfx_setPixel is a no-op in the Linux platform test build, so we cannot
* verify pixel output. We verify the returned text_size instead, which
* exercises the layout logic without display hardware.
* ----------------------------------------------------------------------- */
TEST_CASE("gfx_printBufferClipped text_size.y for single-line text",
"[gfx][text]")
{
color_t white = { 255, 255, 255, 255 };
/*
* Single-line: start.y does not advance, so text_size.y should equal
* the raw glyph height of the last character rendered (line_h). It
* must be > 0 and <= TT_Y_ADVANCE.
*/
point_t start = { 0, 20 };
point_t sz = gfx_printBufferClipped(start, FONT_SIZE_5PT, TEXT_ALIGN_LEFT,
white, "Hi", (uint16_t)WIDE, 0, 127);
REQUIRE(sz.y > 0);
REQUIRE(sz.y <= TT_Y_ADVANCE);
}
TEST_CASE("gfx_printBufferClipped text_size.y grows for two lines",
"[gfx][text]")
{
color_t white = { 255, 255, 255, 255 };
point_t start = { 0, 20 };
point_t sz1 = gfx_printBufferClipped(start, FONT_SIZE_5PT, TEXT_ALIGN_LEFT,
white, "Hi", (uint16_t)WIDE, 0, 127);
point_t sz2 = gfx_printBufferClipped(start, FONT_SIZE_5PT, TEXT_ALIGN_LEFT,
white, "Hi\nHi", (uint16_t)WIDE, 0,
127);
/* Two lines must be taller than one. */
REQUIRE(sz2.y > sz1.y);
/* The difference should equal exactly one yAdvance. */
REQUIRE((sz2.y - sz1.y) == TT_Y_ADVANCE);
}
TEST_CASE("gfx_printBufferClipped negative start.y does not corrupt result",
"[gfx][text]")
{
color_t white = { 255, 255, 255, 255 };
/* Scrolled position: top of text block is above the visible area. */
point_t start_neg = { 0, -10 };
point_t start_pos = { 0, 0 };
point_t sz_neg = gfx_printBufferClipped(start_neg, FONT_SIZE_5PT,
TEXT_ALIGN_LEFT, white, "Hi",
(uint16_t)WIDE, 0, 127);
point_t sz_pos = gfx_printBufferClipped(start_pos, FONT_SIZE_5PT,
TEXT_ALIGN_LEFT, white, "Hi",
(uint16_t)WIDE, 0, 127);
/* Layout is the same regardless of vertical scroll offset. */
REQUIRE(sz_neg.y == sz_pos.y);
REQUIRE(sz_neg.y > 0);
}
TEST_CASE("gfx_printBufferClipped negative clip_top_y does not corrupt result",
"[gfx][text]")
{
color_t white = { 255, 255, 255, 255 };
/* Pixels above row 0 must be suppressed without corrupting layout. */
point_t start = { 0, 10 };
point_t sz = gfx_printBufferClipped(start, FONT_SIZE_5PT, TEXT_ALIGN_LEFT,
white, "Hi", (uint16_t)WIDE, -10, 127);
REQUIRE(sz.y > 0);
REQUIRE(sz.y <= TT_Y_ADVANCE);
}
TEST_CASE("get_line_size exact-fit glyph is included in line width",
"[gfx][text]")
{
color_t white = { 255, 255, 255, 255 };
point_t start = { 0, 10 };
/* TomThumb 'A' xAdvance=4: with max_x=4 the glyph exactly fills the line. */
point_t sz = gfx_printBufferClipped(start, FONT_SIZE_5PT, TEXT_ALIGN_LEFT,
white, "A", 4, 0, 127);
REQUIRE(sz.x == 4);
}
TEST_CASE("gfx_printBufferClipped text_size.x is max line width, not last line",
"[gfx][text]")
{
color_t white = { 255, 255, 255, 255 };
point_t start = { 0, 10 };
/*
* First line "Hello" is wider than second line "Hi".
* text_size.x must reflect the wider first line, not the shorter last.
*/
point_t sz_wide = gfx_printBufferClipped(start, FONT_SIZE_5PT,
TEXT_ALIGN_LEFT, white, "Hello",
(uint16_t)WIDE, 0, 127);
point_t sz_multi =
gfx_printBufferClipped(start, FONT_SIZE_5PT, TEXT_ALIGN_LEFT, white,
"Hello\nHi", (uint16_t)WIDE, 0, 127);
REQUIRE(sz_multi.x == sz_wide.x);
}
TEST_CASE("gfx_printBufferClipped text_size.x when second line is wider",
"[gfx][text]")
{
color_t white = { 255, 255, 255, 255 };
point_t start = { 0, 10 };
/* Second line "Hello" is wider than first line "Hi". */
point_t sz_wide = gfx_printBufferClipped(start, FONT_SIZE_5PT,
TEXT_ALIGN_LEFT, white, "Hello",
(uint16_t)WIDE, 0, 127);
point_t sz_multi =
gfx_printBufferClipped(start, FONT_SIZE_5PT, TEXT_ALIGN_LEFT, white,
"Hi\nHello", (uint16_t)WIDE, 0, 127);
REQUIRE(sz_multi.x == sz_wide.x);
}
TEST_CASE("gfx_printBufferClipped glyph at clip_bot_y baseline is not skipped",
"[gfx][text]")
{
color_t white = { 255, 255, 255, 255 };
/* Baseline at clip_bot_y+1: all glyph pixels still fall within window. */
int16_t base_y = 20;
int16_t clip_bot = base_y - 1;
point_t start = { 0, base_y };
point_t sz = gfx_printBufferClipped(start, FONT_SIZE_5PT, TEXT_ALIGN_LEFT,
white, "A", (uint16_t)WIDE, 0,
clip_bot);
REQUIRE(sz.y > 0);
}
TEST_CASE("gfx_printBufferClipped TEXT_ALIGN_RIGHT resets per line",
"[gfx][text]")
{
color_t white = { 255, 255, 255, 255 };
point_t start = { 0, 10 };
/*
* With TEXT_ALIGN_RIGHT each line's start.x should be computed from
* that line's own width. The returned text_size.x must equal the
* width of the widest line, same as for LEFT alignment.
*/
point_t sz_right =
gfx_printBufferClipped(start, FONT_SIZE_5PT, TEXT_ALIGN_RIGHT, white,
"Hello\nHi", (uint16_t)WIDE, 0, 127);
point_t sz_left =
gfx_printBufferClipped(start, FONT_SIZE_5PT, TEXT_ALIGN_LEFT, white,
"Hello\nHi", (uint16_t)WIDE, 0, 127);
/* Max line width is alignment-independent. */
REQUIRE(sz_right.x == sz_left.x);
}