Providing a means to programmatically get glyph dimensions
Pharap opened this issue · 12 comments
I often find myself forgetting the size of the built-in font's glyphs.
Perhaps it would be a good idea to introduce a means to acquire these programmatically?
E.g. constexpr uint8_t getGlyphWidth() const and constexpr uint8_t getGlyphHeight() const.
Among other things, this would help when calculating text positions, such as when centring text.
Note that I choose 'glyph' rather than 'font' to be technically accurate, though this term may be unfamiliar with many users.
(typography, computing) A visual representation of a letter, character, or symbol, in a specific font and style.
Given your suggested uses, I assume you would want them to give the number of pixels occupied by a glyph when write() is used. I.e. 6 for width and 8 for height. Technically, the font is considered to be 5 x 7 with one extra height pixel for a descender.
Yes, glyph isn't a term that has been used in the library. Instead, character has been used. Although it may be slightly ambiguous, possibly being mistaken as referring to a C++ character type variable, I think getCharacterWidth() and getCharacterHeight() may be better.
Again for your suggested uses, consider that the current selected text size, as set by the setTextSize() function, will affect the rendered size of a glyph. Should the value returned be based on the current text size (meaning the functions could no longer be constexpr)? Or, would the developer have to do something like (getCharacterWidth() * getTextSize()), or create new functions? like:
constexpr uint8_t getSize2CharacterWidth() { return arduboy.getCharacterWidth() * 2 }
Should the value returned be based on the current text size (meaning the functions could no longer be
constexpr)?
I guess another possibility would be to make the functions constexpr but with a (possibly optional) size argument.
constexpr uint8_t getCharacterWidth(uint8_t textSize)
If the argument was constant then the function would be evaluated at compile time. If the argument was not a constant, the function would be compiled.
Given your suggested uses, I assume you would want them to give the number of pixels occupied by a glyph when write() is used.
I'm on the fence about which behaviour would be better.
Semantically returning the exact glyph dimensions makes more sense, and it may make more sense for some use cases, but for the use cases I've got in mind I'd expect including the spacing to be more useful.
I guess another possibility would be to make the functions constexpr but with a (possibly optional) size argument.
That probably makes more sense.
And if this approach is used, I think it's definitely worth making the argument optional. In my experience very few people actually change the font size, so being able to call getCharacterWidth without an argument to get the default size of the font would be useful. It would also have the added bonus of making arduboy.getCharacterWidth() * arduboy.getTextSize() a redundant but valid alternative to arduboy.getCharacterWidth(arduboy.getTextSize()).
but for the use cases I've got in mind I'd expect including the spacing to be more useful.
I tend to agree with doing it this way. I think most use cases would want this. For the rare exception, it's not hard to subtract 1 (and include a constexpr in your own code for it, if desired).
Is this something you'd like to have relatively soon, like for the current game jam? If so, I don't mind doing a quick point release just for this.
Assuming the names will be getCharacterWidth and getCharacterHeight and will be members of the Arduboy2 class and each will have both 0 and 1 argument versions, what would you suggest the declarations and definitions be?
Is this something you'd like to have relatively soon, like for the current game jam?
It was a game that I'm writing that spurred the idea, but I can also make do without it using hardcoded constants if you'd prefer to wait until more features have accrued or to spend more time contemplating the addition.
Assuming the names will be getCharacterWidth and getCharacterHeight and will be members of the Arduboy2 class and each will have both 0 and 1 argument versions, what would you suggest the declarations and definitions be?
Being constexpr of course limits them to being defined in the header (i.e. they cannot be defined in the corresponding .cpp file). I believe it would be possible to declare the function without a defintition in the class definition and provide the function definitions afterwards if you'd prefer to have the declaration and definition separate, but in this case I don't think doing so would provide any particular benefit.
The most obvious definitions would be:
static constexpr uint8_t getCharacterWidth(uint8_t textSize = 1)
{
return (6 * textSize);
}
static constexpr uint8_t getCharacterHeight(uint8_t textSize = 1)
{
return (8 * textSize);
}Theoretically the library could be ported to a system in which font dimensions could exceed the limits of a uint8_t, but that seems unlikely as it would be quite impractical (and at that point the font rendering code may not even function properly), hence uint8_t seems to be sufficient.
One thought that has crossed my mind is that it may be worth having named constants instead of hardcoded values in the functions themselves, e.g.
static constexpr uint8_t characterWidth = 6;
static constexpr uint8_t characterHeight = 8;
static constexpr uint8_t getCharacterWidth(uint8_t textSize = 1)
{
return (characterWidth * textSize);
}
static constexpr uint8_t getCharacterHeight(uint8_t textSize = 1)
{
return (characterHeight * textSize);
}I can't honestly think of any practical reason for doing so.
(I suppose they might be of use for replacing some of the 'magic numbers' in drawChar?)
Though perhaps if characterWidth and characterHeight were the exact dimensions without the spacing accounted for, they may be more useful?
static constexpr uint8_t characterWidth = 5;
static constexpr uint8_t characterHeight = 7;
static constexpr uint8_t getCharacterWidth(uint8_t textSize = 1)
{
return ((characterWidth + 1) * textSize);
}
static constexpr uint8_t getCharacterHeight(uint8_t textSize = 1)
{
return ((characterHeight + 1) * textSize);
}Going off-topic briefly, I had a look at drawChar and noticed what appears to be a discrepency in the clipping test:
Lines 1270 to 1273 in 5d154f8
It seems that while the top clipping test includes the line spacing, the left clipping test does not include the character spacing.
static constexpr uint8_t getCharacterWidth(uint8_t textSize = 1)
For some reason I was thinking you would need to overload the function, with a 0 argument version and a 1 argument version, but you made me realise that since it's a constexpr, there would be no program size impact on just using a single function with an optional argument.
static constexpr uint8_t characterHeight = 7;
The font is really 8 bits high, so it should be = 8. It's classified as a 5 x 7 bit font since with the classic ASCII character set the bottom bit is only used for descenders (mostly on lower case alpha characters) and actually occupies the one pixel space between lines with traditional single line spacing.
It seems that while the top clipping test includes the line spacing, the left clipping test does not include the character spacing.
The previous point explains this. The font is actually 8 bits high by 5 bits wide.
it may be worth having named constants instead of hardcoded values
I was considering the same. I'm also not sure if it's worth it or whether to include the horizontal space. I'll think about it.
While trying to decide whether getCharacterWidth() should include the end of character spacing (1 * textSize pixels), I realised there's something else that should be considered:
I believe the goal of providing these functions is so that character size values don't have to be hard coded (as determined from documentation), for the purposes of calculating string or other text block sizes to determine proper positioning. However, for certain calculations, such as to centre or right justify a string, you generally don't want to include the trailing intercharacter spacing added to the last character. (Of course, you do want to include all the spacing between characters.)
Therefore, regardless of whether getCharacterWidth() includes the spacing or not, we need to know how many pixels it is, so it can either be removed from the end or added between each character, when calculating. If we want to avoid having to refer to the documentation and hard code the spacing value, I think another function should be provided for this:
getCharacterSpace() or getCharacterSpacing() or getIntercharacterSpace() or getIntercharacterSpacing()
Do you have any preferences for one of these or something else? I (only slightly) favour getIntercharacterSpace().
The function would take the same textSize argument as the others:
static constexpr uint8_t getIntercharacterSpace(uint8_t textSize = 1)
{
return 1 * textSize;
}Although I've decided to use constexpr variables for characterWidth and characterHeight, I think I would just hard code 1 (pixel) for the size 1 spacing value because (I think) the text functions currently are coded only to work with a space value of 1.
With getIntercharacterSpace() available, I think getCharacterWidth() should give the width without the space (i.e. 5). So, to determine the number of pixels wide a string would be, you could use:
stringPixelWidth = numberOfCharacters *
(arduboy.getCharacterWidth(textSize) + arduboy.getIntercharacterSpace(textSize)) -
arduboy.getIntercharacterSpace(textSize);An equivalent getInterlineSpace() function could also be provided but for the Arduboy it would always return 0.
Your thoughts on all of this?
Apologies for the delay in replying, until today I had been preoccupied and had not had a suitable moment to read through and consider this.
However, for certain calculations, such as to centre or right justify a string, you generally don't want to include the trailing intercharacter spacing added to the last character.
A valid point.
Do you have any preferences for one of these or something else?
Although it's less precise, I'm in favour of getCharacterSpacing() because it's a bit less of a mouthful and it follows a kind of pattern (getCharacterWidth(), getCharacterSpacing()).
As a bonus, I would imagine that people who don't read documentation are more likely to stumble upon getCharacterSpacing() accidentally.
(Which might avoid the odd question & lecture here and there.)
the text functions currently are coded only to work with a space value of 1
That's true, though I'm not sure that's necessarily a good reason to not have a corresponding constexpr variable. I think it would be worth having it in the interest of consistency if nothing else. (Of course, one less 'magic number' is always welcome.)
With getIntercharacterSpace() available, I think getCharacterWidth() should give the width without the space (i.e. 5)
In this case I'm somewhat ambivalent.
On the one hand I think manually incorporating the space is more cumbersome, but at the same time I think it makes sense.
Either way I'm anticipating that these will be hidden behind helper functions (as they would be in my use case) rather than something that's used extensively throughout a user's code so it's probably not going to be a big issue.
The only reason I can think of in favour of including the space is that runtime calculations with a textSize greater than 1 would theoretically require one less multiply.
However, using the size without the space would seem to make more sense semantically.
An equivalent getInterlineSpace() function could also be provided but for the Arduboy it would always return 0.
It would probably be a good idea to include it anyway. For consistency and for added semantic value.
Although it's less precise, I'm in favour of
getCharacterSpacing()
Then I'll use that. You're right that it helps with grouping, especially when viewed in alphabetical order.
the text functions currently are coded only to work with a space value of 1
That's true, though I'm not sure that's necessarily a good reason to not have a corresponding
constexprvariable.
I was planning to make the characterWidth and characterHeight variables protected members. They wouldn't be visible from the API. But as you suggested, I'll use them to replace the hard coded values in write() and drawChar(), so their usefulness would only be from this aspect. If I were to have a corresponding characterSpacing (= 1) variable, and likewise used it to replace the hard coding, the code would break if it were changed to something other than 1. Therefore, it could be a gotcha for porting or borrowing the code.
Ideally, the code should be changed to work with an intercharacter spacing other than 1. After a quick glance at the code, I think the only thing that programmatically has to be changed is the if (i == 5) line in drawChar(). It would be changed to
if (i >= 5) (now if (i >= characterWidth)). However I wouldn't want to do this if it increases code size. (Well, maybe 2 or 4 bytes increase would be acceptable.)
I'll do some tests. If this works, and code size doesn't change when value 1 is used, then I'll consider including protected member
static constexpr uint8_t characterSpacing = 1;
runtime calculations with a
textSizegreater than1would theoretically require one less multiply.
Doing it the other way, you would sometimes need the extra multiply for trailing space elimination. I think it will be very rare that the current text size isn't known at the time of using these functions. (After all, we are making them constexpr.) If size/speed is an issue you would try to make sure they're being called with a constant parameter.
I've never been too concerned about, or spent much time, optimising the text functions, anyway. I've assumed that if size becomes critical, then the developer would have to use Arduboy2Base and provide their own optimised text rendering.
An equivalent
getInterlineSpacing()function could also be provided but for the Arduboy it would always return 0.
It would probably be a good idea to include it anyway. For consistency and for added semantic value.
As with what I've said above, there isn't any code in the text functions to handle interline spacing (assumed to be 0). It may be possible to add it in a way that it would be optimised out with a value of 0, but I don't really want to spend the time to work on this, given that it would probably never be used (other than maybe "borrowed" for new font rendering code).
However, as you said "for consistency and for added semantic value", maybe I'll leave out a protected member
static constexpr uint8_t lineSpacing = 0;
and hard code the 0 return value in getLineSpacing().
Then I'll use that. You're right that it helps with grouping, especially when viewed in alphabetical order.
I hadn't considered the alphabetical order aspect, that's another small bonus.
I was planning to make the characterWidth and characterHeight variables protected members.
That's what I was expecting. Even if they aren't part of the public API, they are potentially helpful to implementers and maintainers, and to anyone who takes an interest in the library's internals.
(I'm suddenly reminded of the embedded code I've seen where addresses of great significance are simply reinterpret_casted from raw hexadecimal integers rather than enshrined as a clearly named constant.)
the code would break if it were changed to something other than 1
In what way? What would the consequences be?
Is it simply that the logic of write as it presently stands would not handle values other than 1?
However I wouldn't want to do this if it increases code size. (Well, maybe 2 or 4 bytes increase would be acceptable.)
I would have expected the compiler to be smart enough to realise that it could check for the inverse (i < characterWidth), which should require a single branch, much as == or != would be, and hence there should be no increase.
But of course that's just theory, and naturally I can't predict the knock-on effect it might have.
As Grace Hopper once said:
"One accurate measurement is worth a thousand expert opinions."
If size/speed is an issue you would try to make sure they're being called with a constant parameter.
True, caching them in an array would be relatively cheap.
The size of the screen limits the practical size the text can grow to,
so there would theoretically be an upper bound on the array size due to impracticality.
However, as you said "for consistency and for added semantic value", maybe I'll leave out a protected member
static constexpr uint8_t lineSpacing = 0;
and hard code the 0 return value in getLineSpacing().
That is more or less the solution I was imagining.
By 'semantic value' I mean that people can use it in their own code to make it clear that they're factoring in the line spacing, even if that line spacing is actually zero.
Taking that further, if someone were to create another font library,
they could implement these functions in a way that does allow them to produce values other than 0.
Hence Arduboy2 and that library would then (theoretically) be 'drop in replacements' for each other, and both would behave correctly using the same code.
To get technical, it would allow users to make use of (constrained) parametric polymorphism.
(Of course, such a library may never be written, but it never hurts to consider the possibilities.)
In what way? What would the consequences be?
Is it simply that the logic of 'write' as it presently stands would not handle values other than 1?
Correct (but actually for drawChar() not write()). If I'm correct in my understanding of the code, after a quick review, I could replace all the hard coded 1's that refer to the intercharacter space with variable characterSpacing but if characterSpacing is greater than 1, only the first "space" column of pixels would be blank. Subsequent columns would contain columns from the next character in the array. (It would index into the next character and display that data.) Testing for the index being >= instead of == should fix this problem.
Even if they aren't part of the public API, they are potentially helpful to implementers and maintainers, and to anyone who takes an interest in the library's internals.
So my concern is that, a person "borrowing" the internals will look at the code and say: "variable characterSpacing (along with characterWidth and characterHeight) are being used by the write() and drawChar() functions that I want to copy for use in my derived text renderer". They will assume: "I can just change any of these variables and the code I copied will still work properly. This would not be the case for characterSpacing.
I hope that by leaving the hardcoded 1's as they are and also hardcoding a 1 for getCharacterSpacing(), it would imply to a person examining the code that there's a reason there's no characterSpacing variable as there are characterWidth and characterHeight variables. The same goes for not having a lineSpacing variable and just hardcoding a 0 in getLineSpacing().
Anyway, if I can fix the character spacing code with no or trivial impact on code size, then it's only having a lineSpacing protected variable that would apply to the above.
Note that I would put comments in the code, close to the new variables, to the effect of:
"There is no lineSpacing equivalent of characterSpacing because the code does not support adding interline spacing."
By 'semantic value' I mean that people can use it in their own code to make it clear that they're factoring in the line spacing, even if that line spacing is actually zero.
Yes, what you described is exactly what I assumed you meant by 'semantic value'. I will try to point this out as the reason for having getLineSpacing(), in its documentation.