This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

• TTF_GetFontKerningSize err code fix (patch.diff, text/plain, 2014-11-10 11:39:02 +0000, 1406 bytes)
• proposed function prototype for SDL_GetFontKerningSize (proposed_SDL_GetFontKerningSize-2.diff, text/plain, 2014-12-19 18:52:37 +0000, 1887 bytes)
• additional documentation notes for SDL_GetFontKerningSize (SDL_ttf.h) (proposed_SDL_GetFontKerningSize-3.diff, text/plain, 2014-12-21 02:08:32 +0000, 1076 bytes)

Reported in version: unspecified
Reported for operating system, platform: All, x86

Comments on the original bug report:

On 2014-11-10 11:39:02 +0000, Jeffrey Carpenter wrote:

Created attachment 1929
TTF_GetFontKerningSize err code fix

Hello,

I believe I've stumbled upon a bug in TTF_GetFontKerningSize -- the recently patched one at 275:86d0c63699f4 (from bug 2572) -- while writing unit tests in my text rendering class. The function returns -1 when an error occurs, but this error code conflicts with the valid range of possible kerning pair offsets.

I discovered the problem while using OpenSans-Regular.ttf at a point size of 72, with the text "Hello, World" (how terribly ironic, I thought!). The mismatch occurs between the glyph character 'W' and 'o'.

I've attached a patch that resolves the problem for me.

Thanks,
Jeffrey Carpenter i8degrees@gmail.com

On 2014-12-19 08:55:24 +0000, Yohann Ferreira wrote:

Hi there,

I confirm this patch is useful as certain glyphs pairs can return -1, which is a perfectly valid return value.

Can someone push that?

On 2014-12-19 13:14:44 +0000, Sylvain wrote:

Yes, it looks like a bug.

But, maybe the prototype of the function should be changed :

0/-1 on success/error

a pointer to get the value.

On 2014-12-19 13:33:51 +0000, Jeffrey Carpenter wrote:

Sylvain,

I seem to recall considering the same prototype change to a pointer. I feel like outputting the value as a pointer would be result in a less error prone API and additional consistency with other SDL functions.

The only reason I had at the time for leaving it as-is was ... perhaps backwards compatibility with the previous function prototypes, but I think that's a moot point; the original function prototype did not work for me at all before bug # 2572 was resolved (not sure if it ever functioned for anybody?), and none of these changes have actually been released yet.

I'm in favor of changing the prototype function signature. I'll try to get around to submitting a proposed patch diff with that change here before the end of the year.

Cheers,
Jeffrey Carpenter i8degrees@gmail.com

On 2014-12-19 14:27:20 +0000, Yohann Ferreira wrote:

Hi :)

not sure if it ever functioned for anybody?
As pointed out by the patch author in bug # 2572, IIRC the previous function was asking glyph indeces parameters you simply couldn't get from the SDL API. So, indeed I can't see how this function could have been used, except maybe within SDL_ttf.

I'm also quite fine with getting the return value using a reference parameter/pointer to get the value. This should be hopefully rather trivial to fix.

Yohann

On 2014-12-19 17:58:24 +0000, Jeffrey Carpenter wrote:

Hi Yohann!

Excellent point, thanks for checking that. I haven't been back through the history logs in some time :-)

The change is indeed quite trivial. I would like to also look at possibly replacing the internal calls that get kerning pairs directly with TTF_GetFontKerningSize instead (in a separate diff patch) so that it would be easier for future maintenance; there are no internal calls being made to the function at the moment (forcing one to pull in external tests).

Cheers,
Jeffrey Carpenter i8degrees@gmail.com

On 2014-12-19 18:52:37 +0000, Jeffrey Carpenter wrote:

Created attachment 1976
proposed function prototype for SDL_GetFontKerningSize

On 2014-12-19 18:53:35 +0000, Jeffrey Carpenter wrote:

(In reply to Jeffrey Carpenter from comment # 6)

Created attachment 1976 [details]
proposed function prototype for SDL_GetFontKerningSize

Oops, I got trigger happy wee bit soon :-P

...So I got bored and found a bit of time to make the proposed change to return the kerning pair size by passed in pointer. I tested this patch diff using this bit of
temporary test code inside showfont.c -- right before the call to draw_scene():

int kpair = 0;
if( TTF_GetFontKerningSize(font, 87, 111, &kpair) != 0 ) {
fprintf(stderr, "Couldn't obtain kerning pair size: %s\n", TTF_GetError() );
TTF_CloseFont(font);
cleanup(2);
}

SDL_assert(kpair == -1);
// printf("kerning pair size: %d\n", kpair);

$ DYLD_LIBRARY_PATH=/Users/jeff/opt/SDL2/lib ./showfont /Library/Fonts/Arial.ttf 72 "Hello, World"

Works as intended on my system. Built against SDL2 hg rev 9278. I haven't done any external testing yet, but I'll get around to that sooner or later! :-) Let me know if the attached patch diff gives you any problem, or if you spot anything else!

Cheers,
Jeffrey Carpenter i8degrees@gmail.com

On 2014-12-19 21:54:06 +0000, Yohann Ferreira wrote:

Looks and works fine here. :D

I'd simply add this in the function doc:
(Based on: http://www.freetype.org/freetype2/docs/glyphs/glyphs-4.html)

Kerning distances are expressed in pixels. They are usually oriented in the X axis, which means that a negative value indicates that two glyphs must be set closer in a horizontal layout.

On 2014-12-21 02:08:32 +0000, Jeffrey Carpenter wrote:

Created attachment 1979
additional documentation notes for SDL_GetFontKerningSize (SDL_ttf.h)

(In reply to Yohann Ferreira from comment # 8)

Looks and works fine here. :D

I'd simply add this in the function doc:
(Based on: http://www.freetype.org/freetype2/docs/glyphs/glyphs-4.html)

Kerning distances are expressed in pixels. They are usually oriented in the
X axis, which means that a negative value indicates that two glyphs must be
set closer in a horizontal layout.

I thank you for your suggestion! I do agree, and thus have attached another proposed patch diff with the changes made to the documentation notes in SDL_ttf.h.

To whomever is applying these patch diffs, they will probably have to be applied sequentially, in the order that they were created, for a clean merge.

Thanks!
Jeffrey Carpenter i8degrees@gmail.com

On 2017-09-10 06:09:11 +0000, Sam Lantinga wrote:

These patches look good for the next release that updates the ABI, thanks!