DebugTools: finish ResourceManager singleton deprecation.

mosra · Oct 21, 2019 · 607b3a1 · 607b3a1
1 parent 8c7c2c5
commit 607b3a1
Show file tree

Hide file tree

Showing 11 changed files with 83 additions and 66 deletions.
diff --git a/doc/changelog.dox b/doc/changelog.dox
@@ -773,9 +773,11 @@ See also:
 -   Passing @cpp nullptr @ce to @ref ImageView constructors is deprecated and
     will print a warning at runtime. Use a constructor without the @p data
     parameter instead.
--   The @cpp ResourceManager::instance() @ce singleton is deprecated as it
-    makes some use cases harder than they should be. Make your own singleton
-    or explicitly pass a @ref ResourceManager reference around instead.
+-   The @cpp ResourceManager::instance() @ce singleton (and its implicit use in
+    @ref DebugTools::ForceRenderer and @ref DebugTools::ObjectRenderer) is
+    deprecated as it makes some use cases harder than they should be. Make your
+    own singleton or explicitly pass a @ref ResourceManager reference around
+    instead.
 -   @ref Platform::BasicScreen::application() now returns a reference instead
     of a pointer and together with @ref Platform::BasicScreen::redraw() checks
     that the screen is actually added to the application instead of returning

diff --git a/doc/snippets/MagnumDebugTools-gl.cpp b/doc/snippets/MagnumDebugTools-gl.cpp
@@ -58,43 +58,43 @@ DebugTools::ResourceManager manager;
 SceneGraph::DrawableGroup3D debugDrawables;
 
 // Create renderer options which will be referenced later by "my" resource key
-DebugTools::ResourceManager::instance().set("my",
-    DebugTools::ObjectRendererOptions{}.setSize(0.3f));
+manager.set("my", DebugTools::ObjectRendererOptions{}.setSize(0.3f));
 
 // Create debug renderer for given object, use "my" options for it. The
 // renderer is automatically added to the object features and also to
 // specified drawable group.
