Bug 1510490 - Rejigger shader concatenation to avoid allocating and operate on a callback. r=mattwoodrow
☠☠ backed out by b425a0d640af ☠ ☠
authorBobby Holley <bobbyholley@gmail.com>
Sat, 01 Dec 2018 03:05:58 +0000
changeset 505508 c02d08e9dd38e073fe23f50b029a44f572dffbdb
parent 505507 6fafd118a82a58aa39d9a4c26107a28f631e3a94
child 505509 ee215fdef53fbccacf05ef299e2b3367100645bf
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmattwoodrow
bugs1510490
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1510490 - Rejigger shader concatenation to avoid allocating and operate on a callback. r=mattwoodrow Depends on D13440 Differential Revision: https://phabricator.services.mozilla.com/D13441
gfx/wr/webrender/src/device/gl.rs
--- a/gfx/wr/webrender/src/device/gl.rs
+++ b/gfx/wr/webrender/src/device/gl.rs
@@ -9,16 +9,17 @@ use api::TextureTarget;
 use api::VoidPtrToSizeFn;
 #[cfg(any(feature = "debug_renderer", feature="capture"))]
 use api::ImageDescriptor;
 use euclid::Transform3D;
 use gleam::gl;
 use internal_types::{FastHashMap, LayerIndex, RenderTargetInfo};
 use log::Level;
 use smallvec::SmallVec;
+use std::borrow::Cow;
 use std::cell::RefCell;
 use std::cmp;
 use std::collections::hash_map::Entry;
 use std::fs::File;
 use std::io::Read;
 use std::marker::PhantomData;
 use std::mem;
 use std::os::raw::c_void;
@@ -177,92 +178,114 @@ fn get_shader_version(gl: &gl::Gl) -> &'
     match gl.get_type() {
         gl::GlType::Gl => SHADER_VERSION_GL,
         gl::GlType::Gles => SHADER_VERSION_GLES,
     }
 }
 
 // Get a shader string by name, from the built in resources or
 // an override path, if supplied.
-fn get_shader_source(shader_name: &str, base_path: &Option<PathBuf>) -> Option<String> {
+fn get_shader_source(shader_name: &str, base_path: &Option<PathBuf>) -> Option<Cow<'static, str>> {
     if let Some(ref base) = *base_path {
         let shader_path = base.join(&format!("{}.glsl", shader_name));
         if shader_path.exists() {
             let mut source = String::new();
             File::open(&shader_path)
                 .unwrap()
                 .read_to_string(&mut source)
                 .unwrap();
-            return Some(source);
+            return Some(Cow::Owned(source));
         }
     }
 
     shader_source::SHADERS
         .get(shader_name)
-        .map(|s| s.to_string())
+        .map(|s| Cow::Borrowed(*s))
 }
 
 // Parse a shader string for imports. Imports are recursively processed, and
-// prepended to the list of outputs.
-fn parse_shader_source(source: String, base_path: &Option<PathBuf>, output: &mut String) {
+// prepended to the output stream.
+fn parse_shader_source<F: FnMut(&str)>(source: Cow<'static, str>, base_path: &Option<PathBuf>, output: &mut F) {
     for line in source.lines() {
         if line.starts_with(SHADER_IMPORT) {
             let imports = line[SHADER_IMPORT.len() ..].split(',');
 
             // For each import, get the source, and recurse.
             for import in imports {
                 if let Some(include) = get_shader_source(import, base_path) {
                     parse_shader_source(include, base_path, output);
                 }
             }
         } else {
-            output.push_str(line);
-            output.push_str("\n");
+            output(line);
+            output("\n");
         }
     }
 }
 
+/// Creates heap-allocated strings for both vertex and fragment shaders. Public
+/// to be accessible to tests.
 pub fn build_shader_strings(
+     gl_version_string: &str,
+     features: &str,
+     base_filename: &str,
+     override_path: &Option<PathBuf>,
+) -> (String, String) {
+    let mut vs_source = String::new();
+    build_shader_string(
+        gl_version_string,
+        features,
+        SHADER_KIND_VERTEX,
+        base_filename,
+        override_path,
+        |s| vs_source.push_str(s),
+    );
+
+    let mut fs_source = String::new();
+    build_shader_string(
+        gl_version_string,
+        features,
+        SHADER_KIND_FRAGMENT,
+        base_filename,
+        override_path,
+        |s| fs_source.push_str(s),
+    );
+
+    (vs_source, fs_source)
+}
+
+/// Walks the given shader string and applies the output to the provided
+/// callback. Assuming an override path is not used, does no heap allocation
+/// and no I/O.
+fn build_shader_string<F: FnMut(&str)>(
     gl_version_string: &str,
     features: &str,
+    kind: &str,
     base_filename: &str,
     override_path: &Option<PathBuf>,
-) -> (String, String) {
-    // Construct a list of strings to be passed to the shader compiler.
-    let mut vs_source = String::new();
-    let mut fs_source = String::new();
-
+    mut output: F,
+) {
     // GLSL requires that the version number comes first.
-    vs_source.push_str(gl_version_string);
-    fs_source.push_str(gl_version_string);
+    output(gl_version_string);
 
     // Insert the shader name to make debugging easier.
     let name_string = format!("// {}\n", base_filename);
-    vs_source.push_str(&name_string);
-    fs_source.push_str(&name_string);
+    output(&name_string);
 
     // Define a constant depending on whether we are compiling VS or FS.
-    vs_source.push_str(SHADER_KIND_VERTEX);
-    fs_source.push_str(SHADER_KIND_FRAGMENT);
+    output(kind);
 
     // Add any defines that were passed by the caller.
-    vs_source.push_str(features);
-    fs_source.push_str(features);
+    output(features);
 
     // Parse the main .glsl file, including any imports
     // and append them to the list of sources.
-    let mut shared_result = String::new();
     if let Some(shared_source) = get_shader_source(base_filename, override_path) {
-        parse_shader_source(shared_source, override_path, &mut shared_result);
+        parse_shader_source(shared_source, override_path, &mut output);
     }
-
-    vs_source.push_str(&shared_result);
-    fs_source.push_str(&shared_result);
-
-    (vs_source, fs_source)
 }
 
 pub trait FileWatcherHandler: Send {
     fn file_changed(&self, path: PathBuf);
 }
 
 impl VertexAttributeKind {
     fn size_in_bytes(&self) -> u32 {