From 2d04b22abaed4d1723105a4d959228f4d3e242a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vladim=C3=ADr=20Vondru=C5=A1?= Date: Sun, 17 Feb 2019 12:42:02 +0100 Subject: [PATCH] Make it possible to pass Containers::Pointer to ResourceManager APIs. Also cleaned up the test from naked new. This still needs to be reworked to not do manual memory management inside (and then deprecate the raw pointer versions), but for now this has to suffice. --- doc/changelog.dox | 3 ++ src/Magnum/AbstractResourceLoader.h | 10 ++++ src/Magnum/ResourceManager.h | 23 +++++++++ src/Magnum/Test/ResourceManagerTest.cpp | 64 ++++++++++++------------- 4 files changed, 68 insertions(+), 32 deletions(-) diff --git a/doc/changelog.dox b/doc/changelog.dox index 8071a3ee8..bbd3a4361 100644 --- a/doc/changelog.dox +++ b/doc/changelog.dox @@ -58,6 +58,9 @@ See also: @subsection changelog-latest-changes Changes and improvements +- The @ref ResourceManager class now accepts also + @ref Corrade::Containers::Pointer instances in addition to raw pointers + @subsubsection changelog-latest-changes-audio Audio library - The @ref Audio::AnyImporter "AnyAudioImporter" plugin now correctly diff --git a/src/Magnum/AbstractResourceLoader.h b/src/Magnum/AbstractResourceLoader.h index 4081912a9..a5699eb8c 100644 --- a/src/Magnum/AbstractResourceLoader.h +++ b/src/Magnum/AbstractResourceLoader.h @@ -164,6 +164,11 @@ template class AbstractResourceLoader { */ void set(ResourceKey key, T* data, ResourceDataState state, ResourcePolicy policy); + /** @overload */ + void set(ResourceKey key, Containers::Pointer data, ResourceDataState state, ResourcePolicy policy) { + return set(key, data.release(), state, policy); + } + /** @overload */ template void set(ResourceKey key, U&& data, ResourceDataState state, ResourcePolicy policy) { set(key, new typename std::decay::type(std::forward(data)), state, policy); @@ -179,6 +184,11 @@ template class AbstractResourceLoader { set(key, data, ResourceDataState::Final, ResourcePolicy::Resident); } + /** @overload */ + void set(ResourceKey key, Containers::Pointer data) { + set(key, data.release()); + } + /** @overload */ template void set(ResourceKey key, U&& data) { set(key, new typename std::decay::type(std::forward(data))); diff --git a/src/Magnum/ResourceManager.h b/src/Magnum/ResourceManager.h index 93c146c44..29036bb09 100644 --- a/src/Magnum/ResourceManager.h +++ b/src/Magnum/ResourceManager.h @@ -30,6 +30,7 @@ */ #include +#include #include "Magnum/Resource.h" @@ -345,6 +346,12 @@ template class ResourceManager: private Implementation::Resource return *this; } + /** @overload */ + template ResourceManager& set(ResourceKey key, Containers::Pointer&& data, ResourceDataState state, ResourcePolicy policy) { + set(key, data.release(), state, policy); + return *this; + } + /** @overload */ template ResourceManager& set(ResourceKey key, U&& data, ResourceDataState state, ResourcePolicy policy) { return set(key, new typename std::decay::type(std::forward(data)), state, policy); @@ -361,6 +368,11 @@ template class ResourceManager: private Implementation::Resource return set(key, data, ResourceDataState::Final, ResourcePolicy::Resident); } + /** @overload */ + template ResourceManager& set(ResourceKey key, Containers::Pointer&& data) { + return set(key, data.release()); + } + /** @overload */ template ResourceManager& set(ResourceKey key, U&& data) { return set(key, new typename std::decay::type(std::forward(data))); @@ -385,6 +397,12 @@ template class ResourceManager: private Implementation::Resource return *this; } + /** @overload */ + template ResourceManager& setFallback(Containers::Pointer&& data) { + setFallback(data.release()); + return *this; + } + /** @overload */ template ResourceManager& setFallback(U&& data) { return setFallback(new typename std::decay::type(std::forward(data))); @@ -457,6 +475,11 @@ template class ResourceManager: private Implementation::Resource return *this; } + /** @overload */ + template ResourceManager& setLoader(Containers::Pointer>&& loader) { + return setLoader(loader.release()); + } + private: template void freeInternal(Implementation::ResourceTypePack) { free(); diff --git a/src/Magnum/Test/ResourceManagerTest.cpp b/src/Magnum/Test/ResourceManagerTest.cpp index 0ea2caab3..1fe9f9757 100644 --- a/src/Magnum/Test/ResourceManagerTest.cpp +++ b/src/Magnum/Test/ResourceManagerTest.cpp @@ -101,7 +101,7 @@ void ResourceManagerTest::state() { void ResourceManagerTest::stateFallback() { { ResourceManager rm; - rm.setFallback(new Data); + rm.setFallback(Containers::pointer()); Resource data = rm.get("data"); CORRADE_VERIFY(data); @@ -174,15 +174,15 @@ void ResourceManagerTest::basic() { } void ResourceManagerTest::residentPolicy() { - ResourceManager* rm = new ResourceManager; - - rm->set("blah", new Data, ResourceDataState::Mutable, ResourcePolicy::Resident); - CORRADE_COMPARE(Data::count, 1); + { + ResourceManager rm; - rm->free(); - CORRADE_COMPARE(Data::count, 1); + rm.set("blah", Containers::pointer(), ResourceDataState::Mutable, ResourcePolicy::Resident); + CORRADE_COMPARE(Data::count, 1); - delete rm; + rm.free(); + CORRADE_COMPARE(Data::count, 1); + } CORRADE_COMPARE(Data::count, 0); } @@ -192,7 +192,7 @@ void ResourceManagerTest::referenceCountedPolicy() { ResourceKey dataRefCountKey("dataRefCount"); /* Resource is deleted after all references are removed */ - rm.set(dataRefCountKey, new Data, ResourceDataState::Final, ResourcePolicy::ReferenceCounted); + rm.set(dataRefCountKey, Containers::pointer(), ResourceDataState::Final, ResourcePolicy::ReferenceCounted); CORRADE_COMPARE(rm.count(), 1); { Resource data = rm.get(dataRefCountKey); @@ -205,7 +205,7 @@ void ResourceManagerTest::referenceCountedPolicy() { /* Reference counted resources which were not used once will stay loaded until free() is called */ - rm.set(dataRefCountKey, new Data, ResourceDataState::Final, ResourcePolicy::ReferenceCounted); + rm.set(dataRefCountKey, Containers::pointer(), ResourceDataState::Final, ResourcePolicy::ReferenceCounted); CORRADE_COMPARE(rm.count(), 1); CORRADE_COMPARE(rm.state(dataRefCountKey), ResourceState::Final); CORRADE_COMPARE(rm.referenceCount(dataRefCountKey), 0); @@ -223,7 +223,7 @@ void ResourceManagerTest::manualPolicy() { /* Manual free */ { - rm.set(dataKey, new Data, ResourceDataState::Mutable, ResourcePolicy::Manual); + rm.set(dataKey, Containers::pointer(), ResourceDataState::Mutable, ResourcePolicy::Manual); Resource data = rm.get(dataKey); rm.free(); } @@ -234,21 +234,21 @@ void ResourceManagerTest::manualPolicy() { CORRADE_COMPARE(rm.count(), 0); CORRADE_COMPARE(Data::count, 0); - rm.set(dataKey, new Data, ResourceDataState::Mutable, ResourcePolicy::Manual); + rm.set(dataKey, Containers::pointer(), ResourceDataState::Mutable, ResourcePolicy::Manual); CORRADE_COMPARE(rm.count(), 1); CORRADE_COMPARE(Data::count, 1); } void ResourceManagerTest::defaults() { ResourceManager rm; - rm.set("data", new Data); + rm.set("data", Containers::pointer()); CORRADE_COMPARE(rm.state("data"), ResourceState::Final); } void ResourceManagerTest::clear() { ResourceManager rm; - rm.set("blah", new Data); + rm.set("blah", Containers::pointer()); CORRADE_COMPARE(Data::count, 1); rm.free(); @@ -281,7 +281,7 @@ void ResourceManagerTest::loader() { IntResourceLoader(): resource(ResourceManager::instance().get("data")) {} void load() { - set("hello", 773, ResourceDataState::Final, ResourcePolicy::Resident); + set("hello", Containers::pointer(773), ResourceDataState::Final, ResourcePolicy::Resident); setNotFound("world"); } @@ -298,38 +298,38 @@ void ResourceManagerTest::loader() { Resource resource; }; - auto rm = new ResourceManager; - auto loader = new IntResourceLoader; - rm->setLoader(loader); - { - Resource data = rm->get("data"); - Resource hello = rm->get("hello"); - Resource world = rm->get("world"); + ResourceManager rm; + Containers::Pointer loaderPtr{Containers::InPlaceInit}; + IntResourceLoader& loader = *loaderPtr; + rm.setLoader(std::move(loaderPtr)); + + Resource data = rm.get("data"); + Resource hello = rm.get("hello"); + Resource world = rm.get("world"); CORRADE_COMPARE(data.state(), ResourceState::NotLoaded); CORRADE_COMPARE(hello.state(), ResourceState::Loading); CORRADE_COMPARE(world.state(), ResourceState::Loading); - CORRADE_COMPARE(loader->requestedCount(), 2); - CORRADE_COMPARE(loader->loadedCount(), 0); - CORRADE_COMPARE(loader->notFoundCount(), 0); - CORRADE_COMPARE(loader->name(ResourceKey("hello")), "hello"); + CORRADE_COMPARE(loader.requestedCount(), 2); + CORRADE_COMPARE(loader.loadedCount(), 0); + CORRADE_COMPARE(loader.notFoundCount(), 0); + CORRADE_COMPARE(loader.name(ResourceKey("hello")), "hello"); - loader->load(); + loader.load(); CORRADE_COMPARE(hello.state(), ResourceState::Final); CORRADE_COMPARE(*hello, 773); CORRADE_COMPARE(world.state(), ResourceState::NotFound); - CORRADE_COMPARE(loader->requestedCount(), 2); - CORRADE_COMPARE(loader->loadedCount(), 1); - CORRADE_COMPARE(loader->notFoundCount(), 1); + CORRADE_COMPARE(loader.requestedCount(), 2); + CORRADE_COMPARE(loader.loadedCount(), 1); + CORRADE_COMPARE(loader.notFoundCount(), 1); /* Verify that the loader is deleted at proper time */ - rm->set("data", new Data); + rm.set("data", Containers::pointer()); CORRADE_COMPARE(Data::count, 1); } - delete rm; CORRADE_COMPARE(Data::count, 0); }