From b5e416f8cd52f69ff1edd832ee10bac550544ef6 Mon Sep 17 00:00:00 2001 From: Thorbjørn Lindeijer Date: Fri, 9 Feb 2024 14:31:05 +0100 Subject: Some cleanups in UpdaterWindow and BrowserBox Doing some cleanups before working towards optimizing this code. Removed needless additional wrapping code in BrowserBox::addRow, since the text will be relayouted anyway. Simplified layouting code a little. For example, there's no need to keep track of the number of wrapped lines. Use more optimal data structures, like an std::deque for the text rows and a plain std::vector for the line parts. Both have less fragmentation than an std::list. --- src/gui/updaterwindow.cpp | 37 +++++------ src/gui/updaterwindow.h | 17 +++-- src/gui/widgets/browserbox.cpp | 138 ++++++++++++----------------------------- src/gui/widgets/browserbox.h | 57 +++++------------ 4 files changed, 79 insertions(+), 170 deletions(-) diff --git a/src/gui/updaterwindow.cpp b/src/gui/updaterwindow.cpp index bedaa9da..b26119e6 100644 --- a/src/gui/updaterwindow.cpp +++ b/src/gui/updaterwindow.cpp @@ -51,9 +51,9 @@ const std::string txtUpdateFile = "resources2.txt"; /** * Load the given file into a vector of updateFiles. */ -std::vector loadXMLFile(const std::string &fileName) +std::vector loadXMLFile(const std::string &fileName) { - std::vector files; + std::vector files; XML::Document doc(fileName, false); xmlNodePtr rootNode = doc.rootNode(); @@ -69,15 +69,12 @@ std::vector loadXMLFile(const std::string &fileName) if (!xmlStrEqual(fileNode->name, BAD_CAST "update")) continue; - updateFile file; + UpdateFile file; file.name = XML::getProperty(fileNode, "file", ""); file.hash = XML::getProperty(fileNode, "hash", ""); file.type = XML::getProperty(fileNode, "type", "data"); file.desc = XML::getProperty(fileNode, "description", ""); - if (XML::getProperty(fileNode, "required", "yes") == "yes") - file.required = true; - else - file.required = false; + file.required = XML::getProperty(fileNode, "required", "yes") == "yes"; files.push_back(file); } @@ -85,9 +82,9 @@ std::vector loadXMLFile(const std::string &fileName) return files; } -std::vector loadTxtFile(const std::string &fileName) +std::vector loadTxtFile(const std::string &fileName) { - std::vector files; + std::vector files; std::ifstream fileHandler; fileHandler.open(fileName.c_str(), std::ios::in); @@ -99,7 +96,7 @@ std::vector loadTxtFile(const std::string &fileName) fileHandler.getline(name, 256, ' '); fileHandler.getline(hash, 50); - updateFile thisFile; + UpdateFile thisFile; thisFile.name = name; thisFile.hash = hash; thisFile.type = "data"; @@ -179,11 +176,11 @@ UpdaterWindow::~UpdaterWindow() free(mMemoryBuffer); } -void UpdaterWindow::setProgress(float p) +void UpdaterWindow::setProgress(float progress) { // Do delayed progress bar update, since Guichan isn't thread-safe MutexLocker lock(&mDownloadMutex); - mDownloadProgress = p; + mDownloadProgress = progress; } void UpdaterWindow::setLabel(const std::string &str) @@ -359,10 +356,11 @@ void UpdaterWindow::loadUpdates() { ResourceManager *resman = ResourceManager::getInstance(); - if (!mUpdateFiles.size()) - { // updates not downloaded + if (mUpdateFiles.empty()) + { + // updates not downloaded mUpdateFiles = loadXMLFile(mUpdatesDir + "/" + xmlUpdateFile); - if (!mUpdateFiles.size()) + if (mUpdateFiles.empty()) { logger->log("Warning this server does not have a" " %s file falling back to %s", xmlUpdateFile.c_str(), @@ -371,10 +369,9 @@ void UpdaterWindow::loadUpdates() } } - for (mUpdateIndex = 0; mUpdateIndex < mUpdateFiles.size(); mUpdateIndex++) + for (const UpdateFile &file : mUpdateFiles) { - resman->addToSearchPath(mUpdatesDir + "/" - + mUpdateFiles[mUpdateIndex].name, false); + resman->addToSearchPath(mUpdatesDir + "/" + file.name, false); } } @@ -435,7 +432,7 @@ void UpdaterWindow::logic() if (mCurrentFile == xmlUpdateFile) { mUpdateFiles = loadXMLFile(mUpdatesDir + "/" + xmlUpdateFile); - if (mUpdateFiles.size() == 0) + if (mUpdateFiles.empty()) { logger->log("Warning this server does not have a %s" " file falling back to %s", @@ -462,7 +459,7 @@ void UpdaterWindow::logic() { if (mUpdateIndex < mUpdateFiles.size()) { - updateFile thisFile = mUpdateFiles[mUpdateIndex]; + UpdateFile thisFile = mUpdateFiles[mUpdateIndex]; if (!thisFile.required) { // This statement checks to see if the file type is music, and if download-music is true diff --git a/src/gui/updaterwindow.h b/src/gui/updaterwindow.h index 6c69cd9b..2d1b059b 100644 --- a/src/gui/updaterwindow.h +++ b/src/gui/updaterwindow.h @@ -39,14 +39,13 @@ class Button; class ProgressBar; class ScrollArea; -struct updateFile +struct UpdateFile { - public: - std::string name; - std::string hash; - std::string type; - bool required; - std::string desc; + std::string name; + std::string hash; + std::string type; + bool required; + std::string desc; }; /** @@ -76,7 +75,7 @@ class UpdaterWindow : public Window, public gcn::ActionListener, /** * Set's progress bar status */ - void setProgress(float p); + void setProgress(float progress); /** * Set's label above progress @@ -176,7 +175,7 @@ private: Net::Download *mDownload = nullptr; /** List of files to download. */ - std::vector mUpdateFiles; + std::vector mUpdateFiles; /** Index of the file to be downloaded. */ unsigned int mUpdateIndex = 0; diff --git a/src/gui/widgets/browserbox.cpp b/src/gui/widgets/browserbox.cpp index bc424962..7b5d40bb 100644 --- a/src/gui/widgets/browserbox.cpp +++ b/src/gui/widgets/browserbox.cpp @@ -164,61 +164,8 @@ void BrowserBox::addRow(const std::string &row) setWidth(w); } - // TODO: Optimize! There's no point in wrapping all text rows again, just - // do the one that was just added (but take into account discarded rows?). - if (mMode == AUTO_WRAP) - { - unsigned int wrapCount = 0; - unsigned int nextChar; - const char *tilde = "~"; - int tildeWidth = font->getWidth(tilde); - int x = 0; - - for (auto row : mTextRows) - { - for (unsigned int j = 0; j < row.size(); ++j) - { - std::string character = row.substr(j, 1); - x += font->getWidth(character); - nextChar = j + 1; - - // Wraping between words (at blank spaces) - if (nextChar < row.size() && row.at(nextChar) == ' ') - { - int nextSpacePos = static_cast( - row.find(" ", (nextChar + 1))); - if (nextSpacePos <= 0) - nextSpacePos = static_cast(row.size()) - 1; - - int nextWordWidth = font->getWidth( - row.substr(nextChar, - (nextSpacePos - nextChar))); - - if (x + nextWordWidth + 10 > getWidth()) - { - x = 15; // Indent in new line - ++wrapCount; - ++j; - } - } - // Wrapping looong lines (brutal force) - else if (x + 2 * tildeWidth > getWidth()) - { - x = 15; // Ident in new line - ++wrapCount; - } - } - } - - setHeight(lineHeight * (mTextRows.size() + wrapCount - 1) - + fontHeight); - } - else - { - setHeight(lineHeight * (mTextRows.size() - 1) + fontHeight); - } mUpdateTime = 0; - updateHeight(); + maybeRelayoutText(); } void BrowserBox::clearRows() @@ -228,7 +175,7 @@ void BrowserBox::clearRows() setWidth(0); setHeight(0); mSelectedLink = -1; - updateHeight(); + maybeRelayoutText(); } struct MouseOverLink @@ -269,14 +216,14 @@ void BrowserBox::mouseMoved(gcn::MouseEvent &event) void BrowserBox::draw(gcn::Graphics *graphics) { - gcn::ClipRectangle cr = graphics->getCurrentClipArea(); + const gcn::ClipRectangle &cr = graphics->getCurrentClipArea(); mYStart = cr.y - cr.yOffset; int yEnd = mYStart + cr.height; if (mYStart < 0) mYStart = 0; - if (getWidth() != mWidth) - updateHeight(); + if (getWidth() != mLastLayoutWidth) + maybeRelayoutText(); if (mOpaque) { @@ -308,11 +255,11 @@ void BrowserBox::draw(gcn::Graphics *graphics) } } - for (auto &part : mLineParts) + for (const auto &part : mLineParts) { - if (part.getY() + 50 < mYStart) + if (part.y + 50 < mYStart) continue; - if (part.getY() > yEnd) + if (part.y > yEnd) break; // Use the correct font @@ -322,69 +269,64 @@ void BrowserBox::draw(gcn::Graphics *graphics) if (mShadows) { graphics->setColor(Theme::getThemeColor(Theme::SHADOW, - part.getColor().a / 2)); + part.color.a / 2)); + if (mOutline) - { - graphics->drawText(part.getText(), part.getX() + 2, - part.getY() + 2); - } + graphics->drawText(part.text, part.x + 2, part.y + 2); else - { - graphics->drawText(part.getText(), part.getX() + 1, - part.getY() + 1); - } + graphics->drawText(part.text, part.x + 1, part.y + 1); } if (mOutline) { // Text outline graphics->setColor(Theme::getThemeColor(Theme::OUTLINE, - part.getColor().a / 4)); - graphics->drawText(part.getText(), part.getX() + 1, part.getY()); - graphics->drawText(part.getText(), part.getX() - 1, part.getY()); - graphics->drawText(part.getText(), part.getX(), part.getY() + 1); - graphics->drawText(part.getText(), part.getX(), part.getY() - 1); + part.color.a / 4)); + graphics->drawText(part.text, part.x + 1, part.y); + graphics->drawText(part.text, part.x - 1, part.y); + graphics->drawText(part.text, part.x, part.y + 1); + graphics->drawText(part.text, part.x, part.y - 1); } // the main text - graphics->setColor(part.getColor()); - graphics->drawText(part.getText(), part.getX(), part.getY()); + graphics->setColor(part.color); + graphics->drawText(part.text, part.x, part.y); } - - return; } -int BrowserBox::calcHeight() +/** + * Relayouts all text rows. + */ +void BrowserBox::relayoutText() { - int x = 0, y = 0; - int wrappedLines = 0; + int y = 0; unsigned link = 0; - gcn::Font *font = getFont(); + const gcn::Font *font = getFont(); const int fontHeight = font->getHeight(); const int minusWidth = font->getWidth("-"); const int tildeWidth = font->getWidth("~"); int lineHeight = fontHeight; - if (auto *ttf = dynamic_cast(font)) - lineHeight = ttf->getLineHeight(); + if (auto *trueTypeFont = dynamic_cast(font)) + lineHeight = trueTypeFont->getLineHeight(); gcn::Color selColor = Theme::getThemeColor(Theme::TEXT); const gcn::Color &textColor = Theme::getThemeColor(Theme::TEXT); mLineParts.clear(); - for (auto row : mTextRows) + for (const auto &row : mTextRows) { bool wrapped = false; - x = 0; + int x = 0; // Check for separator lines if (row.find("---", 0) == 0) { for (x = 0; x < getWidth(); x++) { - mLineParts.push_back(LinePart(x, y, selColor, "-")); + mLineParts.push_back(LinePart { x, y, selColor, "-" }); x += minusWidth - 2; } @@ -438,7 +380,6 @@ int BrowserBox::calcHeight() } else { - switch (c) { case '1': selColor = RED; break; @@ -456,6 +397,7 @@ int BrowserBox::calcHeight() } } + // Update the position of the links if (c == '<' && link < mLinks.size()) { const int size = @@ -520,8 +462,8 @@ int BrowserBox::calcHeight() if (forced) { x -= tildeWidth; // Remove the wrap-notifier accounting - mLineParts.push_back(LinePart(getWidth() - tildeWidth, - y, selColor, "~")); + mLineParts.push_back(LinePart { getWidth() - tildeWidth, + y, selColor, "~" }); end++; // Skip to the next character } else @@ -530,10 +472,9 @@ int BrowserBox::calcHeight() } wrapped = true; - wrappedLines++; } - mLineParts.push_back(LinePart(x, y, selColor, part)); + mLineParts.push_back(LinePart { x, y, selColor, part }); const int partWidth = font->getWidth(part); if (mMode == AUTO_WRAP && partWidth == 0) @@ -541,19 +482,20 @@ int BrowserBox::calcHeight() x += partWidth; } + y += lineHeight; } - return (mTextRows.size() + wrappedLines - 1) * lineHeight + fontHeight; + + mLastLayoutWidth = getWidth(); + setHeight(y); } -void BrowserBox::updateHeight() +void BrowserBox::maybeRelayoutText() { if (mAlwaysUpdate || !mUpdateTime || std::abs(mUpdateTime - tick_time) > 10 || mTextRows.size() < 3) { - mWidth = getWidth(); - mHeight = calcHeight(); - setHeight(mHeight); + relayoutText(); mUpdateTime = tick_time; } } diff --git a/src/gui/widgets/browserbox.h b/src/gui/widgets/browserbox.h index 6d09f77b..83cdef7c 100644 --- a/src/gui/widgets/browserbox.h +++ b/src/gui/widgets/browserbox.h @@ -26,7 +26,7 @@ #include #include -#include +#include #include class LinkHandler; @@ -38,30 +38,12 @@ struct BrowserLink std::string caption; }; -class LinePart +struct LinePart { - public: - LinePart(int x, int y, gcn::Color color, std::string text) : - mX(x), mY(y), mColor(color), mText(text) - { - } - - int getX() const - { return mX; } - - int getY() const - { return mY; } - - const std::string &getText() const - { return mText; } - - const gcn::Color &getColor() const - { return mColor; } - - private: - int mX, mY; - gcn::Color mColor; - std::string mText; + int x; + int y; + gcn::Color color; + std::string text; }; /** @@ -132,14 +114,14 @@ class BrowserBox : public gcn::Widget, */ void draw(gcn::Graphics *graphics) override; - void updateHeight(); + void maybeRelayoutText(); /** * BrowserBox modes. */ enum { - AUTO_SIZE = 0, + AUTO_SIZE, AUTO_WRAP /**< Maybe it needs a fix or to be redone. */ }; @@ -176,27 +158,17 @@ class BrowserBox : public gcn::Widget, BACKGROUND = 2 }; - using TextRows = std::list; - - TextRows &getRows() - { return mTextRows; } - void setAlwaysUpdate(bool n) { mAlwaysUpdate = n; } private: - int calcHeight(); - - using TextRowIterator = TextRows::iterator; - TextRows mTextRows; + void relayoutText(); + int layoutTextRow(const std::string &row, int y); - using LinePartList = std::list; - using LinePartIterator = LinePartList::iterator; - LinePartList mLineParts; + std::deque mTextRows; - using Links = std::vector; - using LinkIterator = Links::iterator; - Links mLinks; + std::vector mLineParts; + std::vector mLinks; LinkHandler *mLinkHandler = nullptr; unsigned int mMode; @@ -207,8 +179,7 @@ class BrowserBox : public gcn::Widget, bool mUseLinksAndUserColors = true; int mSelectedLink = -1; unsigned int mMaxRows = 0; - int mHeight = 0; - int mWidth = 0; + int mLastLayoutWidth = 0; int mYStart = 0; int mUpdateTime = -1; bool mAlwaysUpdate = true; -- cgit v1.2.3-70-g09d2