Fix RenderTexture copy constructor not creating independent copies

ComputationalBiomechanicsLab · Aug 12, 2024 · b3fc4fb · b3fc4fb
1 parent 24b988b
commit b3fc4fb
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 1 deletion.
diff --git a/src/oscar/Graphics/GraphicsImplementation.cpp b/src/oscar/Graphics/GraphicsImplementation.cpp
@@ -2465,6 +2465,10 @@ osc::RenderBuffer::RenderBuffer(
     impl_{std::make_unique<Impl>(descriptor, buffer_type)}
 {}
 
+osc::RenderBuffer::RenderBuffer(const RenderBuffer& src) :
+    impl_{std::make_unique<Impl>(*src.impl_)}
+{}
+
 osc::RenderBuffer::~RenderBuffer() noexcept = default;
 
 class osc::RenderTexture::Impl final {
@@ -2481,6 +2485,29 @@ class osc::RenderTexture::Impl final {
         depth_buffer_{std::make_shared<RenderBuffer>(descriptor, RenderBufferType::Depth)}
     {}
 
+    // note: independent `RenderTexture::Impl` should have independent data, so value-copy
+    //       the underlying `RenderBuffer` here
+    Impl(const Impl& src) :
+        color_buffer_{std::make_shared<RenderBuffer>(*src.color_buffer_)},
+        depth_buffer_{std::make_shared<RenderBuffer>(*src.depth_buffer_)}
+    {}
+
+    Impl(Impl&&) noexcept = default;
+
+    Impl& operator=(const Impl& src)
+    {
+        if (&src == this) {
+            return *this;
+        }
+        color_buffer_ = std::make_shared<RenderBuffer>(*src.color_buffer_);
+        depth_buffer_ = std::make_shared<RenderBuffer>(*src.depth_buffer_);
+        return *this;
+    }
+
+    Impl& operator=(Impl&&) noexcept = default;
+
+    ~Impl() noexcept = default;
+
     Vec2i dimensions() const
     {
         return color_buffer_->impl_->dimensions();

diff --git a/src/oscar/Graphics/RenderBuffer.h b/src/oscar/Graphics/RenderBuffer.h
@@ -13,7 +13,7 @@ namespace osc
             const RenderTextureDescriptor&,
             RenderBufferType
         );
-        RenderBuffer(const RenderBuffer&) = delete;
+        RenderBuffer(const RenderBuffer&);
         RenderBuffer(RenderBuffer&&) noexcept = delete;
         RenderBuffer& operator=(const RenderBuffer&) = delete;
         RenderBuffer& operator=(RenderBuffer&&) noexcept = delete;

diff --git a/tests/testoscar/Graphics/TestRenderTexture.cpp b/tests/testoscar/Graphics/TestRenderTexture.cpp
@@ -194,3 +194,35 @@ TEST(RenderTexture, UpdDepthBufferReturnsNonNullPtr)
 
     ASSERT_NE(rt.upd_depth_buffer(), nullptr);
 }
+
+TEST(RenderTexture, upd_color_buffer_returns_independent_RenderBuffers_from_copies)
+{
+    // this popped up while developing the `LearnOpenGL/CSM` tab implementation, where
+    // it was using a pattern like:
+    //
+    //     std::vector<RenderTexture> shadowmaps(num_cascades, RenderTexture{common_params});
+    //
+    // that pattern wasn't creating independent shadowmaps because the underlying `RenderBuffer`s
+    // were being reference-copied, rather than value-copied
+
+    RenderTexture rt;
+    RenderTexture copy{rt};
+
+    ASSERT_NE(copy.upd_color_buffer(), rt.upd_color_buffer());
+}
+
+TEST(RenderTexture, upd_depth_buffer_returns_independent_RenderBuffers_from_copies)
+{
+    // this popped up while developing the `LearnOpenGL/CSM` tab implementation, where
+    // it was using a pattern like:
+    //
+    //     std::vector<RenderTexture> shadowmaps(num_cascades, RenderTexture{common_params});
+    //
+    // that pattern wasn't creating independent shadowmaps because the underlying `RenderBuffer`s
+    // were being reference-copied, rather than value-copied
+
+    RenderTexture rt;
+    RenderTexture copy{rt};
+
+    ASSERT_NE(copy.upd_depth_buffer(), rt.upd_depth_buffer());
+}