From 9bc661adc8e98f13d818ae0e22c43a78b39acdd8 Mon Sep 17 00:00:00 2001 From: Zach Pomerantz Date: Wed, 6 Jan 2016 11:42:58 -0800 Subject: [PATCH] Cleanup for style/dead code --- .../render-utils/src/MeshPartPayload.cpp | 38 +++++++++---------- libraries/render-utils/src/MeshPartPayload.h | 6 +-- .../render-utils/src/RenderDeferredTask.h | 18 ++++++--- libraries/render-utils/src/ShapeRender.cpp | 3 +- libraries/render-utils/src/ShapeRender.h | 5 ++- libraries/render/src/render/DrawTask.cpp | 4 +- libraries/render/src/render/DrawTask.h | 8 +--- libraries/render/src/render/Scene.h | 5 ++- libraries/render/src/render/Shape.h | 36 ++++++++++-------- 9 files changed, 67 insertions(+), 56 deletions(-) diff --git a/libraries/render-utils/src/MeshPartPayload.cpp b/libraries/render-utils/src/MeshPartPayload.cpp index 53feaa5a44..7b1f6887f1 100644 --- a/libraries/render-utils/src/MeshPartPayload.cpp +++ b/libraries/render-utils/src/MeshPartPayload.cpp @@ -129,14 +129,14 @@ void MeshPartPayload::bindMesh(gpu::Batch& batch) const { } } -void MeshPartPayload::bindMaterial(gpu::Batch& batch, const ShapePipeline::LocationsPointer locations) const { +void MeshPartPayload::bindMaterial(gpu::Batch& batch, const Shape::Pipeline::LocationsPointer locations) const { if (!_drawMaterial) { return; } auto textureCache = DependencyManager::get(); - batch.setUniformBuffer(ShapePipeline::Slot::MATERIAL_GPU, _drawMaterial->getSchemaBuffer()); + batch.setUniformBuffer(Shape::Pipeline::Slot::MATERIAL_GPU, _drawMaterial->getSchemaBuffer()); auto materialKey = _drawMaterial->getKey(); auto textureMaps = _drawMaterial->getTextureMaps(); @@ -146,44 +146,44 @@ void MeshPartPayload::bindMaterial(gpu::Batch& batch, const ShapePipeline::Locat if (materialKey.isDiffuseMap()) { auto diffuseMap = textureMaps[model::MaterialKey::DIFFUSE_MAP]; if (diffuseMap && diffuseMap->isDefined()) { - batch.setResourceTexture(ShapePipeline::Slot::DIFFUSE_MAP, diffuseMap->getTextureView()); + batch.setResourceTexture(Shape::Pipeline::Slot::DIFFUSE_MAP, diffuseMap->getTextureView()); if (!diffuseMap->getTextureTransform().isIdentity()) { diffuseMap->getTextureTransform().getMatrix(texcoordTransform[0]); } } else { - batch.setResourceTexture(ShapePipeline::Slot::DIFFUSE_MAP, textureCache->getGrayTexture()); + batch.setResourceTexture(Shape::Pipeline::Slot::DIFFUSE_MAP, textureCache->getGrayTexture()); } } else { - batch.setResourceTexture(ShapePipeline::Slot::DIFFUSE_MAP, textureCache->getWhiteTexture()); + batch.setResourceTexture(Shape::Pipeline::Slot::DIFFUSE_MAP, textureCache->getWhiteTexture()); } // Normal map if (materialKey.isNormalMap()) { auto normalMap = textureMaps[model::MaterialKey::NORMAL_MAP]; if (normalMap && normalMap->isDefined()) { - batch.setResourceTexture(ShapePipeline::Slot::NORMAL_MAP, normalMap->getTextureView()); + batch.setResourceTexture(Shape::Pipeline::Slot::NORMAL_MAP, normalMap->getTextureView()); // texcoord are assumed to be the same has diffuse } else { - batch.setResourceTexture(ShapePipeline::Slot::NORMAL_MAP, textureCache->getBlueTexture()); + batch.setResourceTexture(Shape::Pipeline::Slot::NORMAL_MAP, textureCache->getBlueTexture()); } } else { - batch.setResourceTexture(ShapePipeline::Slot::NORMAL_MAP, nullptr); + batch.setResourceTexture(Shape::Pipeline::Slot::NORMAL_MAP, nullptr); } // TODO: For now gloss map is used as the "specular map in the shading, we ll need to fix that if (materialKey.isGlossMap()) { auto specularMap = textureMaps[model::MaterialKey::GLOSS_MAP]; if (specularMap && specularMap->isDefined()) { - batch.setResourceTexture(ShapePipeline::Slot::SPECULAR_MAP, specularMap->getTextureView()); + batch.setResourceTexture(Shape::Pipeline::Slot::SPECULAR_MAP, specularMap->getTextureView()); // texcoord are assumed to be the same has diffuse } else { - batch.setResourceTexture(ShapePipeline::Slot::SPECULAR_MAP, textureCache->getBlackTexture()); + batch.setResourceTexture(Shape::Pipeline::Slot::SPECULAR_MAP, textureCache->getBlackTexture()); } } else { - batch.setResourceTexture(ShapePipeline::Slot::SPECULAR_MAP, nullptr); + batch.setResourceTexture(Shape::Pipeline::Slot::SPECULAR_MAP, nullptr); } // TODO: For now lightmaop is piped into the emissive map unit, we need to fix that and support for real emissive too @@ -191,7 +191,7 @@ void MeshPartPayload::bindMaterial(gpu::Batch& batch, const ShapePipeline::Locat auto lightmapMap = textureMaps[model::MaterialKey::LIGHTMAP_MAP]; if (lightmapMap && lightmapMap->isDefined()) { - batch.setResourceTexture(ShapePipeline::Slot::LIGHTMAP_MAP, lightmapMap->getTextureView()); + batch.setResourceTexture(Shape::Pipeline::Slot::LIGHTMAP_MAP, lightmapMap->getTextureView()); auto lightmapOffsetScale = lightmapMap->getLightmapOffsetScale(); batch._glUniform2f(locations->emissiveParams, lightmapOffsetScale.x, lightmapOffsetScale.y); @@ -200,10 +200,10 @@ void MeshPartPayload::bindMaterial(gpu::Batch& batch, const ShapePipeline::Locat lightmapMap->getTextureTransform().getMatrix(texcoordTransform[1]); } } else { - batch.setResourceTexture(ShapePipeline::Slot::LIGHTMAP_MAP, textureCache->getGrayTexture()); + batch.setResourceTexture(Shape::Pipeline::Slot::LIGHTMAP_MAP, textureCache->getGrayTexture()); } } else { - batch.setResourceTexture(ShapePipeline::Slot::LIGHTMAP_MAP, nullptr); + batch.setResourceTexture(Shape::Pipeline::Slot::LIGHTMAP_MAP, nullptr); } // Texcoord transforms ? @@ -212,7 +212,7 @@ void MeshPartPayload::bindMaterial(gpu::Batch& batch, const ShapePipeline::Locat } } -void MeshPartPayload::bindTransform(gpu::Batch& batch, const ShapePipeline::LocationsPointer locations) const { +void MeshPartPayload::bindTransform(gpu::Batch& batch, const Shape::Pipeline::LocationsPointer locations) const { batch.setModelTransform(_drawTransform); } @@ -377,7 +377,7 @@ ShapeKey ModelMeshPartPayload::getShapeKey() const { int vertexCount = mesh.vertices.size(); if (vertexCount == 0) { // sanity check - return ShapeKey::Builder::invalid(); + return ShapeKey::Builder::invalid(); // FIXME } @@ -431,16 +431,16 @@ void ModelMeshPartPayload::bindMesh(gpu::Batch& batch) const { } } -void ModelMeshPartPayload::bindTransform(gpu::Batch& batch, const ShapePipeline::LocationsPointer locations) const { +void ModelMeshPartPayload::bindTransform(gpu::Batch& batch, const Shape::Pipeline::LocationsPointer locations) const { // Still relying on the raw data from the model const Model::MeshState& state = _model->_meshStates.at(_meshIndex); Transform transform; if (state.clusterBuffer) { if (_model->_cauterizeBones) { - batch.setUniformBuffer(ShapePipeline::Slot::SKINNING_GPU, state.cauterizedClusterBuffer); + batch.setUniformBuffer(Shape::Pipeline::Slot::SKINNING_GPU, state.cauterizedClusterBuffer); } else { - batch.setUniformBuffer(ShapePipeline::Slot::SKINNING_GPU, state.clusterBuffer); + batch.setUniformBuffer(Shape::Pipeline::Slot::SKINNING_GPU, state.clusterBuffer); } } else { if (_model->_cauterizeBones) { diff --git a/libraries/render-utils/src/MeshPartPayload.h b/libraries/render-utils/src/MeshPartPayload.h index 7d693dcbe9..1e6e6214ae 100644 --- a/libraries/render-utils/src/MeshPartPayload.h +++ b/libraries/render-utils/src/MeshPartPayload.h @@ -46,8 +46,8 @@ public: // ModelMeshPartPayload functions to perform render void drawCall(gpu::Batch& batch) const; virtual void bindMesh(gpu::Batch& batch) const; - virtual void bindMaterial(gpu::Batch& batch, const render::ShapePipeline::LocationsPointer locations) const; - virtual void bindTransform(gpu::Batch& batch, const render::ShapePipeline::LocationsPointer locations) const; + virtual void bindMaterial(gpu::Batch& batch, const render::Shape::Pipeline::LocationsPointer locations) const; + virtual void bindTransform(gpu::Batch& batch, const render::Shape::Pipeline::LocationsPointer locations) const; // Payload resource cached values model::MeshPointer _drawMesh; @@ -89,7 +89,7 @@ public: // ModelMeshPartPayload functions to perform render void bindMesh(gpu::Batch& batch) const override; - void bindTransform(gpu::Batch& batch, const render::ShapePipeline::LocationsPointer locations) const override; + void bindTransform(gpu::Batch& batch, const render::Shape::Pipeline::LocationsPointer locations) const override; void initCache(); diff --git a/libraries/render-utils/src/RenderDeferredTask.h b/libraries/render-utils/src/RenderDeferredTask.h index 051f19c133..01e9ab2a16 100755 --- a/libraries/render-utils/src/RenderDeferredTask.h +++ b/libraries/render-utils/src/RenderDeferredTask.h @@ -51,29 +51,35 @@ public: }; class DrawOpaqueDeferred { - ShapeRender _renderer; public: void run(const render::SceneContextPointer& sceneContext, const render::RenderContextPointer& renderContext, const render::ItemIDsBounds& inItems); typedef render::Job::ModelI JobModel; + +protected: + ShapeRender _renderer; }; class DrawTransparentDeferred { - ShapeRender _renderer; public: void run(const render::SceneContextPointer& sceneContext, const render::RenderContextPointer& renderContext, const render::ItemIDsBounds& inItems); typedef render::Job::ModelI JobModel; + +protected: + ShapeRender _renderer; }; class DrawStencilDeferred { - static gpu::PipelinePointer _opaquePipeline; //lazy evaluation hence mutable public: static const gpu::PipelinePointer& getOpaquePipeline(); void run(const render::SceneContextPointer& sceneContext, const render::RenderContextPointer& renderContext); typedef render::Job::Model JobModel; + +protected: + static gpu::PipelinePointer _opaquePipeline; //lazy evaluation hence mutable }; class DrawBackgroundDeferred { @@ -84,14 +90,16 @@ public: }; class DrawOverlay3D { - static gpu::PipelinePointer _opaquePipeline; //lazy evaluation hence mutable - ShapeRender _renderer; public: static const gpu::PipelinePointer& getOpaquePipeline(); void run(const render::SceneContextPointer& sceneContext, const render::RenderContextPointer& renderContext); typedef render::Job::Model JobModel; + +protected: + static gpu::PipelinePointer _opaquePipeline; //lazy evaluation hence mutable + ShapeRender _renderer; }; class Blit { diff --git a/libraries/render-utils/src/ShapeRender.cpp b/libraries/render-utils/src/ShapeRender.cpp index c68664b69a..c4ac9f3c2b 100644 --- a/libraries/render-utils/src/ShapeRender.cpp +++ b/libraries/render-utils/src/ShapeRender.cpp @@ -38,7 +38,8 @@ #include "model_translucent_frag.h" ShapeRender::ShapeRender() { - // TODO: There is probably a cleaner way to init the pipeline that in a derived class + // TODO: Move pipeline initialization to those Jobs using ShapeRender + // such that they own their own pipelines and it is done only once if (_pipelineLib.empty()) { initPipeline(); } diff --git a/libraries/render-utils/src/ShapeRender.h b/libraries/render-utils/src/ShapeRender.h index a3f131f21d..fc174896bb 100644 --- a/libraries/render-utils/src/ShapeRender.h +++ b/libraries/render-utils/src/ShapeRender.h @@ -16,14 +16,15 @@ #include class ShapeRender : public render::Shape { - static model::MaterialPointer _collisionHullMaterial; - public: ShapeRender(); static void initPipeline(); const PipelinePointer pickPipeline(RenderArgs* args, const Key& key) const override; static model::MaterialPointer getCollisionHullMaterial(); + +protected: + static model::MaterialPointer _collisionHullMaterial; }; #endif // hifi_render_utils_Shape_h diff --git a/libraries/render/src/render/DrawTask.cpp b/libraries/render/src/render/DrawTask.cpp index f66b6461b9..83caaa72b7 100755 --- a/libraries/render/src/render/DrawTask.cpp +++ b/libraries/render/src/render/DrawTask.cpp @@ -223,12 +223,12 @@ void render::renderLights(const SceneContextPointer& sceneContext, const RenderC void renderShape(RenderArgs* args, const Shape& shapeContext, Item& item) { assert(item.getKey().isShape()); const auto& key = item.getShapeKey(); - if (key.isValid() && key.hasPipeline()) { + if (key.isValid() && !key.hasOwnPipeline()) { args->_pipeline = shapeContext.pickPipeline(args, key); if (args->_pipeline) { item.render(args); } - } else if (!key.hasPipeline()) { + } else if (key.hasOwnPipeline()) { item.render(args); } else { qDebug() << "Item could not be rendered: invalid key ?" << key; diff --git a/libraries/render/src/render/DrawTask.h b/libraries/render/src/render/DrawTask.h index d3741a07a7..eef4247b17 100755 --- a/libraries/render/src/render/DrawTask.h +++ b/libraries/render/src/render/DrawTask.h @@ -103,12 +103,6 @@ public: } public: - class Context { - public: - virtual const ShapePipeline pickPipeline(RenderArgs* args, const ShapeKey& key) = 0; - }; - using ContextPointer = std::shared_ptr; - class Concept { std::string _name; bool _isEnabled = true; @@ -274,7 +268,7 @@ public: typedef Job::ModelIO JobModel; }; -class DrawLight : public Shape { +class DrawLight { public: void run(const SceneContextPointer& sceneContext, const RenderContextPointer& renderContext); diff --git a/libraries/render/src/render/Scene.h b/libraries/render/src/render/Scene.h index 90625dfd14..bff75ebcd8 100644 --- a/libraries/render/src/render/Scene.h +++ b/libraries/render/src/render/Scene.h @@ -350,7 +350,10 @@ template int payloadGetLayer(const std::shared_ptr& payloadData) { template void payloadRender(const std::shared_ptr& payloadData, RenderArgs* args) { } // Shape type interface -template const ShapeKey shapeGetShapeKey(const std::shared_ptr& payloadData) { return ShapeKey::Builder::noPipeline(); } +// This allows shapes to characterize their pipeline via a ShapeKey, to be picked with a subclass of Shape. +// When creating a new shape payload you need to create a specialized version, or the ShapeKey will be ownPipeline, +// implying that the shape will setup its own pipeline without the use of the ShapeKey. +template const ShapeKey shapeGetShapeKey(const std::shared_ptr& payloadData) { return ShapeKey::Builder::ownPipeline(); } template class Payload : public Item::PayloadInterface { public: diff --git a/libraries/render/src/render/Shape.h b/libraries/render/src/render/Shape.h index 8fc93d76af..4950af1da4 100644 --- a/libraries/render/src/render/Shape.h +++ b/libraries/render/src/render/Shape.h @@ -31,7 +31,7 @@ public: SHADOW, WIREFRAME, - NO_PIPELINE, + OWN_PIPELINE, INVALID, NUM_FLAGS, // Not a valid flag @@ -62,10 +62,10 @@ public: Builder& withDepthOnly() { _flags.set(DEPTH_ONLY); return (*this); } Builder& withShadow() { _flags.set(SHADOW); return (*this); } Builder& withWireframe() { _flags.set(WIREFRAME); return (*this); } - Builder& withoutPipeline() { _flags.set(NO_PIPELINE); return (*this); } + Builder& withOwnPipeline() { _flags.set(OWN_PIPELINE); return (*this); } Builder& invalidate() { _flags.set(INVALID); return (*this); } - static const ShapeKey noPipeline() { return Builder().withoutPipeline(); } + static const ShapeKey ownPipeline() { return Builder().withOwnPipeline(); } static const ShapeKey invalid() { return Builder().invalidate(); } }; ShapeKey(const Builder& builder) : ShapeKey(builder._flags) {} @@ -81,7 +81,7 @@ public: bool isShadow() const { return _flags[SHADOW]; } bool isWireFrame() const { return _flags[WIREFRAME]; } - bool hasPipeline() const { return !_flags[NO_PIPELINE]; } + bool hasOwnPipeline() const { return _flags[OWN_PIPELINE]; } bool isValid() const { return !_flags[INVALID]; } // Hasher for use in unordered_maps @@ -101,18 +101,22 @@ public: inline QDebug operator<<(QDebug debug, const ShapeKey& renderKey) { if (renderKey.isValid()) { - debug << "[ShapeKey:" - << "hasLightmap:" << renderKey.hasLightmap() - << "hasTangents:" << renderKey.hasTangents() - << "hasSpecular:" << renderKey.hasSpecular() - << "hasEmissive:" << renderKey.hasEmissive() - << "isTranslucent:" << renderKey.isTranslucent() - << "isSkinned:" << renderKey.isSkinned() - << "isStereo:" << renderKey.isStereo() - << "isDepthOnly:" << renderKey.isDepthOnly() - << "isShadow:" << renderKey.isShadow() - << "isWireFrame:" << renderKey.isWireFrame() - << "]"; + if (renderKey.hasOwnPipeline()) { + debug << "[ShapeKey: OWN_PIPELINE]"; + } else { + debug << "[ShapeKey:" + << "hasLightmap:" << renderKey.hasLightmap() + << "hasTangents:" << renderKey.hasTangents() + << "hasSpecular:" << renderKey.hasSpecular() + << "hasEmissive:" << renderKey.hasEmissive() + << "isTranslucent:" << renderKey.isTranslucent() + << "isSkinned:" << renderKey.isSkinned() + << "isStereo:" << renderKey.isStereo() + << "isDepthOnly:" << renderKey.isDepthOnly() + << "isShadow:" << renderKey.isShadow() + << "isWireFrame:" << renderKey.isWireFrame() + << "]"; + } } else { debug << "[ShapeKey: INVALID]"; }