From 7e98357070866a59be35da4710ef68308dfb41a6 Mon Sep 17 00:00:00 2001 From: hans Date: Fri, 18 Sep 2026 14:39:07 +0200 Subject: [PATCH] Load source images in the background with a progress window Decoding a batch of photographs took seconds on the main thread, so the window froze with no sign of what was happening. loadImages now takes an optional LoadProgress (per-file callback plus a cancel flag) and hands files out from a shared cursor, so one large file cannot stall the queue behind it. The GUI decodes off the main thread and shows a progress window with the count, the current file and a Cancel button; it only appears if the batch is still running after 250 ms. Headless mode keeps a "reading n/total" line on stderr when it is attached to a terminal. Co-Authored-By: Claude Opus 5 --- README.md | 11 +++ src/core/image.cpp | 47 +++++++++--- src/core/image.hpp | 15 +++- src/gui/window.cpp | 166 ++++++++++++++++++++++++++++++++++++++++++- src/gui/window.hpp | 29 +++++++- src/main.cpp | 21 +++++- tests/core_tests.cpp | 43 +++++++++++ 7 files changed, 317 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index b96f569..640bd59 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,13 @@ build/effector -o out.png -s 1920x1080 \ Any option can also be passed when launching the interface; it becomes the starting state (images, canvas size, effect settings, seed). +Reading a batch of large photographs takes seconds, so it never blocks the +interface: decoding runs in the background and a progress window shows how many +files are done, which one is being read, and a Cancel button that stops the +batch. Small batches finish before the window would be drawn, so it only appears +if the decode is still going after 250 ms. On a terminal the headless mode keeps +a single `reading n/total` line updated on stderr; redirected output stays clean. + ## Effects ### scanlines @@ -113,3 +120,7 @@ are decoded in parallel too, which is what dominates a headless run. A 7680×4320 canvas at `--strip-height 2` from three multi-megapixel JPEGs renders in ~35 ms, against ~73 ms for the Python prototype; the interface reports the render time of the current preview in its status line. + +Decoding threads take the next file from a shared cursor rather than a fixed +slice of the list, so one very large file cannot stall the files queued behind +it and progress advances at a steady rate. diff --git a/src/core/image.cpp b/src/core/image.cpp index bfbabea..44ee3a0 100644 --- a/src/core/image.cpp +++ b/src/core/image.cpp @@ -2,14 +2,14 @@ #include -#include "core/parallel.hpp" - #include +#include #include #include #include #include #include +#include namespace effector { namespace { @@ -108,30 +108,55 @@ void Image::save(const std::string& path) const { } std::vector loadImages(const std::vector& paths, - std::vector& failures) { + std::vector& failures, + const LoadProgress* progress) { // Decoding is the dominant cost of a headless run, and the files are - // independent, so they are decoded in parallel. Slots keep the result in - // the order the paths were given. + // 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. std::vector slots(paths.size()); std::vector errors(paths.size()); + std::atomic cursor{0}; + std::atomic done{0}; - parallelFor(paths.size(), [&](std::size_t begin, std::size_t end) { - for (std::size_t i = begin; i < end; ++i) { + unsigned hardware = std::thread::hardware_concurrency(); + if (hardware == 0) hardware = 1; + const std::size_t workers = std::min(hardware, std::max(paths.size(), 1)); + + 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]); + } } - }, 1); + }; + + 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(); std::vector images; images.reserve(paths.size()); for (std::size_t i = 0; i < paths.size(); ++i) { - if (errors[i].empty()) { - images.push_back(std::move(slots[i])); - } else { + if (!errors[i].empty()) { failures.push_back({paths[i], errors[i]}); + } else if (!slots[i].empty()) { + images.push_back(std::move(slots[i])); } } return images; diff --git a/src/core/image.hpp b/src/core/image.hpp index 9e85d3a..8ed43ea 100644 --- a/src/core/image.hpp +++ b/src/core/image.hpp @@ -1,7 +1,9 @@ #pragma once +#include #include #include +#include #include #include @@ -49,8 +51,19 @@ struct LoadFailure { std::string message; }; +// 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. +struct LoadProgress { + std::function onFile; + std::atomic* cancel = nullptr; +}; + // Loads every path, collecting rather than throwing on individual failures. +// When `progress->cancel` is set the remaining paths are skipped, so the result +// is short and should be discarded rather than used. std::vector loadImages(const std::vector& paths, - std::vector& failures); + std::vector& failures, + const LoadProgress* progress = nullptr); } // namespace effector diff --git a/src/gui/window.cpp b/src/gui/window.cpp index 854ee3e..c6acd1e 100644 --- a/src/gui/window.cpp +++ b/src/gui/window.cpp @@ -38,6 +38,8 @@ EffectorWindow::EffectorWindow(const Options& initial) { seed_ = Rng::randomSeed(); renderDone_.connect(sigc::mem_fun(*this, &EffectorWindow::onRenderFinished)); + loadProgress_.connect(sigc::mem_fun(*this, &EffectorWindow::onLoadProgress)); + loadFinished_.connect(sigc::mem_fun(*this, &EffectorWindow::onLoadFinished)); buildUi(); applyInitial(initial); @@ -48,6 +50,9 @@ EffectorWindow::EffectorWindow(const Options& initial) { EffectorWindow::~EffectorWindow() { debounce_.disconnect(); + loadWindowDelay_.disconnect(); + loadCancel_.store(true, std::memory_order_relaxed); + if (loader_.joinable()) loader_.join(); if (worker_.joinable()) worker_.join(); } @@ -62,6 +67,7 @@ void EffectorWindow::buildUi() { addButton->set_tooltip_text("Add images"); addButton->signal_clicked().connect(sigc::mem_fun(*this, &EffectorWindow::onAddImages)); header->pack_start(*addButton); + addButton_ = addButton; std::vector titles; for (const auto& effect : effects_) titles.push_back(effect->title()); @@ -330,8 +336,94 @@ void EffectorWindow::mutateSources(Mutate&& mutate) { } void EffectorWindow::addPaths(const std::vector& paths) { + if (paths.empty()) return; + if (loading_) { // a second selection arrived mid-batch; take it afterwards + loadQueue_.insert(loadQueue_.end(), paths.begin(), paths.end()); + return; + } + + loading_ = true; + loadCancel_.store(false, std::memory_order_relaxed); + loadDone_.store(0, std::memory_order_relaxed); + loadTotal_ = paths.size(); + { + std::lock_guard lock(loadMutex_); + loadCurrent_.clear(); + } + if (addButton_ != nullptr) addButton_->set_sensitive(false); + + // A couple of small files decode faster than a window can be drawn, so the + // progress window only appears if the batch is still going after a moment. + loadWindowDelay_.disconnect(); + loadWindowDelay_ = Glib::signal_timeout().connect( + [this] { + showLoadWindow(); + return false; + }, + 250); + + loader_ = std::thread([this, paths] { + LoadProgress progress; + progress.cancel = &loadCancel_; + progress.onFile = [this](std::size_t done, std::size_t, const std::string& path) { + { + std::lock_guard lock(loadMutex_); + loadCurrent_ = path; + } + loadDone_.store(done, std::memory_order_relaxed); + loadProgress_.emit(); + }; + + std::vector failures; + auto images = loadImages(paths, failures, &progress); + { + std::lock_guard lock(loadMutex_); + loadedImages_ = std::move(images); + loadFailures_ = std::move(failures); + } + loadFinished_.emit(); + }); +} + +void EffectorWindow::onLoadProgress() { + const std::size_t done = loadDone_.load(std::memory_order_relaxed); + std::string current; + { + std::lock_guard lock(loadMutex_); + current = loadCurrent_; + } + 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_)); + } + if (loadFileLabel_ != nullptr && !current.empty()) { + loadFileLabel_->set_text(std::filesystem::path(current).filename().string()); + } +} + +void EffectorWindow::onLoadFinished() { + if (loader_.joinable()) loader_.join(); + loading_ = false; + loadWindowDelay_.disconnect(); + closeLoadWindow(); + if (addButton_ != nullptr) addButton_->set_sensitive(true); + + std::vector images; std::vector failures; - auto images = loadImages(paths, failures); + { + std::lock_guard lock(loadMutex_); + images = std::move(loadedImages_); + failures = std::move(loadFailures_); + loadedImages_.clear(); + loadFailures_.clear(); + } + + if (loadCancel_.load(std::memory_order_relaxed)) { + // A cancelled batch is incomplete, so none of it is taken. + loadQueue_.clear(); + showError("Loading cancelled."); + return; + } for (const auto& image : images) appendSourceRow(image.path()); mutateSources([&images](SourceList& list) { @@ -349,6 +441,78 @@ void EffectorWindow::addPaths(const std::vector& paths) { clearError(); } requestRender(); + + if (!loadQueue_.empty()) { + auto queued = std::move(loadQueue_); + loadQueue_.clear(); + addPaths(queued); + } +} + +void EffectorWindow::showLoadWindow() { + if (!loading_ || loadWindow_) return; + + loadWindow_ = std::make_unique(); + loadWindow_->set_title("Reading images"); + loadWindow_->set_transient_for(*this); + loadWindow_->set_modal(true); + loadWindow_->set_resizable(false); + loadWindow_->set_default_size(420, -1); + + auto* box = Gtk::make_managed(Gtk::Orientation::VERTICAL, 12); + box->set_margin(18); + + auto* heading = Gtk::make_managed("Reading source images…"); + heading->set_xalign(0.0F); + heading->add_css_class("heading"); + box->append(*heading); + + auto* bar = Gtk::make_managed(); + bar->set_show_text(true); + bar->set_text("0 of " + std::to_string(loadTotal_)); + box->append(*bar); + loadBar_ = bar; + + auto* file = Gtk::make_managed(""); + file->set_xalign(0.0F); + file->set_ellipsize(Pango::EllipsizeMode::MIDDLE); + file->add_css_class("dim-label"); + box->append(*file); + loadFileLabel_ = file; + + auto* cancel = Gtk::make_managed("Cancel"); + cancel->set_halign(Gtk::Align::END); + cancel->signal_clicked().connect(sigc::mem_fun(*this, &EffectorWindow::cancelLoad)); + box->append(*cancel); + + // Closing the window means the same thing as pressing Cancel, but the + // window stays up until the decoding threads have actually stopped. + loadWindow_->signal_close_request().connect( + [this] { + cancelLoad(); + return true; + }, + false); + + loadWindow_->set_child(*box); + onLoadProgress(); // show whatever finished while the window was pending + loadWindow_->present(); +} + +void EffectorWindow::closeLoadWindow() { + loadBar_ = nullptr; + loadFileLabel_ = nullptr; + if (!loadWindow_) return; + loadWindow_->set_hide_on_close(false); + loadWindow_->hide(); + loadWindow_.reset(); +} + +void EffectorWindow::cancelLoad() { + if (!loading_) return; + loadCancel_.store(true, std::memory_order_relaxed); + if (loadBar_ != nullptr) loadBar_->set_text("Cancelling…"); + if (loadFileLabel_ != nullptr) loadFileLabel_->set_text(""); } void EffectorWindow::appendSourceRow(const std::string& path) { diff --git a/src/gui/window.hpp b/src/gui/window.hpp index 1884e64..72d267d 100644 --- a/src/gui/window.hpp +++ b/src/gui/window.hpp @@ -2,6 +2,7 @@ #include +#include #include #include #include @@ -34,7 +35,12 @@ private: void onAddImages(); void onImagesChosen(const Glib::RefPtr& result, const Glib::RefPtr& dialog); - void addPaths(const std::vector& paths); + void addPaths(const std::vector& paths); // decodes in the background + void onLoadProgress(); // dispatcher: one file finished decoding + void onLoadFinished(); // dispatcher: the batch is done or was cancelled + void showLoadWindow(); + void closeLoadWindow(); + void cancelLoad(); void appendSourceRow(const std::string& path); void removeSource(Gtk::ListBoxRow* row); @@ -65,6 +71,22 @@ private: std::uint64_t seed_ = 0; bool suspendRender_ = true; + // Source loading. Decoding a batch of large photographs takes seconds, so + // it runs off the main thread and reports into a small progress window. + std::thread loader_; + Glib::Dispatcher loadProgress_; + Glib::Dispatcher loadFinished_; + std::atomic loadCancel_{false}; + std::atomic loadDone_{0}; + std::size_t loadTotal_ = 0; + std::mutex loadMutex_; + std::string loadCurrent_; + std::vector loadedImages_; + std::vector loadFailures_; + std::vector loadQueue_; // chosen while another batch was loading + bool loading_ = false; + sigc::connection loadWindowDelay_; + // Render job Glib::Dispatcher renderDone_; std::thread worker_; @@ -78,6 +100,11 @@ private: // Widgets Gtk::Label banner_; + Gtk::Button* addButton_ = nullptr; + // Built fresh with the progress window, and null whenever it is closed. + std::unique_ptr loadWindow_; + Gtk::ProgressBar* loadBar_ = nullptr; + Gtk::Label* loadFileLabel_ = nullptr; Gtk::ListBox sourceList_; Gtk::SpinButton widthSpin_; Gtk::SpinButton heightSpin_; diff --git a/src/main.cpp b/src/main.cpp index 816dd15..8f7af92 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -1,7 +1,11 @@ +#include + #include #include +#include #include #include +#include #include "cli/options.hpp" #include "core/effect.hpp" @@ -16,7 +20,22 @@ int renderHeadless(const effector::Options& options) { const auto effect = effector::effectByName(options.effect); std::vector failures; - auto sources = effector::loadImages(options.images, failures); + + // Decoding a batch of photographs takes seconds; on a terminal keep a + // single line updated so the wait is not silent. Redirected output stays + // clean, since the line would only clutter a log. + effector::LoadProgress progress; + std::mutex progressMutex; + const bool showProgress = isatty(fileno(stderr)) != 0 && options.images.size() > 1; + if (showProgress) { + 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 << " " + << std::filesystem::path(path).filename().string() << std::flush; + }; + } + auto sources = effector::loadImages(options.images, failures, showProgress ? &progress : nullptr); + if (showProgress) std::cerr << "\r\033[K" << std::flush; for (const auto& failure : failures) { std::cerr << "effector: skipping " << failure.path << ": " << failure.message << "\n"; } diff --git a/tests/core_tests.cpp b/tests/core_tests.cpp index cc9b2ba..d913488 100644 --- a/tests/core_tests.cpp +++ b/tests/core_tests.cpp @@ -1,9 +1,11 @@ // Small self-contained checks for the pieces that have no GUI dependency. +#include #include #include #include #include +#include #include #include @@ -144,6 +146,47 @@ int main() { check(threw, "missing file throws"); } + std::cout << "batch loading\n"; + { + const auto dir = std::filesystem::temp_directory_path() / "effector_test_batch"; + std::filesystem::create_directories(dir); + std::vector paths; + for (int i = 0; i < 6; ++i) { + const auto path = dir / ("src" + std::to_string(i) + ".png"); + gradient(32, 24, i + 1).save(path.string()); + paths.push_back(path.string()); + } + paths.push_back((dir / "missing.png").string()); + + std::atomic calls{0}; + std::size_t lastTotal = 0; + std::mutex mutex; + LoadProgress progress; + progress.onFile = [&](std::size_t done, std::size_t total, const std::string&) { + std::lock_guard lock(mutex); + ++calls; + lastTotal = total; + check(done >= 1 && done <= total, "progress step is within the batch"); + }; + + std::vector failuresOut; + const auto images = loadImages(paths, failuresOut, &progress); + 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(lastTotal == paths.size(), "progress reports the batch size"); + check(images.front().path() == paths.front(), "input order is preserved"); + + std::atomic cancel{true}; + LoadProgress cancelled; + cancelled.cancel = &cancel; + std::vector ignored; + const auto none = loadImages(paths, ignored, &cancelled); + check(none.empty(), "a cancelled batch loads nothing"); + + std::filesystem::remove_all(dir); + } + std::cout << (failures == 0 ? "\nall checks passed\n" : "\n" + std::to_string(failures) + " failed\n"); return failures == 0 ? 0 : 1; }