Skip to content
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

Add support for color fonts #735

Merged
merged 37 commits into from
Apr 16, 2024
Merged

Conversation

LaurenzV
Copy link
Contributor

@LaurenzV LaurenzV commented Apr 10, 2024

Not polished code yet, but it should be enough to test it.

Oh, and top-to-bottom text also doesn't quite work yet. 😅

@LaurenzV LaurenzV mentioned this pull request Apr 11, 2024
@LaurenzV
Copy link
Contributor Author

So I just tried adding support for SVG and the positioning seems to work out-of-the-box (for horizontal writing), so the issue definitely seems limited to bitmap emojis...

@RazrFalcon
Copy link
Collaborator

Thanks, will take a look.

@LaurenzV
Copy link
Contributor Author

COLRv0 works now, too!

<svg id="svg1" viewBox="0 0 200 200" xmlns="http://www.w3.org/2000/svg"
      font-size="25">
    <title>Emojis</title>

    <path id="crosshair" d="M 20 100 L 180 100 M 100 20 L 100 180"
          stroke="gray" stroke-width="0.5"/>

    <text id="text1" x="0" y="50" font-family="Segoe UI Emoji">Hii😀 😁how </text>
    <text id="text1" x="0" y="90" font-family="Twitter Color Emoji">Hii😀😁how </text>
    <text id="text1" x="0" y="130" font-family="Apple Color Emoji">Hii😀😁how </text>
    <text id="text1" x="0" y="160" font-family="Noto Color Emoji">Hii😀😁how </text>

    <!-- image frame -->
    <rect id="frame" x="1" y="1" width="198" height="198" fill="none" stroke="black"/>
</svg>

Our output:
test

Chrome:
image

@LaurenzV
Copy link
Contributor Author

LaurenzV commented Apr 11, 2024

Here is a toy sbix font we can use to debug: https://github.com/simoncozens/test-fonts?tab=readme-ov-file#cff-and-sbixotf

If I install it locally and open the following SVG:

<svg id="svg1" viewBox="0 0 200 200" xmlns="http://www.w3.org/2000/svg"
      font-size="25">
    <title>Emojis</title>

    <path id="crosshair" d="M 20 100 L 180 100 M 100 20 L 100 180"
          stroke="gray" stroke-width="0.5"/>

    <text id="text1" x="0" y="100" font-family="CFF Outlines and SBIX">AAA</text>

    <!-- image frame -->
    <rect id="frame" x="1" y="1" width="198" height="198" fill="none" stroke="black"/>
</svg>

I get the same behaviour on Firefox and Chrome. I'm trying to change different glyph metrics and recompile the font with fonttools, but none of them have an impact on the y offset. So if you feel like looking into it, this might help.

EDIT: I tried changing different glyph metrics, and apart from x and y offset, none of them have any effect on the positioning... But in the original font the y offset is already 0, so that can't be where it is coming from.

@RazrFalcon
Copy link
Collaborator

Thanks for looking into it.

@LaurenzV
Copy link
Contributor Author

Looks like we are not the only ones troubled by this: harfbuzz/harfbuzz#2679 (comment)

So can we ignore it for now?

@RazrFalcon
Copy link
Collaborator

We should at least try to use the same hack everyone else is using.

@yisibl
Copy link
Contributor

yisibl commented Apr 12, 2024

COLRv0 works now, too!

So Cool! Thank you for all you have done for this.

@LaurenzV
Copy link
Contributor Author

LaurenzV commented Apr 12, 2024

We should at least try to use the same hack everyone else is using.

Well, guess what:

Consider the following SVG with the font I mentioned above:

<svg id="svg1" viewBox="0 0 200 200" xmlns="http://www.w3.org/2000/svg"
      font-size="64">
    <title>Emojis</title>

    <path id="crosshair" d="M 20 100 L 180 100 M 100 20 L 100 180"
          stroke="gray" stroke-width="0.5"/>

    <text id="text1" x="0" y="100" font-family="CFF Outlines and SBIX">AAA</text>

    <!-- image frame -->
    <rect id="frame" x="1" y="1" width="198" height="198" fill="none" stroke="black"/>
