From e4a8e87098e6626687a5eef37a7e661bbebbf376 Mon Sep 17 00:00:00 2001 From: Jonas Strandstedt Date: Wed, 1 Oct 2014 15:30:39 +0200 Subject: [PATCH] Using new ghoul ProgramObject functionality - Fixed risk of memory leak in SceneGraph (deleting shaders) --- ext/ghoul | 2 +- include/openspace/engine/openspaceengine.h | 3 - include/openspace/scenegraph/scenegraph.h | 6 + include/openspace/util/shadercreator.h | 60 ------- shaders/ABuffer/abufferAddToBuffer.hglsl | 2 + src/abuffer/abuffer.cpp | 13 +- src/engine/openspaceengine.cpp | 18 -- src/rendering/renderablefieldlines.cpp | 3 +- src/scenegraph/scenegraph.cpp | 117 ++++++++----- src/util/shadercreator.cpp | 195 --------------------- 10 files changed, 89 insertions(+), 330 deletions(-) delete mode 100644 include/openspace/util/shadercreator.h delete mode 100644 src/util/shadercreator.cpp diff --git a/ext/ghoul b/ext/ghoul index 5a8a150468..44fec6ce6f 160000 --- a/ext/ghoul +++ b/ext/ghoul @@ -1 +1 @@ -Subproject commit 5a8a150468b2e0c8b31f7e3238c1c5abbd667799 +Subproject commit 44fec6ce6f9f214269c764f4a950dab8ccc127a4 diff --git a/include/openspace/engine/openspaceengine.h b/include/openspace/engine/openspaceengine.h index 50e3ee55a2..4973bb4606 100644 --- a/include/openspace/engine/openspaceengine.h +++ b/include/openspace/engine/openspaceengine.h @@ -45,7 +45,6 @@ // #define FLARE_ONLY #include -#include #define ABUFFER_SINGLE_LINKED 1 #define ABUFFER_FIXED 2 @@ -76,7 +75,6 @@ public: InteractionHandler& interactionHandler(); RenderEngine& renderEngine(); scripting::ScriptEngine& scriptEngine(); - ShaderCreator& shaderBuilder(); // SGCT callbacks bool initializeGL(); @@ -116,7 +114,6 @@ private: ghoul::opencl::CLContext _context; sgct::SharedVector _synchronizationBuffer; - ShaderCreator _shaderBuilder; }; #define OsEng (openspace::OpenSpaceEngine::ref()) diff --git a/include/openspace/scenegraph/scenegraph.h b/include/openspace/scenegraph/scenegraph.h index 07544bbd99..869f375e5c 100644 --- a/include/openspace/scenegraph/scenegraph.h +++ b/include/openspace/scenegraph/scenegraph.h @@ -28,6 +28,8 @@ // std includes #include #include +#include +#include #include #include @@ -118,6 +120,10 @@ private: std::map _allNodes; std::string _sceneGraphToLoad; + + std::mutex _programUpdateLock; + std::set _programsToUpdate; + std::vector _programs; }; } // namespace openspace diff --git a/include/openspace/util/shadercreator.h b/include/openspace/util/shadercreator.h deleted file mode 100644 index 61ddecf338..0000000000 --- a/include/openspace/util/shadercreator.h +++ /dev/null @@ -1,60 +0,0 @@ -/***************************************************************************************** - * * - * OpenSpace * - * * - * Copyright (c) 2014 * - * * - * Permission is hereby granted, free of charge, to any person obtaining a copy of this * - * software and associated documentation files (the "Software"), to deal in the Software * - * without restriction, including without limitation the rights to use, copy, modify, * - * merge, publish, distribute, sublicense, and/or sell copies of the Software, and to * - * permit persons to whom the Software is furnished to do so, subject to the following * - * conditions: * - * * - * The above copyright notice and this permission notice shall be included in all copies * - * or substantial portions of the Software. * - * * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, * - * INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A * - * PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT * - * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF * - * CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE * - * OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. * - ****************************************************************************************/ - -#ifndef __SHADERCREATOR_H__ -#define __SHADERCREATOR_H__ - - -#include - -#include - -namespace openspace { -class ShaderCreator { - -public: - ShaderCreator(); - ~ShaderCreator(); - - void createSourceFile(bool b); - void sourceFileExtension(const std::string& extension); - void sourceFileHeader(const std::string& header); - - ghoul::opengl::ProgramObject* buildShader(const std::string& name, const std::string& vpath, const std::string& fpath, const std::string& gpath = ""); - -private: - - void _generateSource(const std::string& filename); - std::string _loadSource(const std::string& filename, unsigned int depth = 0); - std::string _generateFilename(const std::string& filename); - - bool _createSourceFile; - std::string _sourceFileExtension; - std::string _sourceFileHeader; - unsigned int _maxDepth; - -}; -} - -#endif \ No newline at end of file diff --git a/shaders/ABuffer/abufferAddToBuffer.hglsl b/shaders/ABuffer/abufferAddToBuffer.hglsl index 92248796c0..b819de22e7 100644 --- a/shaders/ABuffer/abufferAddToBuffer.hglsl +++ b/shaders/ABuffer/abufferAddToBuffer.hglsl @@ -22,6 +22,8 @@ * OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. * ****************************************************************************************/ +#include <${SHADERS}/ABuffer/constants.hglsl> + #if ABUFFER_IMPLEMENTATION == ABUFFER_SINGLE_LINKED layout (binding = 0, r32ui) uniform uimage2D anchorPointerTexture; layout (binding = 1, rgba32ui) uniform uimageBuffer fragmentTexture; diff --git a/src/abuffer/abuffer.cpp b/src/abuffer/abuffer.cpp index 925de91b82..97d06b4a95 100644 --- a/src/abuffer/abuffer.cpp +++ b/src/abuffer/abuffer.cpp @@ -89,8 +89,9 @@ bool ABuffer::initializeABuffer() { addFunc("${SHADERS}/ABuffer/abufferSort.hglsl"); addFunc("${SHADERS}/ABuffer/abufferAddToBuffer.hglsl"); addFunc("${SHADERS}/ABuffer/abufferStruct.hglsl"); - addFunc("${SHADERS}/PowerScaling/powerScaling_fs.hglsl"); - addFunc("${SHADERS}/PowerScaling/powerScaling_vs.hglsl"); + addFunc("${SHADERS}/PowerScaling/powerScaling_fs.hglsl"); + addFunc("${SHADERS}/PowerScaling/powerScaling_vs.hglsl"); + addFunc("${SHADERS}/PowerScaling/powerScalingMath.hglsl"); // ============================ // GEOMETRY (quad) @@ -182,9 +183,11 @@ bool ABuffer::updateShader() { using ghoul::opengl::ShaderObject; using ghoul::opengl::ProgramObject; - ShaderCreator sc = OsEng.shaderBuilder(); - ghoul::opengl::ProgramObject* resolveShader = sc.buildShader("ABuffer resolve", absPath("${SHADERS}/ABuffer/abufferResolveVertex.glsl"), _fragmentShaderPath); - if( ! resolveShader) { + ProgramObject* resolveShader = ProgramObject::Build("ABuffer resolve", + "${SHADERS}/ABuffer/abufferResolveVertex.glsl", + _fragmentShaderPath); + + if( ! resolveShader) { LERROR("Resolve shader not updated"); return false; } diff --git a/src/engine/openspaceengine.cpp b/src/engine/openspaceengine.cpp index 6dca374ecc..36e0d1c53a 100644 --- a/src/engine/openspaceengine.cpp +++ b/src/engine/openspaceengine.cpp @@ -221,17 +221,6 @@ bool OpenSpaceEngine::initialize() //int samples = sqrt(sgct::Engine::instance()->getActiveWindowPtr()->getNumberOfAASamples()); //LDEBUG("samples: " << samples); - int x1, xSize, y1, ySize; - sgct::Engine::instance()->getActiveWindowPtr()->getCurrentViewportPixelCoords(x1, y1, xSize, ySize); - std::string sourceHeader = ""; - sourceHeader += "#define SCREEN_WIDTH " + std::to_string(xSize) + "\n"; - sourceHeader += "#define SCREEN_HEIGHT " + std::to_string(ySize) + "\n"; - sourceHeader += "#define ABUFFER_SINGLE_LINKED " + std::to_string(ABUFFER_SINGLE_LINKED) + "\n"; - sourceHeader += "#define ABUFFER_FIXED " + std::to_string(ABUFFER_FIXED) + "\n"; - sourceHeader += "#define ABUFFER_DYNAMIC " + std::to_string(ABUFFER_DYNAMIC) + "\n"; - sourceHeader += "#define ABUFFER_IMPLEMENTATION " + std::to_string(ABUFFER_IMPLEMENTATION) + "\n"; - _shaderBuilder.createSourceFile(true); - _shaderBuilder.sourceFileHeader(sourceHeader); // Register the filepaths from static function enables easy testing // registerFilePaths(); @@ -357,13 +346,6 @@ ScriptEngine& OpenSpaceEngine::scriptEngine() return _scriptEngine; } - -ShaderCreator& OpenSpaceEngine::shaderBuilder() -{ - // TODO custom assert (ticket #5) - return _shaderBuilder; -} - bool OpenSpaceEngine::initializeGL() { return _renderEngine.initializeGL(); diff --git a/src/rendering/renderablefieldlines.cpp b/src/rendering/renderablefieldlines.cpp index 2c950aa753..7301266bf1 100644 --- a/src/rendering/renderablefieldlines.cpp +++ b/src/rendering/renderablefieldlines.cpp @@ -114,8 +114,7 @@ RenderableFieldlines::RenderableFieldlines(const ghoul::Dictionary& dictionary) _fragmentSourceFile = new ghoul::filesystem::File(fshaderpath, false); - ShaderCreator sc = OsEng.shaderBuilder(); - _fieldlinesProgram = sc.buildShader("FieldlinesProgram", vshaderpath, fshaderpath); + _fieldlinesProgram = ghoul::opengl::ProgramObject::Build("FieldlinesProgram", vshaderpath, fshaderpath); dictionary.getValue("UpdateOnSave", _programUpdateOnSave); diff --git a/src/scenegraph/scenegraph.cpp b/src/scenegraph/scenegraph.cpp index 779c35486f..bc2913ebc7 100644 --- a/src/scenegraph/scenegraph.cpp +++ b/src/scenegraph/scenegraph.cpp @@ -29,7 +29,6 @@ #include #include #include -#include #include #include @@ -47,6 +46,7 @@ #include #include +#include #include #include @@ -149,87 +149,105 @@ SceneGraph::~SceneGraph() bool SceneGraph::initialize() { LDEBUG("Initializing SceneGraph"); - - LDEBUG("Creating ProgramObjects"); + using ghoul::opengl::ShaderObject; using ghoul::opengl::ProgramObject; - ShaderCreator sc = OsEng.shaderBuilder(); ProgramObject* tmpProgram; - typedef std::chrono::high_resolution_clock clock_; - typedef std::chrono::duration > second_; + int x1, xSize, y1, ySize; + sgct::Engine::instance()-> + getActiveWindowPtr()-> + getCurrentViewportPixelCoords(x1, y1, xSize, ySize); - std::chrono::time_point beginning(clock_::now()); + // TODO: Make this file creation dynamic and better in every way + // TODO: If the screen size changes it is enough if this file is regenerated to + // recompile all necessary files + std::ofstream os(absPath("${SHADERS}/ABuffer/constants.hglsl")); + os << "#define SCREEN_WIDTH " << xSize << "\n" + << "#define SCREEN_HEIGHT " << ySize << "\n" + << "#define ABUFFER_SINGLE_LINKED 1\n" + << "#define ABUFFER_FIXED 2\n" + << "#define ABUFFER_DYNAMIC 3\n" + << "#define ABUFFER_IMPLEMENTATION 1\n"; + os.close(); - // pscstandard - tmpProgram = sc.buildShader("pscstandard", - "${SHADERS}/pscstandard_vs.glsl", - "${SHADERS}/pscstandard_fs.glsl"); + // TODO: Figure out why the callback is called twice + ghoul::opengl::ProgramObject::ProgramObjectCallback cb = [this](ghoul::opengl::ProgramObject* program) { + _programUpdateLock.lock(); + _programsToUpdate.insert(program); + _programUpdateLock.unlock(); + }; + + // Start Timing for building SceneGraph shaders + typedef std::chrono::high_resolution_clock clock_; + typedef std::chrono::duration > second_; + std::chrono::time_point beginning(clock_::now()); + + // pscstandard + tmpProgram = ProgramObject::Build("pscstandard", + "${SHADERS}/pscstandard_vs.glsl", + "${SHADERS}/pscstandard_fs.glsl", + cb); if( ! tmpProgram) return false; + _programs.push_back(tmpProgram); OsEng.ref().configurationManager().setValue("pscShader", tmpProgram); // RaycastProgram - tmpProgram = sc.buildShader("RaycastProgram", - "${SHADERS}/exitpoints.vert", - "${SHADERS}/exitpoints.frag"); - if( ! tmpProgram) return false; + tmpProgram = ProgramObject::Build("RaycastProgram", + "${SHADERS}/exitpoints.vert", + "${SHADERS}/exitpoints.frag", + cb); + if (!tmpProgram) return false; + _programs.push_back(tmpProgram); OsEng.ref().configurationManager().setValue("RaycastProgram", tmpProgram); - - // // TwoPassProgram - // tmpProgram = sc.buildShader("TwoPassProgram", - // "${SHADERS}/twopassraycaster.vert", - // "${SHADERS}/twopassraycaster.frag"); - // if( ! tmpProgram) return false; - // tmpProgram->setUniform("texBack", 0); - // tmpProgram->setUniform("texFront", 1); - // tmpProgram->setUniform("texVolume", 2); - // OsEng.ref().configurationManager().setValue("TwoPassProgram", tmpProgram); - - // Quad - tmpProgram = sc.buildShader("Quad", - "${SHADERS}/quadVert.glsl", - "${SHADERS}/quadFrag.glsl"); - if (!tmpProgram) return false; - tmpProgram->setUniform("quadTex", 0); - OsEng.ref().configurationManager().setValue("Quad", tmpProgram); - // Star program - tmpProgram = sc.buildShader("Star", + tmpProgram = ProgramObject::Build("Star", "${SHADERS}/star_vs.glsl", "${SHADERS}/star_fs.glsl", - "${SHADERS}/star_ge.glsl"); + "${SHADERS}/star_ge.glsl", + cb); if (!tmpProgram) return false; + _programs.push_back(tmpProgram); OsEng.ref().configurationManager().setValue("StarProgram", tmpProgram); // Point program - tmpProgram = sc.buildShader("Point", + tmpProgram = ProgramObject::Build("Point", "${SHADERS}/star_vs.glsl", "${SHADERS}/star_fs.glsl", - "${SHADERS}/star_ge.glsl"); + "${SHADERS}/star_ge.glsl", + cb); if (!tmpProgram) return false; + _programs.push_back(tmpProgram); OsEng.ref().configurationManager().setValue("PointProgram", tmpProgram); // Grid program - tmpProgram = sc.buildShader("Grid", + tmpProgram = ProgramObject::Build("Grid", "${SHADERS}/grid_vs.glsl", - "${SHADERS}/grid_fs.glsl"); + "${SHADERS}/grid_fs.glsl", + cb); if (!tmpProgram) return false; + _programs.push_back(tmpProgram); OsEng.ref().configurationManager().setValue("GridProgram", tmpProgram); - - + // Done building shaders double elapsed = std::chrono::duration_cast(clock_::now()-beginning).count(); LINFO("Time to load shaders: " << elapsed); - return true; } bool SceneGraph::deinitialize() { clearSceneGraph(); + + // clean up all programs + _programsToUpdate.clear(); + for (auto program : _programs) { + delete program; + } + _programs.clear(); return true; } @@ -257,6 +275,13 @@ void SceneGraph::evaluate(Camera* camera) void SceneGraph::render(const RenderData& data) { + _programUpdateLock.lock(); + for (auto program : _programsToUpdate) { + LDEBUG("Attempting to recompile " << program->name()); + program->rebuildFromFile(); + } + _programsToUpdate.erase(_programsToUpdate.begin(), _programsToUpdate.end()); + _programUpdateLock.unlock(); if (_root) _root->render(data); } @@ -474,7 +499,8 @@ SceneGraphNode* SceneGraph::sceneGraphNode(const std::string& name) const { } scripting::ScriptEngine::LuaLibrary SceneGraph::luaLibrary() { - scripting::ScriptEngine::LuaLibrary sceneGraphLibrary = { + //scripting::ScriptEngine::LuaLibrary sceneGraphLibrary = { + return { "", { { @@ -498,8 +524,7 @@ scripting::ScriptEngine::LuaLibrary SceneGraph::luaLibrary() { } } }; - - return std::move(sceneGraphLibrary); + //return std::move(sceneGraphLibrary); } } // namespace openspace diff --git a/src/util/shadercreator.cpp b/src/util/shadercreator.cpp deleted file mode 100644 index aaec11b539..0000000000 --- a/src/util/shadercreator.cpp +++ /dev/null @@ -1,195 +0,0 @@ -/***************************************************************************************** - * * - * OpenSpace * - * * - * Copyright (c) 2014 * - * * - * Permission is hereby granted, free of charge, to any person obtaining a copy of this * - * software and associated documentation files (the "Software"), to deal in the Software * - * without restriction, including without limitation the rights to use, copy, modify, * - * merge, publish, distribute, sublicense, and/or sell copies of the Software, and to * - * permit persons to whom the Software is furnished to do so, subject to the following * - * conditions: * - * * - * The above copyright notice and this permission notice shall be included in all copies * - * or substantial portions of the Software. * - * * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, * - * INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A * - * PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT * - * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF * - * CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE * - * OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. * - ****************************************************************************************/ - -#include - -#include -#include -#include - -#include -#include -#include -// #include -// #include - -using ghoul::opengl::ProgramObject; -using ghoul::opengl::ShaderObject; - -namespace { - const std::string _loggerCat = "ShaderCreator"; - const std::string defaultSourceFileExtension = "OpenSpaceGenerated.glsl"; - const std::string separator = "//=====================================================================\n"; - const ShaderObject::ShaderType vsType = ShaderObject::ShaderType::ShaderTypeVertex; - const ShaderObject::ShaderType fsType = ShaderObject::ShaderType::ShaderTypeFragment; - const ShaderObject::ShaderType gsType = ShaderObject::ShaderType::ShaderTypeGeometry; -} - -namespace openspace { -ShaderCreator::ShaderCreator(): - _createSourceFile(false), - _sourceFileExtension(defaultSourceFileExtension), - _sourceFileHeader(""), - _maxDepth(2) -{ - -} - -ShaderCreator::~ShaderCreator() { - -} - -void ShaderCreator::createSourceFile(bool b) { - _createSourceFile = b; -} - -void ShaderCreator::sourceFileExtension(const std::string& extension) { - if(extension != "") { - _sourceFileExtension = extension; - } -} - -void ShaderCreator::sourceFileHeader(const std::string& header) { - _sourceFileHeader = header; -} - -ghoul::opengl::ProgramObject* ShaderCreator::buildShader( - const std::string& name, const std::string& vpath, const std::string& fpath, const std::string& gpath) -{ - std::string vsPath = absPath(vpath); - std::string fsPath = absPath(fpath); - std::string gsPath = absPath(gpath); - - if( ! FileSys.fileExists(vsPath)) - return nullptr; - if( ! FileSys.fileExists(fsPath)) - return nullptr; - if (gsPath != "" && !FileSys.fileExists(gsPath)) - return nullptr; - - if(_createSourceFile) { - _generateSource(vsPath); - _generateSource(fsPath); - - vsPath = _generateFilename(vsPath); - fsPath = _generateFilename(fsPath); - } - - ProgramObject* po = new ProgramObject(name); - ShaderObject* vs = new ShaderObject(vsType, vsPath, name + " Vertex"); - ShaderObject* fs = new ShaderObject(fsType, fsPath, name + " Fragment"); - po->attachObject(vs); - po->attachObject(fs); - - if (gsPath != "") { - _generateSource(gsPath); - gsPath = _generateFilename(gsPath); - ShaderObject* gs = new ShaderObject(gsType, gsPath, name + " Geometry"); - po->attachObject(gs); - } - - if ( po->compileShaderObjects() && po->linkProgramObject()) - return po; - - // unsuccessful compilation, cleanup and return nullptr - delete po; - po = nullptr; - return po; -} - -void ShaderCreator::_generateSource(const std::string& filename) { - std::string generatedSource = ""; - if(_sourceFileHeader != "") - generatedSource += separator + "// HEADER\n" + separator + _sourceFileHeader + "\n"; - generatedSource += _loadSource(filename); - - std::ofstream of(_generateFilename(filename)); - of << generatedSource; - of.close(); -} - -std::string ShaderCreator::_loadSource(const std::string& filename, unsigned int depth) { - std::string contents = "", line; - std::ifstream f(filename); - - // Pre-allocate memory so the return string doesn't have to resize too often - // f.seekg( 0, std::ios::end ); - // unsigned int fsize = f.tellg(); - // f.seekg( 0); - // contents.reserve(fsize*3); - // line.reserve(fsize*3); - - contents += "\n"; - contents += separator; - contents += "// Filename: " + filename + "\n"; - // contents += "// Size: " + std::to_string(fsize) + "\n"; - contents += separator; - if(depth > _maxDepth) { - contents += "// TOO DEEP"; - return contents; - } - - if( ! FileSys.fileExists(filename)){ - contents += "// FILE NOT FOUND\n"; - return contents; - } - - std::regex e1(R"(^\s*#include \"(.+)\"\s*)"); - std::regex e2(R"(^\s*#include <(.+)>\s*)"); - while(std::getline(f, line)) { - std::smatch m; - if(std::regex_search(line, m, e1)) { - using namespace ghoul::filesystem; - std::string includeFilename = m[1]; - File f(filename); - includeFilename = f.directoryName() + FileSystem::PathSeparator + includeFilename; - //includeFilename = filename.substr(0, filename.find_last_of("/")+1) + includeFilename; - line = _loadSource(includeFilename, depth + 1); - } else if(std::regex_search(line, m, e2)) { - std::string includeFilename = m[1]; - line = _loadSource(absPath(includeFilename), depth + 1); - } - - contents += line + "\n"; - } - f.close(); - - return contents; -} - -std::string ShaderCreator::_generateFilename(const std::string& filename) { - // If way of confirming and creating a directory this could be a good solution - // to avoid cluttering in folders - // if(_cacheFolder != "") { - // size_t delimiter = filename.find_last_of("/"); - // std::string file = filename.substr(delimiter+1, filename.length() - delimiter); - // std::string path = filename.substr(0, filename.find_last_of("/")); - - // return path + "/" + _cacheFolder + "/" + file + "." + _sourceFileExtension; - // } - - return filename + "." + _sourceFileExtension; -} - -} \ No newline at end of file