[PATCH v6 00/10] vidconsole: refactoring and support for wider fonts

[Simon Glass]

Hi Dzmitry,

On Thu, 23 Feb 2023 at 11:10, Dzmitry Sankouski <dsankouski at gmail.com> wrote:
>
> Version 6 contains entire rebased patch series.
> New patch 'move vidconsole_get_font_size() to test.h' added.
>
> Version 5 contain minor changes:
> - move common functions to console-core.c file
> - remove static keyword from shared functions
>
> In version 4, only first patch sent, because review fixes to this would add
> large rebase & patch formatting overhead. When it'll receive reviewed tag,
> I'll resent entire rebased series.
>
> Modern mobile phones typically have high pixel density.
> Bootmenu is hardly readable on those with 8x16 font.
>
> This patch series aims to add wider fonts for devices with high ppi.
>
> Add 16x32, 12x22 fonts from linux, and allow font size configuration.
>
> There was significant changes in version 2:
> - fix video tests failures
> - add runtime font size configuration
> - add test for 12x22 font
>
> In version 3,
> 'video console: add select font logic to vidconsole uclass driver'
> patch was removed in favor of already merged patch
> 'video: Add font functions to the vidconsole API'
>
> Dzmitry Sankouski (10):
>   video console: refactoring and optimization
>   video console: add support for fonts wider than 1 byte
>   video console: move 8x16 font data in named header
>   video console: implement multiple fonts configuration
>   video console: move vidconsole_get_font_size() to test.h
>   video console: allow font size configuration at runtime
>   video console: add 12x22 Sun font from linux
>   video console: add 16x32 Terminus font from linux
>   video console: sandbox_defconfig: add 12x22 font
>   video console: add 12x22 console simple font test
>
>  cmd/Kconfig                         |    8 +
>  cmd/Makefile                        |    2 +-
>  cmd/font.c                          |    5 +-
>  common/splash.c                     |   17 +-
>  configs/sandbox_defconfig           |    5 +-
>  drivers/video/Kconfig               |   30 +
>  drivers/video/Makefile              |    6 +
>  drivers/video/console_core.c        |  203 +
>  drivers/video/console_normal.c      |  176 +-
>  drivers/video/console_rotate.c      |  368 +-
>  drivers/video/console_truetype.c    |    8 +-
>  drivers/video/vidconsole_internal.h |  114 +
>  include/cmd/test.h                  |   19 +
>  include/video_console.h             |    9 -
>  include/video_font.h                |   31 +-
>  include/video_font_4x6.h            |   11 +-
>  include/video_font_8x16.h           | 4624 ++++++++++++++++++++
>  include/video_font_data.h           | 4644 +-------------------
>  include/video_font_sun12x22.h       | 6158 +++++++++++++++++++++++++++
>  include/video_font_ter16x32.h       | 2062 +++++++++
>  test/cmd/font.c                     |    1 +
>  test/dm/video.c                     |   41 +
>  22 files changed, 13488 insertions(+), 5054 deletions(-)
>  create mode 100644 drivers/video/console_core.c
>  create mode 100644 drivers/video/vidconsole_internal.h
>  create mode 100644 include/cmd/test.h
>  create mode 100644 include/video_font_8x16.h
>  create mode 100644 include/video_font_sun12x22.h
>  create mode 100644 include/video_font_ter16x32.h

Apart from my comment on the test code, some minor nits here:

If I do this to sandbox_defconfig:

#CONFIG_CONSOLE_TRUETYPE=y
CONFIG_CMD_SELECT_FONT=y
CONFIG_VIDEO_FONT_4X6=y
CONFIG_VIDEO_FONT_SUN12X22=y
CONFIG_VIDEO_FONT_16X32=y

then the 'font list' command crashes. I think
console_simple_get_font() needs to check if it is in range.

Some odd C code I noticed so please check that:

&fonts[0]    - fonts
(&fonts[seq])->name    - fonts[seq].name

Try to keep in 80cols unless it is painful or splits a string.

Could we make the 8x16 font first in the list so it is the default? At
present if 4x6 is enabled too it comes up, which is a bit hard to
read.

It would be great if we could get this over the line soon!

Regards,
Simon