Message ID | 20221226194929.166369-4-dsankouski@gmail.com |
---|---|
State | Changes Requested |
Delegated to: | Anatolij Gustschin |
Headers | show |
Series | vidconsole: refactoring and support for wider fonts | expand |
Hi Dzmitry, On Mon, 26 Dec 2022 at 13:50, Dzmitry Sankouski <dsankouski@gmail.com> wrote: > > Devices with high ppi may benefit from wider fonts. > > Current width implementation is limited by 1 byte, i.e. 8 bits. > New version iterates VIDEO_FONT_BYTE_WIDTH times, to process all > width bytes, thus allowing fonts wider than 1 byte. > > Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com> > --- > drivers/video/console_simple.c | 79 ++++++++++++++++++---------------- > include/video_font_4x6.h | 2 + > include/video_font_data.h | 2 + > 3 files changed, 47 insertions(+), 36 deletions(-) > > diff --git a/drivers/video/console_simple.c b/drivers/video/console_simple.c > index 20087a30af..3ae1fbdc89 100644 > --- a/drivers/video/console_simple.c > +++ b/drivers/video/console_simple.c > @@ -61,30 +61,32 @@ static int fill_char_horizontally(uchar *pfont, void **line, struct video_priv * > { > int ret; > void *dst; > - u8 mask = 0x80; > - > + u8 mask; > ret = check_bpix_support(vid_priv->bpix); > if (ret) > return ret; > > - for (int col = 0; col < VIDEO_FONT_WIDTH; col++) { > - dst = *line; > - for (int row = 0; row < VIDEO_FONT_HEIGHT; row++) { > - u32 value = (pfont[row * VIDEO_FONT_BYTE_WIDTH + col] & mask) ? > - vid_priv->colour_fg : > - vid_priv->colour_bg; > - > - ret = fill_pixel_and_goto_next(&dst, > - value, > - VNBYTES(vid_priv->bpix), > - direction > - ); > + for (int col = 0; col < VIDEO_FONT_BYTE_WIDTH; col++) { > + mask = 0x80; > + for (int bit = 0; bit < 8; bit++) { > + dst = *line; > + for (int row = 0; row < VIDEO_FONT_HEIGHT; row++) { > + u32 value = (pfont[row * VIDEO_FONT_BYTE_WIDTH + col] & mask) ? > + vid_priv->colour_fg : > + vid_priv->colour_bg; > + > + ret = fill_pixel_and_goto_next(&dst, > + value, > + VNBYTES(vid_priv->bpix), > + direction > + ); > + } > + if (direction) > + *line += vid_priv->line_length; > + else > + *line -= vid_priv->line_length; > + mask >>= 1; > } > - if (direction) > - *line += vid_priv->line_length; > - else > - *line -= vid_priv->line_length; > - mask >>= 1; > } > return ret; > } > @@ -101,24 +103,29 @@ static int fill_char_vertically(uchar *pfont, void **line, struct video_priv *vi > return ret; > for (int row = 0; row < VIDEO_FONT_HEIGHT; row++) { > dst = *line; > - uchar bits = pfont[row]; > - > - for (int i = 0; i < VIDEO_FONT_WIDTH; i++) { > - u32 value = (bits & 0x80) ? > - vid_priv->colour_fg : > - vid_priv->colour_bg; > - > - ret = fill_pixel_and_goto_next(&dst, > - value, > - VNBYTES(vid_priv->bpix), > - direction > - ); > - bits <<= 1; > + uchar bits; > + > + for (int i = 0; i < VIDEO_FONT_BYTE_WIDTH; i++) { > + bits = pfont[i]; > + for (int bit = 0; bit < 8; bit++) { > + u32 value = (bits & 0x80) ? > + vid_priv->colour_fg : > + vid_priv->colour_bg; > + > + ret = fill_pixel_and_goto_next(&dst, > + value, > + VNBYTES(vid_priv->bpix), > + direction > + ); > + bits <<= 1; > + } > } > if (direction) > *line -= vid_priv->line_length; > else > *line += vid_priv->line_length; > + > + pfont += VIDEO_FONT_BYTE_WIDTH; > } > return ret; > } > @@ -177,7 +184,7 @@ static int console_putc_xy(struct udevice *dev, uint x_frac, uint y, char ch) > int pbytes = VNBYTES(vid_priv->bpix); > int x, linenum, ret; > void *start, *line; > - uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_HEIGHT; > + uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_CHAR_PIXEL_DATA_SIZE; Too long :-) How about VIDEO_FONT_PIXEL_BYTES ? > > if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac) > return -EAGAIN; > @@ -286,7 +293,7 @@ static int console_putc_xy_1(struct udevice *dev, uint x_frac, uint y, char ch) > int pbytes = VNBYTES(vid_priv->bpix); > int x, linenum, ret; > void *start, *line; > - uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_HEIGHT; > + uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_CHAR_PIXEL_DATA_SIZE; > > if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac) > return -EAGAIN; > @@ -357,7 +364,7 @@ static int console_putc_xy_2(struct udevice *dev, uint x_frac, uint y, char ch) > int pbytes = VNBYTES(vid_priv->bpix); > int linenum, x, ret; > void *start, *line; > - uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_HEIGHT; > + uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_CHAR_PIXEL_DATA_SIZE; > > if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac) > return -EAGAIN; > @@ -432,7 +439,7 @@ static int console_putc_xy_3(struct udevice *dev, uint x_frac, uint y, char ch) > int pbytes = VNBYTES(vid_priv->bpix); > int linenum, x, ret; > void *start, *line; > - uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_HEIGHT; > + uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_CHAR_PIXEL_DATA_SIZE; > > if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac) > return -EAGAIN; > diff --git a/include/video_font_4x6.h b/include/video_font_4x6.h > index c7e6351b64..f1eb2af861 100644 > --- a/include/video_font_4x6.h > +++ b/include/video_font_4x6.h > @@ -43,7 +43,9 @@ __END__; > > #define VIDEO_FONT_CHARS 256 > #define VIDEO_FONT_WIDTH 4 > +#define VIDEO_FONT_BYTE_WIDTH VIDEO_FONT_WIDTH / 8 + (VIDEO_FONT_WIDTH % 8 > 0) Please add brackets around the whole thing. > #define VIDEO_FONT_HEIGHT 6 > +#define VIDEO_FONT_CHAR_PIXEL_DATA_SIZE VIDEO_FONT_HEIGHT * VIDEO_FONT_BYTE_WIDTH > #define VIDEO_FONT_SIZE (VIDEO_FONT_CHARS * VIDEO_FONT_HEIGHT) > > static unsigned char video_fontdata[VIDEO_FONT_SIZE] = { > diff --git a/include/video_font_data.h b/include/video_font_data.h > index 6e64198d1a..6a575e6d15 100644 > --- a/include/video_font_data.h > +++ b/include/video_font_data.h > @@ -11,7 +11,9 @@ > > #define VIDEO_FONT_CHARS 256 > #define VIDEO_FONT_WIDTH 8 > +#define VIDEO_FONT_BYTE_WIDTH VIDEO_FONT_WIDTH / 8 + (VIDEO_FONT_WIDTH % 8 > 0) > #define VIDEO_FONT_HEIGHT 16 > +#define VIDEO_FONT_CHAR_PIXEL_DATA_SIZE VIDEO_FONT_HEIGHT * VIDEO_FONT_BYTE_WIDTH > #define VIDEO_FONT_SIZE (VIDEO_FONT_CHARS * VIDEO_FONT_HEIGHT) > > static unsigned char __maybe_unused video_fontdata[VIDEO_FONT_SIZE] = { > -- > 2.30.2 > Can you also please add some tests to video.c for this. Also this should have support for console_rotate.c too. Regards, Simon
On Mon, 26 Dec 2022 22:49:27 +0300 Dzmitry Sankouski <dsankouski@gmail.com> wrote: Hi Dzmitry, > Devices with high ppi may benefit from wider fonts. > > Current width implementation is limited by 1 byte, i.e. 8 bits. > New version iterates VIDEO_FONT_BYTE_WIDTH times, to process all > width bytes, thus allowing fonts wider than 1 byte. In general I think this is probably better than my version, not only because it unifies rotated and normal consoles. Some minor/stylistic comments below. > > Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com> > --- > drivers/video/console_simple.c | 79 ++++++++++++++++++---------------- > include/video_font_4x6.h | 2 + > include/video_font_data.h | 2 + > 3 files changed, 47 insertions(+), 36 deletions(-) > > diff --git a/drivers/video/console_simple.c b/drivers/video/console_simple.c > index 20087a30af..3ae1fbdc89 100644 > --- a/drivers/video/console_simple.c > +++ b/drivers/video/console_simple.c > @@ -61,30 +61,32 @@ static int fill_char_horizontally(uchar *pfont, void **line, struct video_priv * > { > int ret; > void *dst; > - u8 mask = 0x80; > - > + u8 mask; > ret = check_bpix_support(vid_priv->bpix); > if (ret) > return ret; > > - for (int col = 0; col < VIDEO_FONT_WIDTH; col++) { > - dst = *line; > - for (int row = 0; row < VIDEO_FONT_HEIGHT; row++) { > - u32 value = (pfont[row * VIDEO_FONT_BYTE_WIDTH + col] & mask) ? > - vid_priv->colour_fg : > - vid_priv->colour_bg; > - > - ret = fill_pixel_and_goto_next(&dst, > - value, > - VNBYTES(vid_priv->bpix), > - direction > - ); > + for (int col = 0; col < VIDEO_FONT_BYTE_WIDTH; col++) { > + mask = 0x80; > + for (int bit = 0; bit < 8; bit++) { I think it's cleaner to collapse this: for (int col = 0; col < VIDEO_FONT_WIDTH; col++) { if ((col % 8) == 0) mask = 0x80; ... > + dst = *line; > + for (int row = 0; row < VIDEO_FONT_HEIGHT; row++) { > + u32 value = (pfont[row * VIDEO_FONT_BYTE_WIDTH + col] & mask) ? ... and use (col / 8) here. > + vid_priv->colour_fg : > + vid_priv->colour_bg; > + > + ret = fill_pixel_and_goto_next(&dst, > + value, > + VNBYTES(vid_priv->bpix), > + direction > + ); > + } > + if (direction) > + *line += vid_priv->line_length; > + else > + *line -= vid_priv->line_length; > + mask >>= 1; > } > - if (direction) > - *line += vid_priv->line_length; > - else > - *line -= vid_priv->line_length; > - mask >>= 1; > } > return ret; > } > @@ -101,24 +103,29 @@ static int fill_char_vertically(uchar *pfont, void **line, struct video_priv *vi > return ret; > for (int row = 0; row < VIDEO_FONT_HEIGHT; row++) { > dst = *line; > - uchar bits = pfont[row]; > - > - for (int i = 0; i < VIDEO_FONT_WIDTH; i++) { > - u32 value = (bits & 0x80) ? > - vid_priv->colour_fg : > - vid_priv->colour_bg; > - > - ret = fill_pixel_and_goto_next(&dst, > - value, > - VNBYTES(vid_priv->bpix), > - direction > - ); > - bits <<= 1; > + uchar bits; > + > + for (int i = 0; i < VIDEO_FONT_BYTE_WIDTH; i++) { > + bits = pfont[i]; > + for (int bit = 0; bit < 8; bit++) { Same as above, those two loops should become one. > + u32 value = (bits & 0x80) ? > + vid_priv->colour_fg : > + vid_priv->colour_bg; > + > + ret = fill_pixel_and_goto_next(&dst, > + value, > + VNBYTES(vid_priv->bpix), > + direction > + ); > + bits <<= 1; > + } > } > if (direction) > *line -= vid_priv->line_length; > else > *line += vid_priv->line_length; > + > + pfont += VIDEO_FONT_BYTE_WIDTH; > } > return ret; > } > @@ -177,7 +184,7 @@ static int console_putc_xy(struct udevice *dev, uint x_frac, uint y, char ch) > int pbytes = VNBYTES(vid_priv->bpix); > int x, linenum, ret; > void *start, *line; > - uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_HEIGHT; > + uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_CHAR_PIXEL_DATA_SIZE; > > if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac) > return -EAGAIN; > @@ -286,7 +293,7 @@ static int console_putc_xy_1(struct udevice *dev, uint x_frac, uint y, char ch) > int pbytes = VNBYTES(vid_priv->bpix); > int x, linenum, ret; > void *start, *line; > - uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_HEIGHT; > + uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_CHAR_PIXEL_DATA_SIZE; > > if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac) > return -EAGAIN; > @@ -357,7 +364,7 @@ static int console_putc_xy_2(struct udevice *dev, uint x_frac, uint y, char ch) > int pbytes = VNBYTES(vid_priv->bpix); > int linenum, x, ret; > void *start, *line; > - uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_HEIGHT; > + uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_CHAR_PIXEL_DATA_SIZE; > > if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac) > return -EAGAIN; > @@ -432,7 +439,7 @@ static int console_putc_xy_3(struct udevice *dev, uint x_frac, uint y, char ch) > int pbytes = VNBYTES(vid_priv->bpix); > int linenum, x, ret; > void *start, *line; > - uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_HEIGHT; > + uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_CHAR_PIXEL_DATA_SIZE; > > if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac) > return -EAGAIN; > diff --git a/include/video_font_4x6.h b/include/video_font_4x6.h > index c7e6351b64..f1eb2af861 100644 > --- a/include/video_font_4x6.h > +++ b/include/video_font_4x6.h > @@ -43,7 +43,9 @@ __END__; > > #define VIDEO_FONT_CHARS 256 > #define VIDEO_FONT_WIDTH 4 > +#define VIDEO_FONT_BYTE_WIDTH VIDEO_FONT_WIDTH / 8 + (VIDEO_FONT_WIDTH % 8 > 0) I think the canonical way for rounding up is: ((VIDEO_FONT_WIDTH + 7) / 8) And as Simon said: you need parentheses, otherwise it failed for me with a width of 12 (sun12x22 font). > #define VIDEO_FONT_HEIGHT 6 > +#define VIDEO_FONT_CHAR_PIXEL_DATA_SIZE VIDEO_FONT_HEIGHT * VIDEO_FONT_BYTE_WIDTH Those two new lines do not contain any specific data, so can be moved to console_simple.c. > #define VIDEO_FONT_SIZE (VIDEO_FONT_CHARS * VIDEO_FONT_HEIGHT) > > static unsigned char video_fontdata[VIDEO_FONT_SIZE] = { > diff --git a/include/video_font_data.h b/include/video_font_data.h > index 6e64198d1a..6a575e6d15 100644 > --- a/include/video_font_data.h > +++ b/include/video_font_data.h > @@ -11,7 +11,9 @@ > > #define VIDEO_FONT_CHARS 256 > #define VIDEO_FONT_WIDTH 8 > +#define VIDEO_FONT_BYTE_WIDTH VIDEO_FONT_WIDTH / 8 + (VIDEO_FONT_WIDTH % 8 > 0) > #define VIDEO_FONT_HEIGHT 16 > +#define VIDEO_FONT_CHAR_PIXEL_DATA_SIZE VIDEO_FONT_HEIGHT * VIDEO_FONT_BYTE_WIDTH same here, remove from here and use one copy in console_simple.c. Cheers, Andre > #define VIDEO_FONT_SIZE (VIDEO_FONT_CHARS * VIDEO_FONT_HEIGHT) > > static unsigned char __maybe_unused video_fontdata[VIDEO_FONT_SIZE] = {
diff --git a/drivers/video/console_simple.c b/drivers/video/console_simple.c index 20087a30af..3ae1fbdc89 100644 --- a/drivers/video/console_simple.c +++ b/drivers/video/console_simple.c @@ -61,30 +61,32 @@ static int fill_char_horizontally(uchar *pfont, void **line, struct video_priv * { int ret; void *dst; - u8 mask = 0x80; - + u8 mask; ret = check_bpix_support(vid_priv->bpix); if (ret) return ret; - for (int col = 0; col < VIDEO_FONT_WIDTH; col++) { - dst = *line; - for (int row = 0; row < VIDEO_FONT_HEIGHT; row++) { - u32 value = (pfont[row * VIDEO_FONT_BYTE_WIDTH + col] & mask) ? - vid_priv->colour_fg : - vid_priv->colour_bg; - - ret = fill_pixel_and_goto_next(&dst, - value, - VNBYTES(vid_priv->bpix), - direction - ); + for (int col = 0; col < VIDEO_FONT_BYTE_WIDTH; col++) { + mask = 0x80; + for (int bit = 0; bit < 8; bit++) { + dst = *line; + for (int row = 0; row < VIDEO_FONT_HEIGHT; row++) { + u32 value = (pfont[row * VIDEO_FONT_BYTE_WIDTH + col] & mask) ? + vid_priv->colour_fg : + vid_priv->colour_bg; + + ret = fill_pixel_and_goto_next(&dst, + value, + VNBYTES(vid_priv->bpix), + direction + ); + } + if (direction) + *line += vid_priv->line_length; + else + *line -= vid_priv->line_length; + mask >>= 1; } - if (direction) - *line += vid_priv->line_length; - else - *line -= vid_priv->line_length; - mask >>= 1; } return ret; } @@ -101,24 +103,29 @@ static int fill_char_vertically(uchar *pfont, void **line, struct video_priv *vi return ret; for (int row = 0; row < VIDEO_FONT_HEIGHT; row++) { dst = *line; - uchar bits = pfont[row]; - - for (int i = 0; i < VIDEO_FONT_WIDTH; i++) { - u32 value = (bits & 0x80) ? - vid_priv->colour_fg : - vid_priv->colour_bg; - - ret = fill_pixel_and_goto_next(&dst, - value, - VNBYTES(vid_priv->bpix), - direction - ); - bits <<= 1; + uchar bits; + + for (int i = 0; i < VIDEO_FONT_BYTE_WIDTH; i++) { + bits = pfont[i]; + for (int bit = 0; bit < 8; bit++) { + u32 value = (bits & 0x80) ? + vid_priv->colour_fg : + vid_priv->colour_bg; + + ret = fill_pixel_and_goto_next(&dst, + value, + VNBYTES(vid_priv->bpix), + direction + ); + bits <<= 1; + } } if (direction) *line -= vid_priv->line_length; else *line += vid_priv->line_length; + + pfont += VIDEO_FONT_BYTE_WIDTH; } return ret; } @@ -177,7 +184,7 @@ static int console_putc_xy(struct udevice *dev, uint x_frac, uint y, char ch) int pbytes = VNBYTES(vid_priv->bpix); int x, linenum, ret; void *start, *line; - uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_HEIGHT; + uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_CHAR_PIXEL_DATA_SIZE; if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac) return -EAGAIN; @@ -286,7 +293,7 @@ static int console_putc_xy_1(struct udevice *dev, uint x_frac, uint y, char ch) int pbytes = VNBYTES(vid_priv->bpix); int x, linenum, ret; void *start, *line; - uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_HEIGHT; + uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_CHAR_PIXEL_DATA_SIZE; if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac) return -EAGAIN; @@ -357,7 +364,7 @@ static int console_putc_xy_2(struct udevice *dev, uint x_frac, uint y, char ch) int pbytes = VNBYTES(vid_priv->bpix); int linenum, x, ret; void *start, *line; - uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_HEIGHT; + uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_CHAR_PIXEL_DATA_SIZE; if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac) return -EAGAIN; @@ -432,7 +439,7 @@ static int console_putc_xy_3(struct udevice *dev, uint x_frac, uint y, char ch) int pbytes = VNBYTES(vid_priv->bpix); int linenum, x, ret; void *start, *line; - uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_HEIGHT; + uchar *pfont = video_fontdata + (u8)ch * VIDEO_FONT_CHAR_PIXEL_DATA_SIZE; if (x_frac + VID_TO_POS(vc_priv->x_charsize) > vc_priv->xsize_frac) return -EAGAIN; diff --git a/include/video_font_4x6.h b/include/video_font_4x6.h index c7e6351b64..f1eb2af861 100644 --- a/include/video_font_4x6.h +++ b/include/video_font_4x6.h @@ -43,7 +43,9 @@ __END__; #define VIDEO_FONT_CHARS 256 #define VIDEO_FONT_WIDTH 4 +#define VIDEO_FONT_BYTE_WIDTH VIDEO_FONT_WIDTH / 8 + (VIDEO_FONT_WIDTH % 8 > 0) #define VIDEO_FONT_HEIGHT 6 +#define VIDEO_FONT_CHAR_PIXEL_DATA_SIZE VIDEO_FONT_HEIGHT * VIDEO_FONT_BYTE_WIDTH #define VIDEO_FONT_SIZE (VIDEO_FONT_CHARS * VIDEO_FONT_HEIGHT) static unsigned char video_fontdata[VIDEO_FONT_SIZE] = { diff --git a/include/video_font_data.h b/include/video_font_data.h index 6e64198d1a..6a575e6d15 100644 --- a/include/video_font_data.h +++ b/include/video_font_data.h @@ -11,7 +11,9 @@ #define VIDEO_FONT_CHARS 256 #define VIDEO_FONT_WIDTH 8 +#define VIDEO_FONT_BYTE_WIDTH VIDEO_FONT_WIDTH / 8 + (VIDEO_FONT_WIDTH % 8 > 0) #define VIDEO_FONT_HEIGHT 16 +#define VIDEO_FONT_CHAR_PIXEL_DATA_SIZE VIDEO_FONT_HEIGHT * VIDEO_FONT_BYTE_WIDTH #define VIDEO_FONT_SIZE (VIDEO_FONT_CHARS * VIDEO_FONT_HEIGHT) static unsigned char __maybe_unused video_fontdata[VIDEO_FONT_SIZE] = {
Devices with high ppi may benefit from wider fonts. Current width implementation is limited by 1 byte, i.e. 8 bits. New version iterates VIDEO_FONT_BYTE_WIDTH times, to process all width bytes, thus allowing fonts wider than 1 byte. Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com> --- drivers/video/console_simple.c | 79 ++++++++++++++++++---------------- include/video_font_4x6.h | 2 + include/video_font_data.h | 2 + 3 files changed, 47 insertions(+), 36 deletions(-)