Skip to content

Commit

Permalink
Auto merge of #17545 - charlesvdv:lazy-font-loading, r=<try>
Browse files Browse the repository at this point in the history 
Load fonts only when needed

I'm just unsure what to do about the synchronous font loading. It looks like synchronous font loading was just a [temporary patch](https://groups.google.com/d/msg/mozilla.dev.servo/q9QXq4974k0/5xlKe7N1BAAJ). Do you want to keep that features or can I completly remove it ?

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #8342 (github issue number if applicable).
- [x] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17545)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Jun 28, 2017
2 parents 522d24d + 0a2f7f0 commit a2f1dc6
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 45 deletions.
53 changes: 47 additions & 6 deletions components/gfx/font_cache_thread.rs
View file
Expand Up @@ -107,14 +107,17 @@ pub enum Command {
GetFontTemplate(FontFamily, FontTemplateDescriptor, IpcSender<Reply>),
GetLastResortFontTemplate(FontTemplateDescriptor, IpcSender<Reply>),
AddWebFont(LowercaseString, EffectiveSources, IpcSender<()>),
FetchWebFont(LowercaseString, EffectiveSources, IpcSender<()>),
AddDownloadedWebFont(LowercaseString, ServoUrl, Vec<u8>, IpcSender<()>),
FontLoadingFinished(IpcSender<Reply>),
Exit(IpcSender<()>),
}

/// Reply messages sent from the font cache thread to the FontContext caller.
#[derive(Deserialize, Serialize, Debug)]
pub enum Reply {
GetFontTemplateReply(Option<FontTemplateInfo>),
FontLoadingState(bool),
}

/// The font cache thread itself. It maintains a list of reference counted
Expand All @@ -125,10 +128,12 @@ struct FontCache {
generic_fonts: HashMap<FontFamily, LowercaseString>,
local_families: HashMap<LowercaseString, FontTemplates>,
web_families: HashMap<LowercaseString, FontTemplates>,
unfetched_web_fonts: HashMap<LowercaseString, (EffectiveSources, IpcSender<()>)>,
font_context: FontContextHandle,
core_resource_thread: CoreResourceThread,
webrender_api: Option<webrender_traits::RenderApi>,
webrender_fonts: HashMap<Atom, webrender_traits::FontKey>,
font_loading_counter: usize,
}

