Bug 1612440 - Forward glyph dimension and index requests through the scene builder thread. r=gw

This prevents the request from racing ahead of the AddFontInstance message which follows the transaction through the scene builder thread and silently returning zero-sized glyphs.

Differential Revision: https://phabricator.services.mozilla.com/D72906

gfx/wr/webrender/src/render_backend.rs

@@ -872,6 +872,25 @@ impl RenderBackend {
 
                         self.bookkeep_after_frames();
                     },
+                    SceneBuilderResult::GetGlyphDimensions(request) => {
+                        let mut glyph_dimensions = Vec::with_capacity(request.glyph_indices.len());
+                        if let Some(base) = self.resource_cache.get_font_instance(request.key) {
+                            let font = FontInstance::from_base(Arc::clone(&base));
+                            for glyph_index in &request.glyph_indices {
+                                let glyph_dim = self.resource_cache.get_glyph_dimensions(&font, *glyph_index);
+                                glyph_dimensions.push(glyph_dim);
+                            }
+                        }
+                        request.sender.send(glyph_dimensions).unwrap();
+                    }
+                    SceneBuilderResult::GetGlyphIndices(request) => {
+                        let mut glyph_indices = Vec::with_capacity(request.text.len());
+                        for ch in request.text.chars() {
+                            let index = self.resource_cache.get_glyph_index(request.key, ch);
+                            glyph_indices.push(index);
+                        }
+                        request.sender.send(glyph_indices).unwrap();
+                    }
                     SceneBuilderResult::FlushComplete(tx) => {
                         tx.send(()).ok();
                     }
