1357952 - UnscaledFontFreeType does not properly serialize system fonts with WebRender

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect, P3)

Description

[Lee Salzman [:lsalzman]]

As a result of bug 1355931, UnscaledFontFreeType was temporarily regressed with respect to serializing fonts for WebRender. It holds either an FT_Face for webfonts, or a filename/index for system fonts, and was only previously reading fonts by querying tables from the face.

As part of fixing this bug, I made a slight compat-breaking change in an upstream WebRender pull request: https://github.com/servo/webrender/commit/6b8000def1e11fe61a438ab1a71f4f102e1b89fd
The pull request basically just added support for supplying an index along with the add_raw_font API call, so that if we're giving FreeType a font collection instead of a single font, it can properly index said collection. The compat issue was noted in bug 1357392 comment 5 (https://bugzilla.mozilla.org/show_bug.cgi?id=1357392#c5).

Comment 1

[Lee Salzman [:lsalzman]]

Attachment: update WebRenderAPI to allow index in add_raw_font

This handles the proper routing of the font index through WebRenderAPI. This is also dependent on updating our in-tree WebRender to drag in the requisite upstream pull request: https://github.com/servo/webrender/commit/6b8000def1e11fe61a438ab1a71f4f102e1b89fd

Kats or Jeff, how to properly stage this with the necessary tree webrender update?

Comment 2

[Lee Salzman [:lsalzman]]

Attachment: fix UnscaledFontFreeType::GetFontFileData to read file contents when filename is used instead of an FT_Face

Comment 3

[Lee Salzman [:lsalzman]]

(In reply to Lee Salzman [:lsalzman] from comment #2)
> Created attachment 8859798 [details] [diff] [review]
> fix UnscaledFontFreeType::GetFontFileData to read file contents when
> filename is used instead of an FT_Face

Should be noted that I chose to use mmap here as we were already using it in places in thebes in the FT2 font code. I discovered during debugging that font files were usually a couple hundred KB in size up to a MB, so mmaping here to get rid of extra expensive allocations and reads made sense.

Ultimately we may want to work around the clunky GetFontFileData interface entirely so that we oculd supply mmap'd fonts directly to ipdl rather than also copying them into a ByteBuffer downstream. But by then, we may also want to just consider shmem-based font sharing to get rid of the need to ship file contents through pipes entirely. Anyway, wishful thinking for a future time.

For now, this patch should now just bring Linux up to parity with the other platforms that currently fling everything through AddRawFont.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

(In reply to Lee Salzman [:lsalzman] from comment #1)
> Kats or Jeff, how to properly stage this with the necessary tree webrender
> update?

Based on the try pushes in bug 1357392 it seems like I can land the webrender update first (with the index set to 0) and it shouldn't break anything, and then you can land this afterwards whenever you like. I'll try to get the WR update landed today if I can. Tomorrow by latest.

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Or in case you're wondering how to test these patches locally, you'll want to apply the patches from one of the try pushes in bug 1357392 locally and put these patches on top. If you want I can add these patches to a try push pretty trivially as well. Or you could just wait until the WR update lands and then test as you usually do.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

I landed the WR update, so feel free to land these patches.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/projects/graphics/rev/bec7523b36c6
update WebRenderAPI to allow index in add_raw_font. r=jrmuizel
https://hg.mozilla.org/projects/graphics/rev/44890ebd9c5b
fix UnscaledFontFreeType::GetFontFileData to read file contents when filename is used instead of an FT_Face. r=jrmuizel

Comment 8

[Kartikaya Gupta (email:[e-mail])]

https://hg.mozilla.org/mozilla-central/rev/bec7523b36c6
https://hg.mozilla.org/mozilla-central/rev/44890ebd9c5b