Repository navigation
Conversation
added 10 commits
September 6, 2026 11:16
With thousands of images in the list, every repaint of a row rebuilt the HTML name string (including a palette lookup), re-created a QIcon from the SVG resource for the status column and re-read mutable CImage fields that worker threads write during a batch. Because the tree view had non-uniform row heights, QTreeView also asked the delegate for a sizeHint of every column of every row on each relayout. Cache the display strings, the status and the compressed path per CImageTreeItem (refreshed on the GUI thread), keep the five status pixmaps in the model, cache the palette-dependent colour string, and enable setUniformRowHeights(true) — the delegate already returns a fixed height of 24, so this is visually neutral. CImageTreeItem now owns its CImage through a std::shared_ptr so that a preview job can keep the image alive after the row is removed (used by later commits).
CImageSortFilterProxyModel::lessThan called sourceModel()->data() for both sides (rebuilding the HTML name string each time) and naturalLessThan constructed a new QCollator in numeric mode for every comparison. Sorting 5000 rows means tens of thousands of comparisons, so importing a large folder paused for seconds after the progress dialog closed. Compare the cached display name (the exact string that was compared before, so the resulting order is unchanged) and keep one thread_local QCollator. Ratio/info comparisons use the cached values so that a header click during a batch never reads fields a worker thread is writing. CImageTreeModel::contains() scanned every row for each imported file, which made a second import into a non-empty list O(existing x new). Keep a reference-counted QHash of full paths instead (reference-counted so that duplicate rows, which are still allowed, stay consistent on removal).
"Clear" selected every row and then called removeRows(row, 1) once per row. Each call emitted its own rowsRemoved and itemsChanged, so the proxy model rebuilt its mapping and the status bar recomputed the total size for every single row: clearing a 5000-row list took seconds of GUI time. Add CImageTreeModel::removeItems(rows), which groups the rows into contiguous ranges (one beginRemoveRows/endRemoveRows per range) and emits itemsChanged once. Removed items are now deleted (they leaked before). While here, fix removeFiles() for partial selections: it iterated selectedIndexes() (five entries per row) with a row-based index, so removing a non-contiguous selection deleted the wrong rows. Use selectedRows() and map each row once.
During a batch the QFutureWatcher's progressValueChanged signal was connected to CImageTreeModel::emitDataChanged. That signal carries the number of completed items, not a row, so the wrong row was refreshed on every tick; to compensate, a lambda on finished() re-emitted dataChanged for every row of the model, one at a time. With the list sorted by "Saved" each of those emissions makes QSortFilterProxyModel re-sort, and with 5000 rows the GUI thread stalled for hundreds of milliseconds per tick towards the end of the batch (measured: up to 617 ms per stall, 81 stalls above 100 ms). Move the batch orchestration into CImageTreeModel::compress(). Workers push the started/finished row into a mutex-protected queue and post a flush to the model's thread; a single-shot 150 ms timer applies the queued updates, refreshes the per-row caches and emits one dataChanged per contiguous range. flushPendingUpdates() drains the queue synchronously, which compressionFinished() uses before showing the summary. MainWindow disables the proxy's dynamic sorting for the duration of the batch and re-enables it at the end, which sorts the list exactly once. The progress bar/label semantics are unchanged. The QFutureWatcher is now created and connected once instead of being leaked and re-connected on every batch.
Compression ran on QThreadPool::globalInstance(), resized to the thread preference (default: all cores). Preview jobs used the same pool, so selecting a row during a batch queued the preview behind every pending compression, and the next selection called previewWatcher->waitForFinished() on the GUI thread — the window froze until a pool thread became free, which on a long batch means until the end. - Compression gets its own QThreadPool (max threads = the existing preference, priority as before). The default for the preference becomes min(idealThreadCount, 8): libcaesium already parallelises internally and every worker holds a fully decoded image, so 24 threads on a 24-core box mostly oversubscribe and multiply peak memory. Saved preferences are kept. - Previews run on a separate single-thread pool. previewImage() cancels the pending job and starts the new one without waiting; the compression options and the preview cache path are captured on the GUI thread (the old lambda read widgets from a worker) so a settings change while a preview is queued can no longer store a preview under the wrong cache key. - Drops onto the list are disabled while a batch runs (rows appended during a batch were undefined behaviour), and closing the window during a batch cancels it and waits for the in-flight images without blocking the GUI before the cache is cleared.
importFiles() ran entirely on the GUI thread: for every file it opened a QImageReader (the WebP handler reads the whole file), resolved the canonical path twice, hashed the path and pumped the event loop through QProgressDialog::setValue. Folder scanning happened before any progress UI, and for drag-and-drop it happened inside dropEvent(), so Explorer's drag source stayed blocked until the scan finished. Thousands of files on a slow disk meant a long modal "Importing files..." with a frozen window. Add Importer::buildImages() (probe + CImage construction with a cancel flag and a progress callback) and a cancellable scanList() overload, and run scan and probe in a QtConcurrent job with a QPromise for progress. The progress dialog shows immediately in busy mode while scanning, switches to a real range once the file list is known, and Cancel keeps what was imported so far, as before. dropEvent() now only emits the dropped paths. Imports requested while a batch is running are queued and started when it finishes; "import and compress" from the command line compresses when the import completes. Dedupe/skip semantics, folder map and root folder computation are unchanged. CImage: derive the directory from the canonical file path instead of a second filesystem round-trip, and free the QImageReader before throwing ImageTooBigException.
… when not converting CImage::compress() wrote the result to a QTemporaryFile in the output folder and then QFile::copy'd it to the final name, i.e. every output was written twice. Rename it instead: the temp file lives in the same directory, so this is a metadata operation. On Windows QTemporaryFile keeps its handle open, so the rename has to go through the QTemporaryFile object itself (a static QFile::rename fails with "cannot remove source file"); autoRemove is disabled only after a successful rename, otherwise the destructor would delete the final file. The skip-if-bigger path, which publishes the *original* file, keeps copying. Before calling libcaesium the code also looked for a cached preview: getTemporaryPreviewFullPath() enumerates every compression_options key from QSettings, hashes twice and calls mkpath — per image, from every worker thread. When no format conversion happens the copied preview is overwritten by c_compress anyway, so only do that probe when converting (where the temp file is the conversion result and the cached preview replaces it). setCompressedInfo() resolves the canonical path once instead of three times.
Logger::messageHandler opened, appended to and closed the log file for every single message (plus a mkpath check). Batches with many unsupported or failing files log from all worker threads at once, and with antivirus scanning each open this became measurable. Keep one QFile open behind a mutex, flush after every line (so a crash still leaves the log complete), reopen when the date changes, and make closeLogFile() actually close it — the old implementation inspected a fresh, never-opened QFile and was a no-op. Open mode is unchanged (no QIODevice::Text, so line endings stay as they were).
…qt helper A local Windows build could not be started from the build directory: caesium.dll and WinSparkle.dll stayed inside their ExternalProject prefixes and the loader reported "WinSparkle.dll was not found". Copy both after linking, and add scripts/deploy-windows.sh, which runs windeployqt so the build folder is self-contained (Qt DLLs, platform and image-format plugins).
…stress harness The test target only compiled Importer.cpp and had a single case. Add QtTest coverage for the behaviour touched by the previous commits: - CImageTreeModelTests / NewApiModelTests: batched inserts emit one rowsInserted, ranged removal, reference-counted contains(), cached display data, base-folder changes. - SortProxyTests: natural order by name, folder grouping, numeric columns, no relayout for non-sort-column changes, sort cost guard. - ImporterBuildImagesTests: metadata, unsupported/corrupt/empty files, duplicates kept, cancellation, monotonic progress, ownership. - CImageCompressTests (real libcaesium): decodable output, skip-if-bigger with and without an existing output, keep dates, same-folder modes, keep structure, format conversion, failure paths, no temp leftovers, stale preview files never reach the output. - CompressionFlowTests: per-row signals, statuses after a batch, only-failed retry, coalesced dataChanged, synchronous flush, cancellation, empty model, GUI reads during a batch. - LoggerTests: throughput, concurrent writers, close/reopen, directory creation. - StressTests: synthetic corpus (CAESIUM_STRESS_N, default 1000) exercising import, append+sort, dedupe, compress, overwrite, cancel, clear and a memory sample; skipped with CAESIUM_SKIP_STRESS=1. The test binary links the real caesium.dll (copied next to it), embeds the status icon resources, runs with QT_QPA_PLATFORM=offscreen and uses a QApplication. Existing ImporterTests assertions were converted from Q_ASSERT (a no-op in Release) to QVERIFY/QCOMPARE.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
With a few thousand images in the list the GUI thread stutters during a batch and stalls at the end of it; clicking rows while compressing could freeze the window until the batch finished. This series removes the GUI-thread work that scales with the number of rows and moves the remaining blocking calls off the GUI thread. Compression throughput itself is unchanged (it is bound by libcaesium and, in "same folder" mode, by the per-image Recycle Bin operation).
Each commit is self-contained, builds on its own, and its message explains the why. The changes are meant to be behaviour-neutral except where noted below.
What changes, per commit
setUniformRowHeights(true)(delegate already fixed the height at 24).lessThanno longer rebuilds strings or constructs a collator per comparison;contains()uses a ref-counted hash.rowsRemovedper contiguous range; also fixes partial removal deleting the wrong rows (selectedIndexes()indexed by row).progressValueChanged(a count) was used as a row index and a final loop re-emitteddataChangedfor every row; workers now report rows through a queue flushed every 150 ms as ranges; dynamic sorting is paused for the batch and applied once at the end.waitForFinished()on the GUI thread, options/cache path captured on the GUI thread (the old lambda read widgets from a worker).dropEventreleases Explorer right away.closeLogFile()was a no-op.WinSparkle.dll was not found).CAESIUM_STRESS_N, skip withCAESIUM_SKIP_STRESS=1).Behaviour changes to be aware of
multithreading_max_threadsbecomesmin(idealThreadCount, 8)(libcaesium parallelises internally; each worker holds a decoded image). Existing saved preferences are untouched.Measurements (Windows 11, Qt 6.8.2 MinGW, 20 threads, list sorted by "Saved", same-folder overwrite)
GUI-thread latency probed with
SendMessageTimeout(WM_NULL)every 50 ms during the whole run:Baseline stalls cluster at the end of the batch (the per-row
dataChangedloop plus re-sorts by "Saved"). Clicking rows mid-batch on this branch keeps the window responsive and loads the preview of the last click.Unit/integration tests: 76 cases plus the stress harness at N=1000 pass (
ctest --test-dir build/tests).Notes
caesiumthere via the existinglink_directories.CsTests+cteststep is straightforward if you want it.