Rewrite the export as image code a bit

Don't store the observer in the part, just create one when we need it
Remove some mutex/atomics, all happens on the main thread
This commit is contained in:
Albert Astals Cid 2024-07-17 16:16:04 +02:00
parent 8db8ddba5c
commit 2f16843ff1
6 changed files with 70 additions and 106 deletions

View file

@ -667,11 +667,6 @@ Okular::TilesManager *PixmapRequestPrivate::tilesManager() const
return mPage->d->tilesManager(mObserver);
}
PixmapRequestPrivate *PixmapRequestPrivate::get(const PixmapRequest *req)
{
return req->d;
}
void PixmapRequestPrivate::swap()
{
std::swap(mWidth, mHeight);

View file

@ -77,7 +77,10 @@ public:
void swap();
TilesManager *tilesManager() const;
static PixmapRequestPrivate *get(const PixmapRequest *req);
static PixmapRequestPrivate *get(const PixmapRequest *req)
{
return req->d;
}
DocumentObserver *mObserver;
int mPageNumber;

View file

@ -22,7 +22,6 @@
#include <QVBoxLayout>
#include <QWidget>
#include <atomic>
#include <utility>
#include <vector>
@ -30,24 +29,13 @@
#include <KMessageBox>
#include "core/document.h"
#include "core/generator_p.h"
#include "core/observer.h"
#include "core/page.h"
ExportImageDialog::ExportImageDialog(Okular::Document *document, QString *dirPath, ExportImageDocumentObserver *observer, QWidget *parent)
ExportImageDialog::ExportImageDialog(Okular::Document *document, QWidget *parent)
: QDialog(parent)
, m_document(document)
, m_dirPath(dirPath)
, m_observer(observer)
, m_parent(parent)
{
initUI();
}
ExportImageDialog::~ExportImageDialog()
{
}
void ExportImageDialog::initUI()
{
m_imageTypeLabel = new QLabel(i18n("Type:"), this);
m_PNGTypeLabel = new QLabel(i18n("PNG"), this);
@ -60,7 +48,7 @@ void ExportImageDialog::initUI()
m_dirPathBrowseButton = new QPushButton(i18n("..."), this);
m_dirPathBrowseButton->setMaximumSize(30, 30);
m_dirPathBrowseButton->setToolTip(i18n("The images would be exported in a subfolder in this path of the same name as the file"));
connect(m_dirPathBrowseButton, &QPushButton::clicked, this, &ExportImageDialog::searchFileName);
connect(m_dirPathBrowseButton, &QPushButton::clicked, this, &ExportImageDialog::browseClicked);
// Options tab
m_exportRangeGroupBox = new QGroupBox(i18n("Export range"), this);
@ -121,8 +109,8 @@ void ExportImageDialog::initUI()
QDialogButtonBox *buttonBox = new QDialogButtonBox(QDialogButtonBox::Ok | QDialogButtonBox::Cancel, this);
buttonBox->button(QDialogButtonBox::Ok)->setText(i18n("Export"));
connect(buttonBox->button(QDialogButtonBox::Ok), &QPushButton::clicked, this, &ExportImageDialog::exportImage);
connect(buttonBox->button(QDialogButtonBox::Cancel), &QPushButton::clicked, this, [this] { QDialog::done(Canceled); });
connect(buttonBox->button(QDialogButtonBox::Ok), &QPushButton::clicked, this, &ExportImageDialog::okClicked);
connect(buttonBox->button(QDialogButtonBox::Cancel), &QPushButton::clicked, this, [this] { done(Canceled); });
QHBoxLayout *dirPathLayout = new QHBoxLayout;
dirPathLayout->addWidget(m_dirPathLineEdit);
@ -142,7 +130,7 @@ void ExportImageDialog::initUI()
setWindowTitle(i18n("Export Image"));
}
void ExportImageDialog::searchFileName()
void ExportImageDialog::browseClicked()
{
QString dirPath = QFileDialog::getExistingDirectory(this, QString(), QDir::homePath(), QFileDialog::ShowDirsOnly);
if (!(dirPath.isEmpty())) {
@ -150,7 +138,7 @@ void ExportImageDialog::searchFileName()
}
}
void ExportImageDialog::exportImage()
void ExportImageDialog::okClicked()
{
std::vector<std::pair<int, int>> pageRanges;
if (m_allPagesRadioButton->isChecked()) {
@ -171,19 +159,19 @@ void ExportImageDialog::exportImage()
if (range.size() == 1) {
int pageVal = range[0].toInt(&ok);
if (!ok || pageVal < 1 || pageVal > static_cast<int>(m_document->pages())) {
QDialog::done(InvalidOptions);
done(InvalidOptions);
return;
}
pageRanges.emplace_back(pageVal, pageVal);
} else if (range.size() == 2) {
int pageStart = range[0].toInt(&ok);
if (!ok || pageStart < 1 || pageStart > static_cast<int>(m_document->pages())) {
QDialog::done(InvalidOptions);
done(InvalidOptions);
return;
}
int pageEnd = range[1].toInt(&ok);
if (!ok || pageEnd < 1 || pageEnd > static_cast<int>(m_document->pages())) {
QDialog::done(InvalidOptions);
done(InvalidOptions);
return;
}
if (pageStart > pageEnd) {
@ -192,7 +180,7 @@ void ExportImageDialog::exportImage()
pageRanges.emplace_back(pageStart, pageEnd);
}
} else {
QDialog::done(InvalidOptions);
done(InvalidOptions);
return;
}
}
@ -201,15 +189,12 @@ void ExportImageDialog::exportImage()
for (int i = p.first; i <= p.second; i++) {
int width = (int)((m_document->page(i - 1))->width());
int height = (int)((m_document->page(i - 1))->height());
Okular::PixmapRequest *request = new Okular::PixmapRequest(m_observer, i - 1, width, height, 1 /* dpr */, 1, Okular::PixmapRequest::Asynchronous);
m_observer->addToPixmapRequestList(request);
Okular::PixmapRequest *request = new Okular::PixmapRequest(nullptr, i - 1, width, height, 1 /* dpr */, 1, Okular::PixmapRequest::Asynchronous);
m_pixmapRequestList << request;
}
}
*m_dirPath = m_dirPathLineEdit->text();
m_observer->m_document = m_document;
m_observer->m_dirPath = *m_dirPath;
m_observer->m_parent = m_parent;
QDialog::done(Accepted);
done(Accepted);
}
void ExportImageDocumentObserver::notifyPageChanged(int page, int flags)
@ -217,14 +202,12 @@ void ExportImageDocumentObserver::notifyPageChanged(int page, int flags)
if (!(flags & Okular::DocumentObserver::Pixmap)) {
return;
}
getPixmapAndSave(page);
QMetaObject::invokeMethod(this, [this, page] { getPixmapAndSave(page); }, Qt::QueuedConnection);
}
void ExportImageDocumentObserver::getPixmapAndSave(int page)
{
m_progressCanceledMutex.lock();
if (m_progressCanceled) {
m_progressCanceledMutex.unlock();
return;
}
const QPixmap *pixmap = m_document->page(page)->getPixmap(this);
@ -233,28 +216,22 @@ void ExportImageDocumentObserver::getPixmapAndSave(int page)
QString filePath = dir.filePath(fileName);
bool status = pixmap->save(filePath, "PNG");
if (!status) {
KMessageBox::error(m_parent, i18n("Failed to save a file ") + fileName, i18n("Failed"));
KMessageBox::error(m_progressDialog->parentWidget(), i18n("Failed to save a file %1").arg(fileName), i18n("Failed"));
} else {
m_progressValue.fetch_add(1);
int value = m_progressValue.load();
m_progressDialog->setValue(value);
if (value == m_progressDialog->maximum()) {
++m_progressValue;
m_progressDialog->setValue(m_progressValue);
if (m_progressValue == m_progressDialog->maximum()) {
delete m_progressDialog;
}
}
m_progressCanceledMutex.unlock();
}
void ExportImageDocumentObserver::addToPixmapRequestList(Okular::PixmapRequest *request)
void ExportImageDocumentObserver::doExport(const QList<Okular::PixmapRequest *> &pixmapRequestList, Okular::Document *document, const QString &dirPath, QWidget *parent)
{
m_pixmapRequestList << request;
}
m_document = document;
m_dirPath = dirPath;
bool ExportImageDocumentObserver::getOrRequestPixmaps()
{
m_progressCanceledMutex.lock();
m_progressCanceled = false;
m_progressCanceledMutex.unlock();
QFileInfo info(m_document->documentInfo().get(Okular::DocumentInfo::FilePath));
QString baseDirPath = m_dirPath + QDir::separator() + info.baseName();
m_dirPath = baseDirPath;
@ -266,36 +243,26 @@ bool ExportImageDocumentObserver::getOrRequestPixmaps()
}
bool status = dir.mkpath(m_dirPath);
if (!status) {
return false;
KMessageBox::error(parent, i18n("Failed creating folder to save images to: %1").arg(m_dirPath), i18n("Failed"));
return;
}
QList<Okular::PixmapRequest *> requestsToProcess;
m_progressDialog = new QProgressDialog(i18n("Exporting images"), i18n("Cancel"), 0, m_document->pages(), m_parent);
m_progressDialog = new QProgressDialog(i18n("Exporting images"), i18n("Cancel"), 0, pixmapRequestList.count(), parent);
connect(m_progressDialog, &QProgressDialog::canceled, this, &ExportImageDocumentObserver::progressDialogCanceled);
m_progressDialog->show();
m_progressValue.store(0);
for (Okular::PixmapRequest *r : std::as_const(m_pixmapRequestList)) {
// If a page had been requested for export earlier, it might already have an associated pixmap pointer.
// If this is the case, directly get the pixmap pointed to by the same pointer.
if (m_document->page(r->pageNumber())->hasPixmap(r->observer(), r->width(), r->height(), r->normalizedRect())) {
getPixmapAndSave(r->pageNumber());
delete r;
} else {
requestsToProcess << r;
// Request delete not required in this case since Document::requestPixmaps internally does delete the request
// both in case of success and failure.
}
m_progressValue = 0;
for (Okular::PixmapRequest *r : pixmapRequestList) {
// This is because we passed nullptr in PixmapRequest constructor in okClicked
// because we don't want ExportImageDialog to worry about observer
Okular::PixmapRequestPrivate *priv = Okular::PixmapRequestPrivate::get(r);
priv->mObserver = this;
}
m_document->requestPixmaps(requestsToProcess, Okular::Document::PixmapRequestFlag::RemoveAllPrevious);
m_pixmapRequestList.clear();
return true;
m_document->requestPixmaps(pixmapRequestList, Okular::Document::PixmapRequestFlag::RemoveAllPrevious);
m_progressDialog->exec();
}
void ExportImageDocumentObserver::progressDialogCanceled()
{
m_document->cancelPixmapRequests(this);
m_progressCanceledMutex.lock();
m_progressDialog->cancel();
m_progressCanceled = true;
delete m_progressDialog;
m_progressCanceledMutex.unlock();
}

View file

@ -32,20 +32,19 @@ class ExportImageDocumentObserver : public QObject, public Okular::DocumentObser
Q_OBJECT
public:
void notifyPageChanged(int page, int flags) override;
void doExport(const QList<Okular::PixmapRequest *> &pixmapRequestList, Okular::Document *document, const QString &dirPath, QWidget *parent);
private Q_SLOTS:
void progressDialogCanceled();
private:
void getPixmapAndSave(int page);
void addToPixmapRequestList(Okular::PixmapRequest *request);
bool getOrRequestPixmaps();
Okular::Document *m_document;
QString m_dirPath;
QList<Okular::PixmapRequest *> m_pixmapRequestList;
QWidget *m_parent;
QProgressDialog *m_progressDialog;
std::atomic<int> m_progressValue;
int m_progressValue;
bool m_progressCanceled;
QMutex m_progressCanceledMutex;
private Q_SLOTS:
void progressDialogCanceled();
};
class ExportImageDialog : public QDialog
@ -54,14 +53,20 @@ class ExportImageDialog : public QDialog
public:
enum DialogCloseCode { Accepted, Canceled, InvalidOptions };
ExportImageDialog(Okular::Document *document, QString *dirPath, ExportImageDocumentObserver *observer, QWidget *parent = nullptr);
~ExportImageDialog() override;
ExportImageDialog(Okular::Document *document, QWidget *parent = nullptr);
QString dirPath() const
{
return m_dirPathLineEdit->text();
}
QList<Okular::PixmapRequest *> pixmapRequestList() const
{
return m_pixmapRequestList;
}
private:
Okular::Document *m_document;
QString *m_dirPath;
ExportImageDocumentObserver *m_observer;
QWidget *m_parent;
QLabel *m_imageTypeLabel;
QLabel *m_PNGTypeLabel;
@ -80,11 +85,11 @@ private:
QPushButton *m_dirPathBrowseButton;
void initUI();
QList<Okular::PixmapRequest *> m_pixmapRequestList;
private Q_SLOTS:
void searchFileName();
void exportImage();
}; //
void browseClicked();
void okClicked();
};
#endif // _OKULAR_EXPORTIMAGEDIALOG_H_
#endif // _OKULAR_EXPORTIMAGEDIALOG_H_

