Add TTF_GetFontBBox() which returns font's extremum bounding box #538
Conversation
ivan-mogilko
force-pushed
the
sdl2--readftface-bbox
branch
from
dcefe5e to
61c1127
Compare
SDL_ttf.h
Outdated
| * | ||
| * \since This function is available since SDL_ttf 2.26.0. | ||
| */ | ||
| extern DECLSPEC void SDLCALL TTF_GetFontBBox(const TTF_Font *font, TTF_BBox *bbox); |
There was a problem hiding this comment.
Rather than creating a new type, can you have separate pointers for each element that can be optionally filled in?
There was a problem hiding this comment.
Changed the parameter list following TTF_GlyphMetrics as an example.
| * | ||
| * The bounding box defines bounds large enough to contain any glyph from | ||
| * the font. It is expressed in pixel offsets from glyph's origin (0,0), | ||
| * with Y axis pointing upwards. Thus ymax offset may be seen as the |
There was a problem hiding this comment.
This is the opposite of how coordinates are usually done in SDL. Can you double check the other SDL_ttf APIs and see if that's consistent here?
There was a problem hiding this comment.
By "opposite", are you referring to Y axis? I know that SDL has it downwards in e.g. drawing, the upwards Y is FT specific, it has ascender as a positive offset and descender as a negative offset afaik.
But I will check what other functions are doing.
There was a problem hiding this comment.
I can confirm that TTF_FontAscent and TTF_FontDescent functions return these offsets in a similar style: ascent is normally a positive value and descent is a negative value, as if Y axis is pointing up. For properly done fonts their return values also match miny and maxy values returned from TTF_GetFontBBox. (Their values differ for the fonts which have incorrect metrics information in them.)
EDIT: Also checked TTF_GlyphMetrics, and its return values seem fully consistent as well.
There was a problem hiding this comment.
By the way, since there's a TTF_GlyphMetrics function, do you think that TTF_GetFontBBox is a okay name, or should I rename to something more consistent with "GlyphMetrics"? Because the meaning of BBox is essentially "max known glyph metrics".
There was a problem hiding this comment.
Okay, thanks for checking, that seems fine to me. I'm not sure on the name, let me think about it a bit. What's the use case for this function?
There was a problem hiding this comment.
What's the use case for this function?
In my case (mentioned in #536), there are fonts that have incorrect ascent/descent information for some reason (maybe because font's creator made a mistake), but there's still a way to tell how big their characters are using this bounding box info. This helps if we want to precalculate a necessary texture size before first render happened, for example.
|
|
||
| void TTF_GetFontBBox(const TTF_Font *font, TTF_BBox *bbox) | ||
| { | ||
| /* Recalculate FT_Face's bbox from font units to pixels */ |
There was a problem hiding this comment.
You should make sure the font is valid, see the other API functions for examples.
There was a problem hiding this comment.
Added a font pointer check. By the way, I noticed that not all functions have this check, only some do.
ivan-mogilko
force-pushed
the
sdl2--readftface-bbox
branch
from
61c1127 to
17dfc0c
Compare
|
@slouken are there doubts about the function's name, or the function's purpose? |
|
@slouken hello, it's been a while, could you please tell what's the view on this PR? Is it acceptable or not at all? |
ivan-mogilko
force-pushed
the
sdl2--readftface-bbox
branch
from
17dfc0c to
a2bc7eb
Compare
|
We're not currently adding new functions to the SDL2 API. Please feel free to retarget this to main and provide a concrete use case for it, as well as clarification on what measurement functions should be called in what situations if you'd like it to be merged. I'll go ahead and close it for now. |
2 participants
Resolve #536
This adds a new function called TTF_GetFontBBox:
and a struct TTF_BBox (simply a pair of xmin/max and ymin/max offsets).
This function may be used to query font's bbox, which is a bounding box enough to suit any glyph from this font.
Function description is based on FreeType comments to FT_Face's bbox field.
I was not certain which version to mention in comments, so put 2.26.0, as that seems to be the next release number, theoretically?