More feedback implementation

Sources/Plasma/FeatureLib/pfMetalPipeline/CMakeLists.txt

@@ -2,7 +2,6 @@ include(FetchContent)
 
 FetchContent_Declare(
   metalcpp
-  DOWNLOAD_EXTRACT_TIMESTAMP TRUE
   URL_HASH_SHA256 0afd87ca851465191ae4e3980aa036c7e9e02fe32e7c760ac1a74244aae6023b
   URL "https://developer.apple.com/metal/cpp/files/metal-cpp_macOS13.3_iOS16.4.zip"
 )

Sources/Plasma/FeatureLib/pfMetalPipeline/ShaderSrc/ShaderTypes.h

@@ -46,6 +46,8 @@ You can contact Cyan Worlds, Inc. by email legal@cyan.com
 #include <simd/simd.h>
 
 #ifndef __METAL_VERSION__
+#include <type_traits>
+
 typedef _Float16 half;
 typedef __attribute__((__ext_vector_type__(2))) half half2;
 typedef __attribute__((__ext_vector_type__(3))) half half3;
@@ -120,10 +122,16 @@ enum plMetalLayerPassType: uint8_t
 struct plMetalFragmentShaderArgumentBuffer {
     __fp16 alphaThreshold;
 };
+#ifndef __METAL_VERSION__
+static_assert(std::is_trivial_v<plMetalFragmentShaderArgumentBuffer>, "plMetalFragmentShaderArgumentBuffer must be a trivial type!");
+#endif
 
 struct plMetalShadowCastFragmentShaderArgumentBuffer {
     bool pointLightCast;
 };
+#ifndef __METAL_VERSION__
+static_assert(std::is_trivial_v<plMetalShadowCastFragmentShaderArgumentBuffer>, "plMetalShadowCastFragmentShaderArgumentBuffer must be a trivial type!");
+#endif
 
 enum plMetalFragmentShaderTextures {
     FragmentShaderArgumentAttributeTextures = 0,
@@ -143,12 +151,18 @@ struct plMetalShaderLightSource {
     __fp16 quadAtten;
     __fp16 scale;
 };
+#ifndef __METAL_VERSION__
+static_assert(std::is_trivial_v<plMetalShaderLightSource>, "plMetalShaderLightSource must be a trivial type!");
+#endif
 
 typedef struct
 {
     uint32_t UVWSrc;
     matrix_float4x4 transform;
 } UVOutDescriptor;
+#ifndef __METAL_VERSION__
+static_assert(std::is_trivial_v<UVOutDescriptor>, "UVOutDescriptor must be a trivial type!");
+#endif
 
 typedef struct
 {
@@ -180,13 +194,19 @@ typedef struct
     half4 calcFog(float4 camPosition) constant;
 #endif
 } VertexUniforms;
+#ifndef __METAL_VERSION__
+static_assert(std::is_trivial_v<VertexUniforms>, "VertexUniforms must be a trivial type!");
+#endif
 
 #define kMetalMaxLightCount 32
 
 typedef struct {
     uint8_t count;
     plMetalShaderLightSource lampSources[kMetalMaxLightCount];
 } plMetalLights;
+#ifndef __METAL_VERSION__
+static_assert(std::is_trivial_v<plMetalLights>, "plMetalLights must be a trivial type!");
+#endif
 
 typedef struct {
     simd::float3 lightPosition;
@@ -195,6 +215,9 @@ typedef struct {
     float power;
     half opacity;
 } plShadowState;
+#ifndef __METAL_VERSION__
+static_assert(std::is_trivial_v<plShadowState>, "plShadowState must be a trivial type!");
+#endif
 
 #endif /* ShaderTypes_h */
 

Sources/Plasma/FeatureLib/pfMetalPipeline/plMetalDevice.cpp

@@ -1169,7 +1169,7 @@ void plMetalDevice::SubmitCommandBuffer()
     fClearDrawableDepth = 1.0;
 }
 
-MTL::SamplerState* plMetalDevice::SampleStateForClampFlags(hsGMatState::hsGMatClampFlags sampleState)
+MTL::SamplerState* plMetalDevice::SampleStateForClampFlags(hsGMatState::hsGMatClampFlags sampleState) const
 {
     return fSamplerStates[sampleState];
 }
@@ -1256,7 +1256,7 @@ MTL::RenderCommandEncoder* plMetalDevice::CurrentRenderCommandEncoder()
     return fCurrentRenderTargetCommandEncoder;
 }
 
