reapack

Package manager for REAPER
Log | Files | Refs | Submodules | README | LICENSE

commit dcce6c31d69b21f6e660cc6658a2ca5d8bf30404
parent 35611c985c5d5bc253a4962f59e2f4919fe77ff2
Author: cfillion <cfillion@users.noreply.github.com>
Date:   Sun, 27 Aug 2017 15:00:39 -0400

listview: refactor row and cell allocations (but temporarily degrade performance)

FIXME: Now the slow ListView::translate is called for every cell on insertion.

Diffstat:
Msrc/about.cpp | 43++++++++++++++++++++-----------------------
Msrc/browser.cpp | 11++++++-----
Msrc/browser_entry.cpp | 26++++++++++----------------
Msrc/browser_entry.hpp | 2+-
Msrc/listview.cpp | 59++++++++++++++++++++++++++++++++---------------------------
Msrc/listview.hpp | 46+++++++++++++++++++++++++++++-----------------
Msrc/manager.cpp | 23+++++++++--------------
Msrc/obsquery.cpp | 2+-
8 files changed, 108 insertions(+), 104 deletions(-)

diff --git a/src/about.cpp b/src/about.cpp @@ -299,10 +299,10 @@ void AboutIndexDelegate::init(About *dialog) dialog->menu()->addColumn({AUTO_STR("Category"), 142}); dialog->menu()->reserveRows(m_index->categories().size() + 1); - dialog->menu()->addRow({AUTO_STR("<All Packages>")}); + dialog->menu()->createRow()->setCell(0, AUTO_STR("<All Packages>")); for(const Category *cat : m_index->categories()) - dialog->menu()->addRow({make_autostring(cat->name())}); + dialog->menu()->createRow()->setCell(0, make_autostring(cat->name())); dialog->list()->addColumn({AUTO_STR("Package"), 382}); dialog->list()->addColumn({AUTO_STR("Version"), 80, 0, ListView::VersionType}); @@ -371,18 +371,15 @@ void AboutIndexDelegate::updateList(const int index) packages = &m_index->category(catIndex)->packages(); m_dialog->list()->reserveRows(packages->size()); + for(const Package *pkg : *packages) { + size_t c = 0; const Version *lastVer = pkg->lastVersion(); - const auto_string &name = make_autostring(pkg->displayName()); - const auto_string &version = make_autostring(lastVer->name().toString()); - const auto_string &author = make_autostring(lastVer->displayAuthor()); - - ListView::Row row; - row.userData = (void *)pkg; - row.push_back(name); - row.push_back({version, (void *)&lastVer->name()}); - row.push_back(author); - m_dialog->list()->addRow(row); + + auto row = m_dialog->list()->createRow((void *)pkg); + row->setCell(c++, make_autostring(pkg->displayName())); + row->setCell(c++, make_autostring(lastVer->name().toString()), (void *)&lastVer->name()); + row->setCell(c++, make_autostring(lastVer->displayAuthor())); } } @@ -419,7 +416,7 @@ const Package *AboutIndexDelegate::currentPackage() const if(index < 0) return nullptr; else - return (const Package *)m_dialog->list()->row(index).userData; + return (const Package *)m_dialog->list()->row(index)->userData; } void AboutIndexDelegate::findInBrowser() @@ -537,13 +534,13 @@ void AboutPackageDelegate::init(About *dialog) dialog->list()->addColumn({AUTO_STR("Action List"), 84}); dialog->menu()->reserveRows(m_package->versions().size()); + for(const Version *ver : m_package->versions()) { - const auto index = dialog->menu()->addRow({ - {make_autostring(ver->name().toString()), (void *)&ver->name()} - }); + auto row = dialog->menu()->createRow(); + row->setCell(0, make_autostring(ver->name().toString()), (void *)&ver->name()); if(m_current == ver->name()) - dialog->menu()->select(index); + dialog->menu()->select(row->index()); } dialog->menu()->sortByColumn(0, ListView::DescendingOrder); @@ -594,10 +591,10 @@ void AboutPackageDelegate::updateList(const int index) else actionList = "No"; - ListView::Row row{ - make_autostring(src->targetPath().join()), make_autostring(actionList)}; - row.userData = (void *)src; - m_dialog->list()->addRow(row); + size_t c = 0; + auto row = m_dialog->list()->createRow((void *)src); + row->setCell(c++, make_autostring(src->targetPath().join())); + row->setCell(c++, make_autostring(actionList)); } } @@ -606,7 +603,7 @@ bool AboutPackageDelegate::fillContextMenu(Menu &menu, const int index) const if(index < 0) return false; - auto src = (const Source *)m_dialog->list()->row(index).userData; + auto src = (const Source *)m_dialog->list()->row(index)->userData; menu.addAction(AUTO_STR("Copy source URL"), ACTION_COPY_URL); menu.setEnabled(m_current.size() > 0 && FS::exists(src->targetPath()), @@ -637,7 +634,7 @@ const Source *AboutPackageDelegate::currentSource() const if(index < 0) return nullptr; else - return (const Source *)m_dialog->list()->row(index).userData; + return (const Source *)m_dialog->list()->row(index)->userData; } void AboutPackageDelegate::copySourceUrl() diff --git a/src/browser.cpp b/src/browser.cpp @@ -520,7 +520,7 @@ void Browser::fillList() const vector<int> selectedIndexes = m_list->selection(); vector<const Entry *> oldSelection(selectedIndexes.size()); for(size_t i = 0; i < selectedIndexes.size(); i++) - oldSelection[i] = (Entry *)m_list->row(selectedIndexes[i]).userData; + oldSelection[i] = (Entry *)m_list->row(selectedIndexes[i])->userData; m_list->clear(); m_list->reserveRows(m_entries.size()); @@ -532,10 +532,11 @@ void Browser::fillList() const auto &matchingEntryIt = find_if(oldSelection.begin(), oldSelection.end(), [&entry] (const Entry *oldEntry) { return *oldEntry == entry; }); - const int index = m_list->addRow(entry.makeRow()); + auto row = m_list->createRow((void *)&entry); + entry.updateRow(row); if(matchingEntryIt != oldSelection.end()) - m_list->select(index); + m_list->select(row->index()); } m_list->setScroll(scroll); @@ -583,7 +584,7 @@ auto Browser::getEntry(const int index) -> Entry * if(index < 0) return nullptr; else - return (Entry *)m_list->row(index).userData; + return (Entry *)m_list->row(index)->userData; } void Browser::aboutPackage(const int index, const bool focus) @@ -746,7 +747,7 @@ void Browser::updateAction(const int index) updateDisplayLabel(); } else { - m_list->setCell(index, 0, entry->displayState()); + m_list->row(index)->setCell(0, entry->displayState()); if(m_list->sortColumn() == 0) m_list->sort(); diff --git a/src/browser_entry.cpp b/src/browser_entry.cpp @@ -151,25 +151,19 @@ Remote Browser::Entry::remote() const return g_reapack->remote(indexName()); } -ListView::Row Browser::Entry::makeRow() const +void Browser::Entry::updateRow(const ListView::RowPtr &row) const { + size_t c = 0; const Time *time = lastUpdate(); - ListView::Row row; - row.reserve(8); - row.userData = (void *)this; - row.emplace_back(make_autostring(displayState())); - row.emplace_back(make_autostring(displayName())); - row.emplace_back(make_autostring(categoryName())); - row.emplace_back(ListView::Cell{ - make_autostring(displayVersion()), (void *)sortVersion()}); - row.emplace_back(make_autostring(displayAuthor())); - row.emplace_back(make_autostring(displayType())); - row.emplace_back(make_autostring(indexName())); - row.emplace_back(ListView::Cell{time ? - make_autostring(time->toString()) : auto_string(), (void *)time}); - - return row; + row->setCell(c++, make_autostring(displayState())); + row->setCell(c++, make_autostring(displayName())); + row->setCell(c++, make_autostring(categoryName())); + row->setCell(c++, make_autostring(displayVersion()), (void *)sortVersion()); + row->setCell(c++, make_autostring(displayAuthor())); + row->setCell(c++, make_autostring(displayType())); + row->setCell(c++, make_autostring(indexName())); + row->setCell(c++, time ? make_autostring(time->toString()) : auto_string(), (void *)time); } void Browser::Entry::fillMenu(Menu &menu) const diff --git a/src/browser_entry.hpp b/src/browser_entry.hpp @@ -59,7 +59,7 @@ public: const Time *lastUpdate() const; Remote remote() const; - ListView::Row makeRow() const; + void updateRow(const ListView::RowPtr &) const; void fillMenu(Menu &) const; bool test(Flag f) const { return (m_flags & f) != 0; } diff --git a/src/listview.cpp b/src/listview.cpp @@ -72,10 +72,8 @@ int ListView::addColumn(const Column &col) return index; } -int ListView::addRow(const Row &row) +auto ListView::createRow(void *data) -> RowPtr { - assert(row.size() == m_cols.size()); - LVITEM item{}; item.iItem = rowCount(); @@ -84,25 +82,17 @@ int ListView::addRow(const Row &row) ListView_InsertItem(handle(), &item); - for(size_t i = 0; i < m_cols.size(); i++) - updateCellText(item.iItem, i, row[i]); - + RowPtr row = make_shared<Row>(m_cols.size(), data, this); m_rows.push_back(row); - return item.iItem; + return row; } -void ListView::setCell(int row, int index, const Cell &cell) +void ListView::updateCell(int row, int cell) { - updateCellText(translate(row), index, cell); - - m_rows[row][index] = cell; -} - -void ListView::updateCellText(int viewRowIndex, int cellIndex, const Cell &cell) -{ - ListView_SetItemText(handle(), viewRowIndex, cellIndex, - const_cast<auto_char *>(cell.value.c_str())); + const int viewRowIndex = translate(row); + ListView_SetItemText(handle(), viewRowIndex, cell, + const_cast<auto_char *>(m_rows[row]->cell(cell).value.c_str())); } void ListView::removeRow(const int userIndex) @@ -152,9 +142,8 @@ void ListView::sort() const int columnIndex = view->m_sort->column; const Column &column = view->m_cols[columnIndex]; - int ret = column.compare( - view->m_rows[aRow][columnIndex], view->m_rows[bRow][columnIndex]); - + int ret = column.compare(view->row(aRow)->cell(columnIndex), + view->row(bRow)->cell(columnIndex)); if(view->m_sort->order == DescendingOrder) ret = -ret; @@ -546,14 +535,14 @@ void ListView::saveState(Serializer::Data &data) const int ListView::Column::compare(const ListView::Cell &cl, const ListView::Cell &cr) const { if(dataType) { - if(!cl.data) + if(!cl.userData) return -1; - else if(!cr.data) + else if(!cr.userData) return 1; } switch(dataType) { - case NoDataType: { + case UserType: { // arbitrary data or no data: sort by visible text auto_string l = cl.value; boost::algorithm::to_lower(l); @@ -563,12 +552,28 @@ int ListView::Column::compare(const ListView::Cell &cl, const ListView::Cell &cr return l.compare(r); } case VersionType: - return reinterpret_cast<const VersionName *>(cl.data)->compare( - *reinterpret_cast<const VersionName *>(cr.data)); + return reinterpret_cast<const VersionName *>(cl.userData)->compare( + *reinterpret_cast<const VersionName *>(cr.userData)); case TimeType: - return reinterpret_cast<const Time *>(cl.data)->compare( - *reinterpret_cast<const Time *>(cr.data)); + return reinterpret_cast<const Time *>(cl.userData)->compare( + *reinterpret_cast<const Time *>(cr.userData)); + default: + return 0; } return 0; // to make MSVC happy } + +ListView::Row::Row(const size_t size, void *data, ListView *list) + : userData(data), m_cells(new Cell[size]), m_index(list->rowCount()), m_list(list) +{ +} + +void ListView::Row::setCell(const size_t i, const auto_string &val, void *data) +{ + Cell &cell = m_cells[i]; + cell.value = val; + cell.userData = data; + + m_list->updateCell(m_index, i); +} diff --git a/src/listview.hpp b/src/listview.hpp @@ -40,20 +40,40 @@ public: }; enum ColumnDataType { - NoDataType, + UserType, VersionType, TimeType, }; struct Cell { - Cell() : data(nullptr) {} - Cell(const auto_char *val, void *ptr = nullptr) : value(val), data(ptr) {} - Cell(const auto_string &val, void *ptr = nullptr) : value(val), data(ptr) {} + Cell() : userData(nullptr) {} + Cell(const Cell &) = delete; auto_string value; - void *data; + void *userData; }; + class Row { + public: + Row(size_t size, void *data, ListView *); + Row(const Row &) = delete; + ~Row() { delete[] m_cells; } + + void *userData; + + int index() const { return m_index; } + + const Cell &cell(const size_t i) const { return m_cells[i]; } + void setCell(const size_t i, const auto_string &, void *data = nullptr); + + private: + Cell *m_cells; + int m_index; + ListView *m_list; + }; + + typedef std::shared_ptr<Row> RowPtr; + struct Column { auto_string label; int width; @@ -66,22 +86,15 @@ public: typedef std::vector<Column> Columns; - class Row : public std::vector<Cell> { - public: - using std::vector<Cell>::vector; - - void *userData; - }; - typedef boost::signals2::signal<void ()> VoidSignal; typedef boost::signals2::signal<bool (Menu &, int index)> MenuSignal; ListView(HWND handle, const Columns & = {}); - int addRow(const Row &row); void reserveRows(size_t count) { m_rows.reserve(count); } - const Row &row(int index) const { return m_rows[index]; } - void setCell(int row, int index, const Cell &); + RowPtr createRow(void *data = nullptr); + const RowPtr &row(int index) const { return m_rows[index]; } + void updateCell(int row, int cell); void removeRow(int index); int rowCount() const { return (int)m_rows.size(); } bool empty() const { return m_rows.empty(); } @@ -136,7 +149,6 @@ private: }; static int adjustWidth(int); - void updateCellText(int viewRowIndex, int cellIndex, const Cell &); void setExStyle(int style, bool enable); void setSortArrow(bool); void handleDoubleClick(); @@ -147,7 +159,7 @@ private: bool m_customizable; std::vector<Column> m_cols; - std::vector<Row> m_rows; + std::vector<RowPtr> m_rows; boost::optional<Sort> m_sort; boost::optional<Sort> m_defaultSort; diff --git a/src/manager.cpp b/src/manager.cpp @@ -318,7 +318,7 @@ void Manager::refresh() const vector<int> selection = m_list->selection(); vector<string> selected(selection.size()); for(size_t i = 0; i < selection.size(); i++) - selected[i] = from_autostring(m_list->row(selection[i])[0].value); // TODO: use data ptr to Remote + selected[i] = from_autostring(m_list->row(selection[i])->cell(0).value); // TODO: use data ptr to Remote const auto &remotes = g_reapack->config()->remotes; @@ -329,17 +329,14 @@ void Manager::refresh() if(m_uninstall.count(remote)) continue; - ListView::Row row; - row.reserve(3); - row.emplace_back(make_autostring(remote.name())); - row.emplace_back(make_autostring(remote.url())); - row.emplace_back(ListView::Cell{}); - - const int index = m_list->addRow(row); - updateEnabledCell(index, remote); + size_t c = 0; + auto row = m_list->createRow(); + row->setCell(c++, make_autostring(remote.name())); + row->setCell(c++, make_autostring(remote.url())); + updateEnabledCell(row->index(), remote); if(find(selected.begin(), selected.end(), remote.name()) != selected.end()) - m_list->select(index); + m_list->select(row->index()); } m_list->sort(); @@ -347,7 +344,7 @@ void Manager::refresh() void Manager::updateEnabledCell(int index, const Remote &remote) { - m_list->setCell(index, 2, isRemoteEnabled(remote) ? + m_list->row(index)->setCell(2, isRemoteEnabled(remote) ? AUTO_STR("Enabled") : AUTO_STR("Disabled")); } @@ -759,9 +756,7 @@ Remote Manager::getRemote(const int index) const if(index < 0 || index > m_list->rowCount() - 1) return {}; - const ListView::Row &row = m_list->row(index); - const string &remoteName = from_autostring(row[0].value); - + const string &remoteName = from_autostring(m_list->row(index)->cell(0).value); return g_reapack->config()->remotes.get(remoteName); } diff --git a/src/obsquery.cpp b/src/obsquery.cpp @@ -58,7 +58,7 @@ void ObsoleteQuery::onInit() ostringstream stream; stream << entry.remote << '/' << entry.category << '/' << Package::displayName(entry.package, entry.description); - m_list->addRow({make_autostring(stream.str())}); + m_list->createRow()->setCell(0, make_autostring(stream.str())); } m_list->autoSizeHeader();