Changes for better caching and sharing parsed fonts

[martin-kolarik]

In principle I made two changes:

• I replaced generic T in (Font<T: FontTableProvider>) with &dyn FontTableProvider, what allows caching Font instances of different types (OTF, WOFF...).
• I made FontData self consuming in , what allows better separating ReadScope font source from Font itself. Font then can be cached only with keeping single self-referenced data (ReadScope).

I removed generic T from Font without breaking API compatibility, with the help of impl Into<Box<dyn FontTableProvider + 'a>>, which is rather complex. Definitely this could be arranged better if API was changed (what I did not want).

It should also help with #52 and with some discussed points in #72.

[wezm]

Thanks for tackling this. I'm aware it is a source of frustration for some allsorts users and I am keen to come up with a solution. I pulled these changes into the Prince code base (where allsorts originated) and attempted to update things to get it building.

I replaced generic T in (Font<T: FontTableProvider>) with &dyn FontTableProvider I removed generic T from Font

The removal of the type parameter poses an issue for us. In our code we have other methods on the concrete type that is passed into Font that we're able to access through the font_table_provider field on Font. Moving to the trait object makes this difficult.

One workaround would be to add an as_any method to the FontTableProvider trait so that we can downcast back into the concrete type. This is a bit cumbersome and in fact I found this post that covers these exact problems with interior Box<dyn Trait> and the suggestion is to use generics.

Considering the two commits you made, are both actually required to achieve the goal of making caching of fonts easier? It seems like it might be enough just to take the second commit as it stops Font from holding a borrowed FontTableProvider.

[martin-kolarik]

Thanks, I think that for my usecase the second commit should be enough as I do not combine font types. I will try. Anyway, if the second commit is fine, it could be merged alone and it would definitely help as it is.

[wezm]

Thanks, I think that for my usecase the second commit should be enough as I do not combine font types. I will try. Anyway, if the second commit is fine, it could be merged alone and it would definitely help as it is.

Ok, great. Let me know how you go and if it works I'll pull in the second commit.

[martin-kolarik]

Hello, I confirm that for the caching purposes the second commit is enough, it works for me.

[wezm]

Thanks so much for checking that @martin-kolarik. I've done some experimenting to see if the same thing can be achieved as your second commit without changing the existing API. I think I've managed to do that in https://github.com/yeslogic/allsorts/commit/fee74f285d0a8df6c8f452c5a2b78f32d8dba207. If you have time, would you be able to try that version?

[martin-kolarik]

Hello @wezm, nice work, I checked your branch, and it works. I appreciate your help, I could not add into_owned() at considered location, because I do not know the internals.

[wezm]

Hello @wezm, nice work, I checked your branch, and it works.

Fantastic, appreciate you checking that for me. I've published Allsorts 0.11.0 with that change. Are you happy for me to close this PR now?

[martin-kolarik]

Sure, thanks!