[PATCH v3] video: fix positioning in TrueType console

With the patch accurate positioning is possible for mono-typed fonts:
Fix the return value of console_truetype_putc_xy(). The current position is passed as parameter x. Some part of x represents a fractional pixel. The return value represents how much the character position must be advanced. This should only comprise the width of the current character and not the preexisting fractional pixel position.
Characters are not square. As all characters of a mono-type font we can take the width of any character. 'W' as one of the widest ANSI characters provides also a good value for variable width fonts.
The character width must be a float for TrueType.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v3: Use floating point only with the TrueType console. v2: Adjust hash values in tests --- drivers/video/console_truetype.c | 27 ++++++++++++++++++++------- include/video_console.h | 4 ++++ test/dm/video.c | 6 +++--- 3 files changed, 27 insertions(+), 10 deletions(-)
diff --git a/drivers/video/console_truetype.c b/drivers/video/console_truetype.c index d9ad52cce0..6fe238eab3 100644 --- a/drivers/video/console_truetype.c +++ b/drivers/video/console_truetype.c @@ -209,7 +209,7 @@ static int console_truetype_putc_xy(struct udevice *dev, uint x, uint y, int width_frac, linenum; struct pos_info *pos; u8 *bits, *data; - int advance; + int advance, kern_adv; void *start, *line, *sync_start, *sync_end; int row, ret; int bg_r, bg_g, bg_b; @@ -224,8 +224,11 @@ static int console_truetype_putc_xy(struct udevice *dev, uint x, uint y, * this character */ xpos = frac(VID_TO_PIXEL((double)x)); if (vc_priv->last_ch) { - xpos += priv->scale * stbtt_GetCodepointKernAdvance(font, - vc_priv->last_ch, ch); + kern_adv = stbtt_GetCodepointKernAdvance(font, vc_priv->last_ch, + ch); + xpos += priv->scale * kern_adv; + } else { + kern_adv = 0; }
/* @@ -236,8 +239,8 @@ static int console_truetype_putc_xy(struct udevice *dev, uint x, uint y, */ x_shift = xpos - (double)tt_floor(xpos); xpos += advance * priv->scale; - width_frac = (int)VID_TO_POS(xpos); - if (x + width_frac >= vc_priv->xsize_frac) + width_frac = VID_TO_POS(priv->scale * (kern_adv + advance)); + if (x + (int)VID_TO_POS(xpos) >= vc_priv->xsize_frac) return -EAGAIN;
/* Write the current cursor position into history */ @@ -585,20 +588,21 @@ static int console_truetype_probe(struct udevice *dev) struct udevice *vid_dev = dev->parent; struct video_priv *vid_priv = dev_get_uclass_priv(vid_dev); stbtt_fontinfo *font = &priv->font; - int ascent; + int advance, ascent, lsb;
debug("%s: start\n", __func__); + if (vid_priv->font_size) priv->font_size = vid_priv->font_size; else priv->font_size = CONFIG_CONSOLE_TRUETYPE_SIZE; + priv->font_data = console_truetype_find_font(); if (!priv->font_data) { debug("%s: Could not find any fonts\n", __func__); return -EBFONT; }
- vc_priv->x_charsize = priv->font_size; vc_priv->y_charsize = priv->font_size; vc_priv->xstart_frac = VID_TO_POS(2); vc_priv->cols = vid_priv->xsize / priv->font_size; @@ -612,6 +616,15 @@ static int console_truetype_probe(struct udevice *dev)
/* Pre-calculate some things we will need regularly */ priv->scale = stbtt_ScaleForPixelHeight(font, priv->font_size); + + /* Assuming that 'W' is the widest character */ + stbtt_GetCodepointHMetrics(font, 'W', &advance, &lsb); + advance += stbtt_GetCodepointKernAdvance(font, 'W', 'W'); + vc_priv->cols = + (int)VID_TO_POS(vid_priv->xsize - 2) / + (int)VID_TO_POS(advance * priv->scale); + vc_priv->x_charsize = advance * priv->scale; + stbtt_GetFontVMetrics(font, &ascent, 0, 0); priv->baseline = (int)(ascent * priv->scale); debug("%s: ready\n", __func__); diff --git a/include/video_console.h b/include/video_console.h index 06b798ef10..c339dc3956 100644 --- a/include/video_console.h +++ b/include/video_console.h @@ -68,7 +68,11 @@ struct vidconsole_priv { int ycur; int rows; int cols; +#ifdef CONFIG_CONSOLE_TRUETYPE + double x_charsize; +#else int x_charsize; +#endif int y_charsize; int tab_width_frac; int xsize_frac; diff --git a/test/dm/video.c b/test/dm/video.c index 1d29b2d61c..c0ad83521a 100644 --- a/test/dm/video.c +++ b/test/dm/video.c @@ -344,7 +344,7 @@ static int dm_test_video_truetype(struct unit_test_state *uts) ut_assertok(uclass_get_device(UCLASS_VIDEO, 0, &dev)); ut_assertok(uclass_get_device(UCLASS_VIDEO_CONSOLE, 0, &con)); vidconsole_put_string(con, test_string); - ut_asserteq(13001, compress_frame_buffer(uts, dev)); + ut_asserteq(12752, compress_frame_buffer(uts, dev));
return 0; } @@ -365,7 +365,7 @@ static int dm_test_video_truetype_scroll(struct unit_test_state *uts) ut_assertok(uclass_get_device(UCLASS_VIDEO, 0, &dev)); ut_assertok(uclass_get_device(UCLASS_VIDEO_CONSOLE, 0, &con)); vidconsole_put_string(con, test_string); - ut_asserteq(36952, compress_frame_buffer(uts, dev)); + ut_asserteq(36493, compress_frame_buffer(uts, dev));
return 0; } @@ -386,7 +386,7 @@ static int dm_test_video_truetype_bs(struct unit_test_state *uts) ut_assertok(uclass_get_device(UCLASS_VIDEO, 0, &dev)); ut_assertok(uclass_get_device(UCLASS_VIDEO_CONSOLE, 0, &con)); vidconsole_put_string(con, test_string); - ut_asserteq(30747, compress_frame_buffer(uts, dev)); + ut_asserteq(31117, compress_frame_buffer(uts, dev));
return 0; } -- 2.30.2