View file

@ -509,9 +509,6 @@ Part::Part(QObject *parent, const QVariantList &args)
connect(m_reviewsWidget.data(), &Reviews::openAnnotationWindow, m_pageView.data(), &PageView::openAnnotationWindow);
// Initialize an image export document observer
m_exportImageDocumentObserver = new ExportImageDocumentObserver;
// add document observers
m_document->addObserver(this);
m_document->addObserver(m_thumbnailList);
@ -526,7 +523,6 @@ Part::Part(QObject *parent, const QVariantList &args)
m_document->addObserver(m_pageSizeLabel);
m_document->addObserver(m_bookmarkList);
m_document->addObserver(m_signaturePanel);
m_document->addObserver(m_exportImageDocumentObserver);
connect(m_document->bookmarkManager(), &BookmarkManager::saved, this, &Part::slotRebuildBookmarkMenu);
@ -977,7 +973,6 @@ Part::~Part()
delete m_bookmarkList;
delete m_infoTimer;
delete m_signaturePanel;
delete m_exportImageDocumentObserver;
delete m_document;
@ -3475,13 +3470,20 @@ void Part::slotExportAs(QAction *act)
break;
case ExportFormat::Image: {
// In the context of image export, the fileName is actually dirName
ExportImageDialog exportImageDialog(m_document, &fileName, m_exportImageDocumentObserver, widget());
ExportImageDialog exportImageDialog(m_document, widget());
int dialogResult = exportImageDialog.exec();
if (dialogResult == ExportImageDialog::InvalidOptions) {
KMessageBox::information(widget(), i18n("Invalid options have been received."));
break;
} else if (dialogResult == ExportImageDialog::Canceled) {
break;
} else {
Q_ASSERT(dialogResult == ExportImageDialog::Accepted);
ExportImageDocumentObserver imageExportObserver;
m_document->addObserver(&imageExportObserver);
imageExportObserver.doExport(exportImageDialog.pixmapRequestList(), m_document, exportImageDialog.dirPath(), widget());
m_document->removeObserver(&imageExportObserver);
}
break;
}
@ -3500,10 +3502,7 @@ void Part::slotExportAs(QAction *act)
case ExportFormat::PlainText:
saved = m_document->exportToText(fileName);
break;
case ExportFormat::Image: {
saved = m_exportImageDocumentObserver->getOrRequestPixmaps();
break;
}
default:
break;
}

View file

@ -39,7 +39,6 @@
#include "../interfaces/viewerinterface.h"
#include "../kdocumentviewer.h"
#include "exportimagedialog.h"
#include "okularpart_export.h"
class QAction;
@ -56,7 +55,6 @@ class KToggleFullScreenAction;
class QTemporaryFile;
class QAction;
class QJsonObject;
namespace KParts
{
class GUIActivateEvent;
@ -353,9 +351,6 @@ private:
bool m_swapInsteadOfOpening; // if set, the next open operation will replace the backing file (used when reloading just saved files)
bool m_warnedAboutModifyingUnsaveableDocument = false;
// relevant document observers
ExportImageDocumentObserver *m_exportImageDocumentObserver = nullptr;
// main widgets
Sidebar *m_sidebar;
SearchWidget *m_searchWidget;