Output requested images/buffers on probe failure. (#316)

This Cl updates the sample app to allow dumping of image and buffer data
even if the probes fail to execute.

Fixes #314.

3 files changed, 32 insertions, 29 deletions

diff --git a/src/amber.cc b/src/amber.cc
index be49e6a..3ad7e9a 100644
--- a/src/amber.cc
+++ b/src/amber.cc
@@ -79,50 +79,44 @@ amber::Result Amber::ExecuteWithShaderData(const amber::Recipe* recipe,
     return r;
 
   Executor executor;
-  r = executor.Execute(engine.get(), script, shader_data,
-                       opts->pipeline_create_only
-                           ? ExecutionType::kPipelineCreateOnly
-                           : ExecutionType::kExecute);
-  if (!r.IsSuccess()) {
-    // Clean up Vulkan/Dawn objects
-    engine->Shutdown();
-    return r;
-  }
-
+  Result executor_result = executor.Execute(
+      engine.get(), script, shader_data,
+      opts->pipeline_create_only ? ExecutionType::kPipelineCreateOnly
+                                 : ExecutionType::kExecute);
+  // Hold the executor result until the extractions are complete. This will let
+  // us dump any buffers requested even on failure.
+
+  // 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.
   for (BufferInfo& buffer_info : opts->extractions) {
     if (buffer_info.buffer_name == "framebuffer") {
       ResourceInfo info;
       r = engine->GetFrameBufferInfo(&info);
-      if (!r.IsSuccess()) {
-        engine->Shutdown();
-        return r;
-      }
+      if (!r.IsSuccess())
+        break;
+
       buffer_info.width = info.image_info.width;
       buffer_info.height = info.image_info.height;
       r = engine->GetFrameBuffer(&(buffer_info.values));
-      if (!r.IsSuccess()) {
-        engine->Shutdown();
-        return r;
-      }
+      if (!r.IsSuccess())
+        break;
 
       continue;
     }
 
     DescriptorSetAndBindingParser desc_set_and_binding_parser;
     r = desc_set_and_binding_parser.Parse(buffer_info.buffer_name);
-    if (!r.IsSuccess()) {
-      engine->Shutdown();
-      return r;
-    }
+    if (!r.IsSuccess())
+      break;
 
     ResourceInfo info = ResourceInfo();
     r = engine->GetDescriptorInfo(
         desc_set_and_binding_parser.GetDescriptorSet(),
         desc_set_and_binding_parser.GetBinding(), &info);
-    if (!r.IsSuccess()) {
-      engine->Shutdown();
-      return r;
-    }
+    if (!r.IsSuccess())
+      break;
 
     const uint8_t* ptr = static_cast<const uint8_t*>(info.cpu_memory);
     auto& values = buffer_info.values;
@@ -133,6 +127,14 @@ amber::Result Amber::ExecuteWithShaderData(const amber::Recipe* recipe,
     }
   }
 
+  if (!executor_result.IsSuccess()) {
+    engine->Shutdown();
+    return executor_result;
+  }
+  if (!r.IsSuccess()) {
+    engine->Shutdown();
+    return r;
+  }
   return engine->Shutdown();
 }
 
diff --git a/src/executor.cc b/src/executor.cc
index 6f72a87..a680607 100644
--- a/src/executor.cc
+++ b/src/executor.cc
@@ -80,7 +80,7 @@ Result Executor::Execute(Engine* engine,
         return r;
 
       // This must come after processing commands because we require
-      // the frambuffer buffer to be mapped into host memory and have
+      // the framebuffer data to be mapped into host memory and have
       // a valid host-side pointer.
       r = engine->GetFrameBufferInfo(&info);
       if (!r.IsSuccess())
diff --git a/src/vulkan/engine_vulkan.cc b/src/vulkan/engine_vulkan.cc
index 1ae9c8e..1deead3 100644
--- a/src/vulkan/engine_vulkan.cc
+++ b/src/vulkan/engine_vulkan.cc
@@ -452,8 +452,9 @@ Result EngineVulkan::DoProcessCommands() {
 
 Result EngineVulkan::GetFrameBufferInfo(ResourceInfo* info) {
   if (!info)
-    return Result("Vulkan::GetFrameBufferInfo Missing info");
-
+    return Result("Vulkan::GetFrameBufferInfo missing info");
+  if (!pipeline_)
+    return Result("Vulkan::GetFrameBufferIfno missing pipeline");
   if (!pipeline_->IsGraphics())
     return Result("Vulkan::GetFrameBufferInfo for Non-Graphics Pipeline");