Skip to content

Treeitem(Model): Fixes & Optimizations #1067

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
daschuer merged 3 commits into from
Dec 14, 2016
Merged

Treeitem(Model): Fixes & Optimizations #1067

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e495718
Fix memory leak in TreeItem
uklotzde Dec 8, 2016
7d2e0fc
Improve and simplify the design/interface of TreeItem
uklotzde Dec 8, 2016
9eb92d0
Simplify and safeguard memory management of TreeItem(Model)
uklotzde Dec 8, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 7 additions & 11 deletions src/library/autodj/autodjfeature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,14 +56,15 @@ AutoDJFeature::AutoDJFeature(Library* pLibrary,


// Create the "Crates" tree-item under the root item.
TreeItem* root = m_childModel.getItem(QModelIndex());
m_pCratesTreeItem = new TreeItem(tr("Crates"), "", this, root);
auto pRootItem = std::make_unique<TreeItem>(this);
m_pCratesTreeItem = pRootItem->appendChild(tr("Crates"));
m_pCratesTreeItem->setIcon(QIcon(":/images/library/ic_library_crates.png"));
root->appendChild(m_pCratesTreeItem);

// Create tree-items under "Crates".
constructCrateChildModel();

m_childModel.setRootItem(std::move(pRootItem));

// Be notified when the status of crates changes.
connect(&m_crateDao, SIGNAL(added(int)),
this, SLOT(slotCrateAdded(int)));
Expand Down Expand Up @@ -222,13 +223,9 @@ void AutoDJFeature::slotCrateAutoDjChanged(int crateId, bool added) {
// Add our record of this crate-ID and name.
m_crateList.append(qMakePair(crateId, strName));

// Create a tree-item for this crate.
TreeItem* item = new TreeItem(strName, strName, this,
m_pCratesTreeItem);

// Prepare to add it to the "Crates" tree-item.
QList<TreeItem*> lstItems;
lstItems.append(item);
lstItems.append(new TreeItem(this, strName));

// Add it to the "Crates" tree-item.
QModelIndex oCratesIndex = m_childModel.index(0, 0);
Expand Down Expand Up @@ -326,8 +323,7 @@ void AutoDJFeature::constructCrateChildModel() {
m_crateList.append(qMakePair(id, name));

// Create the TreeItem for this crate.
TreeItem* item = new TreeItem(name, name, this, m_pCratesTreeItem);
m_pCratesTreeItem->appendChild(item);
m_pCratesTreeItem->appendChild(name);
}
}

Expand All @@ -337,7 +333,7 @@ void AutoDJFeature::onRightClickChild(const QPoint& globalPos,
m_lastRightClickedIndex = index;

TreeItem *item = static_cast<TreeItem*>(index.internalPointer());
QString crateName = item->dataPath().toString();
QString crateName = item->getLabel();
if (crateName.length() > 0) {
// A crate was right-clicked.
// Bring up the context menu.
Expand Down
35 changes: 13 additions & 22 deletions src/library/banshee/bansheefeature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,25 +91,19 @@ void BansheeFeature::activate() {

m_isActivated = true;

TreeItem* playlist_root = new TreeItem();

QList<struct BansheeDbConnection::Playlist> list = m_connection.getPlaylists();

struct BansheeDbConnection::Playlist playlist;
foreach (playlist, list) {
auto pRootItem = std::make_unique<TreeItem>(this);
QList<BansheeDbConnection::Playlist> playlists = m_connection.getPlaylists();
for (const BansheeDbConnection::Playlist& playlist: playlists) {
qDebug() << playlist.name;
// append the playlist to the child model
TreeItem *item = new TreeItem(playlist.name, playlist.playlistId, this, playlist_root);
playlist_root->appendChild(item);
pRootItem->appendChild(playlist.name, playlist.playlistId);
}
m_childModel.setRootItem(std::move(pRootItem));

if (playlist_root) {
m_childModel.setRootItem(playlist_root);
if (m_isActivated) {
activate();
}
qDebug() << "Banshee library loaded: success";
if (m_isActivated) {
activate();
}
qDebug() << "Banshee library loaded: success";

//calls a slot in the sidebarmodel such that 'isLoading' is removed from the feature title.
m_title = tr("Banshee");
Expand All @@ -123,11 +117,9 @@ void BansheeFeature::activate() {

void BansheeFeature::activateChild(const QModelIndex& index) {
TreeItem *item = static_cast<TreeItem*>(index.internalPointer());
//qDebug() << "BansheeFeature::activateChild " << item->data() << " " << item->dataPath();
QString playlist = item->dataPath().toString();
int playlistID = playlist.toInt();
int playlistID = item->getData().toInt();
if (playlistID > 0) {
qDebug() << "Activating " << item->data().toString();
qDebug() << "Activating " << item->getLabel();
m_pBansheePlaylistModel->setTableModel(playlistID);
emit(showTrackModel(m_pBansheePlaylistModel));
emit(enableCoverArtDisplay(false));
Expand All @@ -141,10 +133,9 @@ TreeItemModel* BansheeFeature::getChildModel() {
void BansheeFeature::appendTrackIdsFromRightClickIndex(QList<TrackId>* trackIds, QString* pPlaylist) {
if (m_lastRightClickedIndex.isValid()) {
TreeItem *item = static_cast<TreeItem*>(m_lastRightClickedIndex.internalPointer());
*pPlaylist = item->data().toString();
QString playlistStId = item->dataPath().toString();
int playlistID = playlistStId.toInt();
qDebug() << "BansheeFeature::appendTrackIdsFromRightClickIndex " << *pPlaylist << " " << playlistStId;
*pPlaylist = item->getLabel();
int playlistID = item->getData().toInt();
qDebug() << "BansheeFeature::appendTrackIdsFromRightClickIndex " << *pPlaylist << " " << playlistID;
if (playlistID > 0) {
BansheePlaylistModel* pPlaylistModelToAdd = new BansheePlaylistModel(this, m_pTrackCollection, &m_connection);
pPlaylistModelToAdd->setTableModel(playlistID);
Expand Down
4 changes: 2 additions & 2 deletions src/library/baseexternallibraryfeature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ void BaseExternalLibraryFeature::appendTrackIdsFromRightClickIndex(QList<TrackId
return;
}

// Qt::UserRole asks TreeItemModel for the TreeItem's dataPath. We need to
// use the dataPath because models with nested playlists need to use the
// Qt::UserRole asks TreeItemModel for the TreeItem's data. We need to
// use the data because models with nested playlists need to use the
// full path/name of the playlist.
*pPlaylist = m_lastRightClickedIndex.data(Qt::UserRole).toString();
QScopedPointer<BaseSqlTableModel> pPlaylistModelToAdd(
Expand Down
19 changes: 9 additions & 10 deletions src/library/baseplaylistfeature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,17 +117,17 @@ BasePlaylistFeature::~BasePlaylistFeature() {

int BasePlaylistFeature::playlistIdFromIndex(QModelIndex index) {
TreeItem* item = static_cast<TreeItem*>(index.internalPointer());
if (item == NULL) {
if (item == nullptr) {
return -1;
}

QString dataPath = item->dataPath().toString();
bool ok = false;
int playlistId = dataPath.toInt(&ok);
if (!ok) {
int playlistId = item->getData().toInt(&ok);
if (ok) {
return playlistId;
} else {
return -1;
}
return playlistId;
}

void BasePlaylistFeature::activate() {
Expand Down Expand Up @@ -621,8 +621,6 @@ QModelIndex BasePlaylistFeature::constructChildModel(int selected_id) {
buildPlaylistList();
QList<TreeItem*> data_list;
int selected_row = -1;
// Access the invisible root item
TreeItem* root = m_childModel.getItem(QModelIndex());

int row = 0;
for (QList<QPair<int, QString> >::const_iterator it = m_playlistList.begin();
Expand All @@ -637,7 +635,7 @@ QModelIndex BasePlaylistFeature::constructChildModel(int selected_id) {
}

// Create the TreeItem whose parent is the invisible root item
TreeItem* item = new TreeItem(playlist_name, QString::number(playlist_id), this, root);
TreeItem* item = new TreeItem(this, playlist_name, playlist_id);
item->setBold(m_playlistsSelectedTrackIsIn.contains(playlist_id));

decorateChild(item, playlist_id);
Expand All @@ -663,7 +661,8 @@ void BasePlaylistFeature::updateChildModel(int selected_id) {

if (selected_id == playlist_id) {
TreeItem* item = m_childModel.getItem(indexFromPlaylistId(playlist_id));
item->setData(playlist_name, QString::number(playlist_id));
item->setLabel(playlist_name);
item->setData(playlist_id);
decorateChild(item, playlist_id);
}

Expand Down Expand Up @@ -701,7 +700,7 @@ void BasePlaylistFeature::slotTrackSelected(TrackPointer pTrack) {
}
m_playlistDao.getPlaylistsTrackIsIn(trackId, &m_playlistsSelectedTrackIsIn);

TreeItem* rootItem = m_childModel.getItem(QModelIndex());
TreeItem* rootItem = m_childModel.getRootItem();
if (rootItem == nullptr) {
return;
}
Expand Down
73 changes: 30 additions & 43 deletions src/library/browse/browsefeature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,16 +54,13 @@ BrowseFeature::BrowseFeature(QObject* parent,
m_proxyModel.setDynamicSortFilter(true);

// The invisible root item of the child model
TreeItem* rootItem = new TreeItem();
auto pRootItem = std::make_unique<TreeItem>(this);

m_pQuickLinkItem = new TreeItem(tr("Quick Links"), QUICK_LINK_NODE, this, rootItem);
rootItem->appendChild(m_pQuickLinkItem);
m_pQuickLinkItem = pRootItem->appendChild(tr("Quick Links"), QUICK_LINK_NODE);

// Create the 'devices' shortcut
#if defined(__WINDOWS__)
TreeItem* devices_link = new TreeItem(
tr("Devices"), DEVICE_NODE, this, rootItem);
rootItem->appendChild(devices_link);
TreeItem* devices_link = pRootItem->appendChild(tr("Devices"), DEVICE_NODE);
// show drive letters
QFileInfoList drives = QDir::drives();
// show drive letters
Expand All @@ -81,28 +78,20 @@ BrowseFeature::BrowseFeature(QObject* parent,
if (display_path.endsWith("/")) {
display_path.chop(1);
}
TreeItem* driveLetter = new TreeItem(
display_path, // Displays C:
drive.filePath(), // Displays C:/
this ,
devices_link);
devices_link->appendChild(driveLetter);
TreeItem* driveLetter =
devices_link->appendChild(
display_path, // Displays C:
drive.filePath()); // Displays C:/
}
#elif defined(__APPLE__)
// Apple hides the base Linux file structure But all devices are mounted at
// /Volumes
TreeItem* devices_link = new TreeItem(
tr("Devices"), "/Volumes/", this, rootItem);
rootItem->appendChild(devices_link);
pRootItem->appendChild(tr("Devices"), "/Volumes/");
#else // LINUX
TreeItem* devices_link = new TreeItem(
tr("Removable Devices"), "/media/", this, rootItem);
rootItem->appendChild(devices_link);
pRootItem->appendChild(tr("Removable Devices"), "/media/");

// show root directory on UNIX-based operating systems
TreeItem* root_folder_item = new TreeItem(
QDir::rootPath(), QDir::rootPath(), this, rootItem);
rootItem->appendChild(root_folder_item);
pRootItem->appendChild(QDir::rootPath(), QDir::rootPath());
#endif

// Just a word about how the TreeItem objects are used for the BrowseFeature:
Expand All @@ -122,12 +111,11 @@ BrowseFeature::BrowseFeature(QObject* parent,
foreach (QString quickLinkPath, m_quickLinkList) {
QString name = extractNameFromPath(quickLinkPath);
qDebug() << "Appending Quick Link: " << name << "---" << quickLinkPath;
TreeItem *item = new TreeItem(name, quickLinkPath, this, m_pQuickLinkItem);
m_pQuickLinkItem->appendChild(item);
m_pQuickLinkItem->appendChild(name, quickLinkPath);
}

// initialize the model
m_childModel.setRootItem(rootItem);
m_childModel.setRootItem(std::move(pRootItem));
}

BrowseFeature::~BrowseFeature() {
Expand All @@ -142,10 +130,10 @@ void BrowseFeature::slotAddQuickLink() {
return;
}

QString spath = m_pLastRightClickedItem->dataPath().toString();
QVariant vpath = m_pLastRightClickedItem->getData();
QString spath = vpath.toString();
QString name = extractNameFromPath(spath);
TreeItem *item = new TreeItem(name, spath, this, m_pQuickLinkItem);
m_pQuickLinkItem->appendChild(item);
m_pQuickLinkItem->appendChild(name, vpath);
m_quickLinkList.append(spath);
saveQuickLinks();
}
Expand All @@ -154,7 +142,7 @@ void BrowseFeature::slotAddToLibrary() {
if (!m_pLastRightClickedItem) {
return;
}
QString spath = m_pLastRightClickedItem->dataPath().toString();
QString spath = m_pLastRightClickedItem->getData().toString();
emit(requestAddDir(spath));

QMessageBox msgBox;
Expand Down Expand Up @@ -188,7 +176,7 @@ void BrowseFeature::slotRemoveQuickLink() {
return;
}

QString spath = m_pLastRightClickedItem->dataPath().toString();
QString spath = m_pLastRightClickedItem->getData().toString();
int index = m_quickLinkList.indexOf(spath);

if (index == -1) {
Expand Down Expand Up @@ -225,10 +213,10 @@ void BrowseFeature::activate() {
// Single clicks will not populate sub folders
void BrowseFeature::activateChild(const QModelIndex& index) {
TreeItem *item = static_cast<TreeItem*>(index.internalPointer());
qDebug() << "BrowseFeature::activateChild " << item->data() << " "
<< item->dataPath();
qDebug() << "BrowseFeature::activateChild " << item->getLabel() << " "
<< item->getData();

QString path = item->dataPath().toString();
QString path = item->getData().toString();
if (path == QUICK_LINK_NODE || path == DEVICE_NODE) {
m_browseModel.setPath(MDir());
} else {
Expand Down Expand Up @@ -258,14 +246,14 @@ void BrowseFeature::onRightClickChild(const QPoint& globalPos, QModelIndex index
return;
}

QString path = item->dataPath().toString();
QString path = item->getData().toString();

if (path == QUICK_LINK_NODE || path == DEVICE_NODE) {
return;
}

QMenu menu(NULL);
if (item->parent()->dataPath().toString() == QUICK_LINK_NODE) {
if (item->parent()->getData().toString() == QUICK_LINK_NODE) {
menu.addAction(m_pRemoveQuickLinkAction);
menu.exec(globalPos);
onLazyChildExpandation(index);
Expand All @@ -292,18 +280,18 @@ void BrowseFeature::onLazyChildExpandation(const QModelIndex& index) {
return;
}

qDebug() << "BrowseFeature::onLazyChildExpandation " << item->data()
<< " " << item->dataPath();
qDebug() << "BrowseFeature::onLazyChildExpandation " << item->getLabel()
<< " " << item->getData();

QString path = item->dataPath().toString();
QString path = item->getData().toString();

// If the item is a build-in node, e.g., 'QuickLink' return
if (path == QUICK_LINK_NODE) {
return;
}

// Before we populate the subtree, we need to delete old subtrees
m_childModel.removeRows(0, item->childCount(), index);
m_childModel.removeRows(0, item->childRows(), index);

// List of subfolders or drive letters
QList<TreeItem*> folders;
Expand All @@ -327,10 +315,9 @@ void BrowseFeature::onLazyChildExpandation(const QModelIndex& index) {
display_path.chop(1);
}
TreeItem* driveLetter = new TreeItem(
display_path, // Displays C:
drive.filePath(), // Displays C:/
this,
item);
display_path, // Displays C:
drive.filePath()); // Displays C:/
folders << driveLetter;
}
} else {
Expand All @@ -352,9 +339,9 @@ void BrowseFeature::onLazyChildExpandation(const QModelIndex& index) {
// Once the items are added to the TreeItemModel,
// the models takes ownership of them and ensures their deletion
TreeItem* folder = new TreeItem(
this,
one.fileName(),
one.absoluteFilePath() + "/",
this, item);
one.absoluteFilePath() + "/");
folders << folder;
}
}
Expand Down
10 changes: 5 additions & 5 deletions src/library/browse/foldertreemodel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,14 @@ bool FolderTreeModel::hasChildren(const QModelIndex& parent) const {
* However, for, buid-in items such as 'Quick Links' there exist
* child items at init time
*/
if(item->dataPath().toString() == QUICK_LINK_NODE)
if(item->getData().toString() == QUICK_LINK_NODE)
return true;
//Can only happen on Windows
if(item->dataPath().toString() == DEVICE_NODE)
if(item->getData().toString() == DEVICE_NODE)
return true;

// In all other cases the dataPath() points to a folder
QString folder = item->dataPath().toString();
// In all other cases the getData() points to a folder
QString folder = item->getData().toString();
return directoryHasChildren(folder);
}

Expand All @@ -63,7 +63,7 @@ bool FolderTreeModel::directoryHasChildren(const QString& path) const {
* QDIR::EntryInfoList returns a full QFileInfolist
*
*
* QDir dir(item->dataPath().toString());
* QDir dir(item->getData().toString());
* QFileInfoList all = dir.entryInfoList(QDir::Dirs | QDir::NoDotAndDotDot);
* return (all.count() > 0);
*
Expand Down
Loading