-CA::MetalDrawable* plMetalDevice::GetCurrentDrawable()
+CA::MetalDrawable* plMetalDevice::GetCurrentDrawable() const
 {
     return fCurrentDrawable;
 }

Sources/Plasma/FeatureLib/pfMetalPipeline/plMetalDevice.h

@@ -162,15 +162,15 @@ class plMetalDevice
     // Currently requires a CA drawable and not a Metal drawable. In since CA drawable is only abstract implementation I know about, not sure where we would find others?
     void                CreateNewCommandBuffer(CA::MetalDrawable* drawable);
     MTL::CommandBuffer* GetCurrentCommandBuffer();
-    CA::MetalDrawable*  GetCurrentDrawable();
+    CA::MetalDrawable*  GetCurrentDrawable() const;
     /// Submit the command buffer to the GPU and draws all the render passes. Clears the current command buffer.
     void                SubmitCommandBuffer();
     void                Clear(bool shouldClearColor, simd_float4 clearColor, bool shouldClearDepth, float clearDepth);
 
     void SetMaxAnsiotropy(uint8_t maxAnsiotropy);
     void SetMSAASampleCount(uint8_t sampleCount);
 
-    MTL::SamplerState* SampleStateForClampFlags(hsGMatState::hsGMatClampFlags sampleState);
+    MTL::SamplerState* SampleStateForClampFlags(hsGMatState::hsGMatClampFlags sampleState) const;
 
     NS::UInteger CurrentTargetSampleCount()
     {

Sources/Plasma/FeatureLib/pfMetalPipeline/plMetalDeviceRef.h

@@ -61,8 +61,8 @@ class plMetalDeviceRef : public hsGDeviceRef
 public:
     void              Unlink();
     void              Link(plMetalDeviceRef** back);
-    plMetalDeviceRef* GetNext() { return fNext; }
-    bool              IsLinked() { return fBack != nullptr; }
+    plMetalDeviceRef* GetNext() const { return fNext; }
+    bool              IsLinked() { return fBack != nullptr; } const
 
     bool HasFlag(uint32_t f) const { return 0 != (fFlags & f); }
     void SetFlag(uint32_t f, bool on)
@@ -212,7 +212,7 @@ class plMetalVertexBufferRef : public plMetalBufferPoolRef
     virtual ~plMetalVertexBufferRef();
 
     void                    Link(plMetalVertexBufferRef** back) { plMetalDeviceRef::Link((plMetalDeviceRef**)back); }
-    plMetalVertexBufferRef* GetNext() const { return (plMetalVertexBufferRef*)fNext; }
+    plMetalVertexBufferRef* const GetNext() const { return (plMetalVertexBufferRef*)fNext; }
 
     void Release() override;
 };
@@ -244,7 +244,7 @@ class plMetalIndexBufferRef : public plMetalBufferPoolRef
     void Release() override;
 
     void                   Link(plMetalIndexBufferRef** back) { plMetalDeviceRef::Link((plMetalDeviceRef**)back); }
-    plMetalIndexBufferRef* GetNext() { return (plMetalIndexBufferRef*)fNext; }
+    plMetalIndexBufferRef* const GetNext() { return (plMetalIndexBufferRef*)fNext; }
     virtual ~plMetalIndexBufferRef();
 
     plMetalIndexBufferRef() : plMetalBufferPoolRef(),
@@ -287,7 +287,7 @@ class plMetalRenderTargetRef : public plMetalTextureRef
     MTL::Texture* fDepthBuffer;
 
     void                    Link(plMetalRenderTargetRef** back) { plMetalDeviceRef::Link((plMetalDeviceRef**)back); }
