From 0c0d307caed58bc1277f825766aad30118155157 Mon Sep 17 00:00:00 2001
From: hultonha <hultonha@amazon.co.uk>
Date: Tue, 13 Jul 2021 14:49:27 +0100
Subject: [PATCH] ensure empty geometry buffers are not submitted for render

Signed-off-by: hultonha <hultonha@amazon.co.uk>
---
 .../Code/Source/Shape/ShapeGeometryUtil.cpp   | 20 ++++----
 .../Code/Source/Shape/TubeShape.cpp           |  9 +++-
 .../Code/Tests/ShapeGeometryUtilTest.cpp      | 47 +++++++++++++++++--
 3 files changed, 61 insertions(+), 15 deletions(-)

diff --git a/Gems/LmbrCentral/Code/Source/Shape/ShapeGeometryUtil.cpp b/Gems/LmbrCentral/Code/Source/Shape/ShapeGeometryUtil.cpp
index 9a52fa49f0..b0a52d4ca1 100644
--- a/Gems/LmbrCentral/Code/Source/Shape/ShapeGeometryUtil.cpp
+++ b/Gems/LmbrCentral/Code/Source/Shape/ShapeGeometryUtil.cpp
@@ -31,22 +31,20 @@ namespace LmbrCentral
         return vertices + 1;
     }
 
-    void DrawShape(
-        AzFramework::DebugDisplayRequests& debugDisplay,
-        const ShapeDrawParams& shapeDrawParams, const ShapeMesh& shapeMesh)
+    void DrawShape(AzFramework::DebugDisplayRequests& debugDisplay, const ShapeDrawParams& shapeDrawParams, const ShapeMesh& shapeMesh)
     {
         if (shapeDrawParams.m_filled)
         {
-            debugDisplay.DrawTrianglesIndexed(
-                shapeMesh.m_vertexBuffer,
-                shapeMesh.m_indexBuffer,
-                shapeDrawParams.m_shapeColor
-            );
+            if (!shapeMesh.m_vertexBuffer.empty() && !shapeMesh.m_indexBuffer.empty())
+            {
+                debugDisplay.DrawTrianglesIndexed(shapeMesh.m_vertexBuffer, shapeMesh.m_indexBuffer, shapeDrawParams.m_shapeColor);
+            }
         }
 
-        debugDisplay.DrawLines(
-            shapeMesh.m_lineBuffer,
-            shapeDrawParams.m_wireColor);
+        if (!shapeMesh.m_lineBuffer.empty())
+        {
+            debugDisplay.DrawLines(shapeMesh.m_lineBuffer, shapeDrawParams.m_wireColor);
+        }
     }
 
     /// Determine if a list of vertices constitute a simple polygon
diff --git a/Gems/LmbrCentral/Code/Source/Shape/TubeShape.cpp b/Gems/LmbrCentral/Code/Source/Shape/TubeShape.cpp
index 13762ad1ed..5c3e416008 100644
--- a/Gems/LmbrCentral/Code/Source/Shape/TubeShape.cpp
+++ b/Gems/LmbrCentral/Code/Source/Shape/TubeShape.cpp
@@ -379,6 +379,13 @@ namespace LmbrCentral
         const float radius, const AZ::u32 capSegments, const AZ::u32 sides,
         AZStd::vector<AZ::Vector3>& vertexBufferOut)
     {
+        if (const size_t segmentCount = spline->GetSegmentCount(); segmentCount == 0)
+        {
+            // clear the buffers so we no longer draw anything
+            vertexBufferOut.clear();
+            return;
+        }
+
         // notes on vert buffer size
         // total end segments
         // 2 verts for each segment
@@ -401,7 +408,7 @@ namespace LmbrCentral
         const size_t numVerts = totalEndSegments + totalSegments + totalLoops;
         vertexBufferOut.resize(numVerts);
 
-        AZ::Vector3* vertices = vertexBufferOut.begin();
+        AZ::Vector3* vertices = vertexBufferOut.data();
 
         // start cap
         auto address = spline->GetAddressByFraction(0.0f);
diff --git a/Gems/LmbrCentral/Code/Tests/ShapeGeometryUtilTest.cpp b/Gems/LmbrCentral/Code/Tests/ShapeGeometryUtilTest.cpp
index b486645694..eb8b0f4c51 100644
--- a/Gems/LmbrCentral/Code/Tests/ShapeGeometryUtilTest.cpp
+++ b/Gems/LmbrCentral/Code/Tests/ShapeGeometryUtilTest.cpp
@@ -6,12 +6,14 @@
  */
 
 #include "LmbrCentral_precompiled.h"
-#include <AzTest/AzTest.h>
 
 #include <AzCore/Component/ComponentApplication.h>
-#include <Shape/ShapeGeometryUtil.h>
-#include <AzCore/UnitTest/TestTypes.h>
 #include <AzCore/Math/Vector3.h>
+#include <AzCore/UnitTest/TestTypes.h>
+#include <AzFramework/Entity/EntityDebugDisplayBus.h>
+#include <AzTest/AzTest.h>
+#include <LmbrCentral/Shape/ShapeComponentBus.h>
+#include <Shape/ShapeGeometryUtil.h>
 
 namespace UnitTest
 {
@@ -91,4 +93,43 @@ namespace UnitTest
 
         EXPECT_TRUE(triangles.size() == 18);
     }
+
+    // test double to record if DrawTrianglesIndexed or DrawLines are called
+    class DebugShapeDebugDisplayRequests : public AzFramework::DebugDisplayRequests
+    {
+    public:
+        void DrawTrianglesIndexed(
+            [[maybe_unused]] const AZStd::vector<AZ::Vector3>& vertices,
+            [[maybe_unused]] const AZStd::vector<AZ::u32>& indices,
+            [[maybe_unused]] const AZ::Color& color) override
+        {
+            m_drawTrianglesIndexedCalled = true;
+        }
+
+        void DrawLines([[maybe_unused]] const AZStd::vector<AZ::Vector3>& lines, [[maybe_unused]] const AZ::Color& color) override
+        {
+            m_drawLinesCalled = true;
+        }
+
+        bool m_drawTrianglesIndexedCalled = false;
+        bool m_drawLinesCalled = false;
+    };
+
+    // DrawShape internally calls DrawTrianglesIndexed and DrawLines - with no geometry
+    // we want to make sure the shape is not submitted to be drawn
+    TEST(ShapeGeometry, Shape_not_attempted_to_be_drawn_with_no_geometry)
+    {
+        using ::testing::Eq;
+
+        // given
+        DebugShapeDebugDisplayRequests debugDisplayRequests;
+
+        // when
+        LmbrCentral::DrawShape(
+            debugDisplayRequests, LmbrCentral::ShapeDrawParams{ AZ::Colors::White, AZ::Colors::White, true }, LmbrCentral::ShapeMesh{});
+
+        // then
+        EXPECT_THAT(debugDisplayRequests.m_drawTrianglesIndexedCalled, Eq(false));
+        EXPECT_THAT(debugDisplayRequests.m_drawLinesCalled, Eq(false));
+    }
 }