Use new unresolvedCodePoints API from skia.

[eyebrowsoffire]

NOTE: this works off of a skia CL that is not merged yet, so we shouldn't merge this until it actually lands in skia and rolls into the engine. See https://skia-review.googlesource.com/c/skia/+/695716

[mdebbar]

Just a reminder to revert this before submitting.

[mdebbar]

I have some questions but the PR looks good to me!

[mdebbar]

If this is going to be a common occurrence, consider making an extension method for it:

extension PointerToList<T extends NativeType> on Pointer<T> {
  List<T> toList(int length) {
    return List<T>.generate(length, (int index) => this[index]);
  }
}

but when I tried this, the analyzer didn't like this[index] apparently because the [] operator is defined as a separate extension for each T 😞

[eyebrowsoffire]

I think probably the reason this is failing is because the T in Pointer<T> the native type, but the T in List<T> is probably the dart type (int in this case). That makes this kind of difficult to create a generic helper for.

[mdebbar]

Quick question: is it possible for paragraphGetUnresolvedCodePoints to return a different result for the same paragraph later? E.g. when extra fallback fonts are downloaded.

I guess what I'm really asking is whether there's ever a case where we need to reset paragraphGetUnresolvedCodePoints to false.

[eyebrowsoffire]

I don't believe it can. The reason is that you can't actually change the list of font families on the text styles or paragraph style once it is a paragraph, that's specified at the ParagraphBuilder stage. Once you've already created the paragraph, the only way to add more fallback fonts to it is by actually rebuilding the paragraph with new text styles.

[mdebbar]

I don't understand what this third outLength parameter is for. Could you clarify please?

[eyebrowsoffire]

outLength is the length of the outCodePoints buffer that we are passing to the API. This is so that the C++ code doesn't overwrite the buffer into some other region of the memory. However... because we actually got that value from the C++ code in the first place, it is true that we can just omit this parameter altogether and the C++ code can always trust that the buffer is exactly the length that is needed. I can remove it if you think it's confusing, I think this is just my standard "C memory safety" guardrails I automatically added.

[mdebbar]

Oh I see. Hmm this is a bit confusing and could use a better name or an API change.

For example, it might be a good idea to split this into 2 APIs:

• bool paragraph_hasUnresolvedCodePoints(Paragraph* paragraph)
• int paragraph_getUnresolvedCodePoints(Paragraph* paragraph, SkUnichar* outCodePoints)

[eyebrowsoffire]

A boolean is actually not good enough, we want the API to return the length of the buffer needed to hold all the code points so we can allocate the correct amount. This pattern is actually pretty common for C APIs, where the user of the API passes in null first, and the API returns the size of the buffer needed to hold the output, and then the user allocates that memory and calls the API a second time with the actual buffer to have it filled.

[mdebbar]

Actually, it looks like you are handling the case of hasUnresolvedCodePoints here. Is there really a need for outLength?