search

revery-ui / revery

:zap: Native, high-performance, cross-platform desktop apps - built with Reason!
https://www.outrunlabs.com/revery/
MIT License
8.06k stars 196 forks source link

reafactor(feat/font/fallback): self-contained generateShapes #957

Closed glennsl closed 4 years ago

glennsl commented 4 years ago

This doesn't even work properly right now, and could use some cleanup, but it shows the basic ideas I wanted to get a cross. The hole resolver has been simplified and is fully contained inside generateShapes, I've also tried to prepare it for using the bounded harfbuzz API. Having the bounds be bytes-based and the loop index be glyph-based is a bit confusing though. And it does need to be threaded through a lot of functions.

I'll keep working on this a bit to try to figure out why it's not working properly, and see if I can find a better way to pass the bounds around. I think it's good enough to run by you for a preliminary review though, or to show it just as an example if you want to go in a different direction.

zbaylin commented 4 years ago

Thanks @glennsl! Your approach is kind of similar to an alternative I had in mind where Hole.resolve returns a "resolution" which is a substring (or bounds) and a maybe typeface. What specifically isn't working, if I may ask?

glennsl commented 4 years ago

Right, that would probably be easier now with the accumulator.

What specifically isn't working, if I may ask?

It's not showing the emoji in the title of the examples app. Not even the unknown char square. So I think it's resolving it correctly but not adding it to the accumulator for some reason...

glennsl commented 4 years ago

Found the bug. I was getting the right glyph but using the font from generateShapes instead of the fallback font when creating the ShapeResult.

glennsl commented 4 years ago

I think this is OK now, although I'm not entirely sure since there doesn't seem to be any tests outside the Hello example and without the updated esy-harfbuzz I'm not sure what works and not. I tried to play with the emoji in the Hello example, but that made Onivim freeze my computer 🙃.

Feel free to merge or tweak this however you want now.

zbaylin commented 4 years ago

Awesome, thanks @glennsl -- it looks great. I'm a little nervous about it freezing your computer, but Im confused about you saying Oni caused it -- was that a typo or are you resolving Oni to this branch?

glennsl commented 4 years ago

Oh, sorry, no it has nothing to do with this PR. I was running Oni normally and just found it a bit ironic that it happened while working on or near something that might eventually fix it. Just a bit of encouragement :)

zbaylin commented 4 years ago

Oh haha now I get it -- sounds good. I'll merge this into my branch and then when #956 is merged I'll use that API