Fix non-existent framebuffer name behavior (#700)

Previously, a non-existent framebuffer name led to malformed PNG files.
Also, any missing framebuffer name caused all following framebuffer
extractions to be skipped unnecessarily, leading to further malformed
PNG files.

Fix #699

samples/amber.cc

@@ -471,6 +471,22 @@ int main(int argc, const char** argv) {
           pos != std::string::npos && image_filename.substr(pos + 1) == "png";
       for (const amber::BufferInfo& buffer_info : amber_options.extractions) {
         if (buffer_info.buffer_name == options.fb_names[i]) {
+          if (buffer_info.values.size() !=
+              (buffer_info.width * buffer_info.height)) {
+            result = amber::Result(
+                "Framebuffer (" + buffer_info.buffer_name + ") size (" +
+                std::to_string(buffer_info.values.size()) +
+                ") != " + "width * height (" +
+                std::to_string(buffer_info.width * buffer_info.height) + ")");
+            break;
+          }
+
+          if (buffer_info.values.empty()) {
+            result = amber::Result("Framebuffer (" + buffer_info.buffer_name +
+                                   ") empty or non-existent.");
+            break;
+          }
+
           if (usePNG) {
 #if AMBER_ENABLE_LODEPNG
             result = png::ConvertToPNG(buffer_info.width, buffer_info.height,

samples/png.cc

@@ -50,16 +50,14 @@ amber::Result ConvertToPNG(uint32_t width,
                            uint32_t height,
                            const std::vector<amber::Value>& values,
                            std::vector<uint8_t>* buffer) {
-  if (values.size() != (width * height)) {
-    return amber::Result("Values size (" + std::to_string(values.size()) +
-                         ") != " + "width * height (" +
-                         std::to_string(width * height) + ")");
-  }
+  assert(values.size() == (width * height) &&
+         "Buffer values != width * height");
+  assert(!values.empty() && "Buffer empty");
 
   std::vector<uint8_t> data;
 
   // Prepare data as lodepng expects it
-  for (amber::Value value : values) {
+  for (const amber::Value& value : values) {
     const uint32_t pixel = value.AsUint32();
     data.push_back(byte2(pixel));  // R
     data.push_back(byte1(pixel));  // G

samples/ppm.cc

@@ -42,11 +42,9 @@ amber::Result ConvertToPPM(uint32_t width,
                            uint32_t height,
                            const std::vector<amber::Value>& values,
                            std::vector<uint8_t>* buffer) {
-  if (values.size() != (width * height)) {
-    return amber::Result("Values size (" + std::to_string(values.size()) +
-                         ") != " + "width * height (" +
-                         std::to_string(width * height) + ")");
-  }
+  assert(values.size() == (width * height) &&
+         "Buffer values != width * height");
+  assert(!values.empty() && "Buffer empty");
 
   // Write PPM header
   std::string image = "P6\n";

src/amber.cc

@@ -183,35 +183,30 @@ amber::Result Amber::ExecuteWithShaderData(const amber::Recipe* recipe,
   // TODO(dsinclair): Figure out how extractions work with multiple pipelines.
   auto* pipeline = script->GetPipelines()[0].get();
 
-  // The dump process holds onto the results and terminates the loop if any dump
-  // fails. This will allow us to validate |extractor_result| first as if the
-  // extractor fails before running the pipeline that will trigger the dumps
-  // to almost always fail.
+  // Try to perform each extraction, copying the buffer data into |buffer_info|.
+  // We do not overwrite |executor_result| if extraction fails.
   for (BufferInfo& buffer_info : opts->extractions) {
     if (buffer_info.is_image_buffer) {
       auto* buffer = script->GetBuffer(buffer_info.buffer_name);
       if (!buffer)
-        break;
+        continue;
 
       buffer_info.width = buffer->GetWidth();
       buffer_info.height = buffer->GetHeight();
-      r = GetFrameBuffer(buffer, &(buffer_info.values));
-      if (!r.IsSuccess())
-        break;
-
+      GetFrameBuffer(buffer, &(buffer_info.values));
       continue;
     }
 
     DescriptorSetAndBindingParser desc_set_and_binding_parser;
     r = desc_set_and_binding_parser.Parse(buffer_info.buffer_name);
     if (!r.IsSuccess())
-      break;
+      continue;
 
     const auto* buffer = pipeline->GetBufferForBinding(
         desc_set_and_binding_parser.GetDescriptorSet(),
         desc_set_and_binding_parser.GetBinding());
     if (!buffer)
-      break;
+      continue;
 
     const uint8_t* ptr = buffer->ValuePtr()->data();
     auto& values = buffer_info.values;