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?

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.

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>

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

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>

Created attachment 1976 [details]
proposed function prototype for SDL_GetFontKerningSize

(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>

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.

Created attachment 1979 [details]
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>

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