-new DebugTools::ObjectRenderer3D{*object, "my", &debugDrawables};
+new DebugTools::ObjectRenderer3D{manager, *object, "my", &debugDrawables};
 /* [debug-tools-renderers] */
 }
 
 {
+DebugTools::ResourceManager manager;
 SceneGraph::Object<SceneGraph::MatrixTransformation3D>* object{};
 SceneGraph::DrawableGroup3D debugDrawables;
 /* [ForceRenderer] */
-DebugTools::ResourceManager::instance().set("my",
-    DebugTools::ForceRendererOptions{}
-        .setSize(5.0f)
-        .setColor(Color3::fromHsv({120.0_degf, 1.0f, 0.7f})));
+manager.set("my", DebugTools::ForceRendererOptions{}
+    .setSize(5.0f)
+    .setColor(Color3::fromHsv({120.0_degf, 1.0f, 0.7f})));
 
 Vector3 force; // taken as a reference, has to be kept in scope
 
 // Create debug renderer for given force, use "my" options for it
-new DebugTools::ForceRenderer3D(*object, {0.3f, 1.5f, -0.7f}, force, "my",
-        &debugDrawables);
+new DebugTools::ForceRenderer3D(manager, *object, {0.3f, 1.5f, -0.7f}, force,
+    "my", &debugDrawables);
 /* [ForceRenderer] */
 }
 
 {
 SceneGraph::Object<SceneGraph::MatrixTransformation3D>* object{};
-SceneGraph::DrawableGroup3D debugDrawables;
 /* [ObjectRenderer] */
+DebugTools::ResourceManager manager;
+SceneGraph::DrawableGroup3D debugDrawables;
+
 // Create some options
-DebugTools::ResourceManager::instance().set("my",
-    DebugTools::ObjectRendererOptions{}.setSize(0.3f));
+manager.set("my", DebugTools::ObjectRendererOptions{}.setSize(0.3f));
 
 // Create debug renderer for given object, use "my" options for it
-new DebugTools::ObjectRenderer3D(*object, "my", &debugDrawables);
+new DebugTools::ObjectRenderer3D{manager, *object, "my", &debugDrawables};
 /* [ObjectRenderer] */
 }
 {

diff --git a/src/Magnum/DebugTools/ForceRenderer.cpp b/src/Magnum/DebugTools/ForceRenderer.cpp
@@ -55,13 +55,13 @@ constexpr UnsignedByte indices[]{
 
 }
 
-template<UnsignedInt dimensions> ForceRenderer<dimensions>::ForceRenderer(SceneGraph::AbstractObject<dimensions, Float>& object, const VectorTypeFor<dimensions, Float>& forcePosition, const VectorTypeFor<dimensions, Float>& force, ResourceKey options, SceneGraph::DrawableGroup<dimensions, Float>* drawables): SceneGraph::Drawable<dimensions, Float>(object, drawables), _forcePosition(forcePosition), _force(force), _options(ResourceManager::instance().get<ForceRendererOptions>(options)) {
+template<UnsignedInt dimensions> ForceRenderer<dimensions>::ForceRenderer(ResourceManager& manager, SceneGraph::AbstractObject<dimensions, Float>& object, const VectorTypeFor<dimensions, Float>& forcePosition, const VectorTypeFor<dimensions, Float>& force, ResourceKey options, SceneGraph::DrawableGroup<dimensions, Float>* drawables): SceneGraph::Drawable<dimensions, Float>(object, drawables), _forcePosition(forcePosition), _force(force), _options(manager.get<ForceRendererOptions>(options)) {
     /* Shader */
-    _shader = ResourceManager::instance().get<GL::AbstractShaderProgram, Shaders::Flat<dimensions>>(shaderKey<dimensions>());
-    if(!_shader) ResourceManager::instance().set<GL::AbstractShaderProgram>(_shader.key(), new Shaders::Flat<dimensions>);
+    _shader = manager.get<GL::AbstractShaderProgram, Shaders::Flat<dimensions>>(shaderKey<dimensions>());
+    if(!_shader) manager.set<GL::AbstractShaderProgram>(_shader.key(), new Shaders::Flat<dimensions>);
 
     /* Mesh and vertex buffer */
-    _mesh = ResourceManager::instance().get<GL::Mesh>("force");
+    _mesh = manager.get<GL::Mesh>("force");
     if(_mesh) return;
 
     /* Create the mesh */
@@ -74,9 +74,15 @@ template<UnsignedInt dimensions> ForceRenderer<dimensions>::ForceRenderer(SceneG
         .addVertexBuffer(std::move(vertexBuffer), 0,
             typename Shaders::Flat<dimensions>::Position(Shaders::Flat<dimensions>::Position::Components::Two))
         .setIndexBuffer(std::move(indexBuffer), 0, GL::MeshIndexType::UnsignedByte, 0, Containers::arraySize(positions));
-    ResourceManager::instance().set(_mesh.key(), std::move(mesh), ResourceDataState::Final, ResourcePolicy::Manual);
+    manager.set(_mesh.key(), std::move(mesh), ResourceDataState::Final, ResourcePolicy::Manual);
 }
 
+#ifdef MAGNUM_BUILD_DEPRECATED
+CORRADE_IGNORE_DEPRECATED_PUSH
+template<UnsignedInt dimensions> ForceRenderer<dimensions>::ForceRenderer(SceneGraph::AbstractObject<dimensions, Float>& object, const VectorTypeFor<dimensions, Float>& forcePosition, const VectorTypeFor<dimensions, Float>& force, ResourceKey options, SceneGraph::DrawableGroup<dimensions, Float>* drawables): ForceRenderer<dimensions>{static_cast<ResourceManager&>(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<UnsignedInt dimensions> ForceRenderer<dimensions>::~ForceRenderer() = default;
 

diff --git a/src/Magnum/DebugTools/ForceRenderer.h b/src/Magnum/DebugTools/ForceRenderer.h
@@ -32,11 +32,12 @@
 #endif
 
 #include "Magnum/Resource.h"
+#include "Magnum/DebugTools/DebugTools.h"
+#include "Magnum/DebugTools/visibility.h"
 #include "Magnum/GL/GL.h"
 #include "Magnum/Math/Color.h"
 #include "Magnum/SceneGraph/Drawable.h"
 #include "Magnum/Shaders/Shaders.h"
-#include "Magnum/DebugTools/visibility.h"
 
 #ifdef MAGNUM_TARGET_GL
 namespace Magnum { namespace DebugTools {
@@ -121,21 +122,36 @@ template<UnsignedInt dimensions> class MAGNUM_DEBUGTOOLS_EXPORT ForceRenderer: p
     public:
         /**
          * @brief Constructor
+         * @param manager       Resource manager instance
          * @param object        Object for which to create debug renderer
          * @param forcePosition Where to render the force, relative to object
-         * @param force         Force vector
+         * @param force         Reference to the force vector
          * @param options       Options resource key. See
          *      @ref DebugTools-ForceRenderer-usage "class documentation" for
          *      more information.
          * @param drawables     Drawable group
          */
-        explicit ForceRenderer(SceneGraph::AbstractObject<dimensions, Float>& object, const VectorTypeFor<dimensions, Float>& forcePosition, const VectorTypeFor<dimensions, Float>& force, ResourceKey options = ResourceKey(), SceneGraph::DrawableGroup<dimensions, Float>* drawables = nullptr);
+        explicit ForceRenderer(ResourceManager& manager, SceneGraph::AbstractObject<dimensions, Float>& object, const VectorTypeFor<dimensions, Float>& forcePosition, const VectorTypeFor<dimensions, Float>& force, ResourceKey options = ResourceKey(), SceneGraph::DrawableGroup<dimensions, Float>* drawables = nullptr);
+
+        /**
+         * You have to pass a reference to an external force vector --- the
+         * renderer doesn't store a copy.
+         */
+        explicit ForceRenderer(ResourceManager&, SceneGraph::AbstractObject<dimensions, Float>&, const VectorTypeFor<dimensions, Float>&, VectorTypeFor<dimensions, Float>&&, ResourceKey = ResourceKey(), SceneGraph::DrawableGroup<dimensions, Float>* = nullptr) = delete;
 
+        #ifdef MAGNUM_BUILD_DEPRECATED
         /**
-         * You have to pass reference to existing force instance, as the
-         * renderer uses the current value when rendering.
+         * @brief Constructor
+         * @deprecated Implicit @ref ResourceManager singleton is deprecated,
+         *      use @ref ForceRenderer(ResourceManager&, SceneGraph::AbstractObject<dimensions, Float>&, const VectorTypeFor<dimensions, Float>&, const VectorTypeFor<dimensions, Float>&, ResourceKey, SceneGraph::DrawableGroup<dimensions, Float>*)
+         *      instead.
          */
-        ForceRenderer(SceneGraph::AbstractObject<dimensions, Float>&, const VectorTypeFor<dimensions, Float>&, VectorTypeFor<dimensions, Float>&&, ResourceKey = ResourceKey(), SceneGraph::DrawableGroup<dimensions, Float>* = nullptr) = delete;
+        explicit CORRADE_DEPRECATED("implicit ResourceManager singleton is deprecated, use a constructor with explicit ResourceManager reference instead") ForceRenderer(SceneGraph::AbstractObject<dimensions, Float>& object, const VectorTypeFor<dimensions, Float>& forcePosition, const VectorTypeFor<dimensions, Float>& force, ResourceKey options = ResourceKey(), SceneGraph::DrawableGroup<dimensions, Float>* drawables = nullptr);
+
+        #ifndef DOXYGEN_GENERATOR_OUTPUT
+        explicit CORRADE_DEPRECATED("implicit ResourceManager singleton is deprecated, use a constructor with explicit DebugTools::ResourceManager reference instead") ForceRenderer(SceneGraph::AbstractObject<dimensions, Float>&, const VectorTypeFor<dimensions, Float>&, VectorTypeFor<dimensions, Float>&&, ResourceKey = ResourceKey(), SceneGraph::DrawableGroup<dimensions, Float>* = nullptr) = delete;
+        #endif
+        #endif
 
         ~ForceRenderer();
 

diff --git a/src/Magnum/DebugTools/ObjectRenderer.cpp b/src/Magnum/DebugTools/ObjectRenderer.cpp
@@ -55,16 +55,22 @@ template<> struct Renderer<3> {
 }
 
 /* Doxygen gets confused when using {} to initialize parent object */
-template<UnsignedInt dimensions> ObjectRenderer<dimensions>::ObjectRenderer(SceneGraph::AbstractObject<dimensions, Float>& object, ResourceKey options, SceneGraph::DrawableGroup<dimensions, Float>* drawables): SceneGraph::Drawable<dimensions, Float>(object, drawables), _options{ResourceManager::instance().get<ObjectRendererOptions>(options)} {
+template<UnsignedInt dimensions> ObjectRenderer<dimensions>::ObjectRenderer(ResourceManager& manager, SceneGraph::AbstractObject<dimensions, Float>& object, ResourceKey options, SceneGraph::DrawableGroup<dimensions, Float>* drawables): SceneGraph::Drawable<dimensions, Float>(object, drawables), _options{manager.get<ObjectRendererOptions>(options)} {
     /* Shader */
-    _shader = ResourceManager::instance().get<GL::AbstractShaderProgram, Shaders::VertexColor<dimensions>>(Renderer<dimensions>::shader());
-    if(!_shader) ResourceManager::instance().set<GL::AbstractShaderProgram>(_shader.key(), new Shaders::VertexColor<dimensions>);
+    _shader = manager.get<GL::AbstractShaderProgram, Shaders::VertexColor<dimensions>>(Renderer<dimensions>::shader());
+    if(!_shader) manager.set<GL::AbstractShaderProgram>(_shader.key(), new Shaders::VertexColor<dimensions>);
 
     /* Mesh */
-    _mesh = ResourceManager::instance().get<GL::Mesh>(Renderer<dimensions>::mesh());
-    if(!_mesh) ResourceManager::instance().set<GL::Mesh>(_mesh.key(), MeshTools::compile(Renderer<dimensions>::meshData()));
+    _mesh = manager.get<GL::Mesh>(Renderer<dimensions>::mesh());
+    if(!_mesh) manager.set<GL::Mesh>(_mesh.key(), MeshTools::compile(Renderer<dimensions>::meshData()));
 }
 
+#ifdef MAGNUM_BUILD_DEPRECATED
+CORRADE_IGNORE_DEPRECATED_PUSH
+template<UnsignedInt dimensions> ObjectRenderer<dimensions>::ObjectRenderer(SceneGraph::AbstractObject<dimensions, Float>& object, ResourceKey options, SceneGraph::DrawableGroup<dimensions, Float>* drawables): ObjectRenderer<dimensions>{static_cast<ResourceManager&>(ResourceManager::instance()), object, options, drawables} {}
+CORRADE_IGNORE_DEPRECATED_POP
+#endif
+
 /* To avoid deleting pointers to incomplete type on destruction of Resource members */
 template<UnsignedInt dimensions> ObjectRenderer<dimensions>::~ObjectRenderer() = default;
 

diff --git a/src/Magnum/DebugTools/ObjectRenderer.h b/src/Magnum/DebugTools/ObjectRenderer.h
@@ -32,6 +32,7 @@
 #endif
 
 #include "Magnum/Resource.h"
+#include "Magnum/DebugTools/DebugTools.h"
 #include "Magnum/DebugTools/visibility.h"
 #include "Magnum/GL/GL.h"
 #include "Magnum/SceneGraph/Drawable.h"
@@ -93,6 +94,7 @@ template<UnsignedInt dimensions> class MAGNUM_DEBUGTOOLS_EXPORT ObjectRenderer:
     public:
         /**
          * @brief Constructor
+         * @param manager   Resource manager instance
          * @param object    Object for which to create debug renderer
          * @param options   Options resource key. See
          *      @ref DebugTools-ObjectRenderer-usage "class documentation" for
@@ -101,7 +103,17 @@ template<UnsignedInt dimensions> class MAGNUM_DEBUGTOOLS_EXPORT ObjectRenderer:
          *
          * The renderer is automatically added to object's features.
          */
-        explicit ObjectRenderer(SceneGraph::AbstractObject<dimensions, Float>& object, ResourceKey options = ResourceKey(), SceneGraph::DrawableGroup<dimensions, Float>* drawables = nullptr);
+        explicit ObjectRenderer(ResourceManager& manager, SceneGraph::AbstractObject<dimensions, Float>& object, ResourceKey options = ResourceKey(), SceneGraph::DrawableGroup<dimensions, Float>* drawables = nullptr);
+
+        #ifdef MAGNUM_BUILD_DEPRECATED
+        /**
+         * @brief Constructor
+         * @deprecated Implicit @ref ResourceManager singleton is deprecated,
+         *      use @ref ObjectRenderer(ResourceManager&, SceneGraph::AbstractObject<dimensions, Float>&, ResourceKey, SceneGraph::DrawableGroup<dimensions, Float>*)
+         *      instead.
+         */
+        explicit CORRADE_DEPRECATED("implicit ResourceManager singleton is deprecated, use a constructor with explicit ResourceManager reference instead") ObjectRenderer(SceneGraph::AbstractObject<dimensions, Float>& object, ResourceKey options = ResourceKey(), SceneGraph::DrawableGroup<dimensions, Float>* drawables = nullptr);
+        #endif
 
         ~ObjectRenderer();
 

diff --git a/src/Magnum/DebugTools/ResourceManager.cpp b/src/Magnum/DebugTools/ResourceManager.cpp
@@ -41,32 +41,11 @@ namespace Implementation {
 
 namespace DebugTools {
 
-namespace {
-    #ifdef CORRADE_BUILD_MULTITHREADED
-    CORRADE_THREAD_LOCAL
-    #endif
-    ResourceManager* resourceManagerInstance = nullptr;
-}
-
-ResourceManager& ResourceManager::instance() {
-    CORRADE_ASSERT(resourceManagerInstance,
-        "DebugTools::ResourceManager::instance(): no instance exists",
-        *resourceManagerInstance);
-    return *resourceManagerInstance;
-}
-
 ResourceManager::ResourceManager() {
-    CORRADE_ASSERT(!resourceManagerInstance,
-        "DebugTools::ResourceManager: another instance is already created", );
-    resourceManagerInstance = this;
-
     setFallback(new ForceRendererOptions);
     setFallback(new ObjectRendererOptions);
 }
 
-ResourceManager::~ResourceManager() {
-    CORRADE_INTERNAL_ASSERT(resourceManagerInstance == this);
-    resourceManagerInstance = nullptr;
-}
+ResourceManager::~ResourceManager() = default;
 
 }}
diff --git a/src/Magnum/DebugTools/ResourceManager.h b/src/Magnum/DebugTools/ResourceManager.h
@@ -65,13 +65,6 @@ information.
 class MAGNUM_DEBUGTOOLS_EXPORT ResourceManager: public Magnum::ResourceManager<Magnum::Implementation::ResourceManagerLocalInstance, GL::AbstractShaderProgram, GL::Buffer, GL::Mesh, GL::MeshView, DebugTools::ForceRendererOptions, DebugTools::ObjectRendererOptions>
 {
     public:
-        /**
-         * @brief Global instance
-         *
-         * Assumes that the instance exists.
-         */
-        static ResourceManager& instance();
-
         explicit ResourceManager();
         ~ResourceManager();
 };

diff --git a/src/Magnum/DebugTools/Test/ForceRendererGLTest.cpp b/src/Magnum/DebugTools/Test/ForceRendererGLTest.cpp
@@ -90,7 +90,7 @@ void ForceRendererGLTest::render2D() {
     SceneGraph::Object<SceneGraph::MatrixTransformation2D> object{&scene};
     object.translate({-1.0f, -1.0f});
     Vector2 force{2.0f, 2.0f};
-    ForceRenderer2D renderer{object, {}, force, "my", &drawables};
+    ForceRenderer2D renderer{manager, object, {}, force, "my", &drawables};
 
     GL::Renderbuffer color;
     color.setStorage(
@@ -146,7 +146,7 @@ void ForceRendererGLTest::render3D() {
         .rotateY(-90.0_degf)
         .translate({-0.5f, -1.0f, 1.0f});
     Vector3 force{2.0f, 2.0f, 0.0f};
-    ForceRenderer3D renderer{object, {}, force, "my", &drawables};
+    ForceRenderer3D renderer{manager, object, {}, force, "my", &drawables};
 
     GL::Renderbuffer color;
     color.setStorage(

diff --git a/src/Magnum/DebugTools/Test/ObjectRendererGLTest.cpp b/src/Magnum/DebugTools/Test/ObjectRendererGLTest.cpp
@@ -91,7 +91,7 @@ void ObjectRendererGLTest::render2D() {
     object
         .rotate(-17.3_degf)
         .translate({-1.0f, -1.0f});
-    ObjectRenderer2D renderer{object, "my", &drawables};
+    ObjectRenderer2D renderer{manager, object, "my", &drawables};
 
     GL::Renderbuffer color;
     color.setStorage(
@@ -136,7 +136,7 @@ void ObjectRendererGLTest::render3D() {
         .rotateZ(17.3_degf)
         .rotateY(45.0_degf)
         .translate({-1.0f, -1.0f, -1.0f});
-    ObjectRenderer3D renderer{object, "my", &drawables};
+    ObjectRenderer3D renderer{manager, object, "my", &drawables};
 
     GL::Renderbuffer color;
     color.setStorage(

diff --git a/src/Magnum/ResourceManager.h b/src/Magnum/ResourceManager.h
@@ -251,6 +251,9 @@ template<class... Types> class ResourceManager: private Implementation::Resource
          * @brief Global instance
          *
          * Assumes that the instance exists.
+         *
+         * @deprecated 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<Types...>& instance();
         #endif