-    plMetalRenderTargetRef* GetNext() { return (plMetalRenderTargetRef*)fNext; }
+    plMetalRenderTargetRef* GetNext() const { return (plMetalRenderTargetRef*)fNext; }
 
     plMetalRenderTargetRef() : fDepthBuffer(nullptr)
     {

Sources/Plasma/FeatureLib/pfMetalPipeline/plMetalMaterialShaderRef.h

@@ -57,22 +57,22 @@ class plLayerInterface;
 class plMetalMaterialShaderRef : public plMetalDeviceRef
 {
 protected:
-    plMetalPipeline *fPipeline;
-    hsGMaterial     *fMaterial;
+    plMetalPipeline*    fPipeline;
+    hsGMaterial*        fMaterial;
     // temporary holder for the fragment shader to use, we don't own this reference
-    MTL::Function   *fFragFunction;
+    MTL::Function*      fFragFunction;
 
 private:
     std::vector<uint32_t>      fPassIndices;
     // FIXME: This should be retained/released
-    MTL::Device               *fDevice;
-    std::vector<MTL::Buffer *> fPassArgumentBuffers;
+    MTL::Device*               fDevice;
+    std::vector<MTL::Buffer*>  fPassArgumentBuffers;
 
 public:
     void                      Link(plMetalMaterialShaderRef **back) { plMetalDeviceRef::Link((plMetalDeviceRef **)back); }
-    plMetalMaterialShaderRef* GetNext() const { return (plMetalMaterialShaderRef *)fNext; }
+    plMetalMaterialShaderRef* GetNext() const { return (plMetalMaterialShaderRef*)fNext; }
 
-    plMetalMaterialShaderRef(hsGMaterial *mat, plMetalPipeline *pipe);
+    plMetalMaterialShaderRef(hsGMaterial* mat, plMetalPipeline* pipe);
     ~plMetalMaterialShaderRef();
 
     void Release() override;
@@ -116,7 +116,7 @@ class plMetalMaterialShaderRef : public plMetalDeviceRef
                              std::vector<plLayerInterface*>* piggybacks,
                              const std::function<plLayerInterface* (plLayerInterface*, uint32_t)>& preEncodeTransform,
                              const std::function<plLayerInterface* (plLayerInterface*, uint32_t)>& postEncodeTransform);
-    bool     ICanEatLayer(plLayerInterface *lay);
+    bool     ICanEatLayer(plLayerInterface* lay);
     uint32_t ILayersAtOnce(uint32_t which);
 
     void                                                 IBuildLayerTexture(MTL::RenderCommandEncoder* encoder, uint32_t offsetFromRootLayer, plLayerInterface* layer);

Sources/Plasma/FeatureLib/pfMetalPipeline/plMetalPipeline.cpp

@@ -367,7 +367,7 @@ void plMetalPipeline::ClearRenderTarget(const hsColorRGBA* col, const float* dep
     if (fView.fRenderState & (kRenderClearColor | kRenderClearDepth)) {
         hsColorRGBA clearColor = col ? *col : GetClearColor();
         float       clearDepth = depth ? *depth : fView.GetClearDepth();
-        fDevice.Clear(fView.fRenderState & kRenderClearColor, {clearColor.r, clearColor.g, clearColor.b, clearColor.a}, fView.fRenderState & kRenderClearDepth, 1.0);
+        fDevice.Clear(fView.fRenderState & kRenderClearColor, { clearColor.r, clearColor.g, clearColor.b, clearColor.a }, fView.fRenderState & kRenderClearDepth, 1.0);
         fState.Reset();
     }
 }
@@ -1361,7 +1361,7 @@ void plMetalPipeline::IRenderProjectionEach(const plRenderPrimFunc& render, hsGM
     // For each projector:
     int k;
     for (k = 0; k < fProjEach.size(); k++) {
-        // Push it's projected texture as a piggyback.
+        // Push its projected texture as a piggyback.
         plLightInfo*              li = fProjEach[k];
         plMetalMaterialShaderRef* mRef = static_cast<plMetalMaterialShaderRef*>(material->GetDeviceRef());
 
@@ -1384,7 +1384,7 @@ void plMetalPipeline::IRenderProjectionEach(const plRenderPrimFunc& render, hsGM
 
         RemoveLayerInterface(&layLightBase, false);
 
-        // Pop it's projected texture off piggyback
+        // Pop its projected texture off piggyback
         IPopProjPiggyBacks();
     }
 }
@@ -1539,11 +1539,11 @@ bool plMetalPipeline::IHandleMaterialPass(hsGMaterial* material, uint32_t pass,
         /*
          The programmable pipeline doesn't do things like set the texture transform matrices,
          In practice, the transforms aren't set and used. Does it matter that the Metal
-         implementation doesn't implemention the full inputs the DX version gets?
+         implementation doesn't implement the full inputs the DX version gets?
 
          If it is implemented, the same checks the DX version does should be also implemented.
          DX will set texture transforms, but then turn them off in the pipeline and manually
-         manipulate texture co-ords in the shader.
+         manipulate texture coords in the shader.
 
          Texture setting should also _maybe_ be reconciled with the "fixed" pipeline. But
          the fixed pipeline uses indirect textures mapped to a buffer. That approach could
@@ -1855,8 +1855,9 @@ void plMetalPipeline::ISetPipeConsts(plShader* shader)
             case plPipeConst::kPointLight3:
             case plPipeConst::kPointLight4:
             case plPipeConst::kColorFilter:
-            case plPipeConst::kMaxType:
-                break;
+            case plPipeConst::kMaxType: {
+                hsAssert(0, "Unimplemented uniform passed to shader");
+            } break;
         }
     }
 }
@@ -2532,7 +2533,7 @@ void plMetalPipeline::PopCurrentLightSources()
 plLayerInterface* plMetalPipeline::IPushOverBaseLayer(plLayerInterface* li)
 {
     if (!li)
-        return nil;
+        return nullptr;
 
     fOverLayerStack.emplace_back(li);
 
@@ -2551,7 +2552,7 @@ plLayerInterface* plMetalPipeline::IPushOverBaseLayer(plLayerInterface* li)
 plLayerInterface* plMetalPipeline::IPopOverBaseLayer(plLayerInterface* li)
 {
     if (!li)
-        return nil;
+        return nullptr;
 
     fForceMatHandle = true;
 
@@ -2569,7 +2570,7 @@ plLayerInterface* plMetalPipeline::IPopOverBaseLayer(plLayerInterface* li)
 plLayerInterface* plMetalPipeline::IPushOverAllLayer(plLayerInterface* li)
 {
     if (!li)
-        return nil;
+        return nullptr;
 
     fOverLayerStack.push_back(li);
 
@@ -2592,7 +2593,7 @@ plLayerInterface* plMetalPipeline::IPushOverAllLayer(plLayerInterface* li)
 plLayerInterface* plMetalPipeline::IPopOverAllLayer(plLayerInterface* li)
 {
     if (!li)
-        return nil;
+        return nullptr;
 
     fForceMatHandle = true;
 
@@ -2638,8 +2639,7 @@ void plMetalPipeline::IPushPiggyBacks(hsGMaterial* mat)
     if (fView.fRenderState & plPipeline::kRenderNoPiggyBacks)
         return;
 
-    int i;
-    for (i = 0; i < mat->GetNumPiggyBacks(); i++) {
+    for (int i = 0; i < mat->GetNumPiggyBacks(); i++) {
         if (!mat->GetPiggyBack(i))
             continue;
 
@@ -2916,7 +2916,7 @@ bool plMetalPipeline::ICreateDynDeviceObjects()
 
     // Create device-specific stuff
     fDebugTextMgr = new plDebugTextManager();
-    if (fDebugTextMgr == nil)
+    if (fDebugTextMgr == nullptr)
         return true;
 
     // Vertex buffers, index buffers, textures, etc.
@@ -2934,7 +2934,7 @@ void plMetalPipeline::IReleaseDynDeviceObjects()
     // We should do this earlier, but the textFont objects don't remove
     // themselves from their parent objects yet
     delete fDebugTextMgr;
-    fDebugTextMgr = nil;
+    fDebugTextMgr = nullptr;
 
     while (fTextFontRefList)
         delete fTextFontRefList;
@@ -3397,10 +3397,10 @@ plRenderTarget* plMetalPipeline::IFindRenderTarget(uint32_t& width, uint32_t& he
 // about that.
 hsGDeviceRef* plMetalPipeline::SharedRenderTargetRef(plRenderTarget* share, plRenderTarget* owner)
 {
-    plMetalRenderTargetRef* ref = nil;
-    MTL::Texture*           depthSurface = nil;
-    MTL::Texture*           texture = nil;
-    MTL::Texture*           cTexture = nil;
+    plMetalRenderTargetRef* ref = nullptr;
+    MTL::Texture*           depthSurface = nullptr;
+    MTL::Texture*           texture = nullptr;
+    MTL::Texture*           cTexture = nullptr;
     int                     i;
     plCubicRenderTarget*    cubicRT;
     uint16_t                width, height;
@@ -3435,7 +3435,7 @@ hsGDeviceRef* plMetalPipeline::SharedRenderTargetRef(plRenderTarget* share, plRe
     // if( !IFindRenderTargetInfo(owner, surfFormat, resType) )
     //{
     //    hsAssert( false, "Error getting renderTarget info" );
-    //    return nil;
+    //    return nullptr;
     //}
 
     /// Create the render target now
@@ -3623,7 +3623,7 @@ void plMetalPipeline::IMakeRenderTargetPools()
                 if (!SharedRenderTargetRef((*pool)[0], rt)) {
                     delete rt;
                     pool->resize(j + 1);
-                    (*pool)[j] = nil;
+                    (*pool)[j] = nullptr;
                     break;
                 }
                 (*pool)[j] = rt;
@@ -3964,7 +3964,7 @@ bool plMetalPipeline::IIsViewLeftHanded()
 }
 
 //// ISetCullMode /////////////////////////////////////////////////////////////
-//  Tests and sets the current winding order cull mode (CW, CCW, or none).
+// Tests and sets the current winding order cull mode (CW, CCW, or none).
 // Will reverse the cull mode as necessary for left handed camera or local to world
 // transforms.
 void plMetalPipeline::ISetCullMode(bool flip)
@@ -3976,7 +3976,7 @@ void plMetalPipeline::ISetCullMode(bool flip)
     }
 }
 
-plMetalDevice* plMetalPipeline::GetMetalDevice()
+plMetalDevice* plMetalPipeline::GetMetalDevice()  const
 {
     return &fDevice;
 }

Sources/Plasma/FeatureLib/pfMetalPipeline/plMetalPipeline.h

@@ -151,7 +151,7 @@ class plMetalPipeline : public pl3DPipeline<plMetalDevice>
     void           IRenderAuxSpan(const plSpan& span, const plAuxSpan* aux);
     void           IRenderAuxSpans(const plSpan& span);
     bool           IHandleMaterialPass(hsGMaterial* material, uint32_t pass, const plSpan* currSpan, const plMetalVertexBufferRef* vRef, const bool allowShaders = true);
-    plMetalDevice* GetMetalDevice();
+    plMetalDevice* GetMetalDevice() const;
 
     // Create and/or Refresh geometry buffers
     void          CheckVertexBufferRef(plGBufferGroup* owner, uint32_t idx) override;

Sources/Plasma/FeatureLib/pfMetalPipeline/plMetalPipelineState.cpp

@@ -269,7 +269,7 @@ MTL::Function* plMetalMaterialPassPipelineState::GetVertexFunction(MTL::Library*
 {
     NS::Error*                   error = nullptr;
     MTL::FunctionConstantValues* constants = MTL::FunctionConstantValues::alloc()->init()->autorelease();
-    this->GetFunctionConstants(constants);
+    GetFunctionConstants(constants);
     MTL::Function* function = library->newFunction(
                                          NS::String::string("pipelineVertexShader", NS::ASCIIStringEncoding),
                                          MakeFunctionConstants(),