From d44aae4f43597db578d73dfb52db3c342b1deaf4 Mon Sep 17 00:00:00 2001 From: ArsenArsen Date: Sun, 2 Jul 2017 20:51:15 +0200 Subject: [PATCH] Fix a memory leak with the crop editor I did not properly handle deleting the editor so it would stay in memory permanently. --- colorpicker/colorpickerscene.cpp | 4 ++-- colorpicker/colorpickerscene.hpp | 2 +- cropeditor/cropeditor.cpp | 22 +++++++++--------- cropeditor/cropeditor.hpp | 5 ++--- cropeditor/cropscene.cpp | 11 ++++----- cropeditor/cropscene.hpp | 6 ++--- cropeditor/cropview.cpp | 9 -------- cropeditor/cropview.hpp | 3 --- cropeditor/drawing/bluritem.cpp | 4 ++-- main.cpp | 2 +- recording/recordingcontroller.cpp | 9 ++++---- screenshotter.cpp | 5 +---- screenshotutil.cpp | 37 +++++++++---------------------- screenshotutil.hpp | 6 ++--- uploaders/uploadersingleton.cpp | 5 ++--- uploaders/uploadersingleton.hpp | 2 +- 16 files changed, 49 insertions(+), 83 deletions(-) diff --git a/colorpicker/colorpickerscene.cpp b/colorpicker/colorpickerscene.cpp index 0180e9b..3cb868f 100644 --- a/colorpicker/colorpickerscene.cpp +++ b/colorpicker/colorpickerscene.cpp @@ -6,7 +6,7 @@ #include #include -ColorPickerScene::ColorPickerScene(QPixmap *pixmap, QWidget *parentWidget) +ColorPickerScene::ColorPickerScene(QPixmap pixmap, QWidget *parentWidget) : QGraphicsScene(), QGraphicsView(this, parentWidget) { setFrameShape(QFrame::NoFrame); // Time taken to solve: A george99g and 38 minutes. setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff); @@ -16,7 +16,7 @@ ColorPickerScene::ColorPickerScene(QPixmap *pixmap, QWidget *parentWidget) setCursor(QCursor(Qt::CrossCursor)); setMouseTracking(true); - pItem = addPixmap(*pixmap); + pItem = addPixmap(pixmap); pItem->setZValue(-2); ellipse = addEllipse(QRectF(QCursor::pos(), QSize(20, 20)), QPen(Qt::cyan), Qt::NoBrush); QFont font("Monospace"); diff --git a/colorpicker/colorpickerscene.hpp b/colorpicker/colorpickerscene.hpp index ab42f83..da4bfc4 100644 --- a/colorpicker/colorpickerscene.hpp +++ b/colorpicker/colorpickerscene.hpp @@ -12,7 +12,7 @@ class ColorPickerScene : public QGraphicsScene, public QGraphicsView { public: - ColorPickerScene(QPixmap *pixmap, QWidget *parentWidget); + ColorPickerScene(QPixmap pixmap, QWidget *parentWidget); void mouseMoveEvent(QGraphicsSceneMouseEvent *event) override; void keyPressEvent(QKeyEvent *event) override; void mouseReleaseEvent(QGraphicsSceneMouseEvent *) override; diff --git a/cropeditor/cropeditor.cpp b/cropeditor/cropeditor.cpp index c2b8636..50c8a58 100644 --- a/cropeditor/cropeditor.cpp +++ b/cropeditor/cropeditor.cpp @@ -9,33 +9,31 @@ #include #include -CropEditor::CropEditor(QPixmap *image, QObject *parent) : QObject(parent) { +CropEditor::CropEditor(QPixmap image, QObject *parent) : QObject(parent) { scene = new CropScene(parent, image); view = new CropView(scene); qreal ratio = QApplication::primaryScreen()->devicePixelRatio(); - pixmapItem = new QGraphicsPixmapItem(*image); + QGraphicsPixmapItem *pixmapItem = new QGraphicsPixmapItem(image); pixmapItem->setZValue(-1); pixmapItem->setScale(1 / ratio); scene->addItem(pixmapItem); - scene->setSceneRect(image->rect()); - view->resize(image->width(), image->height()); - view->setMinimumSize(image->size()); + scene->setSceneRect(image.rect()); + view->resize(image.width(), image.height()); + view->setMinimumSize(image.size()); view->move(0, 0); view->setWindowTitle("KShare Crop Editor"); view->show(); + connect(scene, &CropScene::closedWithRect, this, &CropEditor::crop); } CropEditor::~CropEditor() { - delete scene; - delete view; + scene->deleteLater(); + view->deleteLater(); } void CropEditor::crop(QRect rect) { - QPixmap map = view->grab(rect); - QPixmap *cropp = new QPixmap; - map.swap(*cropp); - delete view; - emit cropped(cropp); + if (rect.isValid()) emit cropped(view->grab(rect)); + deleteLater(); } diff --git a/cropeditor/cropeditor.hpp b/cropeditor/cropeditor.hpp index 9f6cd47..b30d1ff 100644 --- a/cropeditor/cropeditor.hpp +++ b/cropeditor/cropeditor.hpp @@ -11,16 +11,15 @@ class CropEditor : public QObject { Q_OBJECT public: - CropEditor(QPixmap *image, QObject *parent = 0); + CropEditor(QPixmap image, QObject *parent = 0); ~CropEditor(); signals: - QPixmap *cropped(QPixmap *pixmap); + QPixmap *cropped(QPixmap pixmap); private: void crop(QRect rect); CropScene *scene = nullptr; CropView *view = nullptr; - QGraphicsPixmapItem *pixmapItem = nullptr; }; #endif // CROPEDITOR_HPP diff --git a/cropeditor/cropscene.cpp b/cropeditor/cropscene.cpp index 3b92f62..876a96f 100644 --- a/cropeditor/cropscene.cpp +++ b/cropeditor/cropscene.cpp @@ -20,7 +20,7 @@ #include #include -CropScene::CropScene(QObject *parent, QPixmap *pixmap) +CropScene::CropScene(QObject *parent, QPixmap pixmap) : QGraphicsScene(parent), drawingSelectionMaker([] { return nullptr; }), prevButtons(Qt::NoButton), _brush(Qt::SolidPattern), _font(settings::settings().value("font", QFont()).value()) { _pixmap = pixmap; @@ -65,7 +65,7 @@ CropScene::CropScene(QObject *parent, QPixmap *pixmap) }); menu.addAction(settings); - magnifier = addPixmap(pixmap->copy(0, 0, 11, 11).scaled(110, 110)); + magnifier = addPixmap(pixmap.copy(0, 0, 11, 11).scaled(110, 110)); magnifierBox = addRect(magnifier->boundingRect(), QPen(Qt::cyan)); magnifier->setZValue(1); magnifierBox->setZValue(1.1); @@ -257,7 +257,7 @@ void CropScene::contextMenuEvent(QGraphicsSceneContextMenuEvent *e) { } void CropScene::keyReleaseEvent(QKeyEvent *event) { - if (event->key() == Qt::Key_Return || event->key() == Qt::Key_Enter) done(); // Segfault + if (event->key() == Qt::Key_Return || event->key() == Qt::Key_Enter || event->key() == Qt::Key_Escape) done(); } void CropScene::updateMag(QPointF scenePos) { @@ -275,7 +275,7 @@ void CropScene::updateMag(QPointF scenePos) { QPointF magnifierPos = scenePos + QPointF(5, 5); magnifier->setPos(magnifierPos); - magnifier->setPixmap(_pixmap->copy(magnifierTopLeft.x(), magnifierTopLeft.y(), pixCnt, pixCnt).scaled(110, 110)); + magnifier->setPixmap(_pixmap.copy(magnifierTopLeft.x(), magnifierTopLeft.y(), pixCnt, pixCnt).scaled(110, 110)); QPointF bottomRight = magnifierHintBox->sceneBoundingRect().bottomRight(); if (magnifier->sceneBoundingRect().bottom() > bottomRight.y()) bottomRight.setY(magnifier->sceneBoundingRect().bottom()); @@ -319,5 +319,6 @@ void CropScene::done() { magnifierHint->setVisible(false); magnifierHintBox->setVisible(false); emit closedWithRect(rect->rect().toRect()); - } + } else + emit closedWithRect(QRect()); } diff --git a/cropeditor/cropscene.hpp b/cropeditor/cropscene.hpp index dd12340..cb04c74 100644 --- a/cropeditor/cropscene.hpp +++ b/cropeditor/cropscene.hpp @@ -16,13 +16,13 @@ class CropScene; class CropScene : public QGraphicsScene { Q_OBJECT public: - CropScene(QObject *parent, QPixmap *pixmap); + CropScene(QObject *parent, QPixmap pixmap); ~CropScene(); QPen &pen(); QBrush &brush(); QFont &font(); void setDrawingSelection(QString name, std::function drawAction); - QPixmap *pixmap() { + QPixmap pixmap() { return _pixmap; } QGraphicsPolygonItem *polyItm() { @@ -58,7 +58,7 @@ private: bool fullscreen; std::function drawingSelectionMaker; QFlags prevButtons; - QPixmap *_pixmap; + QPixmap _pixmap; QGraphicsRectItem *rect = nullptr; QGraphicsPixmapItem *magnifier = nullptr; QGraphicsRectItem *magnifierBox = nullptr; diff --git a/cropeditor/cropview.cpp b/cropeditor/cropview.cpp index 3bc12e8..c4599c3 100644 --- a/cropeditor/cropview.cpp +++ b/cropeditor/cropview.cpp @@ -10,12 +10,3 @@ CropView::CropView(QGraphicsScene *scene) : QGraphicsView(scene) { setMouseTracking(true); setSizePolicy(QSizePolicy(QSizePolicy::Fixed, QSizePolicy::Fixed)); } - -void CropView::keyPressEvent(QKeyEvent *e) { - if (e->key() == Qt::Key_Escape) { - close(); - e->accept(); - return; - } - QGraphicsView::keyPressEvent(e); -} diff --git a/cropeditor/cropview.hpp b/cropeditor/cropview.hpp index 66492c9..fab9983 100644 --- a/cropeditor/cropview.hpp +++ b/cropeditor/cropview.hpp @@ -7,9 +7,6 @@ class CropView : public QGraphicsView { public: CropView(QGraphicsScene *scene); - -protected: - void keyPressEvent(QKeyEvent *e) override; }; #endif // CROPVIEW_HPP diff --git a/cropeditor/drawing/bluritem.cpp b/cropeditor/drawing/bluritem.cpp index f687dbe..aa2b2a8 100644 --- a/cropeditor/drawing/bluritem.cpp +++ b/cropeditor/drawing/bluritem.cpp @@ -17,14 +17,14 @@ void BlurItem::mouseDragEvent(QGraphicsSceneMouseEvent *e, CropScene *scene) { if (pos.isNull()) { pos = e->scenePos(); rect = scene->addRect(QRect(e->scenePos().toPoint(), QSize(1, 1)), QPen(Qt::cyan), Qt::NoBrush); - pixmap = scene->addPixmap(scene->pixmap()->copy(rect->rect().toRect())); + pixmap = scene->addPixmap(scene->pixmap().copy(rect->rect().toRect())); pixmap->setPos(e->scenePos()); pixmap->setZValue(rect->zValue() - 0.1); pixmap->setGraphicsEffect(effect); } else { QPointF p = e->scenePos(); rect->setRect(QRect(qMin(pos.x(), p.x()), qMin(pos.y(), p.y()), qAbs(pos.x() - p.x()), qAbs(pos.y() - p.y()))); - pixmap->setPixmap(scene->pixmap()->copy(rect->rect().toRect())); + pixmap->setPixmap(scene->pixmap().copy(rect->rect().toRect())); pixmap->setPos(rect->rect().topLeft()); } } diff --git a/main.cpp b/main.cpp index e38db55..ac51471 100644 --- a/main.cpp +++ b/main.cpp @@ -88,7 +88,7 @@ int main(int argc, char *argv[]) { Worker::init(); a.connect(&a, &QApplication::aboutToQuit, Worker::end); a.connect(&a, &QApplication::aboutToQuit, [] { stillAlive = false; }); - screenshotutil::fullscreen()->save("/home/arsen/test.png", "PNG"); + if (!parser.isSet(h)) w.show(); return a.exec(); } diff --git a/recording/recordingcontroller.cpp b/recording/recordingcontroller.cpp index 19d49b1..5320e70 100644 --- a/recording/recordingcontroller.cpp +++ b/recording/recordingcontroller.cpp @@ -78,15 +78,14 @@ void RecordingController::timeout() { time++; int localTime = time * timer.interval() - 3000; if (localTime > 0) { - QPixmap *pp = screenshotutil::fullscreenArea(settings::settings().value("captureCursor", true).toBool(), - area.x(), area.y(), area.width(), area.height()); - QScopedPointer p(pp); + QPixmap pp = screenshotutil::fullscreenArea(settings::settings().value("captureCursor", true).toBool(), + area.x(), area.y(), area.width(), area.height()); WorkerContext *context = new WorkerContext; context->consumer = _context->consumer; context->targetFormat = _context->format; - context->pixmap = *pp; + context->pixmap = pp; frame++; - preview->setPixmap(*pp); + preview->setPixmap(pp); Worker::queue(context); } long second = localTime / 1000 % 60; diff --git a/screenshotter.cpp b/screenshotter.cpp index c583557..18e5655 100644 --- a/screenshotter.cpp +++ b/screenshotter.cpp @@ -10,10 +10,7 @@ void screenshotter::area() { CropEditor *editor = new CropEditor(screenshotutil::fullscreen(settings::settings().value("captureCursor", true).toBool())); - QObject::connect(editor, &CropEditor::cropped, [&](QPixmap *pixmap) { - UploaderSingleton::inst().upload(pixmap); - QScopedPointer(editor); - }); + QObject::connect(editor, &CropEditor::cropped, [&](QPixmap pixmap) { UploaderSingleton::inst().upload(pixmap); }); } void screenshotter::fullscreen() { diff --git a/screenshotutil.cpp b/screenshotutil.cpp index 786f2e1..8e04679 100644 --- a/screenshotutil.cpp +++ b/screenshotutil.cpp @@ -7,55 +7,40 @@ #include #include -QPixmap *screenshotutil::fullscreen(bool cursor) { +QPixmap screenshotutil::fullscreen(bool cursor) { int height = 0, width = 0; for (QScreen *screen : QApplication::screens()) { width += screen->size().width(); int h = screen->size().height(); height = h > height ? h : height; } - QPixmap *image = new QPixmap(width, height); - image->fill(Qt::transparent); - QPainter painter(image); + QPixmap image(width, height); + image.fill(Qt::transparent); + QPainter painter(&image); width = 0; for (QScreen *screen : QApplication::screens()) { - - QPixmap *currentScreen = window(0, screen); - painter.drawPixmap(width, 0, currentScreen->copy()); - delete currentScreen; + QPixmap currentScreen = window(0, screen); + painter.drawPixmap(width, 0, currentScreen); width += screen->size().width(); } - painter.end(); #ifdef PLATFORM_CAPABILITY_CURSOR if (cursor) { - QPixmap *noCursor = image; - QScopedPointer p(noCursor); - QPixmap *withCursor = new QPixmap(*noCursor); - QPainter painter(withCursor); auto cursorData = PlatformBackend::inst().getCursor(); painter.drawPixmap(QCursor::pos() - std::get<0>(cursorData), std::get<1>(cursorData)); - painter.end(); - return withCursor; } + painter.end(); #endif return image; } -QPixmap *screenshotutil::window(WId wid, QScreen *w) { - QPixmap screen = w->grabWindow(wid); - QPixmap *pm = new QPixmap(screen.size()); - screen.swap(*pm); - return pm; +QPixmap screenshotutil::window(WId wid, QScreen *w) { + return w->grabWindow(wid); } void screenshotutil::toClipboard(QString value) { QApplication::clipboard()->setText(value); } -QPixmap *screenshotutil::fullscreenArea(bool cursor, qreal x, qreal y, qreal w, qreal h) { - QPixmap *cropped = new QPixmap; - QPixmap *scr = fullscreen(cursor); - scr->copy(x, y, w, h).swap(*cropped); - delete scr; - return cropped; +QPixmap screenshotutil::fullscreenArea(bool cursor, qreal x, qreal y, qreal w, qreal h) { + return fullscreen(cursor).copy(x, y, w, h); } diff --git a/screenshotutil.hpp b/screenshotutil.hpp index 70e36c1..02808ea 100644 --- a/screenshotutil.hpp +++ b/screenshotutil.hpp @@ -5,9 +5,9 @@ #include namespace screenshotutil { -QPixmap *fullscreen(bool cursor = true); -QPixmap *fullscreenArea(bool cursor = true, qreal x = 0, qreal y = 0, qreal w = -1, qreal h = -1); -QPixmap *window(WId wid, QScreen *w = QApplication::primaryScreen()); +QPixmap fullscreen(bool cursor = true); +QPixmap fullscreenArea(bool cursor = true, qreal x = 0, qreal y = 0, qreal w = -1, qreal h = -1); +QPixmap window(WId wid, QScreen *w = QApplication::primaryScreen()); void toClipboard(QString value); } diff --git a/uploaders/uploadersingleton.cpp b/uploaders/uploadersingleton.cpp index e0e7b1f..359d232 100644 --- a/uploaders/uploadersingleton.cpp +++ b/uploaders/uploadersingleton.cpp @@ -59,7 +59,7 @@ void UploaderSingleton::registerUploader(Uploader *uploader) { emit newUploader(uploader); } -void UploaderSingleton::upload(QPixmap *pixmap) { +void UploaderSingleton::upload(QPixmap pixmap) { auto u = uploaders.value(uploader); if (!u->validate()) { u = uploaders.value("imgur"); @@ -72,12 +72,11 @@ void UploaderSingleton::upload(QPixmap *pixmap) { format.toLower()))); if (file.open(QFile::ReadWrite)) { - pixmap->save(&file, format.toLocal8Bit().constData(), settings::settings().value("imageQuality", -1).toInt()); + pixmap.save(&file, format.toLocal8Bit().constData(), settings::settings().value("imageQuality", -1).toInt()); file.seek(0); u->doUpload(file.readAll(), format); } else notifications::notify("KShare - Failed to save picture", file.errorString(), QSystemTrayIcon::Warning); - delete pixmap; } void UploaderSingleton::upload(QByteArray img, QString format) { diff --git a/uploaders/uploadersingleton.hpp b/uploaders/uploadersingleton.hpp index 92862f1..fc1fb2a 100644 --- a/uploaders/uploadersingleton.hpp +++ b/uploaders/uploadersingleton.hpp @@ -13,7 +13,7 @@ public: return inst; } void registerUploader(Uploader *uploader); - void upload(QPixmap *pixmap); + void upload(QPixmap pixmap); void upload(QByteArray img, QString format); void upload(QFile &img, QString format); void showSettings();