Hi Heinrich,
On Thu, 28 Oct 2021 at 12:01, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
With the patch accurate positioning is possible for mono-typed fonts:
Fix the return value of console_truetype_putc_xy(). The current position is passed as parameter x. Some part of x represents a fractional pixel. The return value represents how much the character position must be advanced. This should only comprise the width of the current character and not the preexisting fractional pixel position.
Characters are not square. As all characters of a mono-type font we can take the width of any character. 'W' as one of the widest ANSI characters provides also a good value for variable width fonts.
The character width must be a float for TrueType.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
v3: Use floating point only with the TrueType console. v2: Adjust hash values in tests
drivers/video/console_truetype.c | 27 ++++++++++++++++++++------- include/video_console.h | 4 ++++ test/dm/video.c | 6 +++--- 3 files changed, 27 insertions(+), 10 deletions(-)
What commit is this based on, please? I don't seem to be able to apply it.
Also, can we avoid using the double and use the fractional int value instead?
diff --git a/drivers/video/console_truetype.c b/drivers/video/console_truetype.c index d9ad52cce0..6fe238eab3 100644 --- a/drivers/video/console_truetype.c +++ b/drivers/video/console_truetype.c @@ -209,7 +209,7 @@ static int console_truetype_putc_xy(struct udevice *dev, uint x, uint y, int width_frac, linenum; struct pos_info *pos; u8 *bits, *data;
int advance;
int advance, kern_adv; void *start, *line, *sync_start, *sync_end; int row, ret; int bg_r, bg_g, bg_b;
@@ -224,8 +224,11 @@ static int console_truetype_putc_xy(struct udevice *dev, uint x, uint y, * this character */ xpos = frac(VID_TO_PIXEL((double)x)); if (vc_priv->last_ch) {
xpos += priv->scale * stbtt_GetCodepointKernAdvance(font,
vc_priv->last_ch, ch);
kern_adv = stbtt_GetCodepointKernAdvance(font, vc_priv->last_ch,
ch);
xpos += priv->scale * kern_adv;
} else {
kern_adv = 0; } /*
@@ -236,8 +239,8 @@ static int console_truetype_putc_xy(struct udevice *dev, uint x, uint y, */ x_shift = xpos - (double)tt_floor(xpos); xpos += advance * priv->scale;
width_frac = (int)VID_TO_POS(xpos);
if (x + width_frac >= vc_priv->xsize_frac)
width_frac = VID_TO_POS(priv->scale * (kern_adv + advance));
if (x + (int)VID_TO_POS(xpos) >= vc_priv->xsize_frac) return -EAGAIN; /* Write the current cursor position into history */
@@ -585,20 +588,21 @@ static int console_truetype_probe(struct udevice *dev) struct udevice *vid_dev = dev->parent; struct video_priv *vid_priv = dev_get_uclass_priv(vid_dev); stbtt_fontinfo *font = &priv->font;
int ascent;
int advance, ascent, lsb; debug("%s: start\n", __func__);
if (vid_priv->font_size) priv->font_size = vid_priv->font_size; else priv->font_size = CONFIG_CONSOLE_TRUETYPE_SIZE;
priv->font_data = console_truetype_find_font(); if (!priv->font_data) { debug("%s: Could not find any fonts\n", __func__); return -EBFONT; }
vc_priv->x_charsize = priv->font_size; vc_priv->y_charsize = priv->font_size; vc_priv->xstart_frac = VID_TO_POS(2); vc_priv->cols = vid_priv->xsize / priv->font_size;
@@ -612,6 +616,15 @@ static int console_truetype_probe(struct udevice *dev)
/* Pre-calculate some things we will need regularly */ priv->scale = stbtt_ScaleForPixelHeight(font, priv->font_size);
/* Assuming that 'W' is the widest character */
stbtt_GetCodepointHMetrics(font, 'W', &advance, &lsb);
advance += stbtt_GetCodepointKernAdvance(font, 'W', 'W');
vc_priv->cols =
(int)VID_TO_POS(vid_priv->xsize - 2) /
(int)VID_TO_POS(advance * priv->scale);
vc_priv->x_charsize = advance * priv->scale;
stbtt_GetFontVMetrics(font, &ascent, 0, 0); priv->baseline = (int)(ascent * priv->scale); debug("%s: ready\n", __func__);
diff --git a/include/video_console.h b/include/video_console.h index 06b798ef10..c339dc3956 100644 --- a/include/video_console.h +++ b/include/video_console.h @@ -68,7 +68,11 @@ struct vidconsole_priv { int ycur; int rows; int cols; +#ifdef CONFIG_CONSOLE_TRUETYPE
double x_charsize;
+#else int x_charsize; +#endif int y_charsize; int tab_width_frac; int xsize_frac; diff --git a/test/dm/video.c b/test/dm/video.c index 1d29b2d61c..c0ad83521a 100644 --- a/test/dm/video.c +++ b/test/dm/video.c @@ -344,7 +344,7 @@ static int dm_test_video_truetype(struct unit_test_state *uts) ut_assertok(uclass_get_device(UCLASS_VIDEO, 0, &dev)); ut_assertok(uclass_get_device(UCLASS_VIDEO_CONSOLE, 0, &con)); vidconsole_put_string(con, test_string);
ut_asserteq(13001, compress_frame_buffer(uts, dev));
ut_asserteq(12752, compress_frame_buffer(uts, dev)); return 0;
} @@ -365,7 +365,7 @@ static int dm_test_video_truetype_scroll(struct unit_test_state *uts) ut_assertok(uclass_get_device(UCLASS_VIDEO, 0, &dev)); ut_assertok(uclass_get_device(UCLASS_VIDEO_CONSOLE, 0, &con)); vidconsole_put_string(con, test_string);
ut_asserteq(36952, compress_frame_buffer(uts, dev));
ut_asserteq(36493, compress_frame_buffer(uts, dev)); return 0;
} @@ -386,7 +386,7 @@ static int dm_test_video_truetype_bs(struct unit_test_state *uts) ut_assertok(uclass_get_device(UCLASS_VIDEO, 0, &dev)); ut_assertok(uclass_get_device(UCLASS_VIDEO_CONSOLE, 0, &con)); vidconsole_put_string(con, test_string);
ut_asserteq(30747, compress_frame_buffer(uts, dev));
ut_asserteq(31117, compress_frame_buffer(uts, dev)); return 0;
}
2.30.2
Regards, Simon

Hi Simon,
On Sun, 31 Oct 2021 17:46:56 -0600 Simon Glass sjg@chromium.org wrote: ...
v3: Use floating point only with the TrueType console. v2: Adjust hash values in tests
drivers/video/console_truetype.c | 27 ++++++++++++++++++++------- include/video_console.h | 4 ++++ test/dm/video.c | 6 +++--- 3 files changed, 27 insertions(+), 10 deletions(-)
What commit is this based on, please? I don't seem to be able to apply it.
there is another patch:
http://patchwork.ozlabs.org/project/uboot/patch/20200930225935.256667-1-xypr...
After applying it first, this v3 patch can be applied and building then works:
https://source.denx.de/u-boot/custodians/u-boot-video/-/commits/oct28
-- Anatolij

Hi Anatolij,
On Sun, 31 Oct 2021 at 18:23, Anatolij Gustschin agust@denx.de wrote:
Hi Simon,
On Sun, 31 Oct 2021 17:46:56 -0600 Simon Glass sjg@chromium.org wrote: ...
v3: Use floating point only with the TrueType console. v2: Adjust hash values in tests
drivers/video/console_truetype.c | 27 ++++++++++++++++++++------- include/video_console.h | 4 ++++ test/dm/video.c | 6 +++--- 3 files changed, 27 insertions(+), 10 deletions(-)
What commit is this based on, please? I don't seem to be able to apply it.
there is another patch:
http://patchwork.ozlabs.org/project/uboot/patch/20200930225935.256667-1-xypr...
After applying it first, this v3 patch can be applied and building then works:
https://source.denx.de/u-boot/custodians/u-boot-video/-/commits/oct28
OK thank you.
Tested on: sandbox Tested-by: Simon Glass sjg@chromium.org
Will await comments on my comments.
Regards, Simon
participants (3)
-
Anatolij Gustschin
-
Heinrich Schuchardt
-
Simon Glass