From 4dd8081a75a2906ace77e578c26ceca6755fe409 Mon Sep 17 00:00:00 2001
From: Huidong Chen <huidong.chen@autodesk.com>
Date: Tue, 4 Feb 2020 19:48:46 -0500
Subject: [PATCH] Code review feedback

---
 lib/render/vp2RenderDelegate/basisCurves.cpp  |  1 +
 .../vp2RenderDelegate/render_delegate.cpp     | 29 +++++++++++++------
 .../BasisCurvesCubicDomain_Cg.xml             |  3 ++
 .../BasisCurvesCubicDomain_GLSL.xml           |  4 +++
 .../BasisCurvesCubicDomain_HLSL.xml           |  4 +++
 .../BasisCurvesCubicFallbackShader.xml        |  3 ++
 .../BasisCurvesLinearDomain_Cg.xml            |  3 ++
 .../BasisCurvesLinearDomain_GLSL.xml          |  4 +++
 .../BasisCurvesLinearDomain_HLSL.xml          |  4 +++
 .../BasisCurvesLinearFallbackShader.xml       |  3 ++
 10 files changed, 49 insertions(+), 9 deletions(-)

diff --git a/lib/render/vp2RenderDelegate/basisCurves.cpp b/lib/render/vp2RenderDelegate/basisCurves.cpp
index b7bcc99639..d96323ead8 100644
--- a/lib/render/vp2RenderDelegate/basisCurves.cpp
+++ b/lib/render/vp2RenderDelegate/basisCurves.cpp
@@ -1,4 +1,5 @@
 //
+// Copyright 2018 Pixar
 // Copyright 2020 Autodesk
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
diff --git a/lib/render/vp2RenderDelegate/render_delegate.cpp b/lib/render/vp2RenderDelegate/render_delegate.cpp
index c5837806c0..9caaf91cf6 100644
--- a/lib/render/vp2RenderDelegate/render_delegate.cpp
+++ b/lib/render/vp2RenderDelegate/render_delegate.cpp
@@ -74,8 +74,17 @@ namespace
     const MString _pointSizeParameterName    = "pointSize";       //!< Shader parameter name
     const MString _structOutputName          = "outSurfaceFinal"; //!< Output struct name of the fallback shader
 
