From 1e51b62c882b5fc1554efb46cb41c3d54157626c Mon Sep 17 00:00:00 2001 From: Jan Dalheimer <jan@dalheimer.de> Date: Sat, 6 Jun 2015 12:30:49 +0200 Subject: NOISSUE Comment and bugfix the Resource system --- logic/resources/IconResourceHandler.cpp | 2 + logic/resources/IconResourceHandler.h | 5 ++- logic/resources/Resource.cpp | 65 +++++++++++++++++++-------- logic/resources/Resource.h | 78 +++++++++++++++++++-------------- logic/resources/ResourceHandler.h | 3 +- logic/resources/ResourceObserver.h | 5 +++ logic/resources/ResourceProxyModel.cpp | 42 ++++++------------ logic/resources/ResourceProxyModel.h | 1 + 8 files changed, 120 insertions(+), 81 deletions(-) (limited to 'logic/resources') diff --git a/logic/resources/IconResourceHandler.cpp b/logic/resources/IconResourceHandler.cpp index d47dcc3d..99353b6c 100644 --- a/logic/resources/IconResourceHandler.cpp +++ b/logic/resources/IconResourceHandler.cpp @@ -15,6 +15,7 @@ void IconResourceHandler::setTheme(const QString &theme) { m_theme = theme; + // notify everyone for (auto handler : m_iconHandlers) { std::shared_ptr<IconResourceHandler> ptr = handler.lock(); @@ -28,6 +29,7 @@ void IconResourceHandler::setTheme(const QString &theme) void IconResourceHandler::init(std::shared_ptr<ResourceHandler> &ptr) { m_iconHandlers.append(std::dynamic_pointer_cast<IconResourceHandler>(ptr)); + // we always have a result, so lets report it now! setResult(get()); } diff --git a/logic/resources/IconResourceHandler.h b/logic/resources/IconResourceHandler.h index dedfecb2..6f933ad4 100644 --- a/logic/resources/IconResourceHandler.h +++ b/logic/resources/IconResourceHandler.h @@ -9,14 +9,17 @@ class IconResourceHandler : public ResourceHandler public: explicit IconResourceHandler(const QString &key); + /// Sets the current theme and notifies all IconResourceHandlers of the change static void setTheme(const QString &theme); private: + // we need to keep track of all IconResourceHandlers so that we can update them if the theme changes void init(std::shared_ptr<ResourceHandler> &ptr) override; + static QList<std::weak_ptr<IconResourceHandler>> m_iconHandlers; QString m_key; static QString m_theme; - static QList<std::weak_ptr<IconResourceHandler>> m_iconHandlers; + // the workhorse, returns QVariantMap (filename => size) for m_key/m_theme QVariant get() const; }; diff --git a/logic/resources/Resource.cpp b/logic/resources/Resource.cpp index 16ed3d2d..3cc6b648 100644 --- a/logic/resources/Resource.cpp +++ b/logic/resources/Resource.cpp @@ -6,12 +6,16 @@ #include "IconResourceHandler.h" #include "ResourceObserver.h" +// definition of static members of Resource QMap<QString, std::function<std::shared_ptr<ResourceHandler>(const QString &)>> Resource::m_handlers; QMap<QPair<int, int>, std::function<QVariant(QVariant)>> Resource::m_transfomers; QMap<QString, std::weak_ptr<Resource>> Resource::m_resources; Resource::Resource(const QString &resource) + : m_resource(resource) { + // register default handlers + // QUESTION: move elsewhere? if (!m_handlers.contains("web")) { registerHandler<WebResourceHandler>("web"); @@ -21,33 +25,48 @@ Resource::Resource(const QString &resource) registerHandler<IconResourceHandler>("icon"); } + // a valid resource identifier has the format <id>:<data> Q_ASSERT(resource.contains(':')); + // "parse" the resource identifier into id and data const QString resourceId = resource.left(resource.indexOf(':')); + const QString resourceData = resource.mid(resource.indexOf(':') + 1); + + // create and set up the handler Q_ASSERT(m_handlers.contains(resourceId)); - m_handler = m_handlers.value(resourceId)(resource.mid(resource.indexOf(':') + 1)); + m_handler = m_handlers.value(resourceId)(resourceData); + Q_ASSERT(m_handler); m_handler->init(m_handler); m_handler->setResource(this); - Q_ASSERT(m_handler); } Resource::~Resource() { qDeleteAll(m_observers); } -Resource::Ptr Resource::create(const QString &resource) +Resource::Ptr Resource::create(const QString &resource, Ptr placeholder) { - Resource::Ptr ptr = m_resources.contains(resource) - ? m_resources.value(resource).lock() + const QString storageId = storageIdentifier(resource, placeholder); + + // do we already have a resource? even if m_resources contains it it might not be valid any longer (weak_ptr) + Resource::Ptr ptr = m_resources.contains(storageId) + ? m_resources.value(storageId).lock() : nullptr; + // did we have one? and is it still valid? if (!ptr) { + /* We don't want Resource to have a public constructor, but std::make_shared needs it, + * so we create a subclass of Resource here that exposes the constructor as public. + * The alternative would be making the allocator for std::make_shared a friend, but it + * differs between different STL implementations, so that would be a pain. + */ struct ConstructableResource : public Resource { explicit ConstructableResource(const QString &resource) : Resource(resource) {} }; ptr = std::make_shared<ConstructableResource>(resource); - m_resources.insert(resource, ptr); + ptr->m_placeholder = placeholder; + m_resources.insert(storageId, ptr); } return ptr; } @@ -56,39 +75,35 @@ Resource::Ptr Resource::applyTo(ResourceObserver *observer) { m_observers.append(observer); observer->setSource(shared_from_this()); // give the observer a shared_ptr for us so we don't get deleted - observer->resourceUpdated(); - return shared_from_this(); + observer->resourceUpdated(); // ask the observer to poll us immediently, we might already have data + return shared_from_this(); // allow chaining } Resource::Ptr Resource::applyTo(QObject *target, const char *property) { - // the cast to ResourceObserver* is required to ensure the right overload gets choosen + // the cast to ResourceObserver* is required to ensure the right overload gets choosen, + // since QObjectResourceObserver also inherits from QObject return applyTo(static_cast<ResourceObserver *>(new QObjectResourceObserver(target, property))); } -Resource::Ptr Resource::placeholder(Resource::Ptr other) -{ - m_placeholder = other; - for (ResourceObserver *observer : m_observers) - { - observer->resourceUpdated(); - } - return shared_from_this(); -} - QVariant Resource::getResourceInternal(const int typeId) const { + // no result (yet), but a placeholder? delegate to the placeholder. if (m_handler->result().isNull() && m_placeholder) { return m_placeholder->getResourceInternal(typeId); } const QVariant variant = m_handler->result(); const auto typePair = qMakePair(int(variant.type()), typeId); + + // do we have an explicit transformer? use it. if (m_transfomers.contains(typePair)) { return m_transfomers.value(typePair)(variant); } else { + // we do not have an explicit transformer, so we just pass the QVariant, which will automatically + // transform some types for us (different numbers to each other etc.) return variant; } } @@ -119,3 +134,15 @@ void Resource::notifyObserverDeleted(ResourceObserver *observer) { m_observers.removeAll(observer); } + +QString Resource::storageIdentifier(const QString &id, Resource::Ptr placeholder) +{ + if (placeholder) + { + return id + '#' + storageIdentifier(placeholder->m_resource, placeholder->m_placeholder); + } + else + { + return id; + } +} diff --git a/logic/resources/Resource.h b/logic/resources/Resource.h index d566b2a2..40a6d871 100644 --- a/logic/resources/Resource.h +++ b/logic/resources/Resource.h @@ -7,24 +7,10 @@ #include <memory> #include "ResourceObserver.h" +#include "TypeMagic.h" class ResourceHandler; -namespace Detail -{ -template <typename T> struct Function : public Function<decltype(&T::operator())> {}; -template <typename Ret, typename Arg> struct Function<Ret(*)(Arg)> : public Function<Ret(Arg)> {}; -template <typename Ret, typename Arg> struct Function<Ret(Arg)> -{ - using ReturnType = Ret; - using Argument = Arg; -}; -template <class C, typename Ret, typename Arg> struct Function<Ret(C::*)(Arg)> : public Function<Ret(Arg)> {}; -template <class C, typename Ret, typename Arg> struct Function<Ret(C::*)(Arg) const> : public Function<Ret(Arg)> {}; -template <typename F> struct Function<F&> : public Function<F> {}; -template <typename F> struct Function<F&&> : public Function<F> {}; -} - /** Frontend class for resources * * Usage: @@ -39,10 +25,11 @@ template <typename F> struct Function<F&&> : public Function<F> {}; * placeholder (if present). This means a resource stays valid while it's still used ("applied to" etc.) * by something. When nothing uses it anymore it gets deleted. * - * \note Always pass resource around using ResourcePtr! Copy and move constructors are disabled for a reason. + * @note Always pass resource around using Resource::Ptr! Copy and move constructors are disabled for a reason. */ class Resource : public std::enable_shared_from_this<Resource> { + // only allow creation from Resource::create and disallow passing around non-pointers explicit Resource(const QString &resource); Resource(const Resource &) = delete; Resource(Resource &&) = delete; @@ -51,11 +38,9 @@ public: ~Resource(); - /// The returned pointer needs to be stored until either Resource::then is called, or it is used as the argument to Resource::placeholder. - static Ptr create(const QString &resource); - - /// This can e.g. be used to set a local icon as the placeholder while a slow (remote) icon is fetched - Ptr placeholder(Ptr other); + /// The returned pointer needs to be stored until either Resource::applyTo or Resource::then is called, or it is passed as + /// a placeholder to Resource::create itself. + static Ptr create(const QString &resource, Ptr placeholder = nullptr); /// Use these functions to specify what should happen when e.g. the resource changes Ptr applyTo(ResourceObserver *observer); @@ -63,30 +48,49 @@ public: template<typename Func> Ptr then(Func &&func) { - using Arg = typename std::remove_cv< - typename std::remove_reference<typename Detail::Function<Func>::Argument>::type - >::type; - return applyTo(new FunctionResourceObserver< - typename Detail::Function<Func>::ReturnType, - Arg, Func - >(std::forward<Func>(func))); + // Arg will be the functions argument with references and cv-qualifiers (const, volatile) removed + using Arg = TypeMagic::CleanType<typename TypeMagic::Function<Func>::Argument>; + // Ret will be the functions return type + using Ret = typename TypeMagic::Function<Func>::ReturnType; + + // FunctionResourceObserver<ReturnType, ArgumentType, FunctionSignature> + return applyTo(new FunctionResourceObserver<Ret, Arg, Func>(std::forward<Func>(func))); } /// Retrieve the currently active resource. If it's type is different from T a conversion will be attempted. template<typename T> T getResource() const { return getResourceInternal(qMetaTypeId<T>()).template value<T>(); } + + /// @internal Used by ResourceObserver and ResourceProxyModel QVariant getResourceInternal(const int typeId) const; + /** Register a new ResourceHandler. T needs to inherit from ResourceHandler + * Usage: Resource::registerHandler<MyResourceHandler>("myid"); + */ template<typename T> static void registerHandler(const QString &id) { m_handlers.insert(id, [](const QString &res) { return std::make_shared<T>(res); }); } + /** Register a new resource transformer + * Resource transformers are functions that are responsible for converting between different types, + * for example converting from a QByteArray to a QPixmap. They are registered "externally" because not + * all types might be available in this library, for example gui types like QPixmap. + * + * Usage: Resource::registerTransformer([](const InputType &type) { return OutputType(type); }); + * This assumes that OutputType has a constructor that takes InputType as an argument. More + * complicated transformers can of course also be registered. + * + * When a ResourceObserver requests a type that's different from the actual resource type, a matching + * transformer will be looked up from the list of transformers. + * @note Only one-stage transforms will be performed (you can't registerTransformers for QString => int + * and int => float and expect QString to automatically be transformed into a float. + */ template<typename Func> static void registerTransformer(Func &&func) { - using Out = typename Detail::Function<Func>::ReturnType; - using In = typename std::remove_cv<typename std::remove_reference<typename Detail::Function<Func>::Argument>::type>::type; + using Out = typename TypeMagic::Function<Func>::ReturnType; + using In = TypeMagic::CleanType<typename TypeMagic::Function<Func>::Argument>; static_assert(!std::is_same<Out, In>::value, "It does not make sense to transform a value to itself"); m_transfomers.insert(qMakePair(qMetaTypeId<In>(), qMetaTypeId<Out>()), [func](const QVariant &in) { @@ -94,23 +98,33 @@ public: }); } -private: +private: // half private, implementation details friend class ResourceHandler; + // the following three functions are called by ResourceHandlers + /** Notifies the observers. They will call Resource::getResourceInternal which will call ResourceHandler::result + * or delegate to it's placeholder. + */ void reportResult(); void reportFailure(const QString &reason); void reportProgress(const int progress); friend class ResourceObserver; + /// Removes observer from the list of observers so that we don't attempt to notify something that doesn't exist void notifyObserverDeleted(ResourceObserver *observer); -private: +private: // truly private QList<ResourceObserver *> m_observers; std::shared_ptr<ResourceHandler> m_handler = nullptr; Ptr m_placeholder = nullptr; + const QString m_resource; + + static QString storageIdentifier(const QString &id, Ptr placeholder = nullptr); + QString storageIdentifier() const; // a list of resource handler factories, registered using registerHandler static QMap<QString, std::function<std::shared_ptr<ResourceHandler>(const QString &)>> m_handlers; // a list of resource transformers, registered using registerTransformer static QMap<QPair<int, int>, std::function<QVariant(QVariant)>> m_transfomers; + // a list of resources so that we can reuse them static QMap<QString, std::weak_ptr<Resource>> m_resources; }; diff --git a/logic/resources/ResourceHandler.h b/logic/resources/ResourceHandler.h index c1105efc..a4f638ec 100644 --- a/logic/resources/ResourceHandler.h +++ b/logic/resources/ResourceHandler.h @@ -17,7 +17,8 @@ public: virtual ~ResourceHandler() {} void setResource(Resource *resource) { m_resource = resource; } - // reimplement this if you need to do something after you have been put in a shared pointer + /// reimplement this if you need to do something after you have been put in a shared pointer + // we do this instead of inheriting from std::enable_shared_from_this virtual void init(std::shared_ptr<ResourceHandler>&) {} QVariant result() const { return m_result; } diff --git a/logic/resources/ResourceObserver.h b/logic/resources/ResourceObserver.h index 27430d42..ef946c32 100644 --- a/logic/resources/ResourceObserver.h +++ b/logic/resources/ResourceObserver.h @@ -51,6 +51,11 @@ private: QMetaProperty m_property; }; +/** Observer for functions, lambdas etc. + * Template arguments: + * * We need Ret and Arg in order to create the std::function + * * We need Func in order to std::forward the function + */ template <typename Ret, typename Arg, typename Func> class FunctionResourceObserver : public ResourceObserver { diff --git a/logic/resources/ResourceProxyModel.cpp b/logic/resources/ResourceProxyModel.cpp index 6ff11367..f026d9a9 100644 --- a/logic/resources/ResourceProxyModel.cpp +++ b/logic/resources/ResourceProxyModel.cpp @@ -5,8 +5,6 @@ #include "Resource.h" #include "ResourceObserver.h" -//Q_DECLARE_METATYPE(QVector<int>) - class ModelResourceObserver : public ResourceObserver { public: @@ -20,6 +18,7 @@ public: { if (m_index.isValid()) { + // the resource changed, pretend to be the model and notify the views of the update. they will re-poll the model which will return the new resource value QMetaObject::invokeMethod(const_cast<QAbstractItemModel *>(m_index.model()), "dataChanged", Qt::QueuedConnection, Q_ARG(QModelIndex, m_index), Q_ARG(QModelIndex, m_index), Q_ARG(QVector<int>, QVector<int>() << m_role)); @@ -39,24 +38,29 @@ ResourceProxyModel::ResourceProxyModel(const int resultTypeId, QObject *parent) QVariant ResourceProxyModel::data(const QModelIndex &proxyIndex, int role) const { const QModelIndex mapped = mapToSource(proxyIndex); + // valid cell that's a Qt::DecorationRole and that contains a non-empty string if (mapped.isValid() && role == Qt::DecorationRole && !mapToSource(proxyIndex).data(role).toString().isEmpty()) { + // do we already have a resource for this index? if (!m_resources.contains(mapped)) { - Resource::Ptr res = Resource::create(mapToSource(proxyIndex).data(role).toString()) - ->applyTo(new ModelResourceObserver(proxyIndex, role)); - - const QVariant placeholder = mapped.data(PlaceholderRole); - if (!placeholder.isNull() && placeholder.type() == QVariant::String) + Resource::Ptr placeholder; + const QVariant placeholderIdentifier = mapped.data(PlaceholderRole); + if (!placeholderIdentifier.isNull() && placeholderIdentifier.type() == QVariant::String) { - res->placeholder(Resource::create(placeholder.toString())); + placeholder = Resource::create(placeholderIdentifier.toString()); } + // create the Resource and apply the observer for models + Resource::Ptr res = Resource::create(mapToSource(proxyIndex).data(role).toString(), placeholder) + ->applyTo(new ModelResourceObserver(proxyIndex, role)); + m_resources.insert(mapped, res); } return m_resources.value(mapped)->getResourceInternal(m_resultTypeId); } + // otherwise fall back to the source model return mapped.data(role); } @@ -70,7 +74,8 @@ void ResourceProxyModel::setSourceModel(QAbstractItemModel *model) { connect(model, &QAbstractItemModel::dataChanged, this, [this](const QModelIndex &tl, const QModelIndex &br, const QVector<int> &roles) { - if (roles.contains(Qt::DecorationRole) || roles.isEmpty()) + // invalidate resources so that they will be re-created + if (roles.contains(Qt::DecorationRole) || roles.contains(PlaceholderRole) || roles.isEmpty()) { const QItemSelectionRange range(tl, br); for (const QModelIndex &index : range.indexes()) @@ -78,25 +83,6 @@ void ResourceProxyModel::setSourceModel(QAbstractItemModel *model) m_resources.remove(index); } } - else if (roles.contains(PlaceholderRole)) - { - const QItemSelectionRange range(tl, br); - for (const QModelIndex &index : range.indexes()) - { - if (m_resources.contains(index)) - { - const QVariant placeholder = index.data(PlaceholderRole); - if (!placeholder.isNull() && placeholder.type() == QVariant::String) - { - m_resources.value(index)->placeholder(Resource::create(placeholder.toString())); - } - else - { - m_resources.value(index)->placeholder(nullptr); - } - } - } - } }); } QIdentityProxyModel::setSourceModel(model); diff --git a/logic/resources/ResourceProxyModel.h b/logic/resources/ResourceProxyModel.h index 9db09545..f9a83d1d 100644 --- a/logic/resources/ResourceProxyModel.h +++ b/logic/resources/ResourceProxyModel.h @@ -20,6 +20,7 @@ public: QVariant data(const QModelIndex &proxyIndex, int role) const override; void setSourceModel(QAbstractItemModel *model) override; + /// Helper function, usage: m_view->setModel(ResourceProxyModel::mixin<QIcon>(m_model)); template <typename T> static QAbstractItemModel *mixin(QAbstractItemModel *model) { -- cgit v1.2.3