FrameGraph API improvements

- de-dup read/writes to the same resource from the same pass

- also, when reading & writing into the same resource from the same pass,
  always order the read before the write, i.e.:
     out = builder.write(r);
     in  = builder.read(out);
  is equivalent to:
     in  = builder.read(r);
     out = builder.write(in);

  note that the following is still not allowed:
     out = builder.write(r);
     in  = builder.read(r);
  
  this is never valid, since it's reading from a resource that has
  been written to. 


- Make FrameGraphResource class attributes private and add
  comparison operators (so they could be sorted and compared).

filament/src/fg/FrameGraph.cpp

@@ -118,13 +118,36 @@ struct PassNode {
 
     // for Builder
     FrameGraphResource read(ResourceNode const& resource) {
-        // just record that we're reading from this resource (at the given version)
+        // don't allow multiple reads of the same resource -- it's just redundant.
+        auto pos = std::find_if(reads.begin(), reads.end(),
+                [&resource](FrameGraphResource cur) { return resource.index == cur.index; });
+        if (pos != reads.end()) {
+            return *pos;
+        }
+
         FrameGraphResource r{ resource.index, resource.version };
+
+        // now figure out if we already recorded a write to this resource, and if so, use the
+        // previous version number to record the read. i.e. pretend the read() was recorded first.
+        pos = std::find_if(writes.begin(), writes.end(),
+                [&resource](FrameGraphResource cur) { return resource.index == cur.index; });
+        if (pos != writes.end()) {
+            r.version--;
+        }
+
+        // just record that we're reading from this resource (at the given version)
         reads.push_back(r);
         return r;
     }
 
     FrameGraphResource write(ResourceNode& resource) {
+        // don't allow multiple writes of the same resource -- it's just redundant.
+        auto pos = std::find_if(writes.begin(), writes.end(),
+                [&resource](FrameGraphResource cur) { return resource.index == cur.index; });
+        if (pos != writes.end()) {
+            return *pos;
+        }
+
         /*
          * We invalidate and rename handles that are writen into, to avoid undefined order
          * access to the resources.
@@ -291,6 +314,8 @@ FrameGraph::Builder::Builder(FrameGraph& fg, PassNode& pass) noexcept
     : mFrameGraph(fg), mPass(pass) {
 }
 
+FrameGraph::Builder::~Builder() noexcept = default;
+
 FrameGraphResource FrameGraph::Builder::createResource(
         const char* name, FrameGraphResource::Descriptor const& desc) noexcept {
     FrameGraph& frameGraph = mFrameGraph;

filament/src/fg/FrameGraph.h

@@ -82,6 +82,7 @@ public:
     private:
         friend class FrameGraph;
         Builder(FrameGraph& fg, fg::PassNode& pass) noexcept;
+        ~Builder() noexcept;
         FrameGraph& mFrameGraph;
         fg::PassNode& mPass;
     };

filament/src/fg/FrameGraphResource.h

@@ -25,7 +25,12 @@
 
 namespace filament {
 
+namespace fg {
+struct PassNode;
+} // namespace fg
+
 class FrameGraph;
+class FrameGraphPassResources;
 
 /*
  * A FrameGraph resource.
@@ -34,7 +39,22 @@ class FrameGraph;
  */
 
 class FrameGraphResource {
+    friend class FrameGraph;
+    friend class FrameGraphPassResources;
+    friend struct fg::PassNode;
+
+    FrameGraphResource(uint16_t index, uint16_t version) noexcept
+            : index(index), version(version) {}
+
+    static constexpr uint16_t UNINITIALIZED = std::numeric_limits<uint16_t>::max();
+    // index to the resource handle
+    uint16_t index = UNINITIALIZED;
+    // version of the resource when it was created. When this version doesn't match the
+    // resource handle's version, this resource has become invalid.
+    uint16_t version = 0;
+
 public:
+    FrameGraphResource() noexcept = default;
 
     struct Descriptor {
         uint32_t width = 1;     // width of resource in pixel
@@ -46,13 +66,19 @@ public:
         driver::TextureFormat format = driver::TextureFormat::RGBA8;    // resource internal format
     };
 
-    static constexpr uint16_t UNINITIALIZED = std::numeric_limits<uint16_t>::max();
-    // index to the resource handle
-    uint16_t index = UNINITIALIZED;
-    // version of the resource when it was created. When this version doesn't match the
-    // resource handle's version, this resource has become invalid.
-    uint16_t version = 0;
     bool isValid() const noexcept { return index != UNINITIALIZED; }
+
+    bool operator < (const FrameGraphResource& rhs) const noexcept {
+        return (index == rhs.index) ? (version < rhs.version) : (index < rhs.index);
+    }
+
+    bool operator == (const FrameGraphResource& rhs) const noexcept {
+        return (index == rhs.index) && (version == rhs.version);
+    }
+
+    bool operator != (const FrameGraphResource& rhs) const noexcept {
+        return !operator==(rhs);
+    }
 };
 
 } // namespace filament

filament/test/filament_framegraph_test.cpp

@@ -63,6 +63,32 @@ TEST(FrameGraphTest, SimpleRenderPass) {
     EXPECT_TRUE(renderPassExecuted);
 }
 
+TEST(FrameGraphTest, SimpleRenderPassReadWrite) {
+
+    FrameGraph fg;
+
+    struct RenderPassData {
+        FrameGraphResource input;
+        FrameGraphResource output;
+    };
+
+    auto& renderPass = fg.addPass<RenderPassData>("Render",
+            [&](FrameGraph::Builder& builder, RenderPassData& data) {
+                auto r = builder.createResource("renderTarget", {});
+                data.output = builder.write(r);
+                data.input = builder.read(data.output);
+            },
+            [=](FrameGraphPassResources const& resources,
+                    RenderPassData const& data,
+                    driver::DriverApi& driver) {
+            });
+
+    fg.present(renderPass.getData().output);
+    fg.compile();
+    fg.export_graphviz(utils::slog.d);
+    fg.execute(driverApi);
+}
+
 
 TEST(FrameGraphTest, SimpleRenderAndPostProcessPasses) {
 
@@ -126,9 +152,7 @@ TEST(FrameGraphTest, SimpleRenderAndPostProcessPasses) {
     EXPECT_TRUE(fg.isValid(postProcessPass.getData().output));
 
     fg.compile();
-
-    fg.export_graphviz(utils::slog.d);
-
+    //fg.export_graphviz(utils::slog.d);
     fg.execute(driverApi);
 
 
@@ -204,12 +228,8 @@ TEST(FrameGraphTest, SimplePassCulling) {
     EXPECT_TRUE(fg.isValid(culledPass.getData().input));
     EXPECT_TRUE(fg.isValid(culledPass.getData().output));
 
-    //fg.export_graphviz(utils::slog.d);
-
     fg.compile();
-
     //fg.export_graphviz(utils::slog.d);
-
     fg.execute(driverApi);
 
     EXPECT_TRUE(renderPassExecuted);
@@ -370,9 +390,7 @@ TEST(FrameGraphTest, ComplexGraph) {
     //fg.moveResource(convolutionPass.getData().output, lightingPass.getData().output);
 
     fg.compile();
-
-    fg.export_graphviz(utils::slog.d);
-
+    //fg.export_graphviz(utils::slog.d);
     fg.execute(driverApi);
 }
 
@@ -406,6 +424,6 @@ TEST(FrameGraphTest, MoveResource) {
     fg.moveResource(renderPass.getData().output, debugPass.getData().input);
 
     fg.compile();
-    fg.export_graphviz(utils::slog.d);
+    //fg.export_graphviz(utils::slog.d);
     fg.execute(driverApi);
 }