</svg>

Here is how it renders on MacOS in Chrome:
image

Here is how it renders on Windows 11 in Chrome, which is exactly the same we have right now in resvg:
image

So what I think is happening here is that Chrome, Firefox etc, use CoreText for rendering text in SVG, which is why on MacOS, they have the same behavior as all other Apple programs (like TextEdit, as mentioned in the issue linked above). However, on Windows they use something else, meaning that we don't have this same behaviour there...

So I still think it's safe if we just ignore this for now, right? The reason why I'm reluctant to add manual corrections here is that right now, layouting doesn't care at all about which type of glyph is used, but if we want to add manual corrections, then we might have to decide on the type of glyph that is used while layouting, which is not so nice in my opinion.

@LaurenzV LaurenzV changed the title Add support for bitmap glyphs Add support for color fonts Apr 12, 2024
@RazrFalcon
Copy link
Collaborator

So I still think it's safe if we just ignore this for now, right?

Sounds good to me.

@LaurenzV
Copy link
Contributor Author

Regarding subsetting, I think I tried that but iirc fonttools didn't support subsetting SVG tables. But I will try again.

So should I disregard TTB writing for now?

@RazrFalcon
Copy link
Collaborator

TTB writing

Writing to SVG? Or do you mean the layout? It's fine.

@yisibl
Copy link
Contributor

yisibl commented Apr 13, 2024

Regarding subsetting, I think I tried that but iirc fonttools didn't support subsetting SVG tables. But I will try again.

I remember that fonttools already support subsetting OT-SVG fonts, see fonttools/fonttools#534 (comment)

Alternatively, you can use nanoemoji to convert SVG files into color fonts in a variety of formats, which will produce test fonts that are small enough to use.

@LaurenzV
Copy link
Contributor Author

I remember that fonttools already support subsetting OT-SVG fonts, see fonttools/fonttools#534 (comment)

Huh okay, I'll try it again then, thanks!

@LaurenzV
Copy link
Contributor Author

I think I'm getting close now. I'll try to investigate the tb writing mode issue again, then clean up and then it should be ready. Hopefully will be done within the next few days.

@LaurenzV
Copy link
Contributor Author

There's still something off, but not sure I will be able to fix it for now, this would require me to make a deep dive into how writing-mode works exactly, which I might do at some point, but I don't think it's worth it to hold up emoji support for this... This is how it looks currently:

test

versus in other browsers:

image

For some reason there is more space in the vertical writing mode... But no idea why. I will add a test case regardless so we don't forget about it.

@LaurenzV LaurenzV marked this pull request as ready for review April 14, 2024 22:00
@LaurenzV
Copy link
Contributor Author

LaurenzV commented Apr 14, 2024

I've added a few small TODOs where I wasn't sure what I should change it to. But otherwise it should be good, I hope.

transform: glyph.svg_transform(),
..Group::empty()
};
// TODO: Probably need to update abs_transform of children?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm still thinking about a better abs_transform implementation. The current one is pretty meh.

..Group::empty()
};
// TODO: Probably need to update abs_transform of children?
group.children.push(Node::Group(Box::new(tree.root)));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we should keep it as Image::SVG. But I guess it can be changed later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah true... Yeah I guess both would work.

@RazrFalcon
Copy link
Collaborator

Thanks! Looks good to me.
I will think about how we can simplify abs_transform handling.

@RazrFalcon
Copy link
Collaborator

Ready to merge?

@LaurenzV
Copy link
Contributor Author

I think so!

@RazrFalcon RazrFalcon merged commit d5d9ff9 into linebender:master Apr 16, 2024
3 checks passed
@LaurenzV LaurenzV deleted the image-glyph branch June 8, 2024 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants