Skip to content

Commit

Permalink
PERF: Implement thumbnail storage to disk to avoid re-rendering after…
Browse files Browse the repository at this point in the history 
… series widget destruction.

PERF: Disable SEG rendering in thumbnails due to limited support and potential performance issues. Replace with a blank thumbnail featuring a document SVG.
BUG: Resolve issue hanging job termination when status is 'Queued'.
BUG: Address issue with FreezeJobsScheduling during shutdown.
BUG: Correct retry functionality when status icon on series widget is clicked.
  • Loading branch information
@Punzo @lassoan
Punzo authored and lassoan committed Jun 11, 2024
1 parent b7e1732 commit 065f4df
Show file tree
Hide file tree
Showing 14 changed files with 325 additions and 279 deletions.
109 changes: 35 additions & 74 deletions Libs/Core/ctkJobScheduler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,17 @@ void ctkJobSchedulerPrivate::init()
void ctkJobSchedulerPrivate::onQueueJobsInThreadPool()
{
Q_Q(ctkJobScheduler);

if (this->FreezeJobsScheduling)
{
QMutexLocker locker(&this->QueueMutex);
if (this->FreezeJobsScheduling)
{
return;
}
}

{
// The QMutexLocker is enclosed within brackets to restrict its scope and
// prevent conflicts with other QMutexLockers within the scheduler's methods.
QMutexLocker locker(&this->QueueMutex);


foreach (QThread::Priority priority, (QList<QThread::Priority>()
<< QThread::Priority::HighestPriority
Expand All @@ -86,6 +91,11 @@ void ctkJobSchedulerPrivate::onQueueJobsInThreadPool()
{
foreach (QSharedPointer<ctkAbstractJob> job, this->JobsQueue)
{
if (this->FreezeJobsScheduling)
{
return;
}

if (job->priority() != priority)
{
continue;
Expand Down Expand Up @@ -130,6 +140,16 @@ bool ctkJobSchedulerPrivate::insertJob(QSharedPointer<ctkAbstractJob> job)
return false;
}

if (this->FreezeJobsScheduling)
{
logger.debug(QString("ctkJobScheduler: job object %1 of type %2 in thread %3 "
"not added to the job list since jobs are being stopped.\n")
.arg(job->jobUID())
.arg(job->className())
.arg(QString::number(reinterpret_cast<quint64>(QThread::currentThreadId())), 16));
return false;
}

logger.debug(QString("ctkJobScheduler: creating job object %1 of type %2 in thread %3.\n")
.arg(job->jobUID())
.arg(job->className())
Expand Down Expand Up @@ -165,35 +185,30 @@ bool ctkJobSchedulerPrivate::insertJob(QSharedPointer<ctkAbstractJob> job)
};

{
// The QMutexLocker is enclosed within brackets to restrict its scope and
// prevent conflicts with other QMutexLockers within the scheduler's methods.
QMutexLocker locker(&this->QueueMutex);
this->JobsQueue.insert(job->jobUID(), job);
this->JobsConnections.insert(job->jobUID(), connections);
}

emit q->jobInitialized(job->toVariant());

if (this->FreezeJobsScheduling)
{
logger.debug(QString("ctkJobScheduler: job object %1 of type %2 in thread %3 "
"not added to the list since jobs are being stopped.\n")
.arg(job->jobUID())
.arg(job->className())
.arg(QString::number(reinterpret_cast<quint64>(QThread::currentThreadId())), 16));
return false;
}

emit this->queueJobsInThreadPool();
return true;
}

//------------------------------------------------------------------------------
bool ctkJobSchedulerPrivate::removeJob(const QString& jobUID)
{
Q_Q(ctkJobScheduler);

logger.debug(QString("ctkJobScheduler: deleting job object %1 in thread %2.\n")
.arg(jobUID)
.arg(QString::number(reinterpret_cast<quint64>(QThread::currentThreadId()), 16)));

{
// The QMutexLocker is enclosed within brackets to restrict its scope and
// prevent conflicts with other QMutexLockers within the scheduler's methods.
QMutexLocker locker(&this->QueueMutex);
QSharedPointer<ctkAbstractJob> job = this->JobsQueue.value(jobUID);
if (!job || !this->JobsConnections.contains(jobUID))
Expand All @@ -214,6 +229,7 @@ bool ctkJobSchedulerPrivate::removeJob(const QString& jobUID)
}

emit this->queueJobsInThreadPool();

return true;
}

Expand Down Expand Up @@ -257,6 +273,8 @@ void ctkJobSchedulerPrivate::removeJobs(const QStringList &jobUIDs)
//------------------------------------------------------------------------------
void ctkJobSchedulerPrivate::removeAllJobs()
{
Q_Q(ctkJobScheduler);

{
// The QMutexLocker is enclosed within brackets to restrict its scope and
// prevent conflicts with other QMutexLockers within the scheduler's methods.
Expand Down Expand Up @@ -558,38 +576,10 @@ void ctkJobScheduler::stopAllJobs(bool stopPersistentJobs)
// In addition, to speedup the cleaning of jobs, we remove them with one call removeJobs,
// instead of using the signal
QMap<QString, QMetaObject::Connection> connections = d->JobsConnections.value(jobUID);
//QObject::disconnect(connections.value("userStopped"));
QObject::disconnect(connections.value("userStopped"));
job->setStatus(ctkAbstractJob::JobStatus::UserStopped);
initializedStoppedJobsUIDs.append(jobUID);
}

// Try to stop jobs with a worker, but still not running.
// (in queue in the QThreadPool, still in the main thread)
foreach (QSharedPointer<ctkAbstractWorker> worker, d->Workers)
{
QSharedPointer<ctkAbstractJob> job = worker->jobShared();
if (!job)
{
continue;
}

QString jobUID = job->jobUID();
if (!d->JobsConnections.contains(jobUID))
{
continue;
}

// trytake stops workers not already running
// these corresponds to jobs with status Queued
if (d->ThreadPool->tryTake(worker.data()))
{
this->deleteWorker(job->jobUID());
QMap<QString, QMetaObject::Connection> connections = d->JobsConnections.value(jobUID);
//QObject::disconnect(connections.value("userStopped"));
job->setStatus(ctkAbstractJob::JobStatus::UserStopped);
initializedStoppedJobsUIDs.append(job->jobUID());
}
}
}

d->removeJobs(initializedStoppedJobsUIDs);
Expand Down Expand Up @@ -650,39 +640,10 @@ void ctkJobScheduler::stopJobsByJobUIDs(const QStringList &jobUIDs)
// In addition, to speedup the cleaning of jobs, we remove them with one call removeJobs,
// instead of using the signal
QMap<QString, QMetaObject::Connection> connections = d->JobsConnections.value(jobUID);
//QObject::disconnect(connections.value("userStopped"));
QObject::disconnect(connections.value("userStopped"));
job->setStatus(ctkAbstractJob::JobStatus::UserStopped);
initializedStoppedJobsUIDs.append(job->jobUID());
}

// Try to stop jobs with a worker, but still not running.
// (in queue in the QThreadPool, still in the main thread)
foreach (QSharedPointer<ctkAbstractWorker> worker, d->Workers)
{
QSharedPointer<ctkAbstractJob> job = worker->jobShared();
if (!job)
{
continue;
}

QString jobUID = job->jobUID();
if (jobUID.isEmpty() || !jobUIDs.contains(jobUID) ||
!d->JobsConnections.contains(jobUID))
{
continue;
}

// trytake stops workers not already running,
// these corresponds to jobs with status Queued
if (d->ThreadPool->tryTake(worker.data()))
{
this->deleteWorker(job->jobUID());
QMap<QString, QMetaObject::Connection> connections = d->JobsConnections.value(jobUID);
//QObject::disconnect(connections.value("userStopped"));
job->setStatus(ctkAbstractJob::JobStatus::UserStopped);
initializedStoppedJobsUIDs.append(job->jobUID());
}
}
}

d->removeJobs(initializedStoppedJobsUIDs);
Expand Down
6 changes: 5 additions & 1 deletion Libs/DICOM/Core/ctkDICOMAbstractThumbnailGenerator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

// Qt includes
#include <QObject>
#include <QVector>

#include "ctkDICOMCoreExport.h"

Expand All @@ -43,7 +44,10 @@ class CTK_DICOM_CORE_EXPORT ctkDICOMAbstractThumbnailGenerator : public QObject
explicit ctkDICOMAbstractThumbnailGenerator(QObject* parent = 0);
virtual ~ctkDICOMAbstractThumbnailGenerator();

virtual bool generateThumbnail(DicomImage* dcmImage, const QString& path ) = 0;
virtual bool generateThumbnail(DicomImage* dcmImage, const QString& path,
QVector<int> color = QVector<int>{169, 169, 169}) = 0;
virtual void generateDocumentThumbnail(const QString &thumbnailPath,
QVector<int> color = QVector<int>{169, 169, 169}) = 0;

protected:
QScopedPointer<ctkDICOMAbstractThumbnailGeneratorPrivate> d_ptr;
Expand Down
100 changes: 69 additions & 31 deletions Libs/DICOM/Core/ctkDICOMDatabase.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -856,34 +856,6 @@ bool ctkDICOMDatabasePrivate::insertPatientStudySeries(const ctkDICOMItem& datas
return databaseWasChanged;
}

//------------------------------------------------------------------------------
bool ctkDICOMDatabasePrivate::storeThumbnailFile(const QString& originalFilePath,
const QString& studyInstanceUID, const QString& seriesInstanceUID, const QString& sopInstanceUID)
{
Q_Q(ctkDICOMDatabase);
if (!this->ThumbnailGenerator)
{
return false;
}
// Create thumbnail here
QString thumbnailPath = q->databaseDirectory() +
"/thumbs/" + this->internalStoragePath(studyInstanceUID, seriesInstanceUID, sopInstanceUID) + ".png";
QFileInfo thumbnailInfo(thumbnailPath);
if (thumbnailInfo.exists() && (thumbnailInfo.lastModified() > QFileInfo(originalFilePath).lastModified()))
{
// thumbnail already exists and up-to-date
return true;
}
QDir destinationDir(thumbnailInfo.dir());
if (!destinationDir.exists())
{
destinationDir.mkpath(".");
}
DicomImage dcmImage(QDir::toNativeSeparators(originalFilePath).toUtf8());
return this->ThumbnailGenerator->generateThumbnail(&dcmImage, thumbnailPath);
}


//------------------------------------------------------------------------------
bool ctkDICOMDatabasePrivate::uidsForDataSet(const ctkDICOMItem& dataset,
QString& patientsName, QString& patientID, QString& studyInstanceUID, QString& seriesInstanceUID)
Expand Down Expand Up @@ -1036,7 +1008,7 @@ void ctkDICOMDatabasePrivate::insert(const ctkDICOMItem& dataset, const QString&
}
if (generateThumbnail)
{
this->storeThumbnailFile(storedFilePath, studyInstanceUID, seriesInstanceUID, sopInstanceUID);
q->storeThumbnailFile(storedFilePath, studyInstanceUID, seriesInstanceUID, sopInstanceUID);
}
}
if (q->isInMemory() && databaseWasChanged)
Expand Down Expand Up @@ -2375,6 +2347,72 @@ QDateTime ctkDICOMDatabase::insertDateTimeForInstance(QString sopInstanceUID)
return result;
}

//------------------------------------------------------------------------------
QString ctkDICOMDatabase::thumbnailPathForInstance(const QString &studyInstanceUID,
const QString &seriesInstanceUID,
const QString &sopInstanceUID)
{
Q_D(ctkDICOMDatabase);
QString thumbnailPath = this->databaseDirectory() +
"/thumbs/" + d->internalStoragePath(studyInstanceUID, seriesInstanceUID, sopInstanceUID) + ".png";

QFileInfo thumbnailInfo(thumbnailPath);
if (thumbnailInfo.exists())
{
// thumbnail exists
return thumbnailPath;
}
else
{
return "";
}
}

//------------------------------------------------------------------------------
bool ctkDICOMDatabase::storeThumbnailFile(const QString &originalFilePath,
const QString &studyInstanceUID,
const QString &seriesInstanceUID,
const QString &sopInstanceUID,
const QString& modality,
QVector<int> color)
{
Q_D(ctkDICOMDatabase);
if (!d->ThumbnailGenerator)
{
return false;
}

QString thumbnailPath = this->databaseDirectory() +
"/thumbs/" + d->internalStoragePath(studyInstanceUID, seriesInstanceUID, sopInstanceUID) + ".png";

QFileInfo thumbnailInfo(thumbnailPath);
if (thumbnailInfo.exists() && (thumbnailInfo.lastModified() > QFileInfo(originalFilePath).lastModified()))
{
// thumbnail already exists and it is up-to-date
return true;
}

QDir destinationDir(thumbnailInfo.dir());
if (!destinationDir.exists())
{
destinationDir.mkpath(".");
}

if (modality == "SEG")
{
// NOTE: currently SEG objects are not fully supported by ctkDICOMThumbnailGenerator,
// The rendering will fail and in addition SEG object can be very large and
// loading the file can be slow. For now, we will just create a blank thumbnail with a document svg.
d->ThumbnailGenerator->generateDocumentThumbnail(thumbnailPath, color);
return true;
}
else
{
DicomImage dcmImage(QDir::toNativeSeparators(originalFilePath).toUtf8());
return d->ThumbnailGenerator->generateThumbnail(&dcmImage, thumbnailPath, color);
}
}

//------------------------------------------------------------------------------
int ctkDICOMDatabase::patientsCount()
{
Expand Down Expand Up @@ -2848,7 +2886,7 @@ void ctkDICOMDatabase::insert(const QList<ctkDICOMDatabase::IndexingResult>& ind

if (generateThumbnail)
{
d->storeThumbnailFile(storedFilePath, studyInstanceUID, seriesInstanceUID, sopInstanceUID);
this->storeThumbnailFile(storedFilePath, studyInstanceUID, seriesInstanceUID, sopInstanceUID);
}
}
}
Expand Down Expand Up @@ -3116,7 +3154,7 @@ void ctkDICOMDatabase::insert(QList<QSharedPointer<ctkDICOMJobResponseSet>> jobR
}
if (generateThumbnail)
{
d->storeThumbnailFile(storedFilePath, studyInstanceUID, seriesInstanceUID, sopInstanceUID);
this->storeThumbnailFile(storedFilePath, studyInstanceUID, seriesInstanceUID, sopInstanceUID);
}
}
}
Expand Down
24 changes: 16 additions & 8 deletions Libs/DICOM/Core/ctkDICOMDatabase.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,15 @@ class CTK_DICOM_CORE_EXPORT ctkDICOMDatabase : public QObject
Q_INVOKABLE QString seriesForFile(QString fileName);
Q_INVOKABLE QString instanceForFile(const QString fileName);
Q_INVOKABLE QDateTime insertDateTimeForInstance(const QString fileName);
Q_INVOKABLE QString thumbnailPathForInstance(const QString& studyInstanceUID,
const QString& seriesInstanceUID,
const QString& sopInstanceUID);
Q_INVOKABLE bool storeThumbnailFile(const QString& originalFilePath,
const QString& studyInstanceUID,
const QString& seriesInstanceUID,
const QString& sopInstanceUID,
const QString& modality = "",
QVector<int> color = QVector<int>{169, 169, 169});

Q_INVOKABLE int patientsCount();
Q_INVOKABLE int studiesCount();
Expand Down Expand Up @@ -263,14 +272,13 @@ class CTK_DICOM_CORE_EXPORT ctkDICOMDatabase : public QObject
/// does only make sense if a full object is received.
/// @param @generateThumbnail If true, a thumbnail is generated.
///
Q_INVOKABLE void insert( const ctkDICOMItem& ctkDataset,
bool storeFile, bool generateThumbnail);
void insert ( DcmItem *item,
bool storeFile = true, bool generateThumbnail = true);
Q_INVOKABLE void insert ( const QString& filePath,
bool storeFile = true, bool generateThumbnail = true,
bool createHierarchy = true,
const QString& destinationDirectoryName = QString() );
Q_INVOKABLE void insert(const ctkDICOMItem& ctkDataset,
bool storeFile, bool generateThumbnail);
void insert (DcmItem *item, bool storeFile = true, bool generateThumbnail = true);
Q_INVOKABLE void insert (const QString& filePath,
bool storeFile = true, bool generateThumbnail = true,
bool createHierarchy = true,
const QString& destinationDirectoryName = QString());
Q_INVOKABLE void insert(const QList<ctkDICOMDatabase::IndexingResult>& indexingResults);
Q_INVOKABLE void insert(QList<QSharedPointer<ctkDICOMJobResponseSet>> jobResponseSets);

Expand Down
4 changes: 0 additions & 4 deletions Libs/DICOM/Core/ctkDICOMDatabase_p.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,10 +72,6 @@ class CTK_DICOM_CORE_EXPORT ctkDICOMDatabasePrivate
/// Returns false in case of an error
bool indexingStatusForFile(const QString& filePath, const QString& sopInstanceUID, bool& datasetInDatabase, bool& datasetUpToDate, QString& databaseFilename);

/// Retrieve thumbnail from file and store in database folder.
bool storeThumbnailFile(const QString& originalFilePath,
const QString& studyInstanceUID, const QString& seriesInstanceUID, const QString& sopInstanceUID);

/// Get basic UIDs for a data set, return true if the data set has all the required tags
bool uidsForDataSet(const ctkDICOMItem& dataset, QString& patientsName, QString& patientID, QString& studyInstanceUID, QString& seriesInstanceUID);
bool uidsForDataSet(QString& patientsName, QString& patientID, QString& studyInstanceUID);
Expand Down
Loading

0 comments on commit 065f4df

Please sign in to comment.