-
Notifications
You must be signed in to change notification settings - Fork 107
Add noto color emoji #369
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Add noto color emoji #369
Conversation
|
I question a lot of this approach. Emoji usage is rather sparse. I don't know that atlassing emojis makes much sense. Kinda think that having them as separate pngs, loaded individually on demand, like we already do with flags and icons may be easier in general. It definitely raises a red flag that with this approach, seemingly the only way to regenerate these assets is through your own fork of a random repository. That needs proper vetting. |
Well, now that it's mentioned, I should add my part... I had a quick look at this PR and, I agree regarding the "fork of a random repository" part, though I also hope we can add a .NET tool that does this so we don't need to fiddle around with the Windows-centric tool in https://github.com/ppy/osu-framework/wiki/Setting-Up-Fonts. Could the setup provided serve as a general replacement for that for all fonts? Maybe since it's all JSON, we could even replace SharpFNT which we've seen overheads in regarding character lookups. |
|
I'm all ears for ditching bmfont, although I'm not sure that script there will be enough to get rid of it. It only kinda works in this context because it's emoji which don't really have font metrics. Like the metrics are hardcoded in the script rather than read from the font file. |
Okay, I agree with your point. If I'm not atlassing these emojis, then I don't think I'll need any extra scripts. Can I just put a bunch of PNG files directly here? I also won't need to change any part of |
Should be fine as long as you also include a script to rasterise noto color emoji to png. Like the one for twemoji flags that we have. |
I've finished up this, and the changes I made to the framework ended up being much simpler! The script to generate the images at osu.Game.Resources/Textures/Emoji/osu_emoji.sh. I went with libvips instead of Inkscape because it's way easier to install – hope you don't mind! |
|
I question this whole line of conversation. We have code framework-side to cache PNGs to raw bytes for faster loads of fonts (png decode is slow), which is quite important for fonts which can be updated (via The atlassing part here does not come with overheads, beyond this initial cache-to-raw decode step (and the disk usage as a result). Unfortunately I can't even see how this looked previously because the old version was force pushed away. |
|
Hey Peppy,
I've been wondering about the purpose of
Regarding the previous atlassing version: osu-framework: ppy/osu-framework@master...EYHN:osu-framework:eyhn/emoji-atlassing Emojis, like other fonts, used GlyphStore and automatically had raw caching. However, since SharpFNT and BMFont formats don't support characters composed of multiple Unicode code points, I additionally defined a JSON file to record the positions of emojis within the texture. I used a Python script (https://github.com/EYHN/emoji-bitmap-font-generator) to generate both the emoji texture atlas and this JSON file. Then, within the framework's protected virtual IFontMetadata CreateFontMetadataFromStream(Stream stream, string filename)
{
if (filename.EndsWith(".fnt", StringComparison.Ordinal) || filename.EndsWith(".bin", StringComparison.Ordinal))
{
return SharpFntFontMetadata.FromStream(stream, FormatHint.Binary, false);
}
if (filename.EndsWith(".json", StringComparison.Ordinal))
{
return JsonFontMetadata.FromStream(stream);
}
throw new NotSupportedException($"Unsupported font format: {filename}");
}I believe the issues with the atlassing version are:
I'd like to hear your opinion on this. |
|
Another consideration is that this is increasing the number of files included in the resources dll by multiple magnitudes. In theory this is a non-issue, but I've never had this many files in a resource dll before so it's uncharted territory. |
That's a good point. I actually looked into it further, and it turns out .NET does iterate through resources in a DLL to find them. And I wrote a benchmark, and unfortunately, adding over 3000+ emoji files did cause a performance hit for the Benchmark: public class BenchmarkResourceDll : BenchmarkTest
{
private ResourceStore<byte[]> store = null!;
public override void SetUp()
{
store = new ResourceStore<byte[]>();
store.AddStore(new DllResourceStore(OsuResources.ResourceAssembly));
}
[Benchmark]
public void BenchmarkToGetResource()
{
for (int i = 0; i < 10; i++)
{
store.Get($"Textures/Gameplay/Fonts/argon-counter-{i}.png");
}
}
}Before
After adding emoji resource
|
|
A 6x increase looks bad but (a) we're still in the microsecond range and (b) a microbenchmark here is not that helpful because it's not actually clear to me how often we do these lookups in practice. As in if the lookups only happen at most once per drawable in BDL or something then that's not a very strong argument to me. Comparing usage time inside an actual game execution would paint a much more useful picture imo. I'm not that hung up on whether atlassing should be done here or not because I was at most thinking out loud when I initially commented here but neither proposition here looks especially winning to me. No atlassing means performance overheads, atlassing means having weird json growths on top of an already painful kink of being anchored to bmfont & co. |
|
There are other solutions as well, such as using a zip (store compression) or similar container to keep all these files in a single container file. |
|
I hope I'm not just throwing you in random directions here on a whim, and hope you benchmarked that |
|
Please take a look at the new implementation, including the updates in ppy/osu#34012 . I'm happy with how the zip implementation turned out. I'm storing the raw RGBA32 data directly in the zip, so at runtime, all we need to do just decompress This even gives us better compression than PNG, The |
|
May I ask for the curiousity why have you picked |
osu.Game.Resources.dll103 MB -> 113 MB_to separate multiple code pointsEmojiStorecode: Add full Emoji support to osu osu#34012