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

Change keying of fonts in the font cache thread #12920

Closed
nox opened this issue Aug 18, 2016 · 15 comments · Fixed by #32303
Closed

Change keying of fonts in the font cache thread #12920

nox opened this issue Aug 18, 2016 · 15 comments · Fixed by #32303

Comments

@nox
Copy link
Contributor

nox commented Aug 18, 2016

AFAICT, fonts are keyed with their family name in the font cache thread. What happens if two pages use the same family name for 2 completely different fonts?

@nox
Copy link
Contributor Author

nox commented Aug 18, 2016

Our font cache seems completely broken to me. The cache keys are family names, which is wrong, but even if it was not, a Web font source could change between 2 requests so we can't use the source as key.

@pcwalton @glennw Would it be bad to just store Web fonts in the layout thread? How then would we feed them to the font context of the paint thread?

@metajack
Copy link
Contributor

@nox It's not thread to thread. In multiprocess, painting happens in the privileged process and layout in unprivileged. So this will cross a process boundary.

@pcwalton
Copy link
Contributor

@nox The font cache being completely broken aligns with what I've seen, namely that nytimes.com fonts can be totally random.

@glennw
Copy link
Member

glennw commented Aug 18, 2016

@nox The idea of storing web fonts in the layout thread sounds quite good. I haven't thought through all the details, maybe there are some gotchas, but it does sound worthwhile. With webrender, layout just needs to pass the font once via a message when it first receives the font, so that should be easy enough. Any thoughts on that idea @pcwalton ?

@nox
Copy link
Contributor Author

nox commented Aug 22, 2016

@metajack Where could I store font handles and be sure that they will be freed on tab navigation, when the font is not needed anymore, etc? How painting, layout, script and style communicate is still quite vague to me.

@metajack
Copy link
Contributor

There is a limit to how many pipelines will be retained even when they are non-active. When they are freed, script and layout will both be shut down.

The only case I'm not 100% clear on is what happens when a script thread is reused. Does layout get rebuilt or just start receiving new display lists? If the latter, that could be a case where we would hold onto fons (if stored in layout) longer than we want.

@jdm
Copy link
Member

jdm commented Aug 22, 2016

Layout threads are never reused.

@nox
Copy link
Contributor Author

nox commented Aug 23, 2016

@jdm Great. What's the lifetime for the paint threads in all that? They don't run in the same process as layout right? So we need to have life font handles in two separate processes, am I wrong?

@jdm
Copy link
Member

jdm commented Aug 23, 2016

Correct, paint and layout threads run in different processes, but should have approximately the same lifetime.

@metajack
Copy link
Contributor

Paint threads will not exist once WR becomes the only renderer. And the WR renderer will have basically a static lifetime.

@nox
Copy link
Contributor Author

nox commented Aug 23, 2016

@metajack Will the WR renderer need to do things with fonts, or will everything that is font-related live in layout?

@metajack
Copy link
Contributor

WR needs font data to rasterize glyphs. Layout needs font data to determine where glyphs will appear and to calculate inline length. They both need it for different purposes.

@nox
Copy link
Contributor Author

nox commented Aug 23, 2016

So we need an invalidation mechanism in WR too, right? Doesn't that mean that at least part of the font cache (and probably HTTP cache) need to live outside of Servo, given WR itself lives outside of Servo?

@metajack
Copy link
Contributor

WR will in theory be in another process.

I suppose we can either a) put the cache in the privileged process or b) put a cache in each process. In the (b) option, we'd just send the font data along with the display list and WR could cache it there. Or if the font is local font, we could just send an identifier so it can be loaded there and then cached.

cc @glennw @pcwalton

@nox
Copy link
Contributor Author

nox commented Aug 24, 2016

@pcwalton It seems that on IRC you mentioned we could make a Weak-like version of OsIpcSharedMemory, especially with mach ports, but I don't see how you can get notified that a vm_allocate'd blob has been freed by everyone.

mrobinson added a commit to mrobinson/servo that referenced this issue May 16, 2024
This moves mangement of web fonts to the per-Layout `FontContext`,
preventing web fonts from being available in different Documents.

Fixes servo#12920.
mrobinson added a commit to mrobinson/servo that referenced this issue May 17, 2024
This moves mangement of web fonts to the per-Layout `FontContext`,
preventing web fonts from being available in different Documents.

Fixes servo#12920.
mrobinson added a commit to mrobinson/servo that referenced this issue May 17, 2024
This moves mangement of web fonts to the per-Layout `FontContext`,
preventing web fonts from being available in different Documents.

Fixes servo#12920.
mrobinson added a commit to mrobinson/servo that referenced this issue May 17, 2024
This moves mangement of web fonts to the per-Layout `FontContext`,
preventing web fonts from being available in different Documents.

Fixes servo#12920.

Co-authored-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
mrobinson added a commit to mrobinson/servo that referenced this issue May 17, 2024
This moves mangement of web fonts to the per-Layout `FontContext`,
preventing web fonts from being available in different Documents.

Fixes servo#12920.

Co-authored-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
mrobinson added a commit to mrobinson/servo that referenced this issue May 17, 2024
This moves mangement of web fonts to the per-Layout `FontContext`,
preventing web fonts from being available in different Documents.

Fixes servo#12920.

Co-authored-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
mrobinson added a commit to mrobinson/servo that referenced this issue May 20, 2024
This moves mangement of web fonts to the per-Layout `FontContext`,
preventing web fonts from being available in different Documents.

Fixes servo#12920.

Co-authored-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
mrobinson added a commit to mrobinson/servo that referenced this issue May 20, 2024
This moves mangement of web fonts to the per-Layout `FontContext`,
preventing web fonts from being available in different Documents.

Fixes servo#12920.

Co-authored-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
mrobinson added a commit to mrobinson/servo that referenced this issue May 20, 2024
This moves mangement of web fonts to the per-Layout `FontContext`,
preventing web fonts from being available in different Documents.

Fixes servo#12920.

Co-authored-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
github-merge-queue bot pushed a commit that referenced this issue May 20, 2024
This moves mangement of web fonts to the per-Layout `FontContext`,
preventing web fonts from being available in different Documents.

Fixes #12920.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Co-authored-by: Mukilan Thiyagarajan <mukilan@igalia.com>
mrobinson added a commit to mrobinson/servo that referenced this issue May 20, 2024
This moves mangement of web fonts to the per-Layout `FontContext`,
preventing web fonts from being available in different Documents.

Fixes servo#12920.

Co-authored-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Signed-off-by: Martin Robinson <mrobinson@igalia.com>
github-merge-queue bot pushed a commit that referenced this issue May 20, 2024
This moves mangement of web fonts to the per-Layout `FontContext`,
preventing web fonts from being available in different Documents.

Fixes #12920.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>
Co-authored-by: Mukilan Thiyagarajan <mukilan@igalia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants