Fix the GetKerningPairAdjustments API.

[pdjonov]

Description of Change

The SkTypeface::getKerningPairAdjustments API states:

If count is non-zero, then the glyphs parameter must point to at least [count] valid glyph IDs, and the adjustments parameter must be sized to at least [count - 1] entries. If the method returns true, then [count-1] entries in the adjustments array will be set. If the method returns false, then no kerning should be applied, and the adjustments array will be in an undefined state (possibly some values may have been written, but none of them should be interpreted as valid values).

However, SkTypeface.GetKerningPairAdjustments returns count entries and further in the case where getKerningPairAdjustments returns false it may return invalid values to the caller. This PR:

• Adds a Span-based overload of GetKerningPairAdjustments that:
  • correctly expects glyphs.Length - 1 output entries (but will accept and ignore more),
  • returns true if SkTypeface::getKerningPairAdjustments wrote valid data into the output span,
  • otherwise clears the output span to zero so that even if invalid data was written, the caller won't see it and will get sensible results even when ignoring the return value.
• Rewrites the original GetKerningPairAdjustments in terms of the new overload.
  • For backwards compatibility, an extra zero element is returned at the end of the array.
  • However, partial/invalid results will no longer be returned (the caller will receive zeroes instead).

Some typefaces are known to never support kerning. Calling this method with all zeros (e.g. getKerningPairAdustments(NULL, 0, NULL)) returns a boolean indicating if the typeface might support kerning. If it returns false, then it will always return false (no kerning) for all possible glyph runs. If it returns true, then it may return true for somne glyph runs.

This patch adds a HasGetKerningPairAdjustments that callers can use to query whether the font contains any kerning information or whether GetKerningPairAdjustments will always return zeroes.

API Changes

Added:

• public bool SkTypeface.HasGetKerningPairAdjustments { get; }
• public bool GetKerningPairAdjustments (ReadOnlySpan<ushort> glyphs, Span<int> adjustments)

Behavioral Changes

public int[] GetKerningPairAdjustments (ReadOnlySpan<ushort> glyphs) will now return zeroes in places where it previously returned partial/invalid data that the Skia documentation says to ignore.

PR Checklist

• Has tests (if omitted, state reason in description)
• Rebased on top of main at time of PR
• Merged related skia PRs
• Changes adhere to coding standard
• Updated documentation

Apologies if this isn't quite up to the project standards. I was initially going to just submit a bug report, but then I thought it might be nicer (and likelier to actually get fixed) if I made a quick PR instead of just complaining. I fully acknowledge that I'm not invested enough in this project to learn all of the conventions properly, and I apologize if this wastes anyone's time cleaning up what I've misunderstood. Y'all can do with this patch whatever you will.

I just think it'd be nice if I could be rid of this abomination:

private unsafe delegate bool SkApi_GetKerningPairAdjustments(IntPtr typeface, UInt16* glyphs, Int32 count, Int32* adjustments);
private static SkApi_GetKerningPairAdjustments s_GetKerningPairAdjustments;

private static SkApi_GetKerningPairAdjustments GetKerningPairAdjustmentsFunc()
{
    if (s_GetKerningPairAdjustments == null)
    {
        var apiBindingsType = typeof(SKTypeface).Assembly.GetType("SkiaSharp.SkiaApi");
        var apiFunc = apiBindingsType.GetMethod("sk_typeface_get_kerning_pair_adjustments", System.Reflection.BindingFlags.Static | System.Reflection.BindingFlags.NonPublic);
        s_GetKerningPairAdjustments = apiFunc.CreateDelegate<SkApi_GetKerningPairAdjustments>();
    }

    return s_GetKerningPairAdjustments;
}

private static unsafe bool HasKerningPairAdjustments(SKTypeface typeface)
{
    var func = GetKerningPairAdjustmentsFunc();
    return func(typeface.Handle, null, 0, null);
}

private static unsafe void GetKerningPairAdjustments(SKTypeface typeface, ReadOnlySpan<ushort> glyphs, Span<int> adjustments)
{
	Debug.Assert(glyphs.Length >= 2 && adjustments.Length == glyphs.Length - 1);

	var func = GetKerningPairAdjustmentsFunc();

	bool res;
	fixed (ushort* g = glyphs)
	fixed (int* a = adjustments)
		res = func(typeface.Handle, g, glyphs.Length, a);

	if (!res)
		adjustments.Clear();
}

[pdjonov]

@dotnet-policy-service agree

[pdjonov]

I don't understand why the tests are failing. The stuff in the logs doesn't look like stuff that I've broken, so I'm taking this PR out of draft.

Again, I made this PR since it seemed a better thing to do than just opening a ticket and hoping someone else would write code for me. But I don't have time to learn what's going on with Run the bootstrapper for tests-android and so on. Do whatever y'all want with the patch. (And sorry again if you'd have preferred a ticket to a half-baked PR.)

[mattleibow]

@pdjonov no, thanks for this PR. It looks very good.

CI is a bit temperamental and I am still trying to figure out why macos and Android keep failing the first run. Android has additional issue with the emulators not always booting and macOS has random test crashes.

But, if the other test platforms all pass then it is highly likely it is just a random failure.

Also, the test were all failing before since main had been broken for some time. But it is all fixed now.

[mattleibow]

Thanks for the PR and the docs.

I looked at the skia source and it seems only the SkTypeface_FreeType (FreeType-based typefaces) support this feature. This probably means just Linux-based OSes will support this (Linux, Android and WASM)

Do you perhaps have a test for this? Not sure if you have a font that you can use to check some things and then just make sure the values get passed around correctly?

[mattleibow]

Going to merge this as tests are hard for this scenario and we can add them later. Even skia does not have tests...