From 4739d39a51c367e22f3587644c86f5c65801385d Mon Sep 17 00:00:00 2001 From: Si Date: Fri, 18 Sep 2026 15:49:16 +0200 Subject: [PATCH] Let effects declare whether they need source images MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Whether an effect works from the loaded photographs is now part of the declarative plug-in contract, next to params(): Effect::needsSources() defaults to true, and blur — which only softens what it is handed — says no and blurs an empty canvas when given nothing. Only the first stage of a cascade ever sees the sources, so chainNeedsSources() just asks the first stage. Both front-ends gate on that instead of on "are there any images": the CLI only refuses an empty batch when the chain wants one, and the interface renders a blur-led chain with no images loaded, saying whichever thing is actually missing. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 6 ++++ src/core/chain.cpp | 5 ++++ src/core/chain.hpp | 5 ++++ src/core/effect.hpp | 5 ++++ src/effects/blur.cpp | 18 ++++++------ src/effects/blur.hpp | 10 ++++--- src/gui/window.cpp | 70 +++++++++++++++++++++++++++----------------- src/gui/window.hpp | 3 ++ src/main.cpp | 2 +- tests/core_tests.cpp | 36 ++++++++++++++++++----- 10 files changed, 112 insertions(+), 48 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 7222430..68143f4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -48,6 +48,12 @@ add a `ParamSpec` instead. Values travel as `ParamValues` (a `map>`), read back with `paramInt`/`paramDouble`/`paramBool`/`paramChoice`. +Whether an effect works from the loaded photographs is part of the same +contract: `Effect::needsSources()` (default true, false for blur). Only the +first stage ever sees the sources, so `chainNeedsSources` (`src/core/chain.cpp`) +just asks the first stage — a chain led by blur renders with no images loaded at +all, and both front-ends gate on that rather than on "are there any images". + Adding an effect: new `src/effects/.{hpp,cpp}` subclassing `Effect`, one line in the registry list in `src/core/registry.cpp`, one line in the `effector_core` sources in `CMakeLists.txt`. Nothing else — it shows up in the diff --git a/src/core/chain.cpp b/src/core/chain.cpp index 0225a2f..bb52ec8 100644 --- a/src/core/chain.cpp +++ b/src/core/chain.cpp @@ -26,6 +26,11 @@ Image renderChain(const std::vector& stages, const std::vector& stages) { + if (stages.empty() || !stages.front().effect) return false; + return stages.front().effect->needsSources(); +} + std::string chainTitle(const std::vector& stages) { std::string text; for (const auto& stage : stages) { diff --git a/src/core/chain.hpp b/src/core/chain.hpp index 028fedc..72d95ef 100644 --- a/src/core/chain.hpp +++ b/src/core/chain.hpp @@ -25,6 +25,11 @@ struct ChainStage { Image renderChain(const std::vector& stages, const std::vector& sources, int width, int height, Rng& rng); +// Whether the chain has to be given source images. Only the first stage ever +// sees them — every later one works on the stage before it — so this is simply +// whether the first effect declares that it needs them. +bool chainNeedsSources(const std::vector& stages); + // "Scanlines → Blur", for window titles and status lines. std::string chainTitle(const std::vector& stages); diff --git a/src/core/effect.hpp b/src/core/effect.hpp index b461b5f..af60c9a 100644 --- a/src/core/effect.hpp +++ b/src/core/effect.hpp @@ -27,6 +27,11 @@ public: virtual std::string description() const = 0; virtual const std::vector& params() const = 0; + // Whether the effect builds its image out of the loaded source images. + // Effects that only transform what they are handed (blur) say no, and a + // chain that starts with one of them needs no sources at all. + virtual bool needsSources() const { return true; } + virtual Image render(const std::vector& sources, int width, int height, Rng& rng, const ParamValues& values) const = 0; diff --git a/src/effects/blur.cpp b/src/effects/blur.cpp index 81b2e0b..e69f0ef 100644 --- a/src/effects/blur.cpp +++ b/src/effects/blur.cpp @@ -100,7 +100,6 @@ void blurVertical(const Image& src, Image& dst, int radius) { Image BlurEffect::render(const std::vector& sources, int width, int height, Rng& rng, const ParamValues& values) const { - if (sources.empty()) throw EffectError("No source images."); if (width < 1 || height < 1) throw EffectError("Canvas size must be at least 1x1 pixels."); const int radius = static_cast(std::max(1L, paramInt(values, "radius", 8))); @@ -108,15 +107,16 @@ Image BlurEffect::render(const std::vector& sources, int width, int heigh const std::string direction = paramChoice(values, "direction", "both"); const bool firstSource = paramBool(values, "first_source", false); - // Every random value is drawn here, before any threading, so that a seed - // always gives the same image. - const std::size_t index = firstSource ? 0 : rng.below(sources.size() - 1); - const Image& src = sources[index]; - const int sx = static_cast(rng.inRange(0, std::max(0, src.width() - width))); - const int sy = static_cast(rng.inRange(0, std::max(0, src.height() - height))); - Image canvas(width, height); - fillFromSource(canvas, src, sx, sy); + if (!sources.empty()) { + // Every random value is drawn here, before any threading, so that a + // seed always gives the same image. + const std::size_t index = firstSource ? 0 : rng.below(sources.size() - 1); + const Image& src = sources[index]; + const int sx = static_cast(rng.inRange(0, std::max(0, src.width() - width))); + const int sy = static_cast(rng.inRange(0, std::max(0, src.height() - height))); + fillFromSource(canvas, src, sx, sy); + } Image scratch(width, height); for (int pass = 0; pass < passes; ++pass) { diff --git a/src/effects/blur.hpp b/src/effects/blur.hpp index 57cf3d2..1fdcc8f 100644 --- a/src/effects/blur.hpp +++ b/src/effects/blur.hpp @@ -4,17 +4,19 @@ namespace effector { -// Fills the canvas with one of the sources and softens it with a separable box -// blur, repeated a few times to approximate a gaussian. +// Softens its input with a separable box blur, repeated a few times to +// approximate a gaussian. It needs no source images of its own: given none it +// blurs an empty canvas, which is what makes it usable as a pure filter stage. class BlurEffect : public Effect { public: std::string name() const override { return "blur"; } std::string title() const override { return "Blur"; } std::string description() const override { - return "Fills the canvas from a randomly chosen source and softens it with a repeated " - "box blur, horizontally, vertically or both."; + return "Softens its input with a repeated box blur, horizontally, vertically or both; " + "with sources loaded it fills the canvas from a randomly chosen one first."; } const std::vector& params() const override; + bool needsSources() const override { return false; } Image render(const std::vector& sources, int width, int height, Rng& rng, const ParamValues& values) const override; diff --git a/src/gui/window.cpp b/src/gui/window.cpp index 5e56157..0c01216 100644 --- a/src/gui/window.cpp +++ b/src/gui/window.cpp @@ -57,7 +57,13 @@ EffectorWindow::EffectorWindow(const Options& initial) { applyInitial(initial); suspendRender_ = false; - if (!sources_->empty()) startRender(); + if (loading_) { + placeholder_.set_text(missingText()); + } else if (canRender()) { + startRender(); + } else { + showPlaceholder(); + } } EffectorWindow::~EffectorWindow() { @@ -569,15 +575,7 @@ void EffectorWindow::removeSource(Gtk::ListBoxRow* row) { } }); sourceList_.remove(*row); - - if (!sources_->empty()) { - requestRender(); - } else { - result_ = Image(); - saveButton_.set_sensitive(false); - previewStack_.set_visible_child("placeholder"); - status_.set_text(""); - } + requestRender(); } void EffectorWindow::onAddImages() { @@ -680,14 +678,6 @@ void EffectorWindow::removeStage(int index) { rebuildChainRows(); buildParamWidgets(); - if (stages_.empty()) { - result_ = Image(); - saveButton_.set_sensitive(false); - previewStack_.set_visible_child("placeholder"); - placeholder_.set_text("Add an effect to start."); - status_.set_text(""); - return; - } requestRender(); } @@ -794,15 +784,40 @@ void EffectorWindow::onSeedEntered() { void EffectorWindow::onRecreate() { seed_ = Rng::randomSeed(); seedEntry_.set_text(std::to_string(seed_)); - if (sources_->empty()) { - showError("Add at least one source image first."); + if (!canRender()) { + showError(missingText()); return; } requestRender(); } +// Whether the effect chain has everything it needs. Only the first stage is +// ever handed the sources, so an effect that does not want them (blur) renders +// happily with none loaded. +bool EffectorWindow::canRender() const { + if (stages_.empty()) return false; + return !sources_->empty() || !chainNeedsSources(stages_); +} + +std::string EffectorWindow::missingText() const { + if (stages_.empty()) return "Add an effect to start."; + return "Add source images to start."; +} + +void EffectorWindow::showPlaceholder() { + result_ = Image(); + saveButton_.set_sensitive(false); + placeholder_.set_text(missingText()); + previewStack_.set_visible_child("placeholder"); + status_.set_text(""); +} + void EffectorWindow::requestRender() { - if (suspendRender_ || sources_->empty() || stages_.empty()) return; + if (suspendRender_) return; + if (!canRender()) { + showPlaceholder(); + return; + } // Spin buttons fire on every step while they are held down; coalesce those // into one render instead of queueing a job per tick. debounce_.disconnect(); @@ -815,7 +830,7 @@ void EffectorWindow::requestRender() { } void EffectorWindow::startRender() { - if (sources_->empty() || stages_.empty()) return; + if (!canRender()) return; if (rendering_) { staleResult_ = true; // a fresh render follows this one return; @@ -893,9 +908,11 @@ void EffectorWindow::onRenderFinished() { std::ostringstream status; status.setf(std::ios::fixed); status.precision(1); - status << chainTitle(stages_) << " · " << result_.width() << "×" << result_.height() << " · " - << sources_->size() << (sources_->size() == 1 ? " source" : " sources") << " · seed " - << seed_ << " · " << millis << " ms"; + status << chainTitle(stages_) << " · " << result_.width() << "×" << result_.height() << " · "; + if (!sources_->empty()) { + status << sources_->size() << (sources_->size() == 1 ? " source" : " sources") << " · "; + } + status << "seed " << seed_ << " · " << millis << " ms"; status_.set_text(status.str()); } @@ -970,8 +987,7 @@ void EffectorWindow::applyProject(const Project& project, const std::string& pat suspendRender_ = false; if (project.sources.empty()) { - placeholder_.set_text("Add source images to start."); - status_.set_text(""); + requestRender(); // a chain that needs no sources still has a picture } else { addPaths(project.sources); // renders once the images are decoded } diff --git a/src/gui/window.hpp b/src/gui/window.hpp index ec0f653..5c70fa5 100644 --- a/src/gui/window.hpp +++ b/src/gui/window.hpp @@ -33,6 +33,9 @@ private: void storeParams(); // widgets -> the selected stage's values void applyInitial(const Options& initial); + bool canRender() const; // a chain, and sources if the chain needs them + std::string missingText() const; // what is still missing before a render + void showPlaceholder(); // drops the preview and says what is missing void requestRender(); // coalesced: restarts the short debounce timer void startRender(); // starts a worker, or marks the result stale void onAddImages(); diff --git a/src/main.cpp b/src/main.cpp index 67defbc..79fe47c 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -40,7 +40,7 @@ int renderHeadless(const effector::Options& options) { for (const auto& failure : failures) { std::cerr << "effector: skipping " << failure.path << ": " << failure.message << "\n"; } - if (sources.empty()) { + if (sources.empty() && effector::chainNeedsSources(stages)) { std::cerr << "effector: no usable source images\n"; return 2; } diff --git a/tests/core_tests.cpp b/tests/core_tests.cpp index d9b47f3..dd1ce27 100644 --- a/tests/core_tests.cpp +++ b/tests/core_tests.cpp @@ -194,13 +194,12 @@ int main() { const auto pinnedAgain = renderBlur(sources, 300, 200, 6, {{"first_source", true}}); check(sameBytes(pinned, pinnedAgain), "first_source is reproducible"); - bool threw = false; - try { - renderBlur({}, 100, 100, 1, {}); - } catch (const EffectError&) { - threw = true; - } - check(threw, "no sources throws"); + // Blur declares that it needs no sources, so with none it softens the + // empty canvas it starts from instead of refusing to run. + const auto blank = renderBlur({}, 100, 100, 1, {}); + bool allBlack = blank.width() == 100 && blank.height() == 100; + for (std::size_t i = 0; i < blank.byteSize(); ++i) allBlack = allBlack && blank.data()[i] == 0; + check(allBlack, "without sources blur renders an empty canvas"); } std::cout << "chain\n"; @@ -241,6 +240,29 @@ int main() { threw = true; } check(threw, "an empty chain throws"); + + // Sources are an effect's own business: scanlines cuts strips out of + // them, blur only softens what it is handed and runs without any. + check(effectByName("scanlines")->needsSources(), "scanlines needs source images"); + check(!effectByName("blur")->needsSources(), "blur needs no source images"); + check(chainNeedsSources({scanlines, blur}), "a chain led by scanlines needs sources"); + check(!chainNeedsSources({blur, scanlines}), + "a chain led by blur needs none, later stages see the stage before them"); + check(!chainNeedsSources({}), "an empty chain needs no sources"); + + Rng bare(5); + const auto sourceless = renderChain({blur}, {}, 200, 150, bare); + check(sourceless.width() == 200 && sourceless.height() == 150, + "blur renders a canvas without sources"); + + bool scanlinesThrew = false; + try { + Rng rng(1); + renderChain({scanlines}, {}, 100, 100, rng); + } catch (const EffectError&) { + scanlinesThrew = true; + } + check(scanlinesThrew, "scanlines still refuses to run without sources"); } std::cout << "project\n";