Merge pull request #625 from GriffinRichards/metatile-leak

Stop leaking tileset metatiles

include/core/tileset.h

@@ -18,6 +18,7 @@ public:
     Tileset() = default;
     Tileset(const Tileset &other);
     Tileset &operator=(const Tileset &other);
+    ~Tileset();
 
 public:
     QString name;
@@ -33,7 +34,6 @@ public:
     QStringList palettePaths;
 
     QList<QImage> tiles;
-    QList<Metatile*> metatiles;
     QHash<int, QString> metatileLabels;
     QList<QList<QRgb>> palettes;
     QList<QList<QRgb>> palettePreviews;
@@ -59,6 +59,19 @@ public:
     bool appendToHeaders(QString root, QString friendlyName, bool usingAsm);
     bool appendToGraphics(QString root, QString friendlyName, bool usingAsm);
     bool appendToMetatiles(QString root, QString friendlyName, bool usingAsm);
+
+    void setMetatiles(const QList<Metatile*> &metatiles);
+    void addMetatile(Metatile* metatile);
+
+    QList<Metatile*> metatiles() const { return m_metatiles; }
+    Metatile* metatileAt(unsigned int i) const { return m_metatiles.at(i); }
+
+    void clearMetatiles();
+    void resizeMetatiles(unsigned int newNumMetatiles);
+    int numMetatiles() const { return m_metatiles.length(); }
+
+private:
+    QList<Metatile*> m_metatiles;
 };
 
 #endif // TILESET_H

src/core/tileset.cpp

@@ -29,8 +29,8 @@ Tileset::Tileset(const Tileset &other)
         tiles.append(tile.copy());
     }
 
-    for (auto *metatile : other.metatiles) {
+    for (auto *metatile : other.m_metatiles) {
-        metatiles.append(new Metatile(*metatile));
+        m_metatiles.append(new Metatile(*metatile));
     }
 }
 
@@ -55,14 +55,42 @@ Tileset &Tileset::operator=(const Tileset &other) {
         tiles.append(tile.copy());
     }
 
-    metatiles.clear();
+    clearMetatiles();
-    for (auto *metatile : other.metatiles) {
+    for (auto *metatile : other.m_metatiles) {
-        metatiles.append(new Metatile(*metatile));
+        m_metatiles.append(new Metatile(*metatile));
     }
 
     return *this;
 }
 
