Add more documentation (#182)

16 files changed, 119 insertions, 62 deletions

diff --git a/src/amberscript/parser.cc b/src/amberscript/parser.cc
index 2e004cb..2917810 100644
--- a/src/amberscript/parser.cc
+++ b/src/amberscript/parser.cc
@@ -27,7 +27,7 @@
 namespace amber {
 namespace amberscript {
 
-Parser::Parser() : amber::Parser(), script_(MakeUnique<Script>()) {}
+Parser::Parser() : amber::Parser() {}
 
 Parser::~Parser() = default;
 
diff --git a/src/amberscript/parser.h b/src/amberscript/parser.h
index 7f0fc05..be922dd 100644
--- a/src/amberscript/parser.h
+++ b/src/amberscript/parser.h
@@ -36,7 +36,6 @@ class Parser : public amber::Parser {
 
   // amber::Parser
   Result Parse(const std::string& data) override;
-  std::unique_ptr<Script> GetScript() override { return std::move(script_); }
 
  private:
   std::string make_error(const std::string& err);
@@ -60,7 +59,6 @@ class Parser : public amber::Parser {
   Result ParsePipelineEntryPoint(Pipeline*);
   Result ParsePipelineShaderOptimizations(Pipeline*);
 
-  std::unique_ptr<Script> script_;
   std::unique_ptr<Tokenizer> tokenizer_;
 };
 
diff --git a/src/buffer.h b/src/buffer.h
index 771ff0c..668dd65 100644
--- a/src/buffer.h
+++ b/src/buffer.h
@@ -94,6 +94,7 @@ class DataBuffer : public Buffer {
   explicit DataBuffer(BufferType type);
   ~DataBuffer() override;
 
+  // Buffer
   bool IsDataBuffer() const override { return true; }
   size_t GetSizeInBytes() const override {
     return GetSize() * datum_type_.SizeInBytes();
@@ -118,6 +119,7 @@ class FormatBuffer : public Buffer {
   explicit FormatBuffer(BufferType type);
   ~FormatBuffer() override;
 
+  // Buffer
   bool IsFormatBuffer() const override { return true; }
   size_t GetSizeInBytes() const override {
     return GetSize() * format_->GetByteSize();
diff --git a/src/engine.h b/src/engine.h
index b45ed36..2a94178 100644
--- a/src/engine.h
+++ b/src/engine.h
@@ -29,8 +29,11 @@
 
 namespace amber {
 
+/// The type of resource being described.
 enum class ResourceInfoType : uint8_t {
+  /// A buffer resource.
   kBuffer = 0,
+  /// An image resource.
   kImage,
 };
 
@@ -40,12 +43,13 @@ struct EngineData {
   uint32_t fence_timeout_ms = 100;
 };
 
+/// Contains information relating to a backing resource from the engine.
 struct ResourceInfo {
   ResourceInfoType type = ResourceInfoType::kBuffer;
 
-  // Key metrics of a 2D image.
-  // For higher dimensions or arrayed images, we would need more strides.
-  // For example, see VkSubresourceLayout.
+  /// Key metrics of a 2D image.
+  /// For higher dimensions or arrayed images, we would need more strides.
+  /// For example, see VkSubresourceLayout.
   struct {
     uint32_t texel_stride = 0;  // Number of bytes for a single texel.
     uint32_t row_stride = 0;  // Number of bytes between successive pixel rows.
@@ -54,106 +58,108 @@ struct ResourceInfo {
     uint32_t depth = 0;
   } image_info;
 
-  // The size in bytes of Vulkan memory pointed by |cpu_memory|.
-  // For the case when it is an image resource, |size_in_bytes| must
-  // be |image_info.row_stride * image_info.height * image_info.depth|.
+  /// The size in bytes of Vulkan memory pointed by |cpu_memory|.
+  /// For the case when it is an image resource, |size_in_bytes| must
+  /// be |image_info.row_stride * image_info.height * image_info.depth|.
   size_t size_in_bytes = 0;
 
-  // If the primitive type of resource is the same with the type
-  // of actual data, the alignment must be properly determined by
-  // Vulkan's internal memory allocation. In these cases, script
-  // writers can assume that there is no alignment issues.
+  /// If the primitive type of resource is the same with the type
+  /// of actual data, the alignment must be properly determined by
+  /// Vulkan's internal memory allocation. In these cases, script
+  /// writers can assume that there is no alignment issues.
   const void* cpu_memory = nullptr;
 };
 
+/// Abstract class which describes a backing engine for Amber.
 class Engine {
  public:
+  /// Creates a new engine of the requested |type|.
   static std::unique_ptr<Engine> Create(EngineType type);
 
   virtual ~Engine();
 
-  // Initialize the engine. The |features| list is a set of required features
-  // for the device. The |extensions| list is a list of required extensions.
+  /// Initialize the engine. The |features| list is a set of required features
+  /// for the device. The |extensions| list is a list of required extensions.
   virtual Result Initialize(const std::vector<Feature>& features,
                             const std::vector<std::string>& extensions) = 0;
 
-  // Initialize the engine with the provided device. The device is _not_ owned
-  // by the engine and should not be destroyed. The |features| and |extensions|
-  // are passed into InitializeWithConfig for validation purposes only. If
-  // possible the engine should verify the provided config specifies the
-  // constraints in |features| and |extensions| and fail if those constrains
-  // are not met.
+  /// Initialize the engine with the provided device. The device is _not_ owned
+  /// by the engine and should not be destroyed. The |features| and |extensions|
+  /// are passed into InitializeWithConfig for validation purposes only. If
+  /// possible the engine should verify the provided config specifies the
+  /// constraints in |features| and |extensions| and fail if those constrains
+  /// are not met.
   virtual Result InitializeWithConfig(
       EngineConfig* config,
       const std::vector<Feature>& features,
       const std::vector<std::string>& extensions) = 0;
 
-  // Shutdown the engine and cleanup any resources.
+  /// Shutdown the engine and cleanup any resources.
   virtual Result Shutdown() = 0;
 
-  // Create graphics pipeline.
+  /// Create graphics pipeline.
   virtual Result CreatePipeline(PipelineType type) = 0;
 
-  // Set the shader of |type| to the binary |data|.
+  /// Set the shader of |type| to the binary |data|.
   virtual Result SetShader(ShaderType type,
                            const std::vector<uint32_t>& data) = 0;
 
-  // Provides the data for a given buffer to be bound at the given location
-  // This is used to declare and populate vertex and index inputs to a graphics
-  // pipeline.
+  /// Provides the data for a given buffer to be bound at the given location
+  /// This is used to declare and populate vertex and index inputs to a graphics
+  /// pipeline.
   virtual Result SetBuffer(BufferType type,
                            uint8_t location,
                            const Format& format,
                            const std::vector<Value>& data) = 0;
 
-  // Execute the clear color command
+  /// Execute the clear color command
   virtual Result DoClearColor(const ClearColorCommand* cmd) = 0;
 
-  // Execute the clear stencil command
+  /// Execute the clear stencil command
   virtual Result DoClearStencil(const ClearStencilCommand* cmd) = 0;
 
-  // Execute the clear depth command
+  /// Execute the clear depth command
   virtual Result DoClearDepth(const ClearDepthCommand* cmd) = 0;
 
-  // Execute the clear command
+  /// Execute the clear command
   virtual Result DoClear(const ClearCommand* cmd) = 0;
 
-  // Execute the draw rect command
+  /// Execute the draw rect command
   virtual Result DoDrawRect(const DrawRectCommand* cmd) = 0;
 
-  // Execute the draw arrays command
+  /// Execute the draw arrays command
   virtual Result DoDrawArrays(const DrawArraysCommand* cmd) = 0;
 
-  // Execute the compute command
+  /// Execute the compute command
   virtual Result DoCompute(const ComputeCommand* cmd) = 0;
 
-  // Execute the entry point command
+  /// Execute the entry point command
   virtual Result DoEntryPoint(const EntryPointCommand* cmd) = 0;
 
-  // Execute the patch command
+  /// Execute the patch command
   virtual Result DoPatchParameterVertices(
       const PatchParameterVerticesCommand* cmd) = 0;
 
-  // Execute the buffer command.
-  // This declares an Amber buffer to be bound to a descriptor.
-  // This covers both Vulkan buffers and images.
+  /// Execute the buffer command.
+  /// This declares an Amber buffer to be bound to a descriptor.
+  /// This covers both Vulkan buffers and images.
   virtual Result DoBuffer(const BufferCommand* cmd) = 0;
 
-  // Run all queued commands and copy frame buffer data to the host
-  // if graphics pipeline.
+  /// Run all queued commands and copy frame buffer data to the host
+  /// if graphics pipeline.
   virtual Result DoProcessCommands() = 0;
 
-  // Get stride, width, height, and memory pointer of frame buffer.
-  // This is only valid if the buffer of framebuffer is mapped into
-  // the host address space. In particular, if we have run
-  // DoProcessCommands() and since then no graphics pipeline drawing
-  // commands have occurred e.g., DoClear, DoDrawArrays, DoDrawRect.
+  /// Get stride, width, height, and memory pointer of frame buffer.
+  /// This is only valid if the buffer of framebuffer is mapped into
+  /// the host address space. In particular, if we have run
+  /// DoProcessCommands() and since then no graphics pipeline drawing
+  /// commands have occurred e.g., DoClear, DoDrawArrays, DoDrawRect.
   virtual Result GetFrameBufferInfo(ResourceInfo* info) = 0;
 
-  // Copy the contents of the resource bound to the given descriptor
-  // and get the resource information e.g., size for buffer, width,
-  // height, depth for image of descriptor given as |descriptor_set|
-  // and |binding|.
+  /// Copy the contents of the resource bound to the given descriptor
+  /// and get the resource information e.g., size for buffer, width,
+  /// height, depth for image of descriptor given as |descriptor_set|
+  /// and |binding|.
   virtual Result GetDescriptorInfo(const uint32_t descriptor_set,
                                    const uint32_t binding,
                                    ResourceInfo* info) = 0;
diff --git a/src/executor.h b/src/executor.h
index 1144b89..a05ca76 100644
--- a/src/executor.h
+++ b/src/executor.h
@@ -22,12 +22,17 @@
 
 namespace amber {
 
+/// The executor is responsible for running the given script against an engine.
 class Executor {
  public:
+  /// Create a new executor.
   Executor();
   ~Executor();
 
-  Result Execute(Engine*, const Script*, const ShaderMap&);
+  /// Executes |script| against |engine|. For each shader described in |script|
+  /// if the shader name exists in |map| the value for that map'd key will be
+  /// used as the shader binary.
+  Result Execute(Engine* engine, const Script* script, const ShaderMap& map);
 
  private:
   Verifier verifier_;
diff --git a/src/feature.h b/src/feature.h
index 785b59d..1dcdd24 100644
--- a/src/feature.h
+++ b/src/feature.h
@@ -17,6 +17,7 @@
 
 namespace amber {
 
+/// List of features that can be requested.
 enum class Feature {
   kUnknown = 0,
   kRobustBufferAccess,
diff --git a/src/format.h b/src/format.h
index 31b1d59..95ebd31 100644
--- a/src/format.h
+++ b/src/format.h
@@ -22,8 +22,10 @@
 
 namespace amber {
 
+/// The format class describes requested image formats. (eg. R8G8B8A8_UINT).
 class Format {
  public:
+  /// Describes an individual component of a format.
   struct Component {
     Component(FormatComponentType t, FormatMode m, uint8_t bits)
         : type(t), mode(m), num_bits(bits) {}
@@ -40,9 +42,11 @@ class Format {
   void SetFormatType(FormatType type) { type_ = type; }
   FormatType GetFormatType() const { return type_; }
 
+  /// Set the number of bytes this format is packed into, if provided.
   void SetPackSize(uint8_t size_in_bytes) {
     pack_size_in_bytes_ = size_in_bytes;
   }
+  /// Retrieves the number of bytes this format is packed into.
   uint8_t GetPackSize() const { return pack_size_in_bytes_; }
 
   void AddComponent(FormatComponentType type, FormatMode mode, uint8_t bits) {
diff --git a/src/parser.cc b/src/parser.cc
index 33d3d05..7e0a6e3 100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -14,9 +14,11 @@
 
 #include "src/parser.h"
 
+#include "src/make_unique.h"
+
 namespace amber {
 
-Parser::Parser() = default;
+Parser::Parser() : script_(MakeUnique<Script>()) {}
 
 Parser::~Parser() = default;
 
diff --git a/src/parser.h b/src/parser.h
index ca22be1..20a1713 100644
--- a/src/parser.h
+++ b/src/parser.h
@@ -17,21 +17,29 @@
 
 #include <memory>
 #include <string>
+#include <utility>
 
 #include "amber/result.h"
 #include "src/script.h"
 
 namespace amber {
 
+/// Base class for an input parser.
 class Parser {
  public:
   virtual ~Parser();
 
+  /// Run the parser against |data|. The result is success if the parse
+  /// completes correctly.
   virtual Result Parse(const std::string& data) = 0;
-  virtual std::unique_ptr<Script> GetScript() = 0;
+
+  /// Retrieve the script which is generated by the parser.
+  std::unique_ptr<Script> GetScript() { return std::move(script_); }
 
  protected:
   Parser();
+
+  std::unique_ptr<Script> script_;
 };
 
 }  // namespace amber
diff --git a/src/script.h b/src/script.h
index 1ebab3a..6933775 100644
--- a/src/script.h
+++ b/src/script.h
@@ -33,13 +33,17 @@
 
 namespace amber {
 
+/// Class representing the script to be run against an engine.
 class Script : public RecipeImpl {
  public:
   Script();
   ~Script() override;
 
+  /// Retrieves information on the shaders in the given script.
   std::vector<ShaderInfo> GetShaderInfo() const override;
 
+  /// Adds |pipeline| to the list of known pipelines. The |pipeline| must have
+  /// a unique name over all pipelines in the script.
   Result AddPipeline(std::unique_ptr<Pipeline> pipeline) {
     if (name_to_pipeline_.count(pipeline->GetName()) > 0)
       return Result("duplicate pipeline name provided");
@@ -49,15 +53,19 @@ class Script : public RecipeImpl {
     return {};
   }
 
+  /// Retrieves the pipeline with |name|, |nullptr| if not found.
   Pipeline* GetPipeline(const std::string& name) const {
     auto it = name_to_pipeline_.find(name);
     return it == name_to_pipeline_.end() ? nullptr : it->second;
   }
 
+  /// Retrieves a list of all pipelines.
   const std::vector<std::unique_ptr<Pipeline>>& GetPipelines() const {
     return pipelines_;
   }
 
+  /// Adds |shader| to the list of known shaders. The |shader| must have a
+  /// unique name over all shaders in the script.
   Result AddShader(std::unique_ptr<Shader> shader) {
     if (name_to_shader_.count(shader->GetName()) > 0)
       return Result("duplicate shader name provided");
@@ -67,15 +75,19 @@ class Script : public RecipeImpl {
     return {};
   }
 
+  /// Retrieves the shader with |name|, |nullptr| if not found.
   Shader* GetShader(const std::string& name) const {
     auto it = name_to_shader_.find(name);
     return it == name_to_shader_.end() ? nullptr : it->second;
   }
 
+  /// Retrieves a list of all shaders.
   const std::vector<std::unique_ptr<Shader>>& GetShaders() const {
     return shaders_;
   }
 
+  /// Adds |buffer| to the list of known buffers. The |buffer| must have a
+  /// unique name over all buffers in the script.
   Result AddBuffer(std::unique_ptr<Buffer> buffer) {
     if (name_to_buffer_.count(buffer->GetName()) > 0)
       return Result("duplicate buffer name provided");
@@ -85,35 +97,49 @@ class Script : public RecipeImpl {
     return {};
   }
 
+  /// Retrieves the buffer with |name|, |nullptr| if not found.
   Buffer* GetBuffer(const std::string& name) const {
     auto it = name_to_buffer_.find(name);
     return it == name_to_buffer_.end() ? nullptr : it->second;
   }
 
+  /// Retrieves a list of all buffers.
   const std::vector<std::unique_ptr<Buffer>>& GetBuffers() const {
     return buffers_;
   }
 
+  /// Adds |feature| to the list of features that must be supported by the
+  /// engine.
   void AddRequiredFeature(Feature feature) {
     engine_info_.required_features.push_back(feature);
   }
+
+  /// Retrieves a list of features required for this script.
   const std::vector<Feature>& RequiredFeatures() const {
     return engine_info_.required_features;
   }
 
+  /// Adds |ext| to the list of extensions that must be supported by the engine.
   void AddRequiredExtension(const std::string& ext) {
     engine_info_.required_extensions.push_back(ext);
   }
+
+  /// Retrieves a list of extensions required for this script.
   const std::vector<std::string>& RequiredExtensions() const {
     return engine_info_.required_extensions;
   }
 
+  /// Retrieves the engine configuration data for this script.
   EngineData& GetEngineData() { return engine_data_; }
+  /// Retrieves the engine configuration data for this script.
   const EngineData& GetEngineData() const { return engine_data_; }
 
+  /// Sets |cmds| to the list of commands to execute against the engine.
   void SetCommands(std::vector<std::unique_ptr<Command>> cmds) {
     commands_ = std::move(cmds);
   }
+
+  /// Retrieves the list of commands to execute against the engine.
   const std::vector<std::unique_ptr<Command>>& GetCommands() const {
     return commands_;
   }
diff --git a/src/shader.h b/src/shader.h
index 724e01c..3b1dee8 100644
--- a/src/shader.h
+++ b/src/shader.h
@@ -21,8 +21,10 @@
 
 namespace amber {
 
+/// Stores information for a shader described in a script.
 class Shader {
  public:
+  /// Create a shader of |type|.
   explicit Shader(ShaderType type);
   ~Shader();
 
diff --git a/src/verifier.h b/src/verifier.h
index 5990c85..0f2190e 100644
--- a/src/verifier.h
+++ b/src/verifier.h
@@ -20,18 +20,25 @@
 
 namespace amber {
 
+/// The verifier is used to validate if a probe command is successful or not.
 class Verifier {
  public:
+  /// Create a verifier.
   Verifier();
   ~Verifier();
 
-  Result Probe(const ProbeCommand*,
+  /// Check |command| against |buf|. The result will be success if the probe
+  /// passes correctly.
+  Result Probe(const ProbeCommand* command,
                uint32_t texel_stride,
                uint32_t row_stride,
                uint32_t frame_width,
                uint32_t frame_height,
                const void* buf);
-  Result ProbeSSBO(const ProbeSSBOCommand*,
+
+  /// Check |command| against |cpu_memory|. The result will be success if the
+  /// probe passes correctly.
+  Result ProbeSSBO(const ProbeSSBOCommand* command,
                    size_t size,
                    const void* cpu_memory);
 };
diff --git a/src/vkscript/parser.cc b/src/vkscript/parser.cc
index dab2b9c..2483b36 100644
--- a/src/vkscript/parser.cc
+++ b/src/vkscript/parser.cc
@@ -152,7 +152,7 @@ Feature NameToFeature(const std::string& name) {
 
 }  // namespace
 
-Parser::Parser() : amber::Parser(), script_(MakeUnique<Script>()) {}
+Parser::Parser() : amber::Parser() {}
 
 Parser::~Parser() = default;
 
diff --git a/src/vkscript/parser.h b/src/vkscript/parser.h
index 5823de3..27d60e6 100644
--- a/src/vkscript/parser.h
+++ b/src/vkscript/parser.h
@@ -35,7 +35,6 @@ class Parser : public amber::Parser {
 
   // amber::Parser
   Result Parse(const std::string& data) override;
-  std::unique_ptr<Script> GetScript() override { return std::move(script_); }
 
   Result ProcessSectionForTesting(const SectionParser::Section& section) {
     return ProcessSection(section);
@@ -49,8 +48,6 @@ class Parser : public amber::Parser {
   Result ProcessIndicesBlock(const SectionParser::Section& section);
   Result ProcessVertexDataBlock(const SectionParser::Section& section);
   Result ProcessTestBlock(const SectionParser::Section& section);
-
-  std::unique_ptr<Script> script_;
 };
 
 }  // namespace vkscript
diff --git a/src/vulkan/pipeline.cc b/src/vulkan/pipeline.cc
index 252f4ea..3a4be57 100644
--- a/src/vulkan/pipeline.cc
+++ b/src/vulkan/pipeline.cc
@@ -96,7 +96,7 @@ Result Pipeline::CreateDescriptorSetLayouts() {
       bindings.back().descriptorCount = 1;
       bindings.back().stageFlags = VK_SHADER_STAGE_ALL;
     }
-    desc_info.bindingCount = bindings.size();
+    desc_info.bindingCount = static_cast<uint32_t>(bindings.size());
     desc_info.pBindings = bindings.data();
 
     if (vkCreateDescriptorSetLayout(device_, &desc_info, nullptr,
@@ -341,7 +341,7 @@ void Pipeline::BindVkDescriptorSets() {
     vkCmdBindDescriptorSets(command_->GetCommandBuffer(),
                             IsGraphics() ? VK_PIPELINE_BIND_POINT_GRAPHICS
                                          : VK_PIPELINE_BIND_POINT_COMPUTE,
-                            pipeline_layout_, i, 1,
+                            pipeline_layout_, static_cast<uint32_t>(i), 1,
                             &descriptor_set_info_[i].vk_desc_set, 0, nullptr);
   }
 }
diff --git a/src/vulkan/pipeline.h b/src/vulkan/pipeline.h
index 3b9b3a1..c087c4e 100644
--- a/src/vulkan/pipeline.h
+++ b/src/vulkan/pipeline.h
@@ -107,7 +107,6 @@ class Pipeline {
   std::vector<VkPipelineShaderStageCreateInfo> shader_stage_info_;
   uint32_t fence_timeout_ms_ = 100;
   bool descriptor_related_objects_already_created_ = false;
-  bool need_sort_descriptors_ = true;
 };
 
 }  // namespace vulkan