From 1e91569763ecedd1cdbf24ae287c513af8d3cbeb Mon Sep 17 00:00:00 2001 From: Xiang Xiao Date: Wed, 27 Jan 2021 02:20:36 -0800 Subject: [PATCH] Fix various issues found in font module (#2040) * fix(font): Correct _LV_STR_SYMBOL_ generation command and add _LV_STR_SYMBOL_BULLET to the list * fix(font): lv_font_load shouldn't call lv_fs_close if lv_fs_open fail * fix(font): read_bits should return 0 not -1 in error case to avoid read_bits_signed waste time to extend the sign bit * fix(font): Correct the return type of read_bits to unsgined int and extend the sign bit more efficient and correct * fix(font): Sync LV_FONT_FMT_TXT_CMAP_ value to binary font spec and then remove the hard code value from source code: https://github.com/lvgl/lv_font_conv/blame/master/doc/font_spec.md#L96 remove zero fields statement too since font_dsc->cmaps already zero at line 334. * fix(font): Improve the performance by reading cmap table by once * fix(font): Improve the loading performance if the header is multipled by 8bits * fix(font): Read loca table in batch if the size is 32bits * fix(font): Load the underline related attributes spec here: https://github.com/lvgl/lv_font_conv/blame/master/doc/font_spec.md#L55-L56 --- src/lv_font/lv_font_fmt_txt.h | 4 +- src/lv_font/lv_font_loader.c | 116 ++++++++++++++++------------------ src/lv_font/lv_symbol_def.h | 5 +- 3 files changed, 59 insertions(+), 66 deletions(-) diff --git a/src/lv_font/lv_font_fmt_txt.h b/src/lv_font/lv_font_fmt_txt.h index 1538c2fa6..f47549f4f 100644 --- a/src/lv_font/lv_font_fmt_txt.h +++ b/src/lv_font/lv_font_fmt_txt.h @@ -47,10 +47,10 @@ typedef struct { /** Format of font character map. */ enum { - LV_FONT_FMT_TXT_CMAP_FORMAT0_TINY, LV_FONT_FMT_TXT_CMAP_FORMAT0_FULL, - LV_FONT_FMT_TXT_CMAP_SPARSE_TINY, LV_FONT_FMT_TXT_CMAP_SPARSE_FULL, + LV_FONT_FMT_TXT_CMAP_FORMAT0_TINY, + LV_FONT_FMT_TXT_CMAP_SPARSE_TINY, }; typedef uint8_t lv_font_fmt_txt_cmap_type_t; diff --git a/src/lv_font/lv_font_loader.c b/src/lv_font/lv_font_loader.c index 27f4c1030..7df581034 100644 --- a/src/lv_font/lv_font_loader.c +++ b/src/lv_font/lv_font_loader.c @@ -48,6 +48,8 @@ typedef struct font_header_bin { uint8_t compression_id; uint8_t subpixels_mode; uint8_t padding; + int16_t underline_position; + uint16_t underline_thickness; } font_header_bin_t; typedef struct cmap_table_bin { @@ -68,7 +70,7 @@ static bool lvgl_load_font(lv_fs_file_t * fp, lv_font_t * font); int32_t load_kern(lv_fs_file_t * fp, lv_font_fmt_txt_dsc_t * font_dsc, uint8_t format, uint32_t start); static int read_bits_signed(bit_iterator_t * it, int n_bits, lv_fs_res_t * res); -static int read_bits(bit_iterator_t * it, int n_bits, lv_fs_res_t * res); +static unsigned int read_bits(bit_iterator_t * it, int n_bits, lv_fs_res_t * res); /********************** * MACROS @@ -95,20 +97,20 @@ lv_font_t * lv_font_load(const char * font_name) if(res == LV_FS_RES_OK) { success = lvgl_load_font(&file, font); - } - if(!success) { - LV_LOG_WARN("Error loading font file: %s\n", font_name); - /* - * When `lvgl_load_font` fails it can leak some pointers. - * All non-null pointers can be assumed as allocated and - * `lv_font_free` should free them correctly. - */ - lv_font_free(font); - font = NULL; - } + if(!success) { + LV_LOG_WARN("Error loading font file: %s\n", font_name); + /* + * When `lvgl_load_font` fails it can leak some pointers. + * All non-null pointers can be assumed as allocated and + * `lv_font_free` should free them correctly. + */ + lv_font_free(font); + font = NULL; + } - lv_fs_close(&file); + lv_fs_close(&file); + } return font; } @@ -194,9 +196,9 @@ static bit_iterator_t init_bit_iterator(lv_fs_file_t * fp) return it; } -static int read_bits(bit_iterator_t * it, int n_bits, lv_fs_res_t * res) +static unsigned int read_bits(bit_iterator_t * it, int n_bits, lv_fs_res_t * res) { - int value = 0; + unsigned int value = 0; while(n_bits--) { it->byte_value = it->byte_value << 1; it->bit_pos--; @@ -205,7 +207,7 @@ static int read_bits(bit_iterator_t * it, int n_bits, lv_fs_res_t * res) it->bit_pos = 7; *res = lv_fs_read(it->fp, &(it->byte_value), 1, NULL); if(*res != LV_FS_RES_OK) { - return -1; + return 0; } } int8_t bit = (it->byte_value & 0x80) ? 1 : 0; @@ -218,11 +220,9 @@ static int read_bits(bit_iterator_t * it, int n_bits, lv_fs_res_t * res) static int read_bits_signed(bit_iterator_t * it, int n_bits, lv_fs_res_t * res) { - int value = read_bits(it, n_bits, res); + unsigned int value = read_bits(it, n_bits, res); if(value & (1 << (n_bits - 1))) { - for(int bit = n_bits; bit < 16; ++bit) { - value |= (1 << bit); - } + value |= ~0u << n_bits; } return value; } @@ -247,16 +247,8 @@ static int read_label(lv_fs_file_t * fp, int start, const char * label) static bool load_cmaps_tables(lv_fs_file_t * fp, lv_font_fmt_txt_dsc_t * font_dsc, uint32_t cmaps_start, cmap_table_bin_t * cmap_table) { - for(unsigned int i = 0; i < font_dsc->cmap_num; ++i) { - if(lv_fs_read(fp, &cmap_table[i], sizeof(cmap_table_bin_t), NULL) != LV_FS_RES_OK) { - return false; - } - - lv_font_fmt_txt_cmap_t * cmap = (lv_font_fmt_txt_cmap_t *) & (font_dsc->cmaps[i]); - - cmap->range_start = cmap_table[i].range_start; - cmap->range_length = cmap_table[i].range_length; - cmap->glyph_id_start = cmap_table[i].glyph_id_start; + if(lv_fs_read(fp, cmap_table, font_dsc->cmap_num * sizeof(cmap_table_bin_t), NULL) != LV_FS_RES_OK) { + return false; } for(unsigned int i = 0; i < font_dsc->cmap_num; ++i) { @@ -267,8 +259,13 @@ static bool load_cmaps_tables(lv_fs_file_t * fp, lv_font_fmt_txt_dsc_t * font_ds lv_font_fmt_txt_cmap_t * cmap = (lv_font_fmt_txt_cmap_t *) & (font_dsc->cmaps[i]); + cmap->range_start = cmap_table[i].range_start; + cmap->range_length = cmap_table[i].range_length; + cmap->glyph_id_start = cmap_table[i].glyph_id_start; + cmap->type = cmap_table[i].format_type; + switch(cmap_table[i].format_type) { - case 0: { + case LV_FONT_FMT_TXT_CMAP_FORMAT0_FULL: { uint8_t ids_size = sizeof(uint8_t) * cmap_table[i].data_entries_count; uint8_t * glyph_id_ofs_list = lv_mem_alloc(ids_size); @@ -278,19 +275,13 @@ static bool load_cmaps_tables(lv_fs_file_t * fp, lv_font_fmt_txt_dsc_t * font_ds return false; } - cmap->type = LV_FONT_FMT_TXT_CMAP_FORMAT0_FULL; cmap->list_length = cmap->range_length; - cmap->unicode_list = NULL; break; } - case 2: - cmap->type = LV_FONT_FMT_TXT_CMAP_FORMAT0_TINY; - cmap->list_length = 0; - cmap->unicode_list = NULL; - cmap->glyph_id_ofs_list = NULL; + case LV_FONT_FMT_TXT_CMAP_FORMAT0_TINY: break; - case 1: - case 3: { + case LV_FONT_FMT_TXT_CMAP_SPARSE_FULL: + case LV_FONT_FMT_TXT_CMAP_SPARSE_TINY: { uint32_t list_size = sizeof(uint16_t) * cmap_table[i].data_entries_count; uint16_t * unicode_list = (uint16_t *) lv_mem_alloc(list_size); @@ -301,20 +292,15 @@ static bool load_cmaps_tables(lv_fs_file_t * fp, lv_font_fmt_txt_dsc_t * font_ds return false; } - if(cmap_table[i].format_type == 1) { + if(cmap_table[i].format_type == LV_FONT_FMT_TXT_CMAP_SPARSE_FULL) { uint16_t * buf = lv_mem_alloc(sizeof(uint16_t) * cmap->list_length); - cmap->type = LV_FONT_FMT_TXT_CMAP_SPARSE_FULL; cmap->glyph_id_ofs_list = buf; if(lv_fs_read(fp, buf, sizeof(uint16_t) * cmap->list_length, NULL) != LV_FS_RES_OK) { return false; } } - else { - cmap->type = LV_FONT_FMT_TXT_CMAP_SPARSE_TINY; - cmap->glyph_id_ofs_list = NULL; - } break; } default: @@ -460,12 +446,20 @@ static int32_t load_glyph(lv_fs_file_t * fp, lv_font_fmt_txt_dsc_t * font_dsc, int next_offset = (i < loca_count - 1) ? glyph_offset[i + 1] : (uint32_t)(glyph_length - 1); int bmp_size = next_offset - glyph_offset[i] - nbits / 8; - for(int k = 0; k < bmp_size; ++k) { - glyph_bmp[cur_bmp_size + k] = read_bits(&bit_it, 8, &res); - if(res != LV_FS_RES_OK) { + if (nbits % 8 == 0) { /* Fast path */ + if(lv_fs_read(fp, &glyph_bmp[cur_bmp_size], bmp_size, NULL) != LV_FS_RES_OK) { return -1; } } + else { + for(int k = 0; k < bmp_size; ++k) { + glyph_bmp[cur_bmp_size + k] = read_bits(&bit_it, 8, &res); + if(res != LV_FS_RES_OK) { + return -1; + } + } + } + cur_bmp_size += bmp_size; } return glyph_length; @@ -508,6 +502,8 @@ static bool lvgl_load_font(lv_fs_file_t * fp, lv_font_t * font) font->get_glyph_dsc = lv_font_get_glyph_dsc_fmt_txt; font->get_glyph_bitmap = lv_font_get_bitmap_fmt_txt; font->subpx = font_header.subpixels_mode; + font->underline_position = font_header.underline_position; + font->underline_thickness = font_header.underline_thickness; font_dsc->bpp = font_header.bits_per_pixel; font_dsc->kern_scale = font_header.kerning_scale; @@ -535,8 +531,8 @@ static bool lvgl_load_font(lv_fs_file_t * fp, lv_font_t * font) bool failed = false; uint32_t * glyph_offset = lv_mem_alloc(sizeof(uint32_t) * (loca_count + 1)); - for(unsigned int i = 0; i < loca_count; ++i) { - if(font_header.index_to_loc_format == 0) { + if(font_header.index_to_loc_format == 0) { + for(unsigned int i = 0; i < loca_count; ++i) { uint16_t offset; if(lv_fs_read(fp, &offset, sizeof(uint16_t), NULL) != LV_FS_RES_OK) { failed = true; @@ -544,20 +540,16 @@ static bool lvgl_load_font(lv_fs_file_t * fp, lv_font_t * font) } glyph_offset[i] = offset; } - else if(font_header.index_to_loc_format == 1) { - uint32_t offset; - if(lv_fs_read(fp, &offset, sizeof(uint32_t), NULL) != LV_FS_RES_OK) { - failed = true; - break; - } - glyph_offset[i] = offset; - } - else { - LV_LOG_WARN("Unknown index_to_loc_format: %d.", font_header.index_to_loc_format); + } + else if(font_header.index_to_loc_format == 1) { + if(lv_fs_read(fp, glyph_offset, loca_count * sizeof(uint32_t), NULL) != LV_FS_RES_OK) { failed = true; - break; } } + else { + LV_LOG_WARN("Unknown index_to_loc_format: %d.", font_header.index_to_loc_format); + failed = true; + } if(failed) { lv_mem_free(glyph_offset); diff --git a/src/lv_font/lv_symbol_def.h b/src/lv_font/lv_symbol_def.h index bc32ec26f..83aab1e24 100644 --- a/src/lv_font/lv_symbol_def.h +++ b/src/lv_font/lv_symbol_def.h @@ -85,11 +85,11 @@ extern "C" { /*------------------------------- * Symbols from "normal" font *-----------------------------*/ -#define LV_SYMBOL_BULLET "\xE2\x80\xA2" /*20042, 0x2022*/ +#define LV_SYMBOL_BULLET "\xE2\x80\xA2" /*20042, 0x2022*/ /* * The following list is generated using - * cat src/lv_misc/lv_symbol_def.h | sed -E -n 's/^#define\s+(LV_SYMBOL_\w+).*"$/ _LV_STR_\1,/p' + * cat src/lv_font/lv_symbol_def.h | sed -E -n 's/^#define\s+LV_(SYMBOL_\w+).*".*$/ _LV_STR_\1,/p' */ enum { _LV_STR_SYMBOL_AUDIO, @@ -150,6 +150,7 @@ enum { _LV_STR_SYMBOL_SD_CARD, _LV_STR_SYMBOL_NEW_LINE, _LV_STR_SYMBOL_DUMMY, + _LV_STR_SYMBOL_BULLET, }; #ifdef __cplusplus