From 8acf85ddb8485701025338d85a21884b6ff31b66 Mon Sep 17 00:00:00 2001 From: Si Date: Sat, 26 Sep 2026 22:42:30 +0200 Subject: [PATCH] Show progress while loading reads the image sizes Before decoding, loadImages reads every header to size the batch against the free memory. That pass was serial, silent and could not be cancelled, so the loading window sat at "0 of N". LoadProgress gains onSized, the headers are read in parallel on the same cursor-driven workers as the decodes, and a cancel stops the pass. The window's bar runs twice, "Checking sizes" then "Decoding", and the CLI prints a sizing line. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 4 ++ src/core/image.cpp | 94 +++++++++++++++++++++++++++----------------- src/core/image.hpp | 7 +++- src/gui/window.cpp | 22 +++++++++-- src/gui/window.hpp | 3 +- src/main.cpp | 5 +++ tests/core_tests.cpp | 9 +++++ 7 files changed, 101 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 963393a..2a3d917 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -52,6 +52,10 @@ commit that made it. The interface shows this file under Changelog in its menu. videos exactly as before. ### Changed +- Loading images shows its first pass too: the progress window counts the + files as their sizes are checked ("Checking sizes: 3 of 40") before it + counts the decodes, the headers are read in parallel, and Cancel works + during that pass as well. The command line shows a `sizing` line likewise. - The video preview plays sooner: it renders every third frame first and plays that draft, each frame held for three, then fills in the frames between while it plays. The finished preview and the saved video are diff --git a/src/core/image.cpp b/src/core/image.cpp index cf37662..5d34f76 100644 --- a/src/core/image.cpp +++ b/src/core/image.cpp @@ -164,14 +164,55 @@ std::size_t imageFilePixels(const std::string& path) { std::vector loadImages(const std::vector& paths, std::vector& failures, const LoadProgress* progress) { + const auto cancelled = [progress] { + return progress != nullptr && progress->cancel != nullptr && + progress->cancel->load(std::memory_order_relaxed); + }; + // Runs `work(i)` for every path on `workers` threads. Threads take the next + // path from a shared cursor rather than a fixed slice, so one huge or slow + // file cannot stall the files behind it and progress arrives at a steady + // rate. A cancel stops every thread before its next path. + const auto forEachPath = [&](std::size_t workers, const auto& work) { + std::atomic cursor{0}; + const auto run = [&] { + for (;;) { + if (cancelled()) return; + const std::size_t i = cursor.fetch_add(1, std::memory_order_relaxed); + if (i >= paths.size()) return; + work(i); + } + }; + std::vector threads; + threads.reserve(workers > 0 ? workers - 1 : 0); + for (std::size_t w = 1; w < workers; ++w) threads.emplace_back(run); + run(); + for (auto& thread : threads) thread.join(); + }; + + unsigned hardware = std::thread::hardware_concurrency(); + if (hardware == 0) hardware = 1; + std::size_t workers = std::min(hardware, std::max(paths.size(), 1)); + // What the batch holds once decoded, and what one decode needs on top // while it runs: the pixbuf (with alpha, at worst) and its rotated copy. + // Reading a header can mean a round trip to a slow disk or a network + // share, so the headers are read in parallel too, and reported. + std::vector pixels(paths.size(), 0); + std::atomic sized{0}; + forEachPath(workers, [&](std::size_t i) { + pixels[i] = imageFilePixels(paths[i]); + if (progress != nullptr && progress->onSized) { + progress->onSized(sized.fetch_add(1, std::memory_order_relaxed) + 1, paths.size(), + paths[i]); + } + }); + if (cancelled()) return {}; + std::size_t kept = 0; std::size_t largest = 0; - for (const auto& path : paths) { - const std::size_t pixels = imageFilePixels(path); - kept += pixels * 3; - largest = std::max(largest, pixels * 8); + for (const std::size_t count : pixels) { + kept += count * 3; + largest = std::max(largest, count * 8); } const std::string what = paths.size() == 1 ? std::string("Loading this image") @@ -180,45 +221,24 @@ std::vector loadImages(const std::vector& paths, const std::size_t spare = usableMemory() - std::min(usableMemory(), kept); // Decoding is the dominant cost of a headless run, and the files are - // independent, so they are decoded in parallel. Threads take the next path - // from a shared cursor rather than a fixed slice, so one huge file cannot - // stall the files behind it and progress arrives at a steady rate. Slots - // keep the result in the order the paths were given. + // independent, so they are decoded in parallel. Slots keep the result in + // the order the paths were given. std::vector slots(paths.size()); std::vector errors(paths.size()); - std::atomic cursor{0}; std::atomic done{0}; - - unsigned hardware = std::thread::hardware_concurrency(); - if (hardware == 0) hardware = 1; - std::size_t workers = std::min(hardware, std::max(paths.size(), 1)); if (largest > 0) workers = std::clamp(spare / largest, 1, workers); - const auto decode = [&] { - for (;;) { - if (progress != nullptr && progress->cancel != nullptr && - progress->cancel->load(std::memory_order_relaxed)) { - return; - } - const std::size_t i = cursor.fetch_add(1, std::memory_order_relaxed); - if (i >= paths.size()) return; - try { - slots[i] = Image::load(paths[i]); - } catch (const std::exception& error) { - errors[i] = error.what(); - } - if (progress != nullptr && progress->onFile) { - progress->onFile(done.fetch_add(1, std::memory_order_relaxed) + 1, paths.size(), - paths[i]); - } + forEachPath(workers, [&](std::size_t i) { + try { + slots[i] = Image::load(paths[i]); + } catch (const std::exception& error) { + errors[i] = error.what(); } - }; - - std::vector threads; - threads.reserve(workers > 0 ? workers - 1 : 0); - for (std::size_t w = 1; w < workers; ++w) threads.emplace_back(decode); - decode(); - for (auto& thread : threads) thread.join(); + if (progress != nullptr && progress->onFile) { + progress->onFile(done.fetch_add(1, std::memory_order_relaxed) + 1, paths.size(), + paths[i]); + } + }); std::vector images; images.reserve(paths.size()); diff --git a/src/core/image.hpp b/src/core/image.hpp index c446df2..24a8f54 100644 --- a/src/core/image.hpp +++ b/src/core/image.hpp @@ -61,9 +61,12 @@ struct LoadFailure { }; // Lets a caller follow a long batch decode and abort it. Both members are -// touched from the decoding threads, so `onFile` must be thread-safe and must -// not block: the GUI only bumps a counter from it and wakes the main thread. +// touched from the decoding threads, so `onSized` and `onFile` must be +// thread-safe and must not block: the GUI only bumps a counter from them and +// wakes the main thread. `onSized` follows the first pass, which reads every +// header to size the batch; `onFile` follows the decodes after it. struct LoadProgress { + std::function onSized; std::function onFile; std::atomic* cancel = nullptr; }; diff --git a/src/gui/window.cpp b/src/gui/window.cpp index a10bb76..3563a26 100644 --- a/src/gui/window.cpp +++ b/src/gui/window.cpp @@ -1388,6 +1388,7 @@ void EffectorWindow::addPaths(const std::vector& paths) { loading_ = true; loadCancel_.store(false, std::memory_order_relaxed); + loadSized_.store(0, std::memory_order_relaxed); loadDone_.store(0, std::memory_order_relaxed); loadTotal_ = paths.size(); { @@ -1409,6 +1410,14 @@ void EffectorWindow::addPaths(const std::vector& paths) { loader_ = std::thread([this, paths] { LoadProgress progress; progress.cancel = &loadCancel_; + progress.onSized = [this](std::size_t sized, std::size_t, const std::string& path) { + { + std::lock_guard lock(loadMutex_); + loadCurrent_ = path; + } + loadSized_.store(sized, std::memory_order_relaxed); + loadProgress_.emit(); + }; progress.onFile = [this](std::size_t done, std::size_t, const std::string& path) { { std::lock_guard lock(loadMutex_); @@ -1438,15 +1447,22 @@ void EffectorWindow::addPaths(const std::vector& paths) { } void EffectorWindow::onLoadProgress() { + if (loadCancel_.load(std::memory_order_relaxed)) return; // keep "Cancelling…" up + const std::size_t sized = loadSized_.load(std::memory_order_relaxed); const std::size_t done = loadDone_.load(std::memory_order_relaxed); std::string current; { std::lock_guard lock(loadMutex_); current = loadCurrent_; } + // The headers are all read before the first decode starts, so the bar + // runs twice: once sizing the batch, then once decoding it. if (loadBar_ != nullptr && loadTotal_ > 0) { - loadBar_->set_fraction(static_cast(done) / static_cast(loadTotal_)); - loadBar_->set_text(std::to_string(done) + " of " + std::to_string(loadTotal_)); + const bool sizing = sized < loadTotal_ && done == 0; + const std::size_t count = sizing ? sized : done; + loadBar_->set_fraction(static_cast(count) / static_cast(loadTotal_)); + loadBar_->set_text((sizing ? "Checking sizes: " : "Decoding: ") + std::to_string(count) + + " of " + std::to_string(loadTotal_)); } if (loadFileLabel_ != nullptr && !current.empty()) { loadFileLabel_->set_text(std::filesystem::path(current).filename().string()); @@ -1552,7 +1568,7 @@ void EffectorWindow::showLoadWindow() { auto* bar = Gtk::make_managed(); bar->set_show_text(true); - bar->set_text("0 of " + std::to_string(loadTotal_)); + bar->set_text("Checking sizes: 0 of " + std::to_string(loadTotal_)); box->append(*bar); loadBar_ = bar; diff --git a/src/gui/window.hpp b/src/gui/window.hpp index e99226c..e17b8b0 100644 --- a/src/gui/window.hpp +++ b/src/gui/window.hpp @@ -70,7 +70,7 @@ private: void onImagesChosen(const Glib::RefPtr& result, const Glib::RefPtr& dialog); void addPaths(const std::vector& paths); // decodes in the background - void onLoadProgress(); // dispatcher: one file finished decoding + void onLoadProgress(); // dispatcher: one file sized or decoded void onLoadFinished(); // dispatcher: the batch is done or was cancelled void showLoadWindow(); void closeLoadWindow(); @@ -219,6 +219,7 @@ private: Glib::Dispatcher loadProgress_; Glib::Dispatcher loadFinished_; std::atomic loadCancel_{false}; + std::atomic loadSized_{0}; // headers read (sizing the batch) std::atomic loadDone_{0}; std::size_t loadTotal_ = 0; std::mutex loadMutex_; diff --git a/src/main.cpp b/src/main.cpp index 77b31b2..0d1cb3e 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -92,6 +92,11 @@ int renderHeadless(const effector::Options& options) { std::mutex progressMutex; const bool showProgress = isatty(fileno(stderr)) != 0 && options.images.size() > 1; if (showProgress) { + progress.onSized = [&](std::size_t done, std::size_t total, const std::string& path) { + std::lock_guard lock(progressMutex); + std::cerr << "\r\033[Ksizing " << done << "/" << total << " " + << std::filesystem::path(path).filename().string() << std::flush; + }; progress.onFile = [&](std::size_t done, std::size_t total, const std::string& path) { std::lock_guard lock(progressMutex); std::cerr << "\r\033[Kreading " << done << "/" << total << " " diff --git a/tests/core_tests.cpp b/tests/core_tests.cpp index 0183e51..ae09c4e 100644 --- a/tests/core_tests.cpp +++ b/tests/core_tests.cpp @@ -1708,9 +1708,16 @@ int main() { paths.push_back((dir / "missing.png").string()); std::atomic calls{0}; + std::atomic sizedCalls{0}; + std::atomic sizedAfterDecode{false}; std::size_t lastTotal = 0; std::mutex mutex; LoadProgress progress; + progress.onSized = [&](std::size_t done, std::size_t total, const std::string&) { + ++sizedCalls; + if (calls.load() > 0) sizedAfterDecode = true; + check(done >= 1 && done <= total, "sizing step is within the batch"); + }; progress.onFile = [&](std::size_t done, std::size_t total, const std::string&) { std::lock_guard lock(mutex); ++calls; @@ -1723,6 +1730,8 @@ int main() { check(images.size() == 6, "every readable image is returned"); check(failuresOut.size() == 1, "the unreadable one is reported"); check(calls.load() == paths.size(), "progress fires once per file"); + check(sizedCalls.load() == paths.size() && !sizedAfterDecode.load(), + "every header is sized, all before the first decode"); check(lastTotal == paths.size(), "progress reports the batch size"); check(images.front().path() == paths.front(), "input order is preserved");