-    //! Name of the fallback shaders
-    const MString _fallbackShaderNames[3] =
+    //! Enum class for fallback shader types
+    enum class FallbackShaderType
+    {
+        Common = 0,
+        BasisCurvesLinear,
+        BasisCurvesCubic,
+        Count
+    };
+
+    //! Array of shader fragment names indexed by FallbackShaderType
+    const MString _fallbackShaderNames[FallbackShaderType::Count] =
     {
         "FallbackShader",
         "BasisCurvesLinearFallbackShader",
@@ -220,12 +229,13 @@ namespace
 
             \return A new or existing copy of shader instance with given color
         */
-        MHWRender::MShaderInstance* GetFallbackShader(const MColor& color, unsigned int index)
+        MHWRender::MShaderInstance* GetFallbackShader(const MColor& color, FallbackShaderType type)
         {
-            if (index >= sizeof(_fallbackShaders) / sizeof(_fallbackShaders[0])) {
+            if (type >= FallbackShaderType::Count) {
                 return nullptr;
             }
 
+            const unsigned int index = static_cast<unsigned int>(type);
             auto& shaderMap = _fallbackShaders[index];
 
             // Look for it first with reader lock
@@ -268,8 +278,9 @@ namespace
     private:
         bool                    _isInitialized { false };  //!< Whether the shader cache is initialized
 
-        MShaderMap              _fallbackShaders[3];          //!< Shader registry used by fallback shaders
-        MShaderMap              _3dSolidShaders;              //!< Shader registry used by fallback shaders
+        //! Shader registry used by fallback shaders
+        MShaderMap              _fallbackShaders[FallbackShaderType::Count];
+        MShaderMap              _3dSolidShaders;
 
         MHWRender::MShaderInstance*  _fallbackCPVShader { nullptr }; //!< Fallback shader with CPV support
         MHWRender::MShaderInstance*  _3dFatPointShader { nullptr };  //!< 3d shader for points
@@ -683,7 +694,7 @@ MString HdVP2RenderDelegate::GetLocalNodeName(const MString& name) const {
 MHWRender::MShaderInstance* HdVP2RenderDelegate::GetFallbackShader(
     const MColor& color) const
 {
-    return sShaderCache.GetFallbackShader(color, 0);
+    return sShaderCache.GetFallbackShader(color, FallbackShaderType::Common);
 }
 
 /*! \brief  Returns a fallback shader instance when no material is bound.
@@ -698,7 +709,7 @@ MHWRender::MShaderInstance* HdVP2RenderDelegate::GetFallbackShader(
 MHWRender::MShaderInstance*
 HdVP2RenderDelegate::GetBasisCurvesLinearFallbackShader(const MColor& color) const
 {
-    return sShaderCache.GetFallbackShader(color, 1);
+    return sShaderCache.GetFallbackShader(color, FallbackShaderType::BasisCurvesLinear);
 }
 
 /*! \brief  Returns a fallback shader instance when no material is bound.
@@ -713,7 +724,7 @@ HdVP2RenderDelegate::GetBasisCurvesLinearFallbackShader(const MColor& color) con
 MHWRender::MShaderInstance*
 HdVP2RenderDelegate::GetBasisCurvesCubicFallbackShader(const MColor& color) const
 {
-    return sShaderCache.GetFallbackShader(color, 2);
+    return sShaderCache.GetFallbackShader(color, FallbackShaderType::BasisCurvesCubic);
 }
 
 /*! \brief  Returns a fallback CPV shader instance when no material is bound.
diff --git a/lib/render/vp2ShaderFragments/BasisCurvesCubicDomain_Cg.xml b/lib/render/vp2ShaderFragments/BasisCurvesCubicDomain_Cg.xml
index e5eda0ee7f..d906dc7300 100644
--- a/lib/render/vp2ShaderFragments/BasisCurvesCubicDomain_Cg.xml
+++ b/lib/render/vp2ShaderFragments/BasisCurvesCubicDomain_Cg.xml
@@ -1,10 +1,13 @@
 <!--
 ========================================================================
 Copyright 2020 Autodesk
+
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
 You may obtain a copy of the License at
+
     http://www.apache.org/licenses/LICENSE-2.0
+
 Unless required by applicable law or agreed to in writing, software
 distributed under the License is distributed on an "AS IS" BASIS,
 WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
diff --git a/lib/render/vp2ShaderFragments/BasisCurvesCubicDomain_GLSL.xml b/lib/render/vp2ShaderFragments/BasisCurvesCubicDomain_GLSL.xml
index 706bfdf84f..37c97f882b 100644
--- a/lib/render/vp2ShaderFragments/BasisCurvesCubicDomain_GLSL.xml
+++ b/lib/render/vp2ShaderFragments/BasisCurvesCubicDomain_GLSL.xml
@@ -1,10 +1,14 @@
 <!--
 ========================================================================
+Copyright 2018 Pixar
 Copyright 2020 Autodesk
+
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
 You may obtain a copy of the License at
+
     http://www.apache.org/licenses/LICENSE-2.0
+
 Unless required by applicable law or agreed to in writing, software
 distributed under the License is distributed on an "AS IS" BASIS,
 WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
diff --git a/lib/render/vp2ShaderFragments/BasisCurvesCubicDomain_HLSL.xml b/lib/render/vp2ShaderFragments/BasisCurvesCubicDomain_HLSL.xml
index 9763f78d58..f9dfad72f0 100644
--- a/lib/render/vp2ShaderFragments/BasisCurvesCubicDomain_HLSL.xml
+++ b/lib/render/vp2ShaderFragments/BasisCurvesCubicDomain_HLSL.xml
@@ -1,10 +1,14 @@
 <!--
 ========================================================================
+Copyright 2018 Pixar
 Copyright 2020 Autodesk
+
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
 You may obtain a copy of the License at
+
     http://www.apache.org/licenses/LICENSE-2.0
+
 Unless required by applicable law or agreed to in writing, software
 distributed under the License is distributed on an "AS IS" BASIS,
 WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
diff --git a/lib/render/vp2ShaderFragments/BasisCurvesCubicFallbackShader.xml b/lib/render/vp2ShaderFragments/BasisCurvesCubicFallbackShader.xml
index 664dec486a..88aa6a11ed 100644
--- a/lib/render/vp2ShaderFragments/BasisCurvesCubicFallbackShader.xml
+++ b/lib/render/vp2ShaderFragments/BasisCurvesCubicFallbackShader.xml
@@ -1,10 +1,13 @@
 <!--
 ========================================================================
 Copyright 2020 Autodesk
+
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
 You may obtain a copy of the License at
+
     http://www.apache.org/licenses/LICENSE-2.0
+
 Unless required by applicable law or agreed to in writing, software
 distributed under the License is distributed on an "AS IS" BASIS,
 WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
diff --git a/lib/render/vp2ShaderFragments/BasisCurvesLinearDomain_Cg.xml b/lib/render/vp2ShaderFragments/BasisCurvesLinearDomain_Cg.xml
index ea2379f87d..4088cd701c 100644
--- a/lib/render/vp2ShaderFragments/BasisCurvesLinearDomain_Cg.xml
+++ b/lib/render/vp2ShaderFragments/BasisCurvesLinearDomain_Cg.xml
@@ -1,10 +1,13 @@
 <!--
 ========================================================================
 Copyright 2020 Autodesk
+
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
 You may obtain a copy of the License at
+
     http://www.apache.org/licenses/LICENSE-2.0
+
 Unless required by applicable law or agreed to in writing, software
 distributed under the License is distributed on an "AS IS" BASIS,
 WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
diff --git a/lib/render/vp2ShaderFragments/BasisCurvesLinearDomain_GLSL.xml b/lib/render/vp2ShaderFragments/BasisCurvesLinearDomain_GLSL.xml
index 1811070d9d..c92f904f72 100644
--- a/lib/render/vp2ShaderFragments/BasisCurvesLinearDomain_GLSL.xml
+++ b/lib/render/vp2ShaderFragments/BasisCurvesLinearDomain_GLSL.xml
@@ -1,10 +1,14 @@
 <!--
 ========================================================================
+Copyright 2018 Pixar
 Copyright 2020 Autodesk
+
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
 You may obtain a copy of the License at
+
     http://www.apache.org/licenses/LICENSE-2.0
+
 Unless required by applicable law or agreed to in writing, software
 distributed under the License is distributed on an "AS IS" BASIS,
 WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
diff --git a/lib/render/vp2ShaderFragments/BasisCurvesLinearDomain_HLSL.xml b/lib/render/vp2ShaderFragments/BasisCurvesLinearDomain_HLSL.xml
index 899c33205b..53025ce8a7 100644
--- a/lib/render/vp2ShaderFragments/BasisCurvesLinearDomain_HLSL.xml
+++ b/lib/render/vp2ShaderFragments/BasisCurvesLinearDomain_HLSL.xml
@@ -1,10 +1,14 @@
 <!--
 ========================================================================
+Copyright 2018 Pixar
 Copyright 2020 Autodesk
+
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
 You may obtain a copy of the License at
+
     http://www.apache.org/licenses/LICENSE-2.0
+
 Unless required by applicable law or agreed to in writing, software
 distributed under the License is distributed on an "AS IS" BASIS,
 WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
diff --git a/lib/render/vp2ShaderFragments/BasisCurvesLinearFallbackShader.xml b/lib/render/vp2ShaderFragments/BasisCurvesLinearFallbackShader.xml
index cab6c0b78c..8086f24c65 100644
--- a/lib/render/vp2ShaderFragments/BasisCurvesLinearFallbackShader.xml
+++ b/lib/render/vp2ShaderFragments/BasisCurvesLinearFallbackShader.xml
@@ -1,10 +1,13 @@
 <!--
 ========================================================================
 Copyright 2020 Autodesk
+
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
 You may obtain a copy of the License at
+
     http://www.apache.org/licenses/LICENSE-2.0
+
 Unless required by applicable law or agreed to in writing, software
 distributed under the License is distributed on an "AS IS" BASIS,
 WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.