From e7aeaf78d006137981193552460d19f188ccf8ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Fri, 14 Feb 2020 12:29:17 +0100 Subject: [PATCH] Remove ResourceManager::instance() singleton deprecated in 2019.10. Due to it being global it was severely limiting multithreaded applications and removing it was the path of least resistance. Good riddance. --- doc/changelog.dox | 4 + src/Magnum/CMakeLists.txt | 1 - src/Magnum/DebugTools/ForceRenderer.cpp | 6 -- src/Magnum/DebugTools/ObjectRenderer.cpp | 6 -- src/Magnum/DebugTools/ResourceManager.cpp | 9 +- src/Magnum/DebugTools/ResourceManager.h | 2 +- src/Magnum/ResourceManager.h | 82 ++----------------- src/Magnum/ResourceManager.hpp | 56 ------------- src/Magnum/Test/CMakeLists.txt | 14 ---- .../Test/ResourceManagerLocalInstanceTest.cpp | 55 ------------- .../ResourceManagerLocalInstanceTestLib.cpp | 45 ---------- .../ResourceManagerLocalInstanceTestLib.h | 50 ----------- 12 files changed, 15 insertions(+), 315 deletions(-) delete mode 100644 src/Magnum/ResourceManager.hpp delete mode 100644 src/Magnum/Test/ResourceManagerLocalInstanceTest.cpp delete mode 100644 src/Magnum/Test/ResourceManagerLocalInstanceTestLib.cpp delete mode 100644 src/Magnum/Test/ResourceManagerLocalInstanceTestLib.h diff --git a/doc/changelog.dox b/doc/changelog.dox index d356bf206..f33b56fa8 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -327,6 +327,10 @@ See also: @p segments parameter isn't divisible by four. Before it mistakenly asserted only if @p segments wasn't divisible by two, and now code that mistakenly used a disallowed value will start asserting. +- @ref ResourceManager singleton accessible through @cpp instance() @ce that + was deprecated in 2019.10 is now removed. Usually a deprecated feature is + kept for at least a year before removal, but in this case it was severely + limiting multithreaded applications and removing it was necessary. - Removed remaining APIs deprecated in version 2018.04: - @cpp Audio::Buffer::Format @ce, use @ref Audio::BufferFormat instead - @cpp Shaders::*Vector::setVectorTexture() @ce, diff --git a/src/Magnum/CMakeLists.txt b/src/Magnum/CMakeLists.txt index a0c9a3b0e..a127eba73 100644 --- a/src/Magnum/CMakeLists.txt +++ b/src/Magnum/CMakeLists.txt @@ -56,7 +56,6 @@ set(Magnum_HEADERS PixelStorage.h Resource.h ResourceManager.h - ResourceManager.hpp Sampler.h Tags.h Timeline.h diff --git a/src/Magnum/DebugTools/ForceRenderer.cpp b/src/Magnum/DebugTools/ForceRenderer.cpp index 89a838c66..9f6ca60bf 100644 --- a/src/Magnum/DebugTools/ForceRenderer.cpp +++ b/src/Magnum/DebugTools/ForceRenderer.cpp @@ -77,12 +77,6 @@ template ForceRenderer::ForceRenderer(Resour manager.set(_mesh.key(), std::move(mesh), ResourceDataState::Final, ResourcePolicy::Manual); } -#ifdef MAGNUM_BUILD_DEPRECATED -CORRADE_IGNORE_DEPRECATED_PUSH -template ForceRenderer::ForceRenderer(SceneGraph::AbstractObject& object, const VectorTypeFor& forcePosition, const VectorTypeFor& force, ResourceKey options, SceneGraph::DrawableGroup* drawables): ForceRenderer{static_cast(ResourceManager::instance()), object, forcePosition, force, options, drawables} {} -CORRADE_IGNORE_DEPRECATED_POP -#endif - /* To avoid deleting pointers to incomplete type on destruction of Resource members */ template ForceRenderer::~ForceRenderer() = default; diff --git a/src/Magnum/DebugTools/ObjectRenderer.cpp b/src/Magnum/DebugTools/ObjectRenderer.cpp index e027c2f56..018ff2055 100644 --- a/src/Magnum/DebugTools/ObjectRenderer.cpp +++ b/src/Magnum/DebugTools/ObjectRenderer.cpp @@ -65,12 +65,6 @@ template ObjectRenderer::ObjectRenderer(Reso if(!_mesh) manager.set(_mesh.key(), MeshTools::compile(Renderer::meshData())); } -#ifdef MAGNUM_BUILD_DEPRECATED -CORRADE_IGNORE_DEPRECATED_PUSH -template ObjectRenderer::ObjectRenderer(SceneGraph::AbstractObject& object, ResourceKey options, SceneGraph::DrawableGroup* drawables): ObjectRenderer{static_cast(ResourceManager::instance()), object, options, drawables} {} -CORRADE_IGNORE_DEPRECATED_POP -#endif - /* To avoid deleting pointers to incomplete type on destruction of Resource members */ template ObjectRenderer::~ObjectRenderer() = default; diff --git a/src/Magnum/DebugTools/ResourceManager.cpp b/src/Magnum/DebugTools/ResourceManager.cpp index df5d85afb..a092e5ef5 100644 --- a/src/Magnum/DebugTools/ResourceManager.cpp +++ b/src/Magnum/DebugTools/ResourceManager.cpp @@ -25,7 +25,6 @@ #include "ResourceManager.h" -#include "Magnum/ResourceManager.hpp" #include "Magnum/DebugTools/ForceRenderer.h" #include "Magnum/DebugTools/ObjectRenderer.h" #include "Magnum/GL/AbstractShaderProgram.h" @@ -33,13 +32,7 @@ #include "Magnum/GL/Mesh.h" #include "Magnum/GL/MeshView.h" -namespace Magnum { - -namespace Implementation { - template struct MAGNUM_DEBUGTOOLS_EXPORT ResourceManagerLocalInstanceImplementation; -} - -namespace DebugTools { +namespace Magnum { namespace DebugTools { ResourceManager::ResourceManager() { setFallback(new ForceRendererOptions); diff --git a/src/Magnum/DebugTools/ResourceManager.h b/src/Magnum/DebugTools/ResourceManager.h index 422c83fed..a8a7ba9fe 100644 --- a/src/Magnum/DebugTools/ResourceManager.h +++ b/src/Magnum/DebugTools/ResourceManager.h @@ -62,7 +62,7 @@ information. @ref MAGNUM_TARGET_GL "TARGET_GL" enabled (done by default). See @ref building-features for more information. */ -class MAGNUM_DEBUGTOOLS_EXPORT ResourceManager: public Magnum::ResourceManager +class MAGNUM_DEBUGTOOLS_EXPORT ResourceManager: public Magnum::ResourceManager { public: explicit ResourceManager(); diff --git a/src/Magnum/ResourceManager.h b/src/Magnum/ResourceManager.h index 58a85e790..e57247563 100644 --- a/src/Magnum/ResourceManager.h +++ b/src/Magnum/ResourceManager.h @@ -159,33 +159,12 @@ template class ResourceManagerData { /* Helper class for defining which real types are in the type pack */ template struct ResourceTypePack {}; -/* Common resource manager implementation with inline internal instance (for - use in user code), definition of internalInstance() is in this header */ -template struct ResourceManagerInlineInstanceImplementation { - static ResourceManager*& internalInstance(); -}; -template struct ResourceManagerImplementation: ResourceManagerInlineInstanceImplementation, ResourceManagerData... { - typedef ResourceTypePack TypePack; -}; - -/* Resource manager implementation with file-local internal instance (for use - in code where the manager is used across library boundaries), definition - of internalInstance() is in ResourceManager.hpp, see it for usage details */ -template struct ResourceManagerLocalInstanceImplementation { - static ResourceManager*& internalInstance(); -}; -struct ResourceManagerLocalInstance; -template struct ResourceManagerImplementation: ResourceManagerLocalInstanceImplementation, ResourceManagerData... { - typedef ResourceTypePack TypePack; -}; - } /** @brief Resource manager -Provides storage for arbitrary set of types, accessible globally using -@ref instance(). +Provides storage for arbitrary set of types. @section ResourceMananger-usage Usage @@ -244,35 +223,16 @@ Basic usage is: /* Due to too much work involved with explicit template instantiation (all Resource combinations, all ResourceManagerData...), this class doesn't have template implementation file. */ -template class ResourceManager: private Implementation::ResourceManagerImplementation... { +template class ResourceManager: private Implementation::ResourceManagerData... { public: - #ifdef MAGNUM_BUILD_DEPRECATED - /** - * @brief Global instance - * - * Assumes that the instance exists. - * - * @m_deprecated_since{2019,10} Implicit @ref ResourceManager singleton - * is deprecated, make your own or pass a reference around instead - */ - static CORRADE_DEPRECATED("implicit ResourceManager singleton is deprecated, make your own or pass a reference around instead") ResourceManager& instance(); - #endif - - /** - * @brief Constructor - * - * Sets global instance pointer to itself. - * @attention Only one instance of given ResourceManager type can be - * created. - * @see @ref instance() - */ + /** @brief Constructor */ explicit ResourceManager(); /** * @brief Destructor * - * Sets global instance pointer to @cpp nullptr @ce. - * @see @ref instance() + * Expects that all resources are not referenced anymore at the point + * of destruction. */ ~ResourceManager(); @@ -413,7 +373,7 @@ template class ResourceManager: private Implementation::Resource * @return Reference to self (for method chaining) */ ResourceManager& free() { - freeInternal(typename Implementation::ResourceManagerImplementation::TypePack{}); + freeInternal(Implementation::ResourceTypePack{}); return *this; } @@ -437,7 +397,7 @@ template class ResourceManager: private Implementation::Resource * referenced. */ ResourceManager& clear() { - clearInternal(typename Implementation::ResourceManagerImplementation::TypePack{}); + clearInternal(Implementation::ResourceTypePack{}); return *this; } @@ -493,11 +453,6 @@ template class ResourceManager: private Implementation::Resource namespace Implementation { -template ResourceManager*& ResourceManagerInlineInstanceImplementation::internalInstance() { - static ResourceManager* _instance(nullptr); - return _instance; -} - template void safeDelete(T* data) { static_assert(sizeof(T) > 0, "Cannot delete pointer to incomplete type"); delete data; @@ -636,29 +591,10 @@ template inline ResourceManagerData::Data::~Data() { } -#ifdef MAGNUM_BUILD_DEPRECATED -template ResourceManager& ResourceManager::instance() { - CORRADE_ASSERT(Implementation::ResourceManagerImplementation::internalInstance(), - "ResourceManager::instance(): no instance exists", - static_cast&>(*Implementation::ResourceManagerImplementation::internalInstance())); - return static_cast&>(*Implementation::ResourceManagerImplementation::internalInstance()); -} -#endif - -template ResourceManager::ResourceManager() { - #ifdef MAGNUM_BUILD_DEPRECATED - CORRADE_ASSERT(!Implementation::ResourceManagerImplementation::internalInstance(), - "ResourceManager::ResourceManager(): another instance is already created", ); - Implementation::ResourceManagerImplementation::internalInstance() = this; - #endif -} +template ResourceManager::ResourceManager() = default; template ResourceManager::~ResourceManager() { - freeLoaders(typename Implementation::ResourceManagerImplementation::TypePack{}); - #ifdef MAGNUM_BUILD_DEPRECATED - CORRADE_INTERNAL_ASSERT(Implementation::ResourceManagerImplementation::internalInstance() == this); - Implementation::ResourceManagerImplementation::internalInstance() = nullptr; - #endif + freeLoaders(typename Implementation::ResourceTypePack{}); } } diff --git a/src/Magnum/ResourceManager.hpp b/src/Magnum/ResourceManager.hpp deleted file mode 100644 index a283b8373..000000000 --- a/src/Magnum/ResourceManager.hpp +++ /dev/null @@ -1,56 +0,0 @@ -#ifndef Magnum_ResourceManager_hpp -#define Magnum_ResourceManager_hpp -/* - This file is part of Magnum. - - Copyright © 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019 - Vladimír Vondruš - - 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 "ResourceManager.h" - -/* - File-local definition of ResourceManager instance holder for use in cases - where the class is used across library boundaries, in which case additional - care must be done to ensure a single static instance. - - Usage: typedef the resource manager with Implementation::ResourceManagerLocalInstance - as a first type and then include this file in a _single_ *.cpp file. - - This symbol is always exported. -*/ - -namespace Magnum { namespace Implementation { - -#ifdef MAGNUM_BUILD_DEPRECATED -template -#ifndef _MSC_VER -CORRADE_VISIBILITY_EXPORT -#endif -ResourceManager*& ResourceManagerLocalInstanceImplementation::internalInstance() { - static ResourceManager* _instance(nullptr); - return _instance; -} -#endif - -}} - -#endif diff --git a/src/Magnum/Test/CMakeLists.txt b/src/Magnum/Test/CMakeLists.txt index 043a0e70b..c205c80f0 100644 --- a/src/Magnum/Test/CMakeLists.txt +++ b/src/Magnum/Test/CMakeLists.txt @@ -48,17 +48,3 @@ set_target_properties( SamplerTest TagsTest PROPERTIES FOLDER "Magnum/Test") - -if(MAGNUM_BUILD_DEPRECATED) - add_library(ResourceManagerLocalInstanceTestLib ${SHARED_OR_STATIC} ResourceManagerLocalInstanceTestLib.cpp) - target_link_libraries(ResourceManagerLocalInstanceTestLib Magnum) - if(NOT BUILD_STATIC) - target_compile_definitions(ResourceManagerLocalInstanceTestLib PRIVATE "ResourceManagerLocalInstanceTestLib_EXPORTS") - endif() - corrade_add_test(ResourceManagerLocalInstanceTest ResourceManagerLocalInstanceTest.cpp LIBRARIES Magnum ResourceManagerLocalInstanceTestLib) - - set_target_properties( - ResourceManagerLocalInstanceTestLib - ResourceManagerLocalInstanceTest - PROPERTIES FOLDER "Magnum/Test") -endif() diff --git a/src/Magnum/Test/ResourceManagerLocalInstanceTest.cpp b/src/Magnum/Test/ResourceManagerLocalInstanceTest.cpp deleted file mode 100644 index 244900981..000000000 --- a/src/Magnum/Test/ResourceManagerLocalInstanceTest.cpp +++ /dev/null @@ -1,55 +0,0 @@ -/* - This file is part of Magnum. - - Copyright © 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019 - Vladimír Vondruš - - 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 "ResourceManagerLocalInstanceTestLib.h" - -namespace Magnum { namespace Test { namespace { - -struct ResourceManagerLocalInstanceTest: TestSuite::Tester { - explicit ResourceManagerLocalInstanceTest(); - - void instance(); - - ResourceManagerWithLocalInstance manager; -}; - -ResourceManagerLocalInstanceTest::ResourceManagerLocalInstanceTest() { - addTests({&ResourceManagerLocalInstanceTest::instance}); -} - -void ResourceManagerLocalInstanceTest::instance() { - CORRADE_IGNORE_DEPRECATED_PUSH - ResourceManagerWithLocalInstance::instance().set("another", 13); - CORRADE_COMPARE(&manager.staticInstance, &manager.instance()); - CORRADE_IGNORE_DEPRECATED_POP - CORRADE_COMPARE(manager.count(), 2); - CORRADE_COMPARE(manager.state("integer"), ResourceState::Final); -} - -}}} - -CORRADE_TEST_MAIN(Magnum::Test::ResourceManagerLocalInstanceTest) diff --git a/src/Magnum/Test/ResourceManagerLocalInstanceTestLib.cpp b/src/Magnum/Test/ResourceManagerLocalInstanceTestLib.cpp deleted file mode 100644 index 16621a8cb..000000000 --- a/src/Magnum/Test/ResourceManagerLocalInstanceTestLib.cpp +++ /dev/null @@ -1,45 +0,0 @@ -/* - This file is part of Magnum. - - Copyright © 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019 - Vladimír Vondruš - - 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 "ResourceManagerLocalInstanceTestLib.h" - -#include "Magnum/ResourceManager.hpp" - -namespace Magnum { - -namespace Implementation { - template struct CORRADE_VISIBILITY_EXPORT ResourceManagerLocalInstanceImplementation; -} - -namespace Test { - -CORRADE_IGNORE_DEPRECATED_PUSH -ResourceManagerWithLocalInstance::ResourceManagerWithLocalInstance(): staticInstance(static_cast(instance())) { - /* Add some stuff */ - set("integer", 42); -} -CORRADE_IGNORE_DEPRECATED_POP - -}} diff --git a/src/Magnum/Test/ResourceManagerLocalInstanceTestLib.h b/src/Magnum/Test/ResourceManagerLocalInstanceTestLib.h deleted file mode 100644 index fbf707f2a..000000000 --- a/src/Magnum/Test/ResourceManagerLocalInstanceTestLib.h +++ /dev/null @@ -1,50 +0,0 @@ -#ifndef Magnum_Test_ResourceManagerLocalInstanceTestLib_h -#define Magnum_Test_ResourceManagerLocalInstanceTestLib_h -/* - This file is part of Magnum. - - Copyright © 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017, 2018, 2019 - Vladimír Vondruš - - 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 "Magnum/ResourceManager.h" - -#ifndef MAGNUM_BUILD_STATIC - #if defined(ResourceManagerLocalInstanceTestLib_EXPORTS) - #define MAGNUM_RESOURCEMANAGERLOCALINSTANCETESTLIB_EXPORT CORRADE_VISIBILITY_EXPORT - #else - #define MAGNUM_RESOURCEMANAGERLOCALINSTANCETESTLIB_EXPORT CORRADE_VISIBILITY_IMPORT - #endif -#else - #define MAGNUM_RESOURCEMANAGERLOCALINSTANCETESTLIB_EXPORT CORRADE_VISIBILITY_STATIC -#endif - -namespace Magnum { namespace Test { - -struct MAGNUM_RESOURCEMANAGERLOCALINSTANCETESTLIB_EXPORT ResourceManagerWithLocalInstance: ResourceManager { - explicit ResourceManagerWithLocalInstance(); - - ResourceManagerWithLocalInstance& staticInstance; -}; - -}} - -#endif