Refine addPipeline signature to be more explicit

libraries/render-utils/src/RenderDeferredTask.cpp

@@ -74,12 +74,7 @@ void ToneMappingDeferred::run(const SceneContextPointer& sceneContext, const Ren
 
 RenderDeferredTask::RenderDeferredTask() : Task() {
     // Prepare the ShapePipelines
-    ShapePlumberPointer shapePlumber = std::make_shared<ShapePlumber>([](gpu::Batch& batch, ShapePipelinePointer pipeline) {
+    ShapePlumberPointer shapePlumber;
-        if ((pipeline->locations->normalFittingMapUnit > -1)) {
-            batch.setResourceTexture(pipeline->locations->normalFittingMapUnit,
-                                     DependencyManager::get<TextureCache>()->getNormalFittingTexture());
-        }
-    });
     initDeferredPipelines(*shapePlumber);
     
     // CPU only, create the list of renderedOpaques items
@@ -513,8 +508,45 @@ int RenderDeferredTask::getToneMappingToneCurve() const {
     }
 }
 
+void pipelineBatchSetter(const ShapePipeline& pipeline, gpu::Batch& batch) {
+    if (pipeline.locations->normalFittingMapUnit > -1) {
+        batch.setResourceTexture(pipeline.locations->normalFittingMapUnit,
+            DependencyManager::get<TextureCache>()->getNormalFittingTexture());
+    }
+}
+
 void initDeferredPipelines(render::ShapePlumber& plumber) {
     using Key = render::ShapeKey;
+    using ShaderPointer = gpu::ShaderPointer;
+
+    auto addPipeline = [&plumber](const Key& key, const ShaderPointer& vertexShader, const ShaderPointer& pixelShader) {
+        auto state = std::make_shared<gpu::State>();
+
+        // Cull backface
+        state->setCullMode(gpu::State::CULL_BACK);
+
+        // Z test depends on transparency
+        state->setDepthTest(true, !key.isTranslucent(), gpu::LESS_EQUAL);
+
+        // Blend if transparent
+        state->setBlendFunction(key.isTranslucent(),
+            // For transparency, keep the highlight intensity
+            gpu::State::ONE, gpu::State::BLEND_OP_ADD, gpu::State::INV_SRC_ALPHA,
+            gpu::State::FACTOR_ALPHA, gpu::State::BLEND_OP_ADD, gpu::State::ONE);
+
+        ShaderPointer program = gpu::Shader::createProgram(vertexShader, pixelShader);
+        plumber.addPipeline(key, program, state, &pipelineBatchSetter);
+
+        // Add a wireframe version
+        if (!key.isWireFrame()) {
+            auto wireFrameKey = Key::Builder(key).withWireframe();
+            auto wireFrameState = std::make_shared<gpu::State>(state->getValues());
+
+            wireFrameState->setFillMode(gpu::State::FILL_LINE);
+
+            plumber.addPipeline(wireFrameKey, program, wireFrameState, &pipelineBatchSetter);
+        }
+    };
 
     // Vertex shaders
     auto modelVertex = gpu::Shader::createVertex(std::string(model_vert));
@@ -539,102 +571,101 @@ void initDeferredPipelines(render::ShapePlumber& plumber) {
     auto modelLightmapNormalSpecularMapPixel = gpu::Shader::createPixel(std::string(model_lightmap_normal_specular_map_frag));
 
     // Fill the pipelineLib
-
+    addPipeline(
-    plumber.addPipeline(
         Key::Builder(),
         modelVertex, modelPixel);
 
-    plumber.addPipeline(
+    addPipeline(
         Key::Builder().withTangents(),
         modelNormalMapVertex, modelNormalMapPixel);
 
-    plumber.addPipeline(
+    addPipeline(
         Key::Builder().withSpecular(),
         modelVertex, modelSpecularMapPixel);
 
-    plumber.addPipeline(
+    addPipeline(
         Key::Builder().withTangents().withSpecular(),
         modelNormalMapVertex, modelNormalSpecularMapPixel);
 
 
-    plumber.addPipeline(
+    addPipeline(
         Key::Builder().withTranslucent(),
         modelVertex, modelTranslucentPixel);
     // FIXME Ignore lightmap for translucents meshpart
-    plumber.addPipeline(
+    addPipeline(
         Key::Builder().withTranslucent().withLightmap(),
         modelVertex, modelTranslucentPixel);
 
-    plumber.addPipeline(
+    addPipeline(
         Key::Builder().withTangents().withTranslucent(),
         modelNormalMapVertex, modelTranslucentPixel);
 
-    plumber.addPipeline(
+    addPipeline(
         Key::Builder().withSpecular().withTranslucent(),
         modelVertex, modelTranslucentPixel);
 
-    plumber.addPipeline(
+    addPipeline(
         Key::Builder().withTangents().withSpecular().withTranslucent(),
         modelNormalMapVertex, modelTranslucentPixel);
 
 
-    plumber.addPipeline(
+    addPipeline(
         Key::Builder().withLightmap(),
         modelLightmapVertex, modelLightmapPixel);
 
-    plumber.addPipeline(
+    addPipeline(
         Key::Builder().withLightmap().withTangents(),
         modelLightmapNormalMapVertex, modelLightmapNormalMapPixel);
 
-    plumber.addPipeline(
+    addPipeline(
         Key::Builder().withLightmap().withSpecular(),
         modelLightmapVertex, modelLightmapSpecularMapPixel);
 
-    plumber.addPipeline(
+    addPipeline(
         Key::Builder().withLightmap().withTangents().withSpecular(),
         modelLightmapNormalMapVertex, modelLightmapNormalSpecularMapPixel);
 
 
-    plumber.addPipeline(
+    addPipeline(
         Key::Builder().withSkinned(),
         skinModelVertex, modelPixel);
 
-    plumber.addPipeline(
+    addPipeline(
         Key::Builder().withSkinned().withTangents(),
         skinModelNormalMapVertex, modelNormalMapPixel);
 
-    plumber.addPipeline(
+    addPipeline(
         Key::Builder().withSkinned().withSpecular(),
         skinModelVertex, modelSpecularMapPixel);
 
-    plumber.addPipeline(
+    addPipeline(
         Key::Builder().withSkinned().withTangents().withSpecular(),
         skinModelNormalMapVertex, modelNormalSpecularMapPixel);
 
 
-    plumber.addPipeline(
+    addPipeline(
         Key::Builder().withSkinned().withTranslucent(),
         skinModelVertex, modelTranslucentPixel);
 
-    plumber.addPipeline(
+    addPipeline(
         Key::Builder().withSkinned().withTangents().withTranslucent(),
         skinModelNormalMapVertex, modelTranslucentPixel);
 
-    plumber.addPipeline(
+    addPipeline(
         Key::Builder().withSkinned().withSpecular().withTranslucent(),
         skinModelVertex, modelTranslucentPixel);
 
-    plumber.addPipeline(
+    addPipeline(
         Key::Builder().withSkinned().withTangents().withSpecular().withTranslucent(),
         skinModelNormalMapVertex, modelTranslucentPixel);
 
 
-    plumber.addPipeline(
+    addPipeline(
         Key::Builder().withDepthOnly(),
         modelShadowVertex, modelShadowPixel);
 
 
-    plumber.addPipeline(
+    addPipeline(
         Key::Builder().withSkinned().withDepthOnly(),
         skinModelShadowVertex, modelShadowPixel);
 }

libraries/render/src/render/ShapePipeline.cpp

@@ -22,54 +22,29 @@ ShapeKey::Filter::Builder::Builder() {
     _mask.set(INVALID);
 }
 
-void defaultStateSetter(ShapeKey key, gpu::State& state) {
+void ShapePlumber::addPipelineHelper(const Filter& filter, ShapeKey key, int bit, const PipelinePointer& pipeline) {
-    // Cull backface
+    // Iterate over all keys
-    state.setCullMode(gpu::State::CULL_BACK);
-
-    // Z test depends on transparency
-    state.setDepthTest(true, !key.isTranslucent(), gpu::LESS_EQUAL);
-
-    // Blend if transparent
-    state.setBlendFunction(key.isTranslucent(),
-        // For transparency, keep the highlight intensity
-        gpu::State::ONE, gpu::State::BLEND_OP_ADD, gpu::State::INV_SRC_ALPHA,
-        gpu::State::FACTOR_ALPHA, gpu::State::BLEND_OP_ADD, gpu::State::ONE);
-
-    // Add a wireframe version
-    if (!key.isWireFrame()) {
-        state.setFillMode(gpu::State::FILL_LINE);
-    }
-}
-
-void defaultBatchSetter(gpu::Batch& batch, ShapePipelinePointer pipeline) {
-}
-
-ShapePlumber::ShapePlumber() : _stateSetter{ defaultStateSetter }, _batchSetter{ defaultBatchSetter } {}
-ShapePlumber::ShapePlumber(BatchSetter batchSetter) : _stateSetter{ defaultStateSetter }, _batchSetter{ batchSetter } {}
-
-void ShapePlumber::addPipelineHelper(const Filter& filter, ShapeKey key, int bit, gpu::ShaderPointer program, LocationsPointer locations) {
-    // Iterate over all keys, toggling only bits set as significant in filter._mask
     if (bit < (int)ShapeKey::FlagBit::NUM_FLAGS) {
-        addPipelineHelper(filter, key, ++bit, program, locations);
+        ++bit;
+        addPipelineHelper(filter, key, bit, pipeline);
         if (filter._mask[bit]) {
+            // Toggle bits set as significant in filter._mask 
             key._flags.flip(bit);
-            addPipelineHelper(filter, key, ++bit, program, locations);
+            addPipelineHelper(filter, key, bit, pipeline);
         }
     } else {
-        auto state = std::make_shared<gpu::State>();
-        _stateSetter(key, *state);
-    
         // Add the brand new pipeline and cache its location in the lib
-        auto pipeline = gpu::Pipeline::create(program, state);
+        _pipelineMap.insert(PipelineMap::value_type(key, pipeline));
-        _pipelineMap.insert(PipelineMap::value_type(key, std::make_shared<Pipeline>(pipeline, locations)));
     }
 }
 
-void ShapePlumber::addPipeline(const Key& key, gpu::ShaderPointer& vertexShader, gpu::ShaderPointer& pixelShader) {
+void ShapePlumber::addPipeline(const Key& key, const gpu::ShaderPointer& program, const gpu::StatePointer& state,
-    addPipeline(Filter{key}, vertexShader, pixelShader);
+        BatchSetter batchSetter) {
+    addPipeline(Filter{key}, program, state, batchSetter);
 }
 
-void ShapePlumber::addPipeline(const Filter& filter, gpu::ShaderPointer& vertexShader, gpu::ShaderPointer& pixelShader) {
+void ShapePlumber::addPipeline(const Filter& filter, const gpu::ShaderPointer& program, const gpu::StatePointer& state,
+        BatchSetter batchSetter) {
     gpu::Shader::BindingSet slotBindings;
     slotBindings.insert(gpu::Shader::Binding(std::string("skinClusterBuffer"), Slot::SKINNING_GPU));
     slotBindings.insert(gpu::Shader::Binding(std::string("materialBuffer"), Slot::MATERIAL_GPU));
@@ -80,7 +55,6 @@ void ShapePlumber::addPipeline(const Filter& filter, gpu::ShaderPointer& vertexS
     slotBindings.insert(gpu::Shader::Binding(std::string("lightBuffer"), Slot::LIGHT_BUFFER));
     slotBindings.insert(gpu::Shader::Binding(std::string("normalFittingMap"), Slot::NORMAL_FITTING_MAP));
 
-    gpu::ShaderPointer program = gpu::Shader::createProgram(vertexShader, pixelShader);
     gpu::Shader::makeProgram(*program, slotBindings);
 
     auto locations = std::make_shared<Locations>();
@@ -96,7 +70,9 @@ void ShapePlumber::addPipeline(const Filter& filter, gpu::ShaderPointer& vertexS
     locations->lightBufferUnit = program->getBuffers().findLocation("lightBuffer");
 
     ShapeKey key{filter._flags};
-    addPipelineHelper(filter, key, 0, program, locations);
+    auto gpuPipeline = gpu::Pipeline::create(program, state);
+    auto shapePipeline = std::make_shared<Pipeline>(gpuPipeline, locations, batchSetter);
+    addPipelineHelper(filter, key, 0, shapePipeline);
 }
 
 const ShapePipelinePointer ShapePlumber::pickPipeline(RenderArgs* args, const Key& key) const {
@@ -114,7 +90,9 @@ const ShapePipelinePointer ShapePlumber::pickPipeline(RenderArgs* args, const Ke
 
     PipelinePointer shapePipeline(pipelineIterator->second);
     auto& batch = args->_batch;
-    _batchSetter(*batch, shapePipeline);
+
+    // Run the pipeline's BatchSetter on the passed in batch
+    shapePipeline->batchSetter(*shapePipeline, *batch);
 
     // Setup the one pipeline (to rule them all)
     batch->setPipeline(shapePipeline->pipeline);

libraries/render/src/render/ShapePipeline.h

@@ -204,12 +204,18 @@ public:
     };
     using LocationsPointer = std::shared_ptr<Locations>;
 
+    using BatchSetter = std::function<void(const ShapePipeline&, gpu::Batch&)>;
+
+    ShapePipeline(gpu::PipelinePointer pipeline, LocationsPointer locations, BatchSetter batchSetter) :
+        pipeline(pipeline), locations(locations), batchSetter(batchSetter) {}
+
     gpu::PipelinePointer pipeline;
     std::shared_ptr<Locations> locations;
 
-    ShapePipeline() : ShapePipeline(nullptr, nullptr) {}
+protected:
-    ShapePipeline(gpu::PipelinePointer pipeline, std::shared_ptr<Locations> locations) :
+    friend class ShapePlumber;
-        pipeline(pipeline), locations(locations) {}
+
+    BatchSetter batchSetter;
 };
 using ShapePipelinePointer = std::shared_ptr<ShapePipeline>;
 
@@ -223,21 +229,18 @@ public:
     using Slot = Pipeline::Slot;
     using Locations = Pipeline::Locations;
     using LocationsPointer = Pipeline::LocationsPointer;
-    using StateSetter = std::function<void(Key, gpu::State&)>;
+    using BatchSetter = Pipeline::BatchSetter;
-    using BatchSetter = std::function<void(gpu::Batch&, PipelinePointer)>;
 
-    ShapePlumber();
+    void addPipeline(const Key& key, const gpu::ShaderPointer& program, const gpu::StatePointer& state,
-    explicit ShapePlumber(BatchSetter batchSetter);
+        BatchSetter batchSetter = nullptr);
+    void addPipeline(const Filter& filter, const gpu::ShaderPointer& program, const gpu::StatePointer& state,
+        BatchSetter batchSetter = nullptr);
 
-    void addPipeline(const Key& key, gpu::ShaderPointer& vertexShader, gpu::ShaderPointer& pixelShader);
-    void addPipeline(const Filter& filter, gpu::ShaderPointer& vertexShader, gpu::ShaderPointer& pixelShader);
     const PipelinePointer pickPipeline(RenderArgs* args, const Key& key) const;
 
 protected:
-    void addPipelineHelper(const Filter& filter, Key key, int bit, gpu::ShaderPointer pipeline, LocationsPointer locations);
+    void addPipelineHelper(const Filter& filter, Key key, int bit, const PipelinePointer& pipeline);
     PipelineMap _pipelineMap;
-    StateSetter _stateSetter;
-    BatchSetter _batchSetter;
 };
 using ShapePlumberPointer = std::shared_ptr<ShapePlumber>;