Commit 06cdf66e authored by Nicolas Carion's avatar Nicolas Carion

Simplify, refactor and test logic for bin tree gestion

parent 40793f40
......@@ -34,7 +34,7 @@ AbstractTreeModel::AbstractTreeModel(QObject *parent)
std::shared_ptr<AbstractTreeModel> AbstractTreeModel::construct(QObject *parent)
{
std::shared_ptr<AbstractTreeModel> self(new AbstractTreeModel(parent));
self->rootItem.reset(new TreeItem(QList<QVariant>(), self));
self->rootItem.reset(new TreeItem(QList<QVariant>(), self, true));
return self;
}
......
......@@ -87,13 +87,13 @@ protected:
/* @brief Send the appropriate notification related to a row that we have appended
@param row is the new element
*/
virtual void notifyRowAppended(const std::shared_ptr<TreeItem> &row);
void notifyRowAppended(const std::shared_ptr<TreeItem> &row);
/* @brief Send the appropriate notification related to a row that we are deleting
@param item is the parent of the row being deleted
@param row is the index of the row being deleted
*/
virtual void notifyRowAboutToDelete(std::shared_ptr<TreeItem> item, int row);
void notifyRowAboutToDelete(std::shared_ptr<TreeItem> item, int row);
/* @brief Send the appropriate notification related to a row that we have appended
@param row is the old element
......
......@@ -25,18 +25,19 @@
#include <numeric>
#include <utility>
TreeItem::TreeItem(const QList<QVariant> &data, const std::shared_ptr<AbstractTreeModel> &model, int id)
TreeItem::TreeItem(const QList<QVariant> &data, const std::shared_ptr<AbstractTreeModel> &model, bool isRoot, int id)
: m_itemData(data)
, m_model(model)
, m_depth(0)
, m_id(id == -1 ? AbstractTreeModel::getNextId() : id)
, m_isInModel(false)
, m_isRoot(isRoot)
{
}
std::shared_ptr<TreeItem> TreeItem::construct(const QList<QVariant> &data, std::shared_ptr<AbstractTreeModel> model, int id)
std::shared_ptr<TreeItem> TreeItem::construct(const QList<QVariant> &data, std::shared_ptr<AbstractTreeModel> model, bool isRoot, int id)
{
std::shared_ptr<TreeItem> self(new TreeItem(data, std::move(model), id));
std::shared_ptr<TreeItem> self(new TreeItem(data, std::move(model), isRoot, id));
baseFinishConstruct(self);
return self;
}
......@@ -45,35 +46,21 @@ std::shared_ptr<TreeItem> TreeItem::construct(const QList<QVariant> &data, std::
void TreeItem::baseFinishConstruct(const std::shared_ptr<TreeItem> &self)
{
qDebug() << "FINISHED constructing " << self->getId();
if (auto ptr = self->m_model.lock()) {
ptr->registerItem(self);
} else {
qDebug() << "Error : construction of treeItem failed because parent model is not available anymore";
Q_ASSERT(false);
if (self->m_isRoot) {
self->registerSelf();
}
}
TreeItem::~TreeItem()
{
if (auto ptr = m_model.lock()) {
ptr->deregisterItem(m_id, this);
}
deregisterSelf();
}
std::shared_ptr<TreeItem> TreeItem::appendChild(const QList<QVariant> &data)
{
if (auto ptr = m_model.lock()) {
ptr->notifyRowAboutToAppend(shared_from_this());
auto child = construct(data, ptr);
child->m_parentItem = shared_from_this();
qDebug() << "appending child" << child->getId() << "to " << m_id;
child->m_depth = m_depth + 1;
int id = child->getId();
m_childItems.push_back(child);
auto it = std::prev(m_childItems.end());
m_iteratorTable[id] = it;
ptr->notifyRowAppended(child);
m_isInModel = true;
auto child = construct(data, ptr, false);
appendChild(child);
return child;
}
qDebug() << "ERROR: Something went wrong when appending child in TreeItem. Model is not available anymore";
......@@ -87,12 +74,11 @@ void TreeItem::appendChild(std::shared_ptr<TreeItem> child)
ptr->notifyRowAboutToAppend(shared_from_this());
child->m_depth = m_depth + 1;
child->m_parentItem = shared_from_this();
qDebug() << "appending child2" << child->getId() << "to " << m_id;
int id = child->getId();
auto it = m_childItems.insert(m_childItems.end(), child);
m_iteratorTable[id] = it;
child->registerSelf();
ptr->notifyRowAppended(child);
m_isInModel = true;
} else {
qDebug() << "ERROR: Something went wrong when appending child in TreeItem. Model is not available anymore";
Q_ASSERT(false);
......@@ -122,7 +108,7 @@ void TreeItem::moveChild(int ix, std::shared_ptr<TreeItem> child)
ptr->notifyRowAppended(child);
m_isInModel = true;
} else {
qDebug() << "ERROR: Something went wrong when inserting child in TreeItem. Model is not available anymore";
qDebug() << "ERROR: Something went wrong when moving child in TreeItem. Model is not available anymore";
Q_ASSERT(false);
}
}
......@@ -140,8 +126,8 @@ void TreeItem::removeChild(const std::shared_ptr<TreeItem> &child)
m_iteratorTable.erase(child->getId());
child->m_depth = 0;
child->m_parentItem.reset();
child->deregisterSelf();
ptr->notifyRowDeleted();
m_isInModel = false;
} else {
qDebug() << "ERROR: Something went wrong when removing child in TreeItem. Model is not available anymore";
Q_ASSERT(false);
......@@ -150,6 +136,7 @@ void TreeItem::removeChild(const std::shared_ptr<TreeItem> &child)
void TreeItem::changeParent(std::shared_ptr<TreeItem> newParent)
{
Q_ASSERT(!m_isRoot);
qDebug() << "changing parent of " << m_id;
if (auto ptr = m_parentItem.lock()) {
ptr->removeChild(shared_from_this());
......@@ -208,11 +195,34 @@ int TreeItem::getId() const
return m_id;
}
void TreeItem::setIsInModel(bool isInModel)
{
m_isInModel = isInModel;
}
bool TreeItem::isInModel() const
{
return m_isInModel;
}
void TreeItem::registerSelf()
{
for (const auto &child : m_childItems) {
child->registerSelf();
}
if (auto ptr = m_model.lock()) {
ptr->registerItem(shared_from_this());
m_isInModel = true;
} else {
qDebug() << "Error : construction of treeItem failed because parent model is not available anymore";
Q_ASSERT(false);
}
}
void TreeItem::deregisterSelf()
{
for (const auto &child : m_childItems) {
child->deregisterSelf();
}
if (m_isInModel) {
if (auto ptr = m_model.lock()) {
ptr->deregisterItem(m_id, this);
m_isInModel = false;
}
}
}
......@@ -28,6 +28,17 @@
#include <unordered_map>
/* @brief This class is a generic class to represent items of a tree-like model
It works in tandem with AbstractTreeModel or one of its derived classes.
There is a registration mechanism that takes place: each TreeItem holds a unique Id
that can allow to retrieve it directly from the model.
A TreeItem registers itself to the model as soon as it gets a proper parent (the node
above it in the hierarchy). This means that upon creation, the TreeItem is NOT
registered, because at this point it doesn't belong to any parent.
The only exception is for the rootItem, which is always registered.
Note that the root is a special object. In particular, it must stay at the root and
must not be declared as the child of any other item.
*/
class AbstractTreeModel;
......@@ -38,16 +49,17 @@ public:
@param data List of data elements (columns) of the created item
@param model Pointer to the model to which this elem belongs to
@param parentItem address of the parent if the child is not orphan
@param isRoot is true if the object is the topmost item of the tree
@param id of the newly created item. If left to -1, the id is assigned automatically
@return a ptr to the constructed item
*/
static std::shared_ptr<TreeItem> construct(const QList<QVariant> &data, std::shared_ptr<AbstractTreeModel> model, int id = -1);
static std::shared_ptr<TreeItem> construct(const QList<QVariant> &data, std::shared_ptr<AbstractTreeModel> model, bool isRoot, int id = -1);
friend class AbstractTreeModel;
protected:
// This is protected. Call construct instead
explicit TreeItem(const QList<QVariant> &data, const std::shared_ptr<AbstractTreeModel> &model, int id = -1);
explicit TreeItem(const QList<QVariant> &data, const std::shared_ptr<AbstractTreeModel> &model, bool isRoot, int id = -1);
public:
virtual ~TreeItem();
......@@ -103,6 +115,9 @@ public:
/* @brief Return the id of the current item*/
int getId() const;
/* @brief Return true if the current item has been registered */
bool isInModel() const;
/* @brief This is similar to the std::accumulate function, except that it
operates on the whole subtree
@param init is the initial value of the operation
......@@ -112,14 +127,16 @@ public:
/* @brief Returns true if the model has been notified about the existence of this object
*/
bool isInModel() const;
void setIsInModel(bool isInModel);
protected:
/* @brief Finish construction of object given its pointer
This is a separated function so that it can be called from derived classes */
static void baseFinishConstruct(const std::shared_ptr<TreeItem> &self);
/* @brief Helper functions to handle registration / deregistration to the model */
void registerSelf();
void deregisterSelf();
std::list<std::shared_ptr<TreeItem>> m_childItems;
std::unordered_map<int, std::list<std::shared_ptr<TreeItem>>::iterator>
m_iteratorTable; // this logs the iterator associated which each child id. This allows easy access of a child based on its id.
......@@ -132,6 +149,7 @@ protected:
int m_id;
bool m_isInModel;
bool m_isRoot;
};
template <class T, class BinaryOperation> T TreeItem::accumulate(T init, BinaryOperation op)
......
......@@ -29,8 +29,8 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
#include <QPainter>
#include <QVariant>
AbstractProjectItem::AbstractProjectItem(PROJECTITEMTYPE type, const QString &id, const std::shared_ptr<ProjectItemModel> &model)
: TreeItem(QList<QVariant>(), std::static_pointer_cast<AbstractTreeModel>(model))
AbstractProjectItem::AbstractProjectItem(PROJECTITEMTYPE type, const QString &id, const std::shared_ptr<ProjectItemModel> &model, bool isRoot)
: TreeItem(QList<QVariant>(), std::static_pointer_cast<AbstractTreeModel>(model), isRoot)
, m_name()
, m_description()
, m_thumbnail(QIcon())
......@@ -47,7 +47,7 @@ AbstractProjectItem::AbstractProjectItem(PROJECTITEMTYPE type, const QString &id
}
AbstractProjectItem::AbstractProjectItem(PROJECTITEMTYPE type, const QDomElement &description, const std::shared_ptr<ProjectItemModel> &model)
: TreeItem(QList<QVariant>(), std::static_pointer_cast<AbstractTreeModel>(model))
: TreeItem(QList<QVariant>(), std::static_pointer_cast<AbstractTreeModel>(model), false)
, m_name()
, m_description()
, m_thumbnail(QIcon())
......
......@@ -54,9 +54,12 @@ public:
/**
* @brief Constructor.
* @param parent parent this item should be added to
* @param type is the type of the bin item
* @param id is the binId
* @param model is the ptr to the item model
* @param isRoot is true if this is the topmost folder
*/
AbstractProjectItem(PROJECTITEMTYPE type, const QString &id, const std::shared_ptr<ProjectItemModel> &model);
AbstractProjectItem(PROJECTITEMTYPE type, const QString &id, const std::shared_ptr<ProjectItemModel> &model, bool isRoot = false);
/**
* @brief Creates a project item upon project load.
* @param description element for this item.
......
......@@ -47,7 +47,7 @@ std::shared_ptr<ProjectFolder> ProjectFolder::construct(const QString &id, const
}
ProjectFolder::ProjectFolder(std::shared_ptr<ProjectItemModel> model)
: AbstractProjectItem(AbstractProjectItem::FolderItem, QString::number(-1), model)
: AbstractProjectItem(AbstractProjectItem::FolderItem, QString::number(-1), model, true)
{
m_name = QStringLiteral("root");
}
......
......@@ -393,32 +393,11 @@ bool ProjectItemModel::requestBinClipDeletion(std::shared_ptr<AbstractProjectIte
int parentId = -1;
if (auto ptr = clip->parent()) parentId = ptr->getId();
int id = clip->getId();
Fun operation = [this, id]() {
/* Deletion simply deregister clip and remove it from parent.
The actual object is not actually deleted, because a shared_pointer to it
is captured by the reverse operation.
Actual deletions occurs when the undo object is destroyed.
*/
auto currentClip = std::static_pointer_cast<AbstractProjectItem>(m_allItems[id].lock());
Q_ASSERT(currentClip);
if (!currentClip) return false;
auto parent = currentClip->parent();
parent->removeChild(currentClip);
return true;
};
Fun reverse = [this, clip, parentId]() {
/* To undo deletion, we reregister the clip */
std::shared_ptr<TreeItem> parent;
if (parentId != -1) {
parent = getItemById(parentId);
}
clip->changeParent(parent);
return true;
};
Fun operation = removeBin_lambda(id);
Fun reverse = addBin_lambda(clip, parentId);
bool res = operation();
if (res) {
UPDATE_UNDO_REDO(operation, reverse, undo, redo);
res = clip->selfSoftDelete(undo, redo);
}
return res;
}
......@@ -436,13 +415,6 @@ void ProjectItemModel::deregisterItem(int id, TreeItem *item)
AbstractTreeModel::deregisterItem(id, item);
}
void ProjectItemModel::notifyRowAppended(const std::shared_ptr<TreeItem> &row)
{
auto binElem = std::static_pointer_cast<AbstractProjectItem>(row);
manageBinClipInsertion(binElem);
AbstractTreeModel::notifyRowAppended(row);
}
void ProjectItemModel::manageBinClipInsertion(const std::shared_ptr<AbstractProjectItem> &binElem)
{
switch(binElem->itemType()) {
......@@ -459,14 +431,6 @@ void ProjectItemModel::manageBinClipInsertion(const std::shared_ptr<AbstractProj
}
}
void ProjectItemModel::notifyRowAboutToDelete(std::shared_ptr<TreeItem> item, int row)
{
auto rowItem = item->child(row);
auto binElem = std::static_pointer_cast<AbstractProjectItem>(rowItem);
manageBinClipDeletion(binElem.get());
AbstractTreeModel::notifyRowAboutToDelete(item, row);
}
void ProjectItemModel::manageBinClipDeletion(AbstractProjectItem *binElem)
{
switch(binElem->itemType()) {
......@@ -505,28 +469,11 @@ bool ProjectItemModel::requestAddFolder(QString &id, const QString &name, const
id = QString::number(getFreeFolderId());
}
std::shared_ptr<ProjectFolder> new_folder = ProjectFolder::construct(id, name, std::static_pointer_cast<ProjectItemModel>(shared_from_this()));
parentFolder->appendChild(new_folder);
Fun operation = addBin_lambda(new_folder, parentFolder->getId());
int folderId = new_folder->getId();
Fun operation = [this, new_folder, parentId]() {
/* Insertion is simply setting the parent of the folder.*/
std::shared_ptr<ProjectFolder> parent = getFolderByBinId(parentId);
if (!parent) {
return false;
}
new_folder->changeParent(parent);
return true;
};
Fun reverse = [this, folderId]() {
/* To undo insertion, we deregister the clip */
auto folder = std::static_pointer_cast<AbstractProjectItem>(m_allItems[folderId].lock());
if (!folder) {
return false;
}
auto parent = folder->parent();
parent->removeChild(folder);
return true;
};
Fun reverse = removeBin_lambda(folderId);
bool res = operation();
Q_ASSERT(new_folder->isInModel());
if (res) {
UPDATE_UNDO_REDO(operation, reverse, undo, redo);
}
......@@ -572,3 +519,39 @@ bool ProjectItemModel::requestRenameFolder(std::shared_ptr<AbstractProjectItem>
}
return res;
}
Fun ProjectItemModel::addBin_lambda(std::shared_ptr<AbstractProjectItem> new_item, int parentId)
{
return [this, new_item, parentId]() {
/* Insertion is simply setting the parent of the item.*/
std::shared_ptr<TreeItem> parent;
if (parentId != -1) {
parent = getItemById(parentId);
if (!parent) {
Q_ASSERT(parent);
return false;
}
}
new_item->changeParent(parent);
return true;
};
}
Fun ProjectItemModel::removeBin_lambda(int binId)
{
return [this, binId]() {
/* Deletion simply deregister clip and remove it from parent.
The actual object is not actually deleted, because a shared_pointer to it
is captured by the reverse operation.
Actual deletions occurs when the undo object is destroyed.
*/
auto binItem = std::static_pointer_cast<AbstractProjectItem>(m_allItems[binId].lock());
Q_ASSERT(binItem);
if (!binItem) {
return false;
}
auto parent = binItem->parent();
parent->removeChild(binItem);
return true;
};
}
......@@ -126,18 +126,6 @@ public:
/* Same functions but pushes the undo object directly */
bool requestRenameFolder(std::shared_ptr<AbstractProjectItem> folder, const QString &name);
/* @brief Manage insertion in the tree hierarchy.
Note that the element has normally already been registered through registerItem,
this function is called when its parent is defined.
@param row is the new element
*/
void notifyRowAppended(const std::shared_ptr<TreeItem> &row) override;
/* @brief Manage deletion in the tree hierarchy
@param item is the parent of the row being deleted
@param row is the index of the row being deleted
*/
void notifyRowAboutToDelete(std::shared_ptr<TreeItem> item, int row) override;
/* @brief Register the existence of a new element
*/
void registerItem(const std::shared_ptr<TreeItem> &item) override;
......@@ -164,6 +152,11 @@ protected:
/* @brief Helper function to generate a lambda that rename a folder */
Fun requestRenameFolder_lambda(std::shared_ptr<AbstractProjectItem> folder, const QString &newName);
/* @brief Helper function to generate a lambda that adds a bin item to view */
Fun addBin_lambda(std::shared_ptr<AbstractProjectItem> new_item, int parentId);
/* @brief Helper function to generate a lambda that removes a bin item from view */
Fun removeBin_lambda(int binId);
public slots:
/** @brief An item in the list was modified, notify */
void onItemUpdated(std::shared_ptr<AbstractProjectItem> item);
......
......@@ -44,7 +44,7 @@ std::shared_ptr<EffectTreeModel> EffectTreeModel::construct(const QString &categ
<< "ID"
<< "Type"
<< "isFav";
self->rootItem = TreeItem::construct(rootData, self);
self->rootItem = TreeItem::construct(rootData, self, true);
QHash<QString, std::shared_ptr<TreeItem>> effectCategory; // category in which each effect should land.
......
......@@ -27,7 +27,7 @@
#include <utility>
AbstractEffectItem::AbstractEffectItem(const QList<QVariant> &data, const std::shared_ptr<AbstractTreeModel> &stack)
: TreeItem(data, stack)
: TreeItem(data, stack, false)
, m_enabled(true)
, m_effectStackEnabled(true)
{
......
......@@ -48,7 +48,7 @@ std::shared_ptr<ProfileTreeModel> ProfileTreeModel::construct(QObject *parent)
<< "sample_aspect_ratio"
<< "fps"
<< "colorspace";
self->rootItem = TreeItem::construct(rootData, self);
self->rootItem = TreeItem::construct(rootData, self, true);
ProfileRepository::get()->refresh();
QVector<QPair<QString, QString>> profiles = ProfileRepository::get()->getAllProfiles();
......
......@@ -38,7 +38,7 @@ std::shared_ptr<TransitionTreeModel> TransitionTreeModel::construct(bool flat, Q
<< "ID"
<< "Type"
<< "isFav";
self->rootItem = TreeItem::construct(rootData, self);
self->rootItem = TreeItem::construct(rootData, self, true);
// We create categories, if requested
std::shared_ptr<TreeItem> compoCategory, transCategory;
......
......@@ -22,26 +22,25 @@ TEST_CASE("Basic tree testing", "[TreeModel]")
SECTION("Item creation Test")
{
auto item = TreeItem::construct(QList<QVariant>{QString("test")}, model);
auto item = TreeItem::construct(QList<QVariant>{QString("test")}, model, false);
int id = item->getId();
REQUIRE(item->depth() == 0);
// check that a valid Id has been assigned
REQUIRE(id != -1);
// check that we can query the item in the model
REQUIRE(model->getItemById(id) == item);
REQUIRE(model->m_allItems.size() == 1);
// check that the item is not yet registered (not valid parent)
REQUIRE(model->m_allItems.size() == 0);
// Assign this to a parent
model->getRoot()->appendChild(item);
// Now the item should be registered, we query it
REQUIRE(model->m_allItems.size() == 1);
REQUIRE(model->getItemById(id) == item);
REQUIRE(item->depth() == 1);
REQUIRE(model->rowCount() == 1);
qDebug() << (void*)model->getRoot().get() << (void*)item.get();
qDebug() << model->getIndexFromItem(item);
REQUIRE(model->rowCount(model->getIndexFromItem(item)) == 0);
REQUIRE(model->m_allItems.size() == 1);
// Retrieve data member
REQUIRE(model->data(model->getIndexFromItem(item), 0) == QStringLiteral("test"));
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment