From 607b3a15fa01b7856a26879fff0d09b57c0e8568 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Mon, 21 Oct 2019 16:52:04 +0200 Subject: [PATCH] DebugTools: finish ResourceManager singleton deprecation. --- doc/changelog.dox | 8 ++++-- doc/snippets/MagnumDebugTools-gl.cpp | 26 ++++++++--------- src/Magnum/DebugTools/ForceRenderer.cpp | 16 +++++++---- src/Magnum/DebugTools/ForceRenderer.h | 28 +++++++++++++++---- src/Magnum/DebugTools/ObjectRenderer.cpp | 16 +++++++---- src/Magnum/DebugTools/ObjectRenderer.h | 14 +++++++++- src/Magnum/DebugTools/ResourceManager.cpp | 23 +-------------- src/Magnum/DebugTools/ResourceManager.h | 7 ----- .../DebugTools/Test/ForceRendererGLTest.cpp | 4 +-- .../DebugTools/Test/ObjectRendererGLTest.cpp | 4 +-- src/Magnum/ResourceManager.h | 3 ++ 11 files changed, 83 insertions(+), 66 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index e5f3140df..bec6f9d05 100644 --- 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 index d1e64f660..a5bc19ca7 100644 --- 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* 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* 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 index 790bcf72f..c626e56d8 100644 --- a/src/Magnum/DebugTools/ForceRenderer.cpp +++ b/src/Magnum/DebugTools/ForceRenderer.cpp @@ -55,13 +55,13 @@ constexpr UnsignedByte indices[]{ } -template ForceRenderer::ForceRenderer(SceneGraph::AbstractObject& object, const VectorTypeFor& forcePosition, const VectorTypeFor& force, ResourceKey options, SceneGraph::DrawableGroup* drawables): SceneGraph::Drawable(object, drawables), _forcePosition(forcePosition), _force(force), _options(ResourceManager::instance().get(options)) { +template ForceRenderer::ForceRenderer(ResourceManager& manager, SceneGraph::AbstractObject& object, const VectorTypeFor& forcePosition, const VectorTypeFor& force, ResourceKey options, SceneGraph::DrawableGroup* drawables): SceneGraph::Drawable(object, drawables), _forcePosition(forcePosition), _force(force), _options(manager.get(options)) { /* Shader */ - _shader = ResourceManager::instance().get>(shaderKey()); - if(!_shader) ResourceManager::instance().set(_shader.key(), new Shaders::Flat); + _shader = manager.get>(shaderKey()); + if(!_shader) manager.set(_shader.key(), new Shaders::Flat); /* Mesh and vertex buffer */ - _mesh = ResourceManager::instance().get("force"); + _mesh = manager.get("force"); if(_mesh) return; /* Create the mesh */ @@ -74,9 +74,15 @@ template ForceRenderer::ForceRenderer(SceneG .addVertexBuffer(std::move(vertexBuffer), 0, typename Shaders::Flat::Position(Shaders::Flat::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 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/ForceRenderer.h b/src/Magnum/DebugTools/ForceRenderer.h index f290ce4a4..4dbb0f5a1 100644 --- 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 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& object, const VectorTypeFor& forcePosition, const VectorTypeFor& force, ResourceKey options = ResourceKey(), SceneGraph::DrawableGroup* drawables = nullptr); + explicit ForceRenderer(ResourceManager& manager, SceneGraph::AbstractObject& object, const VectorTypeFor& forcePosition, const VectorTypeFor& force, ResourceKey options = ResourceKey(), SceneGraph::DrawableGroup* 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&, const VectorTypeFor&, VectorTypeFor&&, ResourceKey = ResourceKey(), SceneGraph::DrawableGroup* = 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&, const VectorTypeFor&, const VectorTypeFor&, ResourceKey, SceneGraph::DrawableGroup*) + * instead. */ - ForceRenderer(SceneGraph::AbstractObject&, const VectorTypeFor&, VectorTypeFor&&, ResourceKey = ResourceKey(), SceneGraph::DrawableGroup* = nullptr) = delete; + explicit CORRADE_DEPRECATED("implicit ResourceManager singleton is deprecated, use a constructor with explicit ResourceManager reference instead") ForceRenderer(SceneGraph::AbstractObject& object, const VectorTypeFor& forcePosition, const VectorTypeFor& force, ResourceKey options = ResourceKey(), SceneGraph::DrawableGroup* 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&, const VectorTypeFor&, VectorTypeFor&&, ResourceKey = ResourceKey(), SceneGraph::DrawableGroup* = nullptr) = delete; + #endif + #endif ~ForceRenderer(); diff --git a/src/Magnum/DebugTools/ObjectRenderer.cpp b/src/Magnum/DebugTools/ObjectRenderer.cpp index 1606c6ba3..891365aa2 100644 --- 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 ObjectRenderer::ObjectRenderer(SceneGraph::AbstractObject& object, ResourceKey options, SceneGraph::DrawableGroup* drawables): SceneGraph::Drawable(object, drawables), _options{ResourceManager::instance().get(options)} { +template ObjectRenderer::ObjectRenderer(ResourceManager& manager, SceneGraph::AbstractObject& object, ResourceKey options, SceneGraph::DrawableGroup* drawables): SceneGraph::Drawable(object, drawables), _options{manager.get(options)} { /* Shader */ - _shader = ResourceManager::instance().get>(Renderer::shader()); - if(!_shader) ResourceManager::instance().set(_shader.key(), new Shaders::VertexColor); + _shader = manager.get>(Renderer::shader()); + if(!_shader) manager.set(_shader.key(), new Shaders::VertexColor); /* Mesh */ - _mesh = ResourceManager::instance().get(Renderer::mesh()); - if(!_mesh) ResourceManager::instance().set(_mesh.key(), MeshTools::compile(Renderer::meshData())); + _mesh = manager.get(Renderer::mesh()); + 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/ObjectRenderer.h b/src/Magnum/DebugTools/ObjectRenderer.h index c2d36e0a2..2765a2eab 100644 --- 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 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 class MAGNUM_DEBUGTOOLS_EXPORT ObjectRenderer: * * The renderer is automatically added to object's features. */ - explicit ObjectRenderer(SceneGraph::AbstractObject& object, ResourceKey options = ResourceKey(), SceneGraph::DrawableGroup* drawables = nullptr); + explicit ObjectRenderer(ResourceManager& manager, SceneGraph::AbstractObject& object, ResourceKey options = ResourceKey(), SceneGraph::DrawableGroup* drawables = nullptr); + + #ifdef MAGNUM_BUILD_DEPRECATED + /** + * @brief Constructor + * @deprecated Implicit @ref ResourceManager singleton is deprecated, + * use @ref ObjectRenderer(ResourceManager&, SceneGraph::AbstractObject&, ResourceKey, SceneGraph::DrawableGroup*) + * instead. + */ + explicit CORRADE_DEPRECATED("implicit ResourceManager singleton is deprecated, use a constructor with explicit ResourceManager reference instead") ObjectRenderer(SceneGraph::AbstractObject& object, ResourceKey options = ResourceKey(), SceneGraph::DrawableGroup* drawables = nullptr); + #endif ~ObjectRenderer(); diff --git a/src/Magnum/DebugTools/ResourceManager.cpp b/src/Magnum/DebugTools/ResourceManager.cpp index aaad66ee1..df5d85afb 100644 --- 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 index 6a1914758..422c83fed 100644 --- 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 { 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 index 797d13920..7707e8cdb 100644 --- a/src/Magnum/DebugTools/Test/ForceRendererGLTest.cpp +++ b/src/Magnum/DebugTools/Test/ForceRendererGLTest.cpp @@ -90,7 +90,7 @@ void ForceRendererGLTest::render2D() { SceneGraph::Object 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 index 909f905f1..f69f06ef6 100644 --- 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 index d2d6f21ab..5c8b348e1 100644 --- a/src/Magnum/ResourceManager.h +++ b/src/Magnum/ResourceManager.h @@ -251,6 +251,9 @@ template 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& instance(); #endif