+Tileset::~Tileset() {
+    clearMetatiles();
+}
+
+void Tileset::clearMetatiles() {
+    qDeleteAll(m_metatiles);
+    m_metatiles.clear();
+}
+
+void Tileset::setMetatiles(const QList<Metatile*> &metatiles) {
+    clearMetatiles();
+    m_metatiles = metatiles;
+}
+
+void Tileset::addMetatile(Metatile* metatile) {
+    m_metatiles.append(metatile);
+}
+
+void Tileset::resizeMetatiles(unsigned int newNumMetatiles) {
+    while (m_metatiles.length() > newNumMetatiles) {
+        delete m_metatiles.takeLast();
+    }
+    const int numTiles = projectConfig.getNumTilesInMetatile();
+    while (m_metatiles.length() < newNumMetatiles) {
+        m_metatiles.append(new Metatile(numTiles));
+    }
+}
+
 Tileset* Tileset::getTileTileset(int tileId, Tileset *primaryTileset, Tileset *secondaryTileset) {
     if (tileId < Project::getNumTilesPrimary()) {
         return primaryTileset;
@@ -89,7 +117,7 @@ Metatile* Tileset::getMetatile(int metatileId, Tileset *primaryTileset, Tileset
         return nullptr;
     }
     int index = Metatile::getIndexInTileset(metatileId);
-    return tileset->metatiles.value(index, nullptr);
+    return tileset->m_metatiles.value(index, nullptr);
 }
 
 // Metatile labels are stored per-tileset. When looking for a metatile label, first search in the tileset
@@ -178,10 +206,10 @@ bool Tileset::metatileIsValid(uint16_t metatileId, Tileset *primaryTileset, Tile
     if (metatileId >= Project::getNumMetatilesTotal())
         return false;
 
-    if (metatileId < Project::getNumMetatilesPrimary() && metatileId >= primaryTileset->metatiles.length())
+    if (metatileId < Project::getNumMetatilesPrimary() && metatileId >= primaryTileset->numMetatiles())
         return false;
 
-    if (metatileId >= Project::getNumMetatilesPrimary() + secondaryTileset->metatiles.length())
+    if (metatileId >= Project::getNumMetatilesPrimary() + secondaryTileset->numMetatiles())
         return false;
 
     return true;

src/mainwindow.cpp

@@ -1445,7 +1445,7 @@ void MainWindow::on_actionNew_Tileset_triggered() {
                 }
                 mt->tiles.append(tile);
             }
-            newSet.metatiles.append(mt);
+            newSet.addMetatile(mt);
         }
         for(int i = 0; i < 16; ++i) {
             QList<QRgb> currentPal;

src/project.cpp

@@ -1013,7 +1013,7 @@ void Project::saveTilesetMetatileAttributes(Tileset *tileset) {
     QFile attrs_file(tileset->metatile_attrs_path);
     if (attrs_file.open(QIODevice::WriteOnly | QIODevice::Truncate)) {
         QByteArray data;
-        for (Metatile *metatile : tileset->metatiles) {
+        for (const auto &metatile : tileset->metatiles()) {
             uint32_t attributes = metatile->getAttributes();
             for (int i = 0; i < projectConfig.metatileAttributesSize; i++)
                 data.append(static_cast<char>(attributes >> (8 * i)));
@@ -1029,7 +1029,7 @@ void Project::saveTilesetMetatiles(Tileset *tileset) {
     if (metatiles_file.open(QIODevice::WriteOnly | QIODevice::Truncate)) {
         QByteArray data;
         int numTiles = projectConfig.getNumTilesInMetatile();
-        for (Metatile *metatile : tileset->metatiles) {
+        for (const auto &metatile : tileset->metatiles()) {
             for (int i = 0; i < numTiles; i++) {
                 uint16_t tile = metatile->tiles.at(i).rawValue();
                 data.append(static_cast<char>(tile));
@@ -1038,7 +1038,7 @@ void Project::saveTilesetMetatiles(Tileset *tileset) {
         }
         metatiles_file.write(data);
     } else {
-        tileset->metatiles.clear();
+        tileset->clearMetatiles();
         logError(QString("Could not open tileset metatiles file '%1'").arg(tileset->metatiles_path));
     }
 }
@@ -1522,16 +1522,16 @@ void Project::loadTilesetMetatiles(Tileset* tileset) {
             }
             metatiles.append(metatile);
         }
-        tileset->metatiles = metatiles;
+        tileset->setMetatiles(metatiles);
     } else {
-        tileset->metatiles.clear();
+        tileset->clearMetatiles();
         logError(QString("Could not open tileset metatiles file '%1'").arg(tileset->metatiles_path));
     }
 
     QFile attrs_file(tileset->metatile_attrs_path);
     if (attrs_file.open(QIODevice::ReadOnly)) {
         QByteArray data = attrs_file.readAll();
-        int num_metatiles = tileset->metatiles.count();
+        int num_metatiles = tileset->numMetatiles();
         int attrSize = projectConfig.metatileAttributesSize;
         int num_metatileAttrs = data.length() / attrSize;
         if (num_metatiles != num_metatileAttrs) {
@@ -1544,7 +1544,7 @@ void Project::loadTilesetMetatiles(Tileset* tileset) {
             uint32_t attributes = 0;
             for (int j = 0; j < attrSize; j++)
                 attributes |= static_cast<unsigned char>(data.at(i * attrSize + j)) << (8 * j);
-            tileset->metatiles.at(i)->setAttributes(attributes);
+            tileset->metatileAt(i)->setAttributes(attributes);
         }
     } else {
         logError(QString("Could not open tileset metatile attributes file '%1'").arg(tileset->metatile_attrs_path));

src/scriptapi/apimap.cpp

@@ -533,13 +533,13 @@ QJSValue MainWindow::getSecondaryTilesetPalettesPreview() {
 int MainWindow::getNumPrimaryTilesetMetatiles() {
     if (!this->editor || !this->editor->map || !this->editor->map->layout || !this->editor->map->layout->tileset_primary)
         return 0;
-    return this->editor->map->layout->tileset_primary->metatiles.length();
+    return this->editor->map->layout->tileset_primary->numMetatiles();
 }
 
 int MainWindow::getNumSecondaryTilesetMetatiles() {
     if (!this->editor || !this->editor->map || !this->editor->map->layout || !this->editor->map->layout->tileset_secondary)
         return 0;
-    return this->editor->map->layout->tileset_secondary->metatiles.length();
+    return this->editor->map->layout->tileset_secondary->numMetatiles();
 }
 
 int MainWindow::getNumPrimaryTilesetTiles() {

src/ui/metatilelayersitem.cpp

@@ -24,7 +24,7 @@ void MetatileLayersItem::draw() {
     QPainter painter(&pixmap);
 
     // Draw tile images
-    int numTiles = projectConfig.getNumTilesInMetatile();
+    int numTiles = qMin(projectConfig.getNumTilesInMetatile(), this->metatile ? this->metatile->tiles.length() : 0);
     for (int i = 0; i < numTiles; i++) {
         Tile tile = this->metatile->tiles.at(i);
         QImage tileImage = getPalettedTileImage(tile.tileId, this->primaryTileset, this->secondaryTileset, tile.palette, true)

src/ui/metatileselector.cpp

@@ -14,8 +14,8 @@ void MetatileSelector::draw() {
         this->setPixmap(QPixmap());
     }
 
-    int primaryLength = this->primaryTileset->metatiles.length();
+    int primaryLength = this->primaryTileset->numMetatiles();
-    int length_ = primaryLength + this->secondaryTileset->metatiles.length();
+    int length_ = primaryLength + this->secondaryTileset->numMetatiles();
     int height_ = length_ / this->numMetatilesWide;
     if (length_ % this->numMetatilesWide != 0) {
         height_++;
@@ -199,10 +199,10 @@ void MetatileSelector::updateExternalSelectedMetatiles() {
 
 uint16_t MetatileSelector::getMetatileId(int x, int y) const {
     int index = y * this->numMetatilesWide + x;
-    if (index < this->primaryTileset->metatiles.length()) {
+    if (index < this->primaryTileset->numMetatiles()) {
         return static_cast<uint16_t>(index);
     } else {
-        return static_cast<uint16_t>(Project::getNumMetatilesPrimary() + index - this->primaryTileset->metatiles.length());
+        return static_cast<uint16_t>(Project::getNumMetatilesPrimary() + index - this->primaryTileset->numMetatiles());
     }
 }
 
@@ -215,7 +215,7 @@ QPoint MetatileSelector::getMetatileIdCoords(uint16_t metatileId) {
 
     int index = metatileId < Project::getNumMetatilesPrimary()
                 ? metatileId
-                : metatileId - Project::getNumMetatilesPrimary() + this->primaryTileset->metatiles.length();
+                : metatileId - Project::getNumMetatilesPrimary() + this->primaryTileset->numMetatiles();
     return QPoint(index % this->numMetatilesWide, index / this->numMetatilesWide);
 }
 

src/ui/tileseteditor.cpp

@@ -32,7 +32,6 @@ TilesetEditor::~TilesetEditor()
     delete tileSelector;
     delete metatileLayersItem;
     delete paletteEditor;
-    delete metatile;
     delete primaryTileset;
     delete secondaryTileset;
     delete metatilesScene;
@@ -41,6 +40,7 @@ TilesetEditor::~TilesetEditor()
     delete selectedTileScene;
     delete metatileLayersScene;
     delete copiedMetatile;
+    this->metatileHistory.clear();
 }
 
 void TilesetEditor::update(Map *map, QString primaryTilesetLabel, QString secondaryTilesetLabel) {
@@ -781,8 +781,8 @@ void TilesetEditor::on_actionChange_Metatiles_Count_triggered()
     secondarySpinBox->setMinimum(1);
     primarySpinBox->setMaximum(Project::getNumMetatilesPrimary());
     secondarySpinBox->setMaximum(Project::getNumMetatilesTotal() - Project::getNumMetatilesPrimary());
-    primarySpinBox->setValue(this->primaryTileset->metatiles.length());
+    primarySpinBox->setValue(this->primaryTileset->numMetatiles());
-    secondarySpinBox->setValue(this->secondaryTileset->metatiles.length());
+    secondarySpinBox->setValue(this->secondaryTileset->numMetatiles());
     form.addRow(new QLabel("Primary Tileset"), primarySpinBox);
     form.addRow(new QLabel("Secondary Tileset"), secondarySpinBox);
 
@@ -792,22 +792,8 @@ void TilesetEditor::on_actionChange_Metatiles_Count_triggered()
     form.addRow(&buttonBox);
 
     if (dialog.exec() == QDialog::Accepted) {
-        int numPrimaryMetatiles = primarySpinBox->value();
+        this->primaryTileset->resizeMetatiles(primarySpinBox->value());
-        int numSecondaryMetatiles = secondarySpinBox->value();
+        this->secondaryTileset->resizeMetatiles(secondarySpinBox->value());
-        int numTiles = projectConfig.getNumTilesInMetatile();
-        while (this->primaryTileset->metatiles.length() > numPrimaryMetatiles) {
-            delete this->primaryTileset->metatiles.takeLast();
-        }
-        while (this->primaryTileset->metatiles.length() < numPrimaryMetatiles) {
-            this->primaryTileset->metatiles.append(new Metatile(numTiles));
-        }
-        while (this->secondaryTileset->metatiles.length() > numSecondaryMetatiles) {
-            delete this->secondaryTileset->metatiles.takeLast();
-        }
-        while (this->secondaryTileset->metatiles.length() < numSecondaryMetatiles) {
-            this->secondaryTileset->metatiles.append(new Metatile(numTiles));
-        }
-
         this->metatileSelector->updateSelectedMetatile();
         this->refresh();
         this->hasUnsavedChanges = true;
@@ -1008,20 +994,20 @@ void TilesetEditor::importTilesetMetatiles(Tileset *tileset, bool primary)
     //       Revisit this when tiles and num metatiles are added to tileset editory history.
     int metatileIdBase = primary ? 0 : Project::getNumMetatilesPrimary();
     for (int i = 0; i < metatiles.length(); i++) {
-        if (i >= tileset->metatiles.length()) {
+        if (i >= tileset->numMetatiles()) {
             break;
         }
 
         uint16_t metatileId = static_cast<uint16_t>(metatileIdBase + i);
         QString prevLabel = Tileset::getOwnedMetatileLabel(metatileId, this->primaryTileset, this->secondaryTileset);
-        Metatile *prevMetatile = new Metatile(*tileset->metatiles.at(i));
+        Metatile *prevMetatile = new Metatile(*tileset->metatileAt(i));
         MetatileHistoryItem *commit = new MetatileHistoryItem(metatileId,
                                                               prevMetatile, new Metatile(*metatiles.at(i)),
                                                               prevLabel, prevLabel);
         metatileHistory.push(commit);
     }
 
-    tileset->metatiles = metatiles;
+    tileset->setMetatiles(metatiles);
     this->refresh();
     this->hasUnsavedChanges = true;
 }
@@ -1125,9 +1111,9 @@ void TilesetEditor::countTileUsage() {
 
     // check primary tilesets that are used with this secondary tileset for
     // reference to secondary tiles in primary metatiles
-    for (Tileset *tileset : primaryTilesets) {
+    for (const auto &tileset : primaryTilesets) {
-        for (Metatile *metatile : tileset->metatiles) {
+        for (const auto &metatile : tileset->metatiles()) {
-            for (Tile tile : metatile->tiles) {
+            for (const auto &tile : metatile->tiles) {
                 if (tile.tileId >= Project::getNumTilesPrimary())
                     this->tileSelector->usedTiles[tile.tileId]++;
             }
@@ -1136,8 +1122,8 @@ void TilesetEditor::countTileUsage() {
 
     // do the opposite for primary tiles in secondary metatiles
     for (Tileset *tileset : secondaryTilesets) {
-        for (Metatile *metatile : tileset->metatiles) {
+        for (const auto &metatile : tileset->metatiles()) {
-            for (Tile tile : metatile->tiles) {
+            for (const auto &tile : metatile->tiles) {
                 if (tile.tileId < Project::getNumTilesPrimary())
                     this->tileSelector->usedTiles[tile.tileId]++;
             }
@@ -1145,15 +1131,15 @@ void TilesetEditor::countTileUsage() {
     }
 
     // check this primary tileset metatiles
-    for (Metatile *metatile : this->primaryTileset->metatiles) {
+    for (const auto &metatile : this->primaryTileset->metatiles()) {
-        for (Tile tile : metatile->tiles) {
+        for (const auto &tile : metatile->tiles) {
             this->tileSelector->usedTiles[tile.tileId]++;
         }
     }
 
     // and the secondary metatiles
-    for (Metatile *metatile : this->secondaryTileset->metatiles) {
+    for (const auto &metatile : this->secondaryTileset->metatiles()) {
-        for (Tile tile : metatile->tiles) {
+        for (const auto &tile : metatile->tiles) {
             this->tileSelector->usedTiles[tile.tileId]++;
         }
     }

src/ui/tileseteditormetatileselector.cpp

@@ -22,24 +22,24 @@ int TilesetEditorMetatileSelector::numRows(int numMetatiles) {
 }
 
 int TilesetEditorMetatileSelector::numRows() {
-    return this->numRows(this->primaryTileset->metatiles.length() + this->secondaryTileset->metatiles.length());
+    return this->numRows(this->primaryTileset->numMetatiles() + this->secondaryTileset->numMetatiles());
 }
 
 QImage TilesetEditorMetatileSelector::buildAllMetatilesImage() {
-    return this->buildImage(0, this->primaryTileset->metatiles.length() + this->secondaryTileset->metatiles.length());
+    return this->buildImage(0, this->primaryTileset->numMetatiles() + this->secondaryTileset->numMetatiles());
 }
 
 QImage TilesetEditorMetatileSelector::buildPrimaryMetatilesImage() {
-    return this->buildImage(0, this->primaryTileset->metatiles.length());
+    return this->buildImage(0, this->primaryTileset->numMetatiles());
 }
 
 QImage TilesetEditorMetatileSelector::buildSecondaryMetatilesImage() {
-    return this->buildImage(Project::getNumMetatilesPrimary(), this->secondaryTileset->metatiles.length());
+    return this->buildImage(Project::getNumMetatilesPrimary(), this->secondaryTileset->numMetatiles());
 }
 
 QImage TilesetEditorMetatileSelector::buildImage(int metatileIdStart, int numMetatiles) {
     int numMetatilesHigh = this->numRows(numMetatiles);
-    int numPrimary = this->primaryTileset->metatiles.length();
+    int numPrimary = this->primaryTileset->numMetatiles();
     int maxPrimary = Project::getNumMetatilesPrimary();
     bool includesPrimary = metatileIdStart < maxPrimary;
 
@@ -96,7 +96,7 @@ void TilesetEditorMetatileSelector::updateSelectedMetatile() {
     if (Tileset::metatileIsValid(metatileId, this->primaryTileset, this->secondaryTileset))
         this->selectedMetatile = metatileId;
     else
-        this->selectedMetatile = Project::getNumMetatilesPrimary() + this->secondaryTileset->metatiles.length() - 1;
+        this->selectedMetatile = Project::getNumMetatilesPrimary() + this->secondaryTileset->numMetatiles() - 1;
     emit selectedMetatileChanged(this->selectedMetatile);
 }
 
@@ -106,10 +106,10 @@ uint16_t TilesetEditorMetatileSelector::getSelectedMetatileId() {
 
 uint16_t TilesetEditorMetatileSelector::getMetatileId(int x, int y) {
     int index = y * this->numMetatilesWide + x;
-    if (index < this->primaryTileset->metatiles.length()) {
+    if (index < this->primaryTileset->numMetatiles()) {
         return static_cast<uint16_t>(index);
     } else {
-        return static_cast<uint16_t>(Project::getNumMetatilesPrimary() + index - this->primaryTileset->metatiles.length());
+        return static_cast<uint16_t>(Project::getNumMetatilesPrimary() + index - this->primaryTileset->numMetatiles());
     }
 }
 
@@ -155,7 +155,7 @@ QPoint TilesetEditorMetatileSelector::getMetatileIdCoords(uint16_t metatileId) {
     }
     int index = metatileId < Project::getNumMetatilesPrimary()
                 ? metatileId
-                : metatileId - Project::getNumMetatilesPrimary() + this->primaryTileset->metatiles.length();
+                : metatileId - Project::getNumMetatilesPrimary() + this->primaryTileset->numMetatiles();
     return QPoint(index % this->numMetatilesWide, index / this->numMetatilesWide);
 }
 
@@ -232,8 +232,8 @@ void TilesetEditorMetatileSelector::drawUnused() {
     QPainter unusedPainter(&metatilesPixmap);
     unusedPainter.setOpacity(0.5);
 
-    int primaryLength = this->primaryTileset->metatiles.length();
+    int primaryLength = this->primaryTileset->numMetatiles();
-    int length_ = primaryLength + this->secondaryTileset->metatiles.length();
+    int length_ = primaryLength + this->secondaryTileset->numMetatiles();
 
     for (int i = 0; i < length_; i++) {
         int tile = i;
@@ -271,8 +271,8 @@ void TilesetEditorMetatileSelector::drawCounts() {
     whitePen.setWidth(1);
     countPainter.setPen(whitePen);
 
-    int primaryLength = this->primaryTileset->metatiles.length();
+    int primaryLength = this->primaryTileset->numMetatiles();
-    int length_ = primaryLength + this->secondaryTileset->metatiles.length();
+    int length_ = primaryLength + this->secondaryTileset->numMetatiles();
 
     for (int i = 0; i < length_; i++) {
         int tile = i;