@@ -1054,24 +1073,11 @@ impl RenderBackend {
             ApiMsg::FlushSceneBuilder(tx) => {
                 self.low_priority_scene_tx.send(SceneBuilderRequest::Flush(tx)).unwrap();
             }
-            ApiMsg::GetGlyphDimensions(instance_key, glyph_indices, tx) => {
+            ApiMsg::GetGlyphDimensions(request) => {
-                let mut glyph_dimensions = Vec::with_capacity(glyph_indices.len());
+                self.scene_tx.send(SceneBuilderRequest::GetGlyphDimensions(request)).unwrap();
-                if let Some(base) = self.resource_cache.get_font_instance(instance_key) {
-                    let font = FontInstance::from_base(Arc::clone(&base));
-                    for glyph_index in &glyph_indices {
-                        let glyph_dim = self.resource_cache.get_glyph_dimensions(&font, *glyph_index);
-                        glyph_dimensions.push(glyph_dim);
-                    }
-                }
-                tx.send(glyph_dimensions).unwrap();
             }
-            ApiMsg::GetGlyphIndices(font_key, text, tx) => {
+            ApiMsg::GetGlyphIndices(request) => {
-                let mut glyph_indices = Vec::new();
+                self.scene_tx.send(SceneBuilderRequest::GetGlyphIndices(request)).unwrap();
-                for ch in text.chars() {
-                    let index = self.resource_cache.get_glyph_index(font_key, ch);
-                    glyph_indices.push(index);
-                }
-                tx.send(glyph_indices).unwrap();
             }
             ApiMsg::CloneApi(sender) => {
                 assert!(!self.namespace_alloc_by_client);

gfx/wr/webrender/src/scene_builder_thread.rs

@@ -6,7 +6,7 @@ use api::{AsyncBlobImageRasterizer, BlobImageRequest, BlobImageParams, BlobImage
 use api::{DocumentId, PipelineId, ApiMsg, FrameMsg, SceneMsg, ResourceUpdate, ExternalEvent};
 use api::{BuiltDisplayList, NotificationRequest, Checkpoint, IdNamespace, QualitySettings};
 use api::{ClipIntern, FilterDataIntern, MemoryReport, PrimitiveKeyKind, SharedFontInstanceMap};
-use api::DocumentLayer;
+use api::{DocumentLayer, GlyphDimensionRequest, GlyphIndexRequest};
 use api::units::*;
 #[cfg(feature = "capture")]
 use crate::capture::CaptureConfig;
@@ -123,6 +123,8 @@ pub enum SceneBuilderRequest {
     ExternalEvent(ExternalEvent),
     AddDocument(DocumentId, DeviceIntSize, DocumentLayer),
     DeleteDocument(DocumentId),
+    GetGlyphDimensions(GlyphDimensionRequest),
+    GetGlyphIndices(GlyphIndexRequest),
     WakeUp,
     Flush(Sender<()>),
     ClearNamespace(IdNamespace),
@@ -150,6 +152,8 @@ pub enum SceneBuilderResult {
     ExternalEvent(ExternalEvent),
     FlushComplete(Sender<()>),
     ClearNamespace(IdNamespace),
+    GetGlyphDimensions(GlyphDimensionRequest),
+    GetGlyphIndices(GlyphIndexRequest),
     Stopped,
     DocumentsForDebugger(String)
 }
@@ -393,6 +397,12 @@ impl SceneBuilderThread {
                 Ok(SceneBuilderRequest::ExternalEvent(evt)) => {
                     self.send(SceneBuilderResult::ExternalEvent(evt));
                 }
+                Ok(SceneBuilderRequest::GetGlyphDimensions(request)) => {
+                    self.send(SceneBuilderResult::GetGlyphDimensions(request))
+                }
+                Ok(SceneBuilderRequest::GetGlyphIndices(request)) => {
+                    self.send(SceneBuilderResult::GetGlyphIndices(request))
+                }
                 Ok(SceneBuilderRequest::Stop) => {
                     self.tx.send(SceneBuilderResult::Stopped).unwrap();
                     // We don't need to send a WakeUp to api_tx because we only

gfx/wr/webrender_api/src/api.rs

@@ -986,13 +986,9 @@ pub enum DebugCommand {
 /// Message sent by the `RenderApi` to the render backend thread.
 pub enum ApiMsg {
     /// Gets the glyph dimensions
-    GetGlyphDimensions(
+    GetGlyphDimensions(font::GlyphDimensionRequest),
-        font::FontInstanceKey,
-        Vec<font::GlyphIndex>,
-        Sender<Vec<Option<font::GlyphDimensions>>>,
-    ),
     /// Gets the glyph indices from a string
-    GetGlyphIndices(font::FontKey, String, Sender<Vec<Option<u32>>>),
+    GetGlyphIndices(font::GlyphIndexRequest),
     /// Adds a new document namespace.
     CloneApi(Sender<IdNamespace>),
     /// Adds a new document namespace.
@@ -1487,20 +1483,28 @@ impl RenderApi {
     /// This means that glyph dimensions e.g. for spaces (' ') will mostly be None.
     pub fn get_glyph_dimensions(
         &self,
-        font: font::FontInstanceKey,
+        key: font::FontInstanceKey,
         glyph_indices: Vec<font::GlyphIndex>,
     ) -> Vec<Option<font::GlyphDimensions>> {
-        let (tx, rx) = channel();
+        let (sender, rx) = channel();
-        let msg = ApiMsg::GetGlyphDimensions(font, glyph_indices, tx);
+        let msg = ApiMsg::GetGlyphDimensions(font::GlyphDimensionRequest {
+            key,
+            glyph_indices,
+            sender
+        });
         self.api_sender.send(msg).unwrap();
         rx.recv().unwrap()
     }
 
     /// Gets the glyph indices for the supplied string. These
     /// can be used to construct GlyphKeys.
-    pub fn get_glyph_indices(&self, font_key: font::FontKey, text: &str) -> Vec<Option<u32>> {
+    pub fn get_glyph_indices(&self, key: font::FontKey, text: &str) -> Vec<Option<u32>> {
-        let (tx, rx) = channel();
+        let (sender, rx) = channel();
-        let msg = ApiMsg::GetGlyphIndices(font_key, text.to_string(), tx);
+        let msg = ApiMsg::GetGlyphIndices(font::GlyphIndexRequest {
+            key,
+            text: text.to_string(),
+            sender,
+        });
         self.api_sender.send(msg).unwrap();
         rx.recv().unwrap()
     }

gfx/wr/webrender_api/src/font.rs

@@ -16,7 +16,7 @@ use std::cmp::Ordering;
 use std::hash::{Hash, Hasher};
 #[cfg(not(target_os = "macos"))]
 use std::path::PathBuf;
-use std::sync::{Arc, RwLock, RwLockReadGuard};
+use std::sync::{Arc, RwLock, RwLockReadGuard, mpsc::Sender};
 use std::collections::HashMap;
 // local imports
 use crate::api::IdNamespace;
@@ -210,6 +210,18 @@ pub struct GlyphDimensions {
     pub advance: f32,
 }
 
+pub struct GlyphDimensionRequest {
+    pub key: FontInstanceKey,
+    pub glyph_indices: Vec<GlyphIndex>,
+    pub sender: Sender<Vec<Option<GlyphDimensions>>>,
+}
+
+pub struct GlyphIndexRequest {
+    pub key: FontKey,
+    pub text: String,
+    pub sender: Sender<Vec<Option<u32>>>,
+}
+
 #[repr(C)]
 #[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, MallocSizeOf, PartialEq, Serialize, Ord, PartialOrd)]
 pub struct FontKey(pub IdNamespace, pub u32);