fn populate_generic_fonts() -> HashMap<FontFamily, LowercaseString> {
Expand Down Expand Up @@ -173,12 +178,19 @@ impl FontCache {
let _ = result.send(Reply::GetFontTemplateReply(Some(font_template)));
}
Command::AddWebFont(family_name, sources, result) => {
self.handle_add_web_font(family_name, sources, result);
self.unfetched_web_fonts.insert(family_name, (sources, result.clone()));
}
Command::FetchWebFont(family_name, sources, result) => {
self.get_web_font_from_source(family_name, sources, result);
}
Command::AddDownloadedWebFont(family_name, url, bytes, result) => {
let templates = &mut self.web_families.get_mut(&family_name).unwrap();
templates.add_template(Atom::from(url.to_string()), Some(bytes));
drop(result.send(()));
self.font_loading_counter -= 1;
result.send(()).unwrap();
}
Command::FontLoadingFinished(result) => {
let _ = result.send(Reply::FontLoadingState(self.font_loading_counter == 0));
}
Command::Exit(result) => {
let _ = result.send(());
Expand All @@ -188,13 +200,14 @@ impl FontCache {
}
}

fn handle_add_web_font(&mut self,
fn get_web_font_from_source(&mut self,
family_name: LowercaseString,
mut sources: EffectiveSources,
sender: IpcSender<()>) {
let src = if let Some(src) = sources.next() {
src
} else {
self.font_loading_counter -= 1;
sender.send(()).unwrap();
return;
};
Expand Down Expand Up @@ -241,7 +254,7 @@ impl FontCache {
FetchResponseMsg::ProcessResponseEOF(response) => {
trace!("@font-face {} EOF={:?}", family_name, response);
if response.is_err() || !*response_valid.lock().unwrap() {
let msg = Command::AddWebFont(family_name.clone(), sources.clone(), sender.clone());
let msg = Command::FetchWebFont(family_name.clone(), sources.clone(), sender.clone());
channel_to_self.send(msg).unwrap();
return;
}
Expand All @@ -253,7 +266,9 @@ impl FontCache {
// FIXME(servo/fontsan#1): get an error message
debug!("Sanitiser rejected web font: \
family={} url={:?}", family_name, url);
let msg = Command::AddWebFont(family_name.clone(), sources.clone(), sender.clone());
let msg = Command::FetchWebFont(family_name.clone(),
sources.clone(),
sender.clone());
channel_to_self.send(msg).unwrap();
return;
},
Expand All @@ -277,9 +292,10 @@ impl FontCache {
templates.add_template(Atom::from(&*path), None);
});
if found {
self.font_loading_counter -= 1;
sender.send(()).unwrap();
} else {
let msg = Command::AddWebFont(family_name, sources, sender);
let msg = Command::FetchWebFont(family_name, sources, sender.clone());
self.channel_to_self.send(msg).unwrap();
}
}
Expand Down Expand Up @@ -335,6 +351,11 @@ impl FontCache {
if self.web_families.contains_key(&family_name) {
let templates = self.web_families.get_mut(&family_name).unwrap();
templates.find_font_for_style(desc, &self.font_context)
} else if self.unfetched_web_fonts.contains_key(&family_name) {
self.font_loading_counter += 1;
let (sources, result) = self.unfetched_web_fonts.remove(&family_name).unwrap();
self.get_web_font_from_source(family_name.clone(), sources, result);
None
} else {
None
}
Expand Down Expand Up @@ -414,10 +435,12 @@ impl FontCacheThread {
generic_fonts: generic_fonts,
local_families: HashMap::new(),
web_families: HashMap::new(),
unfetched_web_fonts: HashMap::new(),
font_context: FontContextHandle::new(),
core_resource_thread: core_resource_thread,
webrender_api: webrender_api,
webrender_fonts: HashMap::new(),
font_loading_counter: 0,
};

cache.refresh_local_families();
Expand All @@ -443,6 +466,7 @@ impl FontCacheThread {
Reply::GetFontTemplateReply(data) => {
data
}
_ => unreachable!(),
}
}

Expand All @@ -460,13 +484,30 @@ impl FontCacheThread {
Reply::GetFontTemplateReply(data) => {
data.unwrap()
}
_ => unreachable!(),
}
}

pub fn add_web_font(&self, family: FamilyName, sources: EffectiveSources, sender: IpcSender<()>) {
self.chan.send(Command::AddWebFont(LowercaseString::new(&family.name), sources, sender)).unwrap();
}

pub fn is_web_font_loading_finished(&self) -> bool {
let (response_chan, response_port) =
ipc::channel().expect("failed to create IPC channel");
self.chan.send(Command::FontLoadingFinished(response_chan)).unwrap();

let reply = response_port.recv()
.expect("failed to receive response to font request");

match reply {
Reply::FontLoadingState(data) => {
data
}
_ => unreachable!(),
}
}

pub fn exit(&self) {
let (response_chan, response_port) = ipc::channel().unwrap();
self.chan.send(Command::Exit(response_chan)).expect("Couldn't send FontCacheThread exit message");
Expand Down
49 changes: 12 additions & 37 deletions components/layout_thread/lib.rs
View file
Expand Up @@ -102,7 +102,6 @@ use std::mem as std_mem;
use std::ops::{Deref, DerefMut};
use std::process;
use std::sync::{Arc, Mutex, MutexGuard};
use std::sync::atomic::{AtomicUsize, Ordering};
use std::sync::mpsc::{Receiver, Sender, channel};
use std::thread;
use style::animation::Animation;
Expand Down Expand Up @@ -190,9 +189,6 @@ pub struct LayoutThread {
/// Receives newly-discovered animations.
new_animations_receiver: Receiver<Animation>,

/// The number of Web fonts that have been requested but not yet loaded.
outstanding_web_fonts: Arc<AtomicUsize>,

/// The root of the flow tree.
root_flow: RefCell<Option<FlowRef>>,

Expand Down Expand Up @@ -391,30 +387,15 @@ fn add_font_face_rules(stylesheet: &Stylesheet,
guard: &SharedRwLockReadGuard,
device: &Device,
font_cache_thread: &FontCacheThread,
font_cache_sender: &IpcSender<()>,
outstanding_web_fonts_counter: &Arc<AtomicUsize>) {
if opts::get().load_webfonts_synchronously {
let (sender, receiver) = ipc::channel().unwrap();
stylesheet.effective_font_face_rules(&device, guard, |rule| {
if let Some(font_face) = rule.font_face() {
let effective_sources = font_face.effective_sources();
font_cache_thread.add_web_font(font_face.family().clone(),
effective_sources,
sender.clone());
receiver.recv().unwrap();
}
})
} else {
stylesheet.effective_font_face_rules(&device, guard, |rule| {
if let Some(font_face) = rule.font_face() {
let effective_sources = font_face.effective_sources();
outstanding_web_fonts_counter.fetch_add(1, Ordering::SeqCst);
font_cache_thread.add_web_font(font_face.family().clone(),
effective_sources,
(*font_cache_sender).clone());
}
})
}
font_cache_sender: &IpcSender<()>) {
stylesheet.effective_font_face_rules(&device, guard, |rule| {
if let Some(font_face) = rule.font_face() {
let effective_sources = font_face.effective_sources();
font_cache_thread.add_web_font(font_face.family().clone(),
effective_sources,
(*font_cache_sender).clone());
}
})
}

impl LayoutThread {
Expand Down Expand Up @@ -459,16 +440,14 @@ impl LayoutThread {
ROUTER.route_ipc_receiver_to_new_mpsc_receiver(ipc_font_cache_receiver);

let stylist = Stylist::new(device, QuirksMode::NoQuirks);
let outstanding_web_fonts_counter = Arc::new(AtomicUsize::new(0));
let ua_stylesheets = &*UA_STYLESHEETS;
let guard = ua_stylesheets.shared_lock.read();
for stylesheet in &ua_stylesheets.user_or_user_agent_stylesheets {
add_font_face_rules(stylesheet,
&guard,
stylist.device(),
&font_cache_thread,
&ipc_font_cache_sender,
&outstanding_web_fonts_counter);
&ipc_font_cache_sender);
}

LayoutThread {
Expand All @@ -493,7 +472,6 @@ impl LayoutThread {
generation: Cell::new(0),
new_animations_sender: new_animations_sender,
new_animations_receiver: new_animations_receiver,
outstanding_web_fonts: outstanding_web_fonts_counter,
root_flow: RefCell::new(None),
document_shared_lock: None,
running_animations: StyleArc::new(RwLock::new(FnvHashMap::default())),
Expand Down Expand Up @@ -633,7 +611,6 @@ impl LayoutThread {
},
Request::FromFontCache => {
let _rw_data = possibly_locked_rw_data.lock();
self.outstanding_web_fonts.fetch_sub(1, Ordering::SeqCst);
font_context::invalidate_font_caches();
self.script_chan.send(ConstellationControlMsg::WebFontLoaded(self.id)).unwrap();
true
Expand Down Expand Up @@ -687,8 +664,7 @@ impl LayoutThread {
}
Msg::GetWebFontLoadState(sender) => {
let _rw_data = possibly_locked_rw_data.lock();
let outstanding_web_fonts = self.outstanding_web_fonts.load(Ordering::SeqCst);
sender.send(outstanding_web_fonts != 0).unwrap();
sender.send(!self.font_cache_thread.is_web_font_loading_finished()).unwrap();
},
Msg::CreateLayoutThread(info) => {
self.create_layout_thread(info)
Expand Down Expand Up @@ -814,8 +790,7 @@ impl LayoutThread {
&guard,
self.stylist.device(),
&self.font_cache_thread,
&self.font_cache_sender,
&self.outstanding_web_fonts);
&self.font_cache_sender);
}

possibly_locked_rw_data.block(rw_data);
Expand Down
15 changes: 13 additions & 2 deletions tests/unit/gfx/font_cache_thread.rs
View file
Expand Up @@ -4,8 +4,10 @@

use cssparser::SourceLocation;
use gfx::font_cache_thread::FontCacheThread;
use gfx::font_template::FontTemplateDescriptor;
use ipc_channel::ipc;
use style::computed_values::font_family::FamilyName;
use style::computed_values::{font_stretch, font_weight};
use style::computed_values::font_family::{FamilyName, FontFamily};
use style::font_face::{FontFaceRuleData, Source};

#[test]
Expand All @@ -31,9 +33,18 @@ fn test_local_web_font() {
};

font_cache_thread.add_web_font(
family_name,
family_name.clone(),
font_face_rule.font_face().unwrap().effective_sources(),
out_chan);

let font_family = FontFamily::FamilyName(family_name);
let font_template_descriptor = FontTemplateDescriptor::new(font_weight::T::Weight400,
font_stretch::T::normal,
false);
let result = font_cache_thread.find_font_template(font_family.clone(), font_template_descriptor.clone());
if let Some(_) = result {
panic!("Should not have a value since we don't even load it yet.");
}

assert_eq!(out_receiver.recv().unwrap(), ());
}

0 comments on commit a2f1dc6

Please sign in to comment.