-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
fonts: Store web fonts in the per-Layout FontContext
#32303
Conversation
🤖 Opened new upstream WPT pull request (web-platform-tests/wpt#46341) with upstreamable changes. |
6d439f1
to
5e73fd0
Compare
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#46341). |
5e73fd0
to
f04e3b6
Compare
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#46341). |
components/gfx/font_context.rs
Outdated
font_template_cache: RwLock<HashMap<FontTemplateCacheKey, Vec<FontTemplateRef>>>, | ||
font_group_cache: | ||
RwLock<HashMap<FontGroupCacheKey, Arc<RwLock<FontGroup>>, BuildHasherDefault<FnvHasher>>>, | ||
font_source: Arc<ReentrantMutex<S>>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be able to remove the Arc around font_source
and resource_threads
.
I'm able to compile and run servo with this patch:
diff --git a/components/gfx/font_context.rs b/components/gfx/font_context.rs
index 560247efe7..0aff4abde2 100644
--- a/components/gfx/font_context.rs
+++ b/components/gfx/font_context.rs
@@ -44,8 +44,8 @@ static SMALL_CAPS_SCALE_FACTOR: f32 = 0.8; // Matches FireFox (see gfxFont.h)
/// paint code. It talks directly to the font cache thread where
/// required.
pub struct FontContext<S: FontSource> {
- font_source: Arc<ReentrantMutex<S>>,
- resource_threads: Arc<ReentrantMutex<CoreResourceThread>>,
+ font_source: ReentrantMutex<S>,
+ resource_threads: ReentrantMutex<CoreResourceThread>,
cache: CachingFontSource<S>,
web_fonts: CrossThreadFontStore,
webrender_font_store: CrossThreadWebRenderFontStore,
@@ -60,11 +60,10 @@ impl<S: FontSource> MallocSizeOf for FontContext<S> {
impl<S: FontSource> FontContext<S> {
pub fn new(font_source: S, resource_threads: ResourceThreads) -> FontContext<S> {
#[allow(clippy::default_constructed_unit_structs)]
- let font_source = Arc::new(ReentrantMutex::new(font_source));
FontContext {
- font_source: font_source.clone(),
- resource_threads: Arc::new(ReentrantMutex::new(resource_threads.core_thread)),
- cache: CachingFontSource::new(font_source),
+ font_source: ReentrantMutex::new(font_source.clone()),
+ resource_threads: ReentrantMutex::new(resource_threads.core_thread.clone()),
+ cache: CachingFontSource::new(font_source.clone()),
web_fonts: Arc::new(RwLock::default()),
webrender_font_store: Arc::new(RwLock::default()),
}
@@ -508,7 +507,7 @@ impl<FCT: FontSource + Send + 'static> RemoteWebFontDownloader<FCT> {
#[derive(Default)]
pub struct CachingFontSource<FCT: FontSource> {
- font_cache_thread: Arc<ReentrantMutex<FCT>>,
+ font_cache_thread: ReentrantMutex<FCT>,
fonts: RwLock<HashMap<FontCacheKey, Option<FontRef>>>,
templates: RwLock<HashMap<FontTemplateCacheKey, Vec<FontTemplateRef>>>,
resolved_font_groups:
@@ -516,9 +515,9 @@ pub struct CachingFontSource<FCT: FontSource> {
}
impl<FCT: FontSource> CachingFontSource<FCT> {
- fn new(font_cache_thread: Arc<ReentrantMutex<FCT>>) -> Self {
+ fn new(font_cache_thread: FCT) -> Self {
Self {
- font_cache_thread,
+ font_cache_thread: ReentrantMutex::new(font_cache_thread),
fonts: Default::default(),
templates: Default::default(),
resolved_font_groups: Default::default(),
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I've gone ahead and integrated your changes.
f04e3b6
to
fce5b56
Compare
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#46341). |
fce5b56
to
9320537
Compare
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#46341). |
9320537
to
0dc3466
Compare
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#46341). |
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>
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>
0dc3466
to
1c9312b
Compare
📝 Transplanted new upstreamable changes to existing upstream WPT pull request (web-platform-tests/wpt#46341). |
This moves mangement of web fonts to the per-Layout
FontContext
,preventing web fonts from being available in different Documents.
Fixes #12920.
Co-authored-by: Mukilan Thiyagarajan mukilan@igalia.com
Signed-off-by: Martin Robinson mrobinson@igalia.com
./mach build -d
does not report any errors./mach test-tidy
does not report any errors