Fix output data alignment and avoid copy

Change the output data container of libshaderc compilation result from
std::string to std::vector<uint32_t> to guarantee the output data alignment.

Change the libshaderc_util::Compiler::Compile() signature and
implementation to avoid extra copies of SPIR-V binary. Also store the
string data of text output in vector<uint32_t>.

3 files changed, 35 insertions, 22 deletions

diff --git a/libshaderc/src/shaderc.cc b/libshaderc/src/shaderc.cc
index 5faf8a7..fc8ed90 100644
--- a/libshaderc/src/shaderc.cc
+++ b/libshaderc/src/shaderc.cc
@@ -315,9 +315,10 @@ shaderc_spv_module_t shaderc_compile_into_spv(
   }
   result->compilation_status = shaderc_compilation_status_invalid_stage;
   bool compilation_succeeded = false;  // In case we exit early.
+  std::vector<uint32_t> compilation_output_data;
+  size_t compilation_output_data_size_in_bytes = 0u;
   if (!compiler->initializer) return result;
   TRY_IF_EXCEPTIONS_ENABLED {
-    std::stringstream output;
     std::stringstream errors;
     size_t total_warnings = 0;
     size_t total_errors = 0;
@@ -327,27 +328,33 @@ shaderc_spv_module_t shaderc_compile_into_spv(
         shaderc_util::string_piece(source_text, source_text + source_text_size);
     StageDeducer stage_deducer(shader_kind);
     if (additional_options) {
-      compilation_succeeded = additional_options->compiler.Compile(
-          source_string, forced_stage, input_file_name_str,
-          // stage_deducer has a flag: error_, which we need to check later.
-          // We need to make this a reference wrapper, so that std::function
-          // won't make a copy for this callable object.
-          std::ref(stage_deducer),
-          InternalFileIncluder(additional_options->get_includer_response,
-                               additional_options->release_includer_response,
-                               additional_options->includer_user_data),
-          &output, &errors, &total_warnings, &total_errors,
-          compiler->initializer);
+      // Depends on return value optimization to avoid extra copy.
+      std::tie(compilation_succeeded, compilation_output_data,
+               compilation_output_data_size_in_bytes) =
+          additional_options->compiler.Compile(
+              source_string, forced_stage, input_file_name_str,
+              // stage_deducer has a flag: error_, which we need to check later.
+              // We need to make this a reference wrapper, so that std::function
+              // won't make a copy for this callable object.
+              std::ref(stage_deducer),
+              InternalFileIncluder(
+                  additional_options->get_includer_response,
+                  additional_options->release_includer_response,
+                  additional_options->includer_user_data),
+              &errors, &total_warnings, &total_errors, compiler->initializer);
     } else {
       // Compile with default options.
-      compilation_succeeded = shaderc_util::Compiler().Compile(
-          source_string, forced_stage, input_file_name_str,
-          std::ref(stage_deducer), InternalFileIncluder(), &output, &errors,
-          &total_warnings, &total_errors, compiler->initializer);
+      std::tie(compilation_succeeded, compilation_output_data,
+               compilation_output_data_size_in_bytes) =
+          shaderc_util::Compiler().Compile(
+              source_string, forced_stage, input_file_name_str,
+              std::ref(stage_deducer), InternalFileIncluder(), &errors,
+              &total_warnings, &total_errors, compiler->initializer);
     }
 
     result->messages = errors.str();
-    result->spirv = output.str();
+    result->output_data = std::move(compilation_output_data);
+    result->output_data_size = compilation_output_data_size_in_bytes;
     result->num_warnings = total_warnings;
     result->num_errors = total_errors;
     if (compilation_succeeded) {
@@ -368,7 +375,7 @@ shaderc_spv_module_t shaderc_compile_into_spv(
 }
 
 size_t shaderc_module_get_length(const shaderc_spv_module_t module) {
-  return module->spirv.size();
+  return module->output_data_size;
 }
 
 size_t shaderc_module_get_num_warnings(const shaderc_spv_module_t module) {
@@ -380,7 +387,7 @@ size_t shaderc_module_get_num_errors(const shaderc_spv_module_t module) {
 }
 
 const char* shaderc_module_get_bytes(const shaderc_spv_module_t module) {
-  return module->spirv.c_str();
+  return reinterpret_cast<const char*>(module->output_data.data());
 }
 
 void shaderc_module_release(shaderc_spv_module_t module) { delete module; }
diff --git a/libshaderc/src/shaderc_cpp_test.cc b/libshaderc/src/shaderc_cpp_test.cc
index 614e027..b70ab38 100644
--- a/libshaderc/src/shaderc_cpp_test.cc
+++ b/libshaderc/src/shaderc_cpp_test.cc
@@ -530,7 +530,7 @@ TEST_F(CppInterface, PreprocessingOnlyModeSecondOverridesDisassemblyMode) {
               HasSubstr("void main(){ }"));
 }
 
-// A shader kind test cases needs: 1) A shader text with or without #pragma
+// A shader kind test case needs: 1) A shader text with or without #pragma
 // annotation, 2) shader_kind.
 struct ShaderKindTestCase {
   const char* shader_;
diff --git a/libshaderc/src/shaderc_private.h b/libshaderc/src/shaderc_private.h
index 598d5c2..7b8bb0b 100644
--- a/libshaderc/src/shaderc_private.h
+++ b/libshaderc/src/shaderc_private.h
@@ -23,8 +23,14 @@
 
 // Described in shaderc.h.
 struct shaderc_spv_module {
-  // SPIR-V binary.
-  std::string spirv;
+  // Compilation output data. In normal compilation mode, it contains the
+  // compiled SPIR-V binary code. In disassembly and preprocessing-only mode, it
+  // contains a null-terminated string which is the text output. For text
+  // output, extra bytes with value 0x00 might be appended to complete the last
+  // uint32_t element.
+  std::vector<uint32_t> output_data;
+  // The size of the output data in term of bytes.
+  size_t output_data_size;
   // Compilation messages.
   std::string messages;
   // Number of errors.