[WIP] OpenType Support #20
Conversation
|
Note, I decided to go the stb_truetype route rather than try and use freetype2. Means we don't have to try and link against freetype (or figure out how to statically compile it), and FBInk already uses other STB libraries, so it was a simple matter of Quality seems acceptable, probably not as nice as freetype2 would be, but good enough. Still have to think about how we handle font loading. Current implementation is for the developer to load the font into memory and pass it to FBInk. It might be nicer if FBInk could accept a font file path. EDIT: And also another thought, which I've got as a TODO in the code, but I'll mention here. We need to figure out how to handle the DPI/PPI value. It is used to calculate the font height in pixels from the size provided in points. The value is currently hard coded for the Aura H2O. |
|
First of all, thanks, this looks fantastic! So, in no particular order:
It made sense for cell-based rendering, both because that's what eips does on Kindle, so I'm used to its quirks, and because it was relatively tame at the beginning, but when you start piling stuff on top of it (centering, dumb line-breaking, whatever, ...), it quickly becomes a giant mess :D. And even for a user, I get that it might sound a bit opaque to use at first (again, used to eips here ;p).
|
|
And as far a glyph caching is concerned, I'm guessing it'll all depend on use-cases and real-world performance. As a vaguely related data point, story time: In the cell renderer, I was originally decoding the font data, then scaling it into a buffer, then rendering that scaled buffer. TL;DR: I'm not sure the type of use-cases this'll likely get warrant the effort for a glyph cache, which might make it a net loss instead of a gain. Keep in mind that this could barely even be called a gut feeling, so, if you want to try stuff in that realm, go ahead ;). |
|
I kind of appreciate the OT struct being on its own, as FBInkConfig is already starting to look like a giant mess ;). |
|
And I definitely get the STB choice, that'd probably have been my first choice since the image loading code, too ;). (Although I am rather well versed in compiling FT :D). |
|
And a final comment in the "not to forget when you're done" box: adding a libunibreak mention to the CREDITS file ;). |
|
Hey, thanks for the feedback! My only concern with passing font paths instead of an already loaded buffer is that I will need to add a function like `fbink_close_ot() to free the allocated memory. On the other hand, it would remove any ambiguity. I'm leaning towards changing the margins to pixels as well. That means it becomes meaningful to return the Y position of the next row of text that callers can set on the next print call. Thanks for the heads up on the macro. I never was that fond of it, but my bitwise-fu isn't always the greatest! As to glyph caching, I'm really not certain. Generating them is certainly more intensive than a simple integer scaling however, as there's floating point math and anti-aliasing going on. On the other hand, even with the current method, text is basically appearing on the screen instantly so... |
|
Hmm, maybe we need a "get viewinfo" function to return basic resolution info (without requiring getting the full state)? Because if I allow the user to specify margins as pixels, they probably would like to know how many pixels are available first! |
|
Yep, having an open/close pair would probably make things more obvious :). I'm not opposed to a specific getter for the resolution and/or viewport, but how were you envisioning it: returning a new struct that only contains a subset of the full state, or setting uint pointers? |
|
Either method could work I suppose. I've gotten used to setting pointers with the STB libraries. |
|
Yeah, that sounds like the least intrusive way to handle that ;). |
|
So... um... erm.... I have a confession to make. I kind of went and added font variant support (bold, italics, bold italics), as well as a very rudimentary "markdown style" parser. Ummm... Oops? :D
I'm pushing what I've done so far. Fair warning though, there are Problems. Namely segfaults. But only with certain combinations and lengths of text. In other words, I probably need to return to school, because I'm obviously having trouble with my sums. :p If you spot any obvious errors, please let me know. My brain has turned to mush... |
|
:D I'll try to take a look at it this afternoon, with my old friend gdb if need be ;). (If you have a test string that crashes reliably, that might help, although I'm guessing it'll crash somewhere in the md parsing stage ;)) |
|
Speaking of crashes, it also crashes with an empty string ;). In the render pass: |
|
FWIW, I'm testing with a quick'n dirty piggyback in fbink_cmd: diff --git a/fbink_cmd.c b/fbink_cmd.c
index fabf953..c2a0193 100644
--- a/fbink_cmd.c
+++ b/fbink_cmd.c
@@ -690,11 +690,23 @@ int
fbink_config.fontname,
fbink_config.fontmult);
}
+ // TTF!
+ FBInkOTConfig cfg = { 0 };
+ //cfg.is_formatted = true;
+ cfg.margins.top = 5;
+ fbink_add_ot_font("/mnt/onboard/fonts/Bookerly-Regular.ttf", FNT_REGULAR);
+ fbink_add_ot_font("/mnt/onboard/fonts/Bookerly-Italic.ttf", FNT_ITALIC);
+ fbink_add_ot_font("/mnt/onboard/fonts/Bookerly-Bold.ttf", FNT_BOLD);
+ fbink_add_ot_font("/mnt/onboard/fonts/Bookerly-BoldItalic.ttf", FNT_BOLD_ITALIC);
+ fbink_print_ot(fbfd, string, &cfg);
+ fbink_free_ot_fonts();
+ /*
if ((linecount = fbink_print(fbfd, string, &fbink_config)) < 0) {
fprintf(stderr, "Failed to print that string!\n");
rv = ERRCODE(EXIT_FAILURE);
goto cleanup;
}
+ */
// NOTE: Don't clobber previous entries if multiple strings were passed...
// We make sure to trust print's return value,
// because it knows how much space it already took up ;). |
|
Haven't managed to break anything else after a few quick tests ;). diff --git a/fbink.c b/fbink.c
index 36f32eb..7d563ea 100644
--- a/fbink.c
+++ b/fbink.c
@@ -2114,8 +2114,8 @@ int
}
// Free an individual OpenType font structure
-void*
- free_ot_font(stbtt_fontinfo* font_info)
+void*
+ free_ot_font(stbtt_fontinfo* font_info)
{
if (font_info) {
free(font_info->data); // This is the font data we loaded
@@ -2712,38 +2712,24 @@ int
// This is **bold** text.
// This is ***bold italic*** text.
// As well as their underscore equivalents
-void
+void
parse_simple_md(char* string, int size, unsigned char* result)
{
int ci = 0;
+ char ch;
bool is_italic = false;
bool is_bold = false;
while (ci < size) {
- switch (string[ci]) {
+ printf("ci: %d (< %d) is %c\n", ci, size, string[ci]);
+ switch (ch = string[ci]) {
case '*':
- if (ci + 1 < size && string[ci + 1] == '*') {
- if (ci + 2 < size && string[ci + 2] == '*') {
- is_bold = !is_bold;
- is_italic = !is_italic;
- result[ci] = CH_IGNORE;
- result[ci + 1] = CH_IGNORE;
- result[ci + 2] = CH_IGNORE;
- ci += 3;
- break;
- }
- is_bold = !is_bold;
- result[ci] = CH_IGNORE;
- result[ci + 1] = CH_IGNORE;
- ci += 2;
- break;
- }
- is_italic = !is_italic;
- result[ci] = CH_IGNORE;
- ci++;
- break;
case '_':
- if (ci + 1 < size && string[ci + 1] == '_') {
- if (ci + 2 < size && string[ci + 2] == '_') {
+ // FIXME: _ handling is problematic, because it'll catch in-word underscores when it shouldn't,
+ // since there's no closing tag ;).
+ if (ci + 1 < size && string[ci + 1] == ch) {
+ printf("ci: %d && ci + 1 == %c\n", ci, ch);
+ if (ci + 2 < size && string[ci + 2] == ch) {
+ printf("ci: %d && ci + 2 == %c\n", ci, ch);
is_bold = !is_bold;
is_italic = !is_italic;
result[ci] = CH_IGNORE;
@@ -2902,7 +2888,7 @@ int
}
// Calculate the maximum number of lines we may have to deal with
unsigned int num_lines = (unsigned int)(viewHeight / (uint32_t)max_height);
-
+
// And allocate the memory for it...
lines = calloc(num_lines, sizeof(FBInkOTLine));
@@ -2921,7 +2907,7 @@ int
LOG("Found linebreaks!");
// Parse our string for formatting, if requested
- fmt_buff = calloc(str_len_bytes, sizeof(char));
+ fmt_buff = calloc(str_len_bytes + 1, sizeof(char));
if (!fmt_buff) {
rv = ERRCODE(EXIT_FAILURE);
goto cleanup;So, basically, just making sure the result buffer is NULL-terminated, and avoiding code duplication between */_ handling. |
|
And with a pretty crappy workaround to the _ thing: diff --git a/fbink.c b/fbink.c
index 36f32eb..1d2ecfc 100644
--- a/fbink.c
+++ b/fbink.c
@@ -2114,8 +2114,8 @@ int
}
// Free an individual OpenType font structure
-void*
- free_ot_font(stbtt_fontinfo* font_info)
+void*
+ free_ot_font(stbtt_fontinfo* font_info)
{
if (font_info) {
free(font_info->data); // This is the font data we loaded
@@ -2712,17 +2712,22 @@ int
// This is **bold** text.
// This is ***bold italic*** text.
// As well as their underscore equivalents
-void
+void
parse_simple_md(char* string, int size, unsigned char* result)
{
int ci = 0;
+ char ch;
bool is_italic = false;
bool is_bold = false;
while (ci < size) {
- switch (string[ci]) {
+ printf("ci: %d (< %d) is %c\n", ci, size, string[ci]);
+ switch (ch = string[ci]) {
case '*':
- if (ci + 1 < size && string[ci + 1] == '*') {
- if (ci + 2 < size && string[ci + 2] == '*') {
+ case '_':
+ if (ci + 1 < size && string[ci + 1] == ch) {
+ printf("ci: %d && ci + 1 == %c\n", ci, ch);
+ if (ci + 2 < size && string[ci + 2] == ch) {
+ printf("ci: %d && ci + 2 == %c\n", ci, ch);
is_bold = !is_bold;
is_italic = !is_italic;
result[ci] = CH_IGNORE;
@@ -2737,25 +2742,10 @@ void
ci += 2;
break;
}
- is_italic = !is_italic;
- result[ci] = CH_IGNORE;
- ci++;
- break;
- case '_':
- if (ci + 1 < size && string[ci + 1] == '_') {
- if (ci + 2 < size && string[ci + 2] == '_') {
- is_bold = !is_bold;
- is_italic = !is_italic;
- result[ci] = CH_IGNORE;
- result[ci + 1] = CH_IGNORE;
- result[ci + 2] = CH_IGNORE;
- ci += 3;
- break;
- }
- is_bold = !is_bold;
- result[ci] = CH_IGNORE;
- result[ci + 1] = CH_IGNORE;
- ci += 2;
+ // Try to avoid flagging a single underscore in the middle of a word.
+ if (ch == '_' && ci > 0 && string[ci - 1] != ' ' && string[ci + 1] != ' ' && string[ci - 1] != ch && string[ci + 1] != ch) {
+ result[ci] = CH_REGULAR;
+ ci++;
break;
}
is_italic = !is_italic;
@@ -2902,7 +2892,7 @@ int
}
// Calculate the maximum number of lines we may have to deal with
unsigned int num_lines = (unsigned int)(viewHeight / (uint32_t)max_height);
-
+
// And allocate the memory for it...
lines = calloc(num_lines, sizeof(FBInkOTLine));
@@ -2921,7 +2911,7 @@ int
LOG("Found linebreaks!");
// Parse our string for formatting, if requested
- fmt_buff = calloc(str_len_bytes, sizeof(char));
+ fmt_buff = calloc(str_len_bytes + 1, sizeof(char));
if (!fmt_buff) {
rv = ERRCODE(EXIT_FAILURE);
goto cleanup; |
|
Given stbtt's native bitmap format, we can get inversion pretty easily, so, here goes: diff --git a/fbink.c b/fbink.c
index 36f32eb..6b2e4dc 100644
--- a/fbink.c
+++ b/fbink.c
@@ -2114,8 +2114,8 @@ int
}
// Free an individual OpenType font structure
-void*
- free_ot_font(stbtt_fontinfo* font_info)
+void*
+ free_ot_font(stbtt_fontinfo* font_info)
{
if (font_info) {
free(font_info->data); // This is the font data we loaded
@@ -2712,17 +2712,22 @@ int
// This is **bold** text.
// This is ***bold italic*** text.
// As well as their underscore equivalents
-void
+void
parse_simple_md(char* string, int size, unsigned char* result)
{
int ci = 0;
+ char ch;
bool is_italic = false;
bool is_bold = false;
while (ci < size) {
- switch (string[ci]) {
+ printf("ci: %d (< %d) is %c\n", ci, size, string[ci]);
+ switch (ch = string[ci]) {
case '*':
- if (ci + 1 < size && string[ci + 1] == '*') {
- if (ci + 2 < size && string[ci + 2] == '*') {
+ case '_':
+ if (ci + 1 < size && string[ci + 1] == ch) {
+ printf("ci: %d && ci + 1 == %c\n", ci, ch);
+ if (ci + 2 < size && string[ci + 2] == ch) {
+ printf("ci: %d && ci + 2 == %c\n", ci, ch);
is_bold = !is_bold;
is_italic = !is_italic;
result[ci] = CH_IGNORE;
@@ -2737,25 +2742,10 @@ void
ci += 2;
break;
}
- is_italic = !is_italic;
- result[ci] = CH_IGNORE;
- ci++;
- break;
- case '_':
- if (ci + 1 < size && string[ci + 1] == '_') {
- if (ci + 2 < size && string[ci + 2] == '_') {
- is_bold = !is_bold;
- is_italic = !is_italic;
- result[ci] = CH_IGNORE;
- result[ci + 1] = CH_IGNORE;
- result[ci + 2] = CH_IGNORE;
- ci += 3;
- break;
- }
- is_bold = !is_bold;
- result[ci] = CH_IGNORE;
- result[ci + 1] = CH_IGNORE;
- ci += 2;
+ // Try to avoid flagging a single underscore in the middle of a word.
+ if (ch == '_' && ci > 0 && string[ci - 1] != ' ' && string[ci + 1] != ' ' && string[ci - 1] != ch && string[ci + 1] != ch) {
+ result[ci] = CH_REGULAR;
+ ci++;
break;
}
is_italic = !is_italic;
@@ -2902,7 +2892,7 @@ int
}
// Calculate the maximum number of lines we may have to deal with
unsigned int num_lines = (unsigned int)(viewHeight / (uint32_t)max_height);
-
+
// And allocate the memory for it...
lines = calloc(num_lines, sizeof(FBInkOTLine));
@@ -2921,7 +2911,7 @@ int
LOG("Found linebreaks!");
// Parse our string for formatting, if requested
- fmt_buff = calloc(str_len_bytes, sizeof(char));
+ fmt_buff = calloc(str_len_bytes + 1, sizeof(char));
if (!fmt_buff) {
rv = ERRCODE(EXIT_FAILURE);
goto cleanup;
@@ -3064,7 +3054,7 @@ int
unsigned char *lnPtr, *glPtr = NULL;
unsigned short start_x, start_y;
// stb_truetype renders glyphs with color inverted to what our blitting functions expect
- unsigned char invert = 0xff;
+ unsigned char invert = cfg->is_inverted ? 0x00 : 0xFF;
// Render!
for (line = 0; lines[line].line_used; line++) {
printf("Line # %d\n", line);
diff --git a/fbink.h b/fbink.h
index 3777e19..8dbe3d0 100644
--- a/fbink.h
+++ b/fbink.h
@@ -198,6 +198,7 @@ typedef struct {
} margins;
bool is_centered; // Horizontal text centering
bool is_formatted; // Is string "formatted"? Bold/Italic support only, markdown like syntax
+ bool is_inverted;
} FBInkOTConfig;
// NOTE: Unless otherwise specified,
@@ -271,7 +272,7 @@ FBINK_API int fbink_printf(int fbfd, const FBInkConfig* fbink_config, const char
__attribute__((format(printf, 3, 4)));
// Print a string using an OpenType font. Note the caller MUST init with fbink_init_ot() FIRST.
-// This function uses margins (as whole number percentages) instead of rows/columns for
+// This function uses margins (as whole number percentages) instead of rows/columns for
// positioning and setting the printable area.
// Returns -(ERANGE) if the provided margins are out of range, or sum to < 100%
// Returns -(ENOSYS) if compiled with MINIMAL
diff --git a/fbink_cmd.c b/fbink_cmd.c
index fabf953..6eb5df1 100644
--- a/fbink_cmd.c
+++ b/fbink_cmd.c
@@ -690,11 +690,23 @@ int
fbink_config.fontname,
fbink_config.fontmult);
}
+ // TTF!
+ FBInkOTConfig cfg = { 0 };
+ cfg.is_formatted = true;
+ cfg.margins.top = 5;
+ fbink_add_ot_font("/mnt/onboard/fonts/Bookerly-Regular.ttf", FNT_REGULAR);
+ fbink_add_ot_font("/mnt/onboard/fonts/Bookerly-Italic.ttf", FNT_ITALIC);
+ fbink_add_ot_font("/mnt/onboard/fonts/Bookerly-Bold.ttf", FNT_BOLD);
+ fbink_add_ot_font("/mnt/onboard/fonts/Bookerly-BoldItalic.ttf", FNT_BOLD_ITALIC);
+ fbink_print_ot(fbfd, string, &cfg);
+ fbink_free_ot_fonts();
+ /*
if ((linecount = fbink_print(fbfd, string, &fbink_config)) < 0) {
fprintf(stderr, "Failed to print that string!\n");
rv = ERRCODE(EXIT_FAILURE);
goto cleanup;
}
+ */
// NOTE: Don't clobber previous entries if multiple strings were passed...
// We make sure to trust print's return value,
// because it knows how much space it already took up ;). |
|
And that's it for now ;). I you have a few test strings that crashes, I'm happy to try that on my end. (I'll probably need to know the font/sizes/margins you were using, though). |
|
Okay, I lied :D. Now with added crappy empty string check! diff --git a/fbink.c b/fbink.c
index 36f32eb..d412f35 100644
--- a/fbink.c
+++ b/fbink.c
@@ -2114,8 +2114,8 @@ int
}
// Free an individual OpenType font structure
-void*
- free_ot_font(stbtt_fontinfo* font_info)
+void*
+ free_ot_font(stbtt_fontinfo* font_info)
{
if (font_info) {
free(font_info->data); // This is the font data we loaded
@@ -2712,17 +2712,22 @@ int
// This is **bold** text.
// This is ***bold italic*** text.
// As well as their underscore equivalents
-void
+void
parse_simple_md(char* string, int size, unsigned char* result)
{
int ci = 0;
+ char ch;
bool is_italic = false;
bool is_bold = false;
while (ci < size) {
- switch (string[ci]) {
+ printf("ci: %d (< %d) is %c\n", ci, size, string[ci]);
+ switch (ch = string[ci]) {
case '*':
- if (ci + 1 < size && string[ci + 1] == '*') {
- if (ci + 2 < size && string[ci + 2] == '*') {
+ case '_':
+ if (ci + 1 < size && string[ci + 1] == ch) {
+ printf("ci: %d && ci + 1 == %c\n", ci, ch);
+ if (ci + 2 < size && string[ci + 2] == ch) {
+ printf("ci: %d && ci + 2 == %c\n", ci, ch);
is_bold = !is_bold;
is_italic = !is_italic;
result[ci] = CH_IGNORE;
@@ -2737,25 +2742,10 @@ void
ci += 2;
break;
}
- is_italic = !is_italic;
- result[ci] = CH_IGNORE;
- ci++;
- break;
- case '_':
- if (ci + 1 < size && string[ci + 1] == '_') {
- if (ci + 2 < size && string[ci + 2] == '_') {
- is_bold = !is_bold;
- is_italic = !is_italic;
- result[ci] = CH_IGNORE;
- result[ci + 1] = CH_IGNORE;
- result[ci + 2] = CH_IGNORE;
- ci += 3;
- break;
- }
- is_bold = !is_bold;
- result[ci] = CH_IGNORE;
- result[ci + 1] = CH_IGNORE;
- ci += 2;
+ // Try to avoid flagging a single underscore in the middle of a word.
+ if (ch == '_' && ci > 0 && string[ci - 1] != ' ' && string[ci + 1] != ' ' && string[ci - 1] != ch && string[ci + 1] != ch) {
+ result[ci] = CH_REGULAR;
+ ci++;
break;
}
is_italic = !is_italic;
@@ -2788,6 +2778,11 @@ int
# pragma GCC diagnostic ignored "-Wconversion"
# pragma GCC diagnostic ignored "-Wbad-function-cast"
+ // Abort if we were passed an empty string
+ if (string[0] == '\0') {
+ return ERRCODE(EXIT_FAILURE);
+ }
+
// Has fbink_init_ot() been called yet?
if (!otInit) {
return ERRCODE(ENODATA);
@@ -2902,7 +2897,7 @@ int
}
// Calculate the maximum number of lines we may have to deal with
unsigned int num_lines = (unsigned int)(viewHeight / (uint32_t)max_height);
-
+
// And allocate the memory for it...
lines = calloc(num_lines, sizeof(FBInkOTLine));
@@ -2921,7 +2916,7 @@ int
LOG("Found linebreaks!");
// Parse our string for formatting, if requested
- fmt_buff = calloc(str_len_bytes, sizeof(char));
+ fmt_buff = calloc(str_len_bytes + 1, sizeof(char));
if (!fmt_buff) {
rv = ERRCODE(EXIT_FAILURE);
goto cleanup;
@@ -3064,7 +3059,7 @@ int
unsigned char *lnPtr, *glPtr = NULL;
unsigned short start_x, start_y;
// stb_truetype renders glyphs with color inverted to what our blitting functions expect
- unsigned char invert = 0xff;
+ unsigned char invert = cfg->is_inverted ? 0x00 : 0xFF;
// Render!
for (line = 0; lines[line].line_used; line++) {
printf("Line # %d\n", line);
diff --git a/fbink.h b/fbink.h
index 3777e19..8dbe3d0 100644
--- a/fbink.h
+++ b/fbink.h
@@ -198,6 +198,7 @@ typedef struct {
} margins;
bool is_centered; // Horizontal text centering
bool is_formatted; // Is string "formatted"? Bold/Italic support only, markdown like syntax
+ bool is_inverted;
} FBInkOTConfig;
// NOTE: Unless otherwise specified,
@@ -271,7 +272,7 @@ FBINK_API int fbink_printf(int fbfd, const FBInkConfig* fbink_config, const char
__attribute__((format(printf, 3, 4)));
// Print a string using an OpenType font. Note the caller MUST init with fbink_init_ot() FIRST.
-// This function uses margins (as whole number percentages) instead of rows/columns for
+// This function uses margins (as whole number percentages) instead of rows/columns for
// positioning and setting the printable area.
// Returns -(ERANGE) if the provided margins are out of range, or sum to < 100%
// Returns -(ENOSYS) if compiled with MINIMAL
diff --git a/fbink_cmd.c b/fbink_cmd.c
index fabf953..6eb5df1 100644
--- a/fbink_cmd.c
+++ b/fbink_cmd.c
@@ -690,11 +690,23 @@ int
fbink_config.fontname,
fbink_config.fontmult);
}
+ // TTF!
+ FBInkOTConfig cfg = { 0 };
+ cfg.is_formatted = true;
+ cfg.margins.top = 5;
+ fbink_add_ot_font("/mnt/onboard/fonts/Bookerly-Regular.ttf", FNT_REGULAR);
+ fbink_add_ot_font("/mnt/onboard/fonts/Bookerly-Italic.ttf", FNT_ITALIC);
+ fbink_add_ot_font("/mnt/onboard/fonts/Bookerly-Bold.ttf", FNT_BOLD);
+ fbink_add_ot_font("/mnt/onboard/fonts/Bookerly-BoldItalic.ttf", FNT_BOLD_ITALIC);
+ fbink_print_ot(fbfd, string, &cfg);
+ fbink_free_ot_fonts();
+ /*
if ((linecount = fbink_print(fbfd, string, &fbink_config)) < 0) {
fprintf(stderr, "Failed to print that string!\n");
rv = ERRCODE(EXIT_FAILURE);
goto cleanup;
}
+ */
// NOTE: Don't clobber previous entries if multiple strings were passed...
// We make sure to trust print's return value,
// because it knows how much space it already took up ;). |
|
(I refrained from using a pointery syntax for that check, because I find it ( |
|
The extra != ch tests in my crappy underscore workaround might actually be superfluous... EDIT: Yeah, probably. diff --git a/fbink.c b/fbink.c
index 36f32eb..7bf4431 100644
--- a/fbink.c
+++ b/fbink.c
@@ -2114,8 +2114,8 @@ int
}
// Free an individual OpenType font structure
-void*
- free_ot_font(stbtt_fontinfo* font_info)
+void*
+ free_ot_font(stbtt_fontinfo* font_info)
{
if (font_info) {
free(font_info->data); // This is the font data we loaded
@@ -2712,17 +2712,22 @@ int
// This is **bold** text.
// This is ***bold italic*** text.
// As well as their underscore equivalents
-void
+void
parse_simple_md(char* string, int size, unsigned char* result)
{
int ci = 0;
+ char ch;
bool is_italic = false;
bool is_bold = false;
while (ci < size) {
- switch (string[ci]) {
+ printf("ci: %d (< %d) is %c\n", ci, size, string[ci]);
+ switch (ch = string[ci]) {
case '*':
- if (ci + 1 < size && string[ci + 1] == '*') {
- if (ci + 2 < size && string[ci + 2] == '*') {
+ case '_':
+ if (ci + 1 < size && string[ci + 1] == ch) {
+ printf("ci: %d && ci + 1 == %c\n", ci, ch);
+ if (ci + 2 < size && string[ci + 2] == ch) {
+ printf("ci: %d && ci + 2 == %c\n", ci, ch);
is_bold = !is_bold;
is_italic = !is_italic;
result[ci] = CH_IGNORE;
@@ -2737,25 +2742,10 @@ void
ci += 2;
break;
}
- is_italic = !is_italic;
- result[ci] = CH_IGNORE;
- ci++;
- break;
- case '_':
- if (ci + 1 < size && string[ci + 1] == '_') {
- if (ci + 2 < size && string[ci + 2] == '_') {
- is_bold = !is_bold;
- is_italic = !is_italic;
- result[ci] = CH_IGNORE;
- result[ci + 1] = CH_IGNORE;
- result[ci + 2] = CH_IGNORE;
- ci += 3;
- break;
- }
- is_bold = !is_bold;
- result[ci] = CH_IGNORE;
- result[ci + 1] = CH_IGNORE;
- ci += 2;
+ // Try to avoid flagging a single underscore in the middle of a word.
+ if (ch == '_' && ci > 0 && string[ci - 1] != ' ' && string[ci + 1] != ' ') {
+ result[ci] = CH_REGULAR;
+ ci++;
break;
}
is_italic = !is_italic;
@@ -2788,6 +2778,11 @@ int
# pragma GCC diagnostic ignored "-Wconversion"
# pragma GCC diagnostic ignored "-Wbad-function-cast"
+ // Abort if we were passed an empty string
+ if (! *string) {
+ return ERRCODE(EXIT_FAILURE);
+ }
+
// Has fbink_init_ot() been called yet?
if (!otInit) {
return ERRCODE(ENODATA);
@@ -2902,7 +2897,7 @@ int
}
// Calculate the maximum number of lines we may have to deal with
unsigned int num_lines = (unsigned int)(viewHeight / (uint32_t)max_height);
-
+
// And allocate the memory for it...
lines = calloc(num_lines, sizeof(FBInkOTLine));
@@ -2921,7 +2916,7 @@ int
LOG("Found linebreaks!");
// Parse our string for formatting, if requested
- fmt_buff = calloc(str_len_bytes, sizeof(char));
+ fmt_buff = calloc(str_len_bytes + 1, sizeof(char));
if (!fmt_buff) {
rv = ERRCODE(EXIT_FAILURE);
goto cleanup;
@@ -3064,7 +3059,7 @@ int
unsigned char *lnPtr, *glPtr = NULL;
unsigned short start_x, start_y;
// stb_truetype renders glyphs with color inverted to what our blitting functions expect
- unsigned char invert = 0xff;
+ unsigned char invert = cfg->is_inverted ? 0x00 : 0xFF;
// Render!
for (line = 0; lines[line].line_used; line++) {
printf("Line # %d\n", line);
diff --git a/fbink.h b/fbink.h
index 3777e19..8dbe3d0 100644
--- a/fbink.h
+++ b/fbink.h
@@ -198,6 +198,7 @@ typedef struct {
} margins;
bool is_centered; // Horizontal text centering
bool is_formatted; // Is string "formatted"? Bold/Italic support only, markdown like syntax
+ bool is_inverted;
} FBInkOTConfig;
// NOTE: Unless otherwise specified,
@@ -271,7 +272,7 @@ FBINK_API int fbink_printf(int fbfd, const FBInkConfig* fbink_config, const char
__attribute__((format(printf, 3, 4)));
// Print a string using an OpenType font. Note the caller MUST init with fbink_init_ot() FIRST.
-// This function uses margins (as whole number percentages) instead of rows/columns for
+// This function uses margins (as whole number percentages) instead of rows/columns for
// positioning and setting the printable area.
// Returns -(ERANGE) if the provided margins are out of range, or sum to < 100%
// Returns -(ENOSYS) if compiled with MINIMAL
diff --git a/fbink_cmd.c b/fbink_cmd.c
index fabf953..6eb5df1 100644
--- a/fbink_cmd.c
+++ b/fbink_cmd.c
@@ -690,11 +690,23 @@ int
fbink_config.fontname,
fbink_config.fontmult);
}
+ // TTF!
+ FBInkOTConfig cfg = { 0 };
+ cfg.is_formatted = true;
+ cfg.margins.top = 5;
+ fbink_add_ot_font("/mnt/onboard/fonts/Bookerly-Regular.ttf", FNT_REGULAR);
+ fbink_add_ot_font("/mnt/onboard/fonts/Bookerly-Italic.ttf", FNT_ITALIC);
+ fbink_add_ot_font("/mnt/onboard/fonts/Bookerly-Bold.ttf", FNT_BOLD);
+ fbink_add_ot_font("/mnt/onboard/fonts/Bookerly-BoldItalic.ttf", FNT_BOLD_ITALIC);
+ fbink_print_ot(fbfd, string, &cfg);
+ fbink_free_ot_fonts();
+ /*
if ((linecount = fbink_print(fbfd, string, &fbink_config)) < 0) {
fprintf(stderr, "Failed to print that string!\n");
rv = ERRCODE(EXIT_FAILURE);
goto cleanup;
}
+ */
// NOTE: Don't clobber previous entries if multiple strings were passed...
// We make sure to trust print's return value,
// because it knows how much space it already took up ;). |
|
And with a private stbtt implementation, which may allow GCC some more optimization opportunities... diff --git a/fbink.c b/fbink.c
index 36f32eb..7439800 100644
--- a/fbink.c
+++ b/fbink.c
@@ -66,6 +66,8 @@
// stb_truetype needs maths, and so do we to round to the nearest pixel
# include <math.h>
# define STB_TRUETYPE_IMPLEMENTATION
+// Make it private, we don't need it anywhere else
+# define STBTT_STATIC
// stb_truetype is.... noisy
# pragma GCC diagnostic push
# pragma GCC diagnostic ignored "-Wunknown-pragmas"
@@ -2114,8 +2116,8 @@ int
}
// Free an individual OpenType font structure
-void*
- free_ot_font(stbtt_fontinfo* font_info)
+void*
+ free_ot_font(stbtt_fontinfo* font_info)
{
if (font_info) {
free(font_info->data); // This is the font data we loaded
@@ -2712,17 +2714,22 @@ int
// This is **bold** text.
// This is ***bold italic*** text.
// As well as their underscore equivalents
-void
+void
parse_simple_md(char* string, int size, unsigned char* result)
{
int ci = 0;
+ char ch;
bool is_italic = false;
bool is_bold = false;
while (ci < size) {
- switch (string[ci]) {
+ printf("ci: %d (< %d) is %c\n", ci, size, string[ci]);
+ switch (ch = string[ci]) {
case '*':
- if (ci + 1 < size && string[ci + 1] == '*') {
- if (ci + 2 < size && string[ci + 2] == '*') {
+ case '_':
+ if (ci + 1 < size && string[ci + 1] == ch) {
+ printf("ci: %d && ci + 1 == %c\n", ci, ch);
+ if (ci + 2 < size && string[ci + 2] == ch) {
+ printf("ci: %d && ci + 2 == %c\n", ci, ch);
is_bold = !is_bold;
is_italic = !is_italic;
result[ci] = CH_IGNORE;
@@ -2737,25 +2744,10 @@ void
ci += 2;
break;
}
- is_italic = !is_italic;
- result[ci] = CH_IGNORE;
- ci++;
- break;
- case '_':
- if (ci + 1 < size && string[ci + 1] == '_') {
- if (ci + 2 < size && string[ci + 2] == '_') {
- is_bold = !is_bold;
- is_italic = !is_italic;
- result[ci] = CH_IGNORE;
- result[ci + 1] = CH_IGNORE;
- result[ci + 2] = CH_IGNORE;
- ci += 3;
- break;
- }
- is_bold = !is_bold;
- result[ci] = CH_IGNORE;
- result[ci + 1] = CH_IGNORE;
- ci += 2;
+ // Try to avoid flagging a single underscore in the middle of a word.
+ if (ch == '_' && ci > 0 && string[ci - 1] != ' ' && string[ci + 1] != ' ') {
+ result[ci] = CH_REGULAR;
+ ci++;
break;
}
is_italic = !is_italic;
@@ -2788,6 +2780,11 @@ int
# pragma GCC diagnostic ignored "-Wconversion"
# pragma GCC diagnostic ignored "-Wbad-function-cast"
+ // Abort if we were passed an empty string
+ if (! *string) {
+ return ERRCODE(EXIT_FAILURE);
+ }
+
// Has fbink_init_ot() been called yet?
if (!otInit) {
return ERRCODE(ENODATA);
@@ -2902,7 +2899,7 @@ int
}
// Calculate the maximum number of lines we may have to deal with
unsigned int num_lines = (unsigned int)(viewHeight / (uint32_t)max_height);
-
+
// And allocate the memory for it...
lines = calloc(num_lines, sizeof(FBInkOTLine));
@@ -2921,7 +2918,7 @@ int
LOG("Found linebreaks!");
// Parse our string for formatting, if requested
- fmt_buff = calloc(str_len_bytes, sizeof(char));
+ fmt_buff = calloc(str_len_bytes + 1, sizeof(char));
if (!fmt_buff) {
rv = ERRCODE(EXIT_FAILURE);
goto cleanup;
@@ -3064,7 +3061,7 @@ int
unsigned char *lnPtr, *glPtr = NULL;
unsigned short start_x, start_y;
// stb_truetype renders glyphs with color inverted to what our blitting functions expect
- unsigned char invert = 0xff;
+ unsigned char invert = cfg->is_inverted ? 0x00 : 0xFF;
// Render!
for (line = 0; lines[line].line_used; line++) {
printf("Line # %d\n", line);
diff --git a/fbink.h b/fbink.h
index 3777e19..8dbe3d0 100644
--- a/fbink.h
+++ b/fbink.h
@@ -198,6 +198,7 @@ typedef struct {
} margins;
bool is_centered; // Horizontal text centering
bool is_formatted; // Is string "formatted"? Bold/Italic support only, markdown like syntax
+ bool is_inverted;
} FBInkOTConfig;
// NOTE: Unless otherwise specified,
@@ -271,7 +272,7 @@ FBINK_API int fbink_printf(int fbfd, const FBInkConfig* fbink_config, const char
__attribute__((format(printf, 3, 4)));
// Print a string using an OpenType font. Note the caller MUST init with fbink_init_ot() FIRST.
-// This function uses margins (as whole number percentages) instead of rows/columns for
+// This function uses margins (as whole number percentages) instead of rows/columns for
// positioning and setting the printable area.
// Returns -(ERANGE) if the provided margins are out of range, or sum to < 100%
// Returns -(ENOSYS) if compiled with MINIMAL
diff --git a/fbink_cmd.c b/fbink_cmd.c
index fabf953..6eb5df1 100644
--- a/fbink_cmd.c
+++ b/fbink_cmd.c
@@ -690,11 +690,23 @@ int
fbink_config.fontname,
fbink_config.fontmult);
}
+ // TTF!
+ FBInkOTConfig cfg = { 0 };
+ cfg.is_formatted = true;
+ cfg.margins.top = 5;
+ fbink_add_ot_font("/mnt/onboard/fonts/Bookerly-Regular.ttf", FNT_REGULAR);
+ fbink_add_ot_font("/mnt/onboard/fonts/Bookerly-Italic.ttf", FNT_ITALIC);
+ fbink_add_ot_font("/mnt/onboard/fonts/Bookerly-Bold.ttf", FNT_BOLD);
+ fbink_add_ot_font("/mnt/onboard/fonts/Bookerly-BoldItalic.ttf", FNT_BOLD_ITALIC);
+ fbink_print_ot(fbfd, string, &cfg);
+ fbink_free_ot_fonts();
+ /*
if ((linecount = fbink_print(fbfd, string, &fbink_config)) < 0) {
fprintf(stderr, "Failed to print that string!\n");
rv = ERRCODE(EXIT_FAILURE);
goto cleanup;
}
+ */
// NOTE: Don't clobber previous entries if multiple strings were passed...
// We make sure to trust print's return value,
// because it knows how much space it already took up ;).
diff --git a/fbink_types.h b/fbink_types.h
index 89d1c5f..42187d2 100644
--- a/fbink_types.h
+++ b/fbink_types.h
@@ -23,6 +23,9 @@
#include <stdbool.h>
#include <stdint.h>
+// NOTE: We need to import stbtt early because we depend on stbtt_fontinfo here
+// We'll want it as static/private, so do that here, because we're importing it earlier than fbink.c
+#define STBTT_STATIC
#include "stb/stb_truetype.h"
// List of flags for device or screen-specific quirks... |
|
Okay, ungluing myself from the screen, for real this time ;). |
|
I'm not sure if that might be an issue here, but I've had weird things happening with libu8 skipping over a single terminating NULL because something looked juicy enough after the end of the buffer ;). |
|
And on the topic of possibly unrelated stuff, here's what's Clang's static analyzer had to say: I tend to take all of that with a grain of salt, but, who knows, sometimes it does catch things ;). |
|
There we go. Changed the eink refresh implementation, and I've also implemented support for the is_cleared and is_flashing options. I think that's about it, apart from the previously discussed DPI matter. |
|
👍 I'll take a final look at it tomorrow then, and I'll handle the dpi & CLI tweaks once it's merged ;). Many thanks for working on this! |
|
Thanks for the assistance! I feel a bit bad for opening this PR so early, but your input has been greatly appreciated, and has hopefully resulted in a much more complete and stable solution. |
|
As you've probably gathered by now, I'm pretty much in the "throw stuff at the wall, and see what sticks" camp, so, really, not a problem ;). |
|
Here we go! :). Thanks! |
|
You're welcome. Although I just noticed a boo-boo I made that got merged. I forgot to reword the fbink_print_ot() API documentation regarding margins. It is still referring to percentages instead of pixels. |
|
Yeah, I noticed, no worries, I'll take care of it ;). |
|
Thanks |
|
One other thing I just noticed, which I haven't yet looked into: with (at least) valign CENTER, when re-using the top margin, subsequent print calls won't be edge-to-edge, the second line will start at 3/4 down the screen ;). |
|
It's that way by design. Alignment is not to center of the viewport, but rather center to whatever the printable area is, which is defined by the margins. It allows for some very creative positioning. But now that you've brought it up, I can also see the potential issue with it. However, as soon as you make multiple print calls for vertical centering, you lose the perfect centering anyway (as previous lines aren't reprinted), so you may as well not use valign, but something like a 50% top margin. TLDR, vertical centering really only makes sense in the context of a single print call (which honors linefeeds, so you can still do multiple lines in one call, if required) |
|
Yep, that makes perfect sense, will just have to jot that down somewhere ;). |
|
I feel a bit bad for not doing a better job of cleaning up the type conversions/casting. My apologies for that, and thank you. Unfortunately, it was the nature of the libraries used that required so much type casting. And in my own code, there was quite a bit of well what type do I use here...? going on. |
|
Well, to be fair, I do use a fairly psychotic amount of warnings, so, that doesn't help ;). And yeah, the fact that stb is engineered by a long-time Win32 games dev relying on a really old VS toolchain means no shiny C99 types (well, no shiny C99 anything, really :D), and a fine balancing act between memory usage and performance when it comes to the choice of types. Which leaves us with a few questionable choices to deal with (signed codepoints, really? :D). TL;DR: What was left was mostly making GCC happy one way or another, don't worry about it too much ;). Didn't really hit any snag when it came to the actual underlying types. |
|
@shermp: Quick question, since I'll probably attempt to look at the AA stuff in esoteric rendering modes over the next few days: What was the reasoning behind inverting the colors if Because if it's only about |
|
Now that I think about it, layer diff could have been made negative and it still would work. The main concern is that when performing the following calculation: the fg should be darker than bg if "black on grey/white" or lighter if "white on black/grey" I was trying to get my head around it, and realized that the two situations were the inverse of each other, hence the inversion. But thinking of it now, a simple negative |
|
Just some musings on the AA side of things, I've just had an idea. Currently, each 'line' is prepared with the background colour, and then the glyphs are rendered onto it, baking the final colour information in the process. What if instead, we treat the line itself as an alpha, and only when painting to the framebuffer do we set the background/foreground colour etc. This would preserve the full AA information in the glyph right up to the final rendering stage. Thoughts? |
|
I think that's what I was thinking of, yeah: keeping the alpha coverage mask we get from stbtt up until the final rendering stage, where we "just" have to alpha blend it (with the usual 0/0xFF shortcuts to avoid the maths). |
|
Okay, started experimenting for real in the ot-blend branch, because trying to figure that out without code & testing was starting to break my brain ^^. So far, so good... (3c7b0e8). Performance is roughly similar, so I'm not even sure I'll bother adding a fast path for B/W||W/B where we could just paint the mask inverted or not... |
|
Looks good! I probably should have taken this approach originally, but I'm still rather a novice at this whole per-pixel image manipulation business... |
|
I nearly threw the Kindle 3 out the window near the end, there... :D. But apart from the monstrosity that is handling those 4bpp fbs, it went fairly okay, once I realized I'd forgot a multiplication... -_-". |
|
Just noticed another potential boo-boo. Where we are painting lines using the loop |
|
Indeed it does, nice catch! |
As @shermp pointed out (#20 (comment)) Thanks!
Note that this was initially an idea floated by @shermp in #20 (#20 (comment)) :).
Hi NiLuJe
I decided to see if I could figure out this whole OpenType/TrueType thing...
I'm opening this pull request relatively early, so I hopefully don't go too far down the wrong track if I've done stuff you don't agree with and/or want changed. Consider this Not Done Yet.
With that out the way, here's what's working:
Centered textOops, this seems a bit broken if I have linebreaks in there.What's not implemented:
I've done away with the rows/columns concept, as it doesn't make much sense when using proportional fonts. I've also currently created a new struct for the OT settings. We may wish to integrate these to the main FBInkConfig struct.
Nothing has been optimized yet. We may want to look at caching glyph bitmaps for example (although, that will increase memory usage).
I also haven't handled any rotation shenanigans, I'm not sure if what I've done requires this.
I look forward to getting your feedback on this. No rush though, you are allowed to sleep!
(Also, if I may make an observation on the current printing code? The rows/columns concept seems to be more trouble than its worth IMHO)