diff --git a/CHANGELOG.md b/CHANGELOG.md index 704d1d4..f828460 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,17 @@ Every release of Effector, newest first. The version lives in one place, the `project()` line of `CMakeLists.txt`, and each release is a `vX.Y.Z` tag on the commit that made it. The interface shows this file under Changelog in its menu. +## [0.6.1] - 2026-09-26 + +### Fixed +- Opening a project whose images would not fit in memory no longer gets + Effector killed by the system. The images are measured before any is read, + and a batch that would not fit is refused with a message saying how much it + needs; the images stay in the project, so saving it keeps them. +- A canvas too large to render in the free memory is refused the same way, + in the interface and on the command line, instead of taking the machine's + memory down with it. + ## [0.6.0] - 2026-09-26 ### Added diff --git a/CLAUDE.md b/CLAUDE.md index 34a77b7..e114c8c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -261,6 +261,21 @@ then copies pixels in parallel. copy-on-write via `mutateSources()`, so a render in flight can never race with an add/remove. +### Memory is checked before it is taken + +Running out of memory does not fail cleanly on Linux: the kernel's OOM killer +ends the program (and often the terminal or session it runs in), leaving no +core dump. So bulk allocations are sized first and refused with an +`OutOfMemoryError` (`src/core/memory.hpp`) when they would take more than three +quarters of `availableMemory()` — the system's MemAvailable, or less under a +cgroup `memory.max`. `loadImages` sizes the whole batch from the file headers +(`imageFilePixels`) and refuses it before decoding anything, and holds the +decodes it runs at once to what is left; `renderChain` and `renderFrames` +refuse a canvas at 24 bytes a pixel (13 measured, plus the interface's copy), +plus any picture a `Path` param loads. The interface keeps a refused batch's +paths in `refusedSources_`, so saving the project never drops them; the tests +fake a small machine with `limitAvailableMemory`. + ### Image buffer `Image` is tightly packed 8-bit RGB with stride exactly `width * 3` — effects diff --git a/CMakeLists.txt b/CMakeLists.txt index 0ea4f7a..0a83737 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,5 +1,5 @@ cmake_minimum_required(VERSION 3.20) -project(effector VERSION 0.6.0 LANGUAGES CXX) +project(effector VERSION 0.6.1 LANGUAGES CXX) set(CMAKE_CXX_STANDARD 20) set(CMAKE_CXX_STANDARD_REQUIRED ON) @@ -35,6 +35,7 @@ add_library(effector_core STATIC src/core/chain.cpp src/core/effect.cpp src/core/image.cpp + src/core/memory.cpp src/core/params.cpp src/core/project.cpp src/core/registry.cpp diff --git a/packaging/arch/PKGBUILD b/packaging/arch/PKGBUILD index bc75631..7728724 100644 --- a/packaging/arch/PKGBUILD +++ b/packaging/arch/PKGBUILD @@ -2,7 +2,7 @@ # so pkgver must follow the project() line of CMakeLists.txt (the version core # test checks it). From this folder: makepkg -si pkgname=effector -pkgver=0.6.0 +pkgver=0.6.1 pkgrel=1 pkgdesc='Image effects app: a GTK4 interface and a headless CLI over pluggable effects' arch=('x86_64' 'aarch64') diff --git a/src/core/chain.cpp b/src/core/chain.cpp index a8ded83..d841f6b 100644 --- a/src/core/chain.cpp +++ b/src/core/chain.cpp @@ -2,6 +2,8 @@ #include +#include "core/memory.hpp" + namespace effector { std::vector enabledStages(const std::vector& stages) { @@ -48,6 +50,28 @@ std::vector runningStages(const std::vector& stag return running; } +// Throws OutOfMemoryError for a render that would not fit. A render holds a +// few canvases at once -- the stage before, the one being drawn, the input +// a transform copies -- and the file it is saved to; 13 bytes a pixel is +// what the heaviest chains were measured at, and the interface keeps one +// more copy to show, so 24 leaves room. A picture a stage loads itself (an +// overlay, a text box's picture) comes on top, with its alpha and pixbuf. +void ensureRenderFits(const std::vector& running, int width, int height, + const StyleSet& styles) { + std::size_t needed = static_cast(std::max(width, 0)) * + static_cast(std::max(height, 0)) * 24; + for (const auto* stage : running) { + const auto values = stageValues(*stage, styles); + for (const auto& spec : stage->effect->params()) { + if (spec.type != ParamSpec::Type::Path) continue; + const std::string path = paramText(values, spec.key, ""); + if (!path.empty()) needed += imageFilePixels(path) * 8; + } + } + ensureMemory(needed, "Rendering a " + std::to_string(width) + " × " + std::to_string(height) + + " canvas"); +} + // Runs running[first, last) on `image`, which holds the sources' place for // the first stage of the chain and the previous stage's output after it. Image runStages(const std::vector& running, std::size_t first, @@ -76,6 +100,7 @@ Image renderChain(const std::vector& stages, const std::vector& stages, const std::vector const VideoSettings& video, const std::function& onFrame) { const auto running = runningStages(stages); + ensureRenderFits(running, width, height, styles); const int count = video.frameCount(); const int fps = std::clamp(video.fps, kMinFps, kMaxFps); diff --git a/src/core/image.cpp b/src/core/image.cpp index aa218a1..cf37662 100644 --- a/src/core/image.cpp +++ b/src/core/image.cpp @@ -11,6 +11,8 @@ #include #include +#include "core/memory.hpp" + namespace effector { namespace { @@ -152,9 +154,31 @@ void Image::save(const std::string& path) const { if (saved == FALSE) throw std::runtime_error(takeError(error)); } +std::size_t imageFilePixels(const std::string& path) { + int width = 0; + int height = 0; + if (gdk_pixbuf_get_file_info(path.c_str(), &width, &height) == nullptr) return 0; + return static_cast(std::max(width, 0)) * static_cast(std::max(height, 0)); +} + std::vector loadImages(const std::vector& paths, std::vector& failures, const LoadProgress* progress) { + // 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. + 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); + } + const std::string what = paths.size() == 1 + ? std::string("Loading this image") + : "Loading these " + std::to_string(paths.size()) + " images"; + ensureMemory(kept + largest, what); + 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 @@ -167,7 +191,8 @@ std::vector loadImages(const std::vector& paths, unsigned hardware = std::thread::hardware_concurrency(); if (hardware == 0) hardware = 1; - const std::size_t workers = std::min(hardware, std::max(paths.size(), 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 (;;) { diff --git a/src/core/image.hpp b/src/core/image.hpp index ffa584d..c446df2 100644 --- a/src/core/image.hpp +++ b/src/core/image.hpp @@ -68,9 +68,18 @@ struct LoadProgress { std::atomic* cancel = nullptr; }; +// The pixel count of an image file from its header alone, without decoding +// it; 0 when the header cannot be read. +std::size_t imageFilePixels(const std::string& path); + // 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. +// +// Before decoding anything the whole batch is sized from the file headers, +// and a batch that would not fit in the free memory throws OutOfMemoryError +// (core/memory.hpp) with nothing loaded; the decodes that run at once are +// also held to what is left over. std::vector loadImages(const std::vector& paths, std::vector& failures, const LoadProgress* progress = nullptr); diff --git a/src/core/memory.cpp b/src/core/memory.cpp new file mode 100644 index 0000000..3e1689a --- /dev/null +++ b/src/core/memory.cpp @@ -0,0 +1,123 @@ +#include "core/memory.hpp" + +#include + +#include +#include +#include +#include +#include +#include + +namespace effector { +namespace { + +std::atomic cap{0}; + +// MemAvailable from /proc/meminfo, falling back on the free pages; 0 when +// neither says. +std::size_t systemAvailable() { + std::ifstream meminfo("/proc/meminfo"); + std::string key; + std::size_t kilobytes = 0; + std::string unit; + while (meminfo >> key >> kilobytes >> unit) { + if (key == "MemAvailable:") return kilobytes * 1024; + } + const long pages = sysconf(_SC_AVPHYS_PAGES); + const long pageSize = sysconf(_SC_PAGESIZE); + if (pages > 0 && pageSize > 0) { + return static_cast(pages) * static_cast(pageSize); + } + return 0; +} + +// A number from a cgroup file, or nothing for "max" or a file that is not +// there. +bool readCount(const std::filesystem::path& file, std::size_t& out) { + std::ifstream in(file); + std::string word; + if (!(in >> word) || word == "max") return false; + try { + out = static_cast(std::stoull(word)); + return true; + } catch (const std::exception&) { + return false; + } +} + +std::size_t inactiveFile(const std::filesystem::path& dir) { + std::ifstream in(dir / "memory.stat"); + std::string key; + std::size_t value = 0; + while (in >> key >> value) { + if (key == "inactive_file") return value; + } + return 0; +} + +// The room left under the tightest memory.max on the way from this +// process's cgroup (v2) up to the root, counting the page cache it could +// drop as room; the largest value when nothing caps it. +std::size_t cgroupAvailable() { + std::size_t room = std::numeric_limits::max(); + std::ifstream self("/proc/self/cgroup"); + std::string line; + while (std::getline(self, line)) { + if (line.rfind("0::", 0) != 0) continue; + // "0::/user.slice/app.slice/x.scope", walked from the scope upwards. + for (std::filesystem::path group = line.substr(4); !group.empty(); + group = group.parent_path()) { + const auto dir = std::filesystem::path("/sys/fs/cgroup") / group; + std::size_t max = 0; + std::size_t current = 0; + if (!readCount(dir / "memory.max", max) || !readCount(dir / "memory.current", current)) { + continue; + } + const std::size_t used = current - std::min(current, inactiveFile(dir)); + room = std::min(room, max - std::min(max, used)); + } + } + return room; +} + +} // namespace + +std::size_t availableMemory() { + std::size_t available = systemAvailable(); + const std::size_t room = cgroupAvailable(); + if (room != std::numeric_limits::max()) { + available = available == 0 ? room : std::min(available, room); + } + const std::size_t capped = cap.load(std::memory_order_relaxed); + if (capped != 0 && (available == 0 || capped < available)) available = capped; + return available; +} + +void limitAvailableMemory(std::size_t bytes) { cap.store(bytes, std::memory_order_relaxed); } + +std::size_t usableMemory() { + const std::size_t available = availableMemory(); + return available == 0 ? std::numeric_limits::max() : available / 4 * 3; +} + +void ensureMemory(std::size_t needed, const std::string& what) { + const std::size_t available = availableMemory(); + if (available == 0 || needed <= available / 4 * 3) return; + throw OutOfMemoryError(what + " needs about " + formatBytes(needed) + + " of memory, more than is safe to take with " + formatBytes(available) + + " free."); +} + +std::string formatBytes(std::size_t bytes) { + char text[32]; + const double gb = static_cast(bytes) / (1024.0 * 1024.0 * 1024.0); + if (gb >= 1.0) { + std::snprintf(text, sizeof text, "%.1f GB", gb); + } else { + std::snprintf(text, sizeof text, "%.0f MB", static_cast(bytes) / (1024.0 * 1024.0)); + } + return text; +} + +} // namespace effector diff --git a/src/core/memory.hpp b/src/core/memory.hpp new file mode 100644 index 0000000..485dfdf --- /dev/null +++ b/src/core/memory.hpp @@ -0,0 +1,39 @@ +#pragma once + +#include +#include +#include + +namespace effector { + +// Thrown instead of starting work that would not fit in memory. Decoding a +// batch of photographs or rendering a huge canvas past what the machine has +// free does not fail cleanly: the kernel's OOM killer ends the program, and +// often others with it. +class OutOfMemoryError : public std::runtime_error { +public: + using std::runtime_error::runtime_error; +}; + +// The bytes this process could still allocate without swapping: the +// system's MemAvailable, or less when a cgroup (a systemd unit, a container) +// caps the process below it. Zero when neither can be read. +std::size_t availableMemory(); + +// Caps what availableMemory() reports, for the tests; 0 lifts the cap. +void limitAvailableMemory(std::size_t bytes); + +// Throws OutOfMemoryError when `needed` more bytes would take more than three +// quarters of availableMemory() -- the rest is left to the desktop and to +// whatever else the program holds. `what` names the work in the message. +// Does nothing when the free memory cannot be read. +void ensureMemory(std::size_t needed, const std::string& what); + +// The bytes ensureMemory() would let through right now; the largest value +// when the free memory cannot be read. +std::size_t usableMemory(); + +// "4.6 GB", "380 MB". +std::string formatBytes(std::size_t bytes); + +} // namespace effector diff --git a/src/gui/window.cpp b/src/gui/window.cpp index 05bd237..d77f2bf 100644 --- a/src/gui/window.cpp +++ b/src/gui/window.cpp @@ -10,6 +10,7 @@ #include "core/canvas.hpp" #include "core/chain.hpp" +#include "core/memory.hpp" #include "core/project.hpp" #include "core/registry.hpp" #include "core/rng.hpp" @@ -1349,11 +1350,19 @@ void EffectorWindow::addPaths(const std::vector& paths) { }; std::vector failures; - auto images = loadImages(paths, failures, &progress); + std::vector images; + std::string refused; + try { + images = loadImages(paths, failures, &progress); + } catch (const OutOfMemoryError& error) { + refused = error.what(); + } { std::lock_guard lock(loadMutex_); loadedImages_ = std::move(images); loadFailures_ = std::move(failures); + loadRefused_ = std::move(refused); + loadRefusedPaths_ = loadRefused_.empty() ? std::vector{} : paths; } loadFinished_.emit(); }); @@ -1384,12 +1393,18 @@ void EffectorWindow::onLoadFinished() { std::vector images; std::vector failures; + std::string refused; + std::vector refusedPaths; { std::lock_guard lock(loadMutex_); images = std::move(loadedImages_); failures = std::move(loadFailures_); + refused = std::move(loadRefused_); + refusedPaths = std::move(loadRefusedPaths_); loadedImages_.clear(); loadFailures_.clear(); + loadRefused_.clear(); + loadRefusedPaths_.clear(); } if (loadCancel_.load(std::memory_order_relaxed)) { @@ -1406,6 +1421,22 @@ void EffectorWindow::onLoadFinished() { dropLoadedImages_ = false; images.clear(); failures.clear(); + refused.clear(); + refusedPaths.clear(); + } + + if (!refused.empty()) { + // Nothing of a refused batch was decoded. Its paths are kept for the + // project file, and whatever was queued behind it still gets its turn. + refusedSources_.insert(refusedSources_.end(), refusedPaths.begin(), refusedPaths.end()); + showError(refused + " They were not loaded, but stay in the project when it is saved. " + "Close other programs, or load fewer or smaller images."); + if (!loadQueue_.empty()) { + auto queued = std::move(loadQueue_); + loadQueue_.clear(); + addPaths(queued); + } + return; } for (const auto& image : images) appendSourceRow(image.path()); @@ -2273,6 +2304,7 @@ Gtk::Widget& EffectorWindow::buildMainMenu() { Project EffectorWindow::currentProject() const { Project project; for (const auto& image : *sources_) project.sources.push_back(image.path()); + project.sources.insert(project.sources.end(), refusedSources_.begin(), refusedSources_.end()); project.stages = stages_; project.styles = styles_; project.width = static_cast(widthSpin_.get_value()); @@ -2320,6 +2352,7 @@ void EffectorWindow::clearSources() { while (auto* row = sourceList_.get_row_at_index(0)) sourceList_.remove(*row); updateSourcesLabel(); mutateSources([](SourceList& list) { list.clear(); }); + refusedSources_.clear(); loadQueue_.clear(); if (loading_) dropLoadedImages_ = true; diff --git a/src/gui/window.hpp b/src/gui/window.hpp index 5172eff..e01cfc0 100644 --- a/src/gui/window.hpp +++ b/src/gui/window.hpp @@ -219,6 +219,12 @@ private: std::string loadCurrent_; std::vector loadedImages_; std::vector loadFailures_; + std::string loadRefused_; // why the batch would not fit in memory, if it would not + std::vector loadRefusedPaths_; + // Sources refused as too large to hold. They are not shown or rendered, + // but a project saved now keeps them, so a refusal never costs the + // project its images. + std::vector refusedSources_; std::vector loadQueue_; // chosen while another batch was loading bool loading_ = false; bool dropLoadedImages_ = false; // the batch in flight belongs to a replaced project diff --git a/tests/core_tests.cpp b/tests/core_tests.cpp index 9b8359b..beb4a84 100644 --- a/tests/core_tests.cpp +++ b/tests/core_tests.cpp @@ -20,6 +20,7 @@ #include "cli/options.hpp" #include "core/canvas.hpp" #include "core/chain.hpp" +#include "core/memory.hpp" #include "core/project.hpp" #include "core/registry.hpp" #include "core/rng.hpp" @@ -1718,6 +1719,72 @@ int main() { std::filesystem::remove_all(dir); } + std::cout << "memory\n"; + { + check(availableMemory() > 0, "the free memory can be read"); + check(formatBytes(std::size_t{5} << 29) == "2.5 GB" && formatBytes(std::size_t{380} << 20) == "380 MB", + "sizes are written in GB and MB"); + + const auto dir = std::filesystem::temp_directory_path() / "effector_test_memory"; + std::filesystem::create_directories(dir); + std::vector paths; + for (int i = 0; i < 3; ++i) { + const auto path = dir / ("src" + std::to_string(i) + ".png"); + gradient(400, 300, i + 1).save(path.string()); + paths.push_back(path.string()); + } + check(imageFilePixels(paths[0]) == 400u * 300 && imageFilePixels((dir / "missing.png").string()) == 0, + "an image is sized from its header"); + + // Three 400 x 300 images hold 1 MB decoded; with 1 MB free they do not fit. + limitAvailableMemory(std::size_t{1} << 20); + std::atomic decoded{0}; + LoadProgress counting; + counting.onFile = [&](std::size_t, std::size_t, const std::string&) { ++decoded; }; + std::vector failures; + bool refused = false; + std::string message; + try { + loadImages(paths, failures, &counting); + } catch (const OutOfMemoryError& error) { + refused = true; + message = error.what(); + } + check(refused, "a batch too large for the free memory is refused"); + check(decoded.load() == 0, "before a single file is decoded"); + check(message.find("Loading these 3 images needs about") == 0 && message.find("with 1 MB free") != std::string::npos, + "and the message says how much is needed and how much is free"); + + const std::vector scanlines = {{effectByName("scanlines"), {}}}; + const std::vector small = {gradient(64, 64, 7)}; + bool renderRefused = false; + try { + Rng rng(1); + renderChain(scanlines, small, 4000, 4000, rng); + } catch (const OutOfMemoryError&) { + renderRefused = true; + } + check(renderRefused, "a render too large for the free memory is refused"); + bool videoRefused = false; + try { + VideoSettings clip; + clip.enabled = true; + renderFrames(scanlines, small, 4000, 4000, 1, {}, clip, [](int, Image&&) { return true; }); + } catch (const OutOfMemoryError&) { + videoRefused = true; + } + check(videoRefused, "and so is a clip"); + Rng smallRng(1); + check(renderChain(scanlines, small, 100, 100, smallRng).width() == 100, + "a render that fits still runs"); + + limitAvailableMemory(0); + failures.clear(); + check(loadImages(paths, failures).size() == 3, "with the memory there the batch loads"); + + std::filesystem::remove_all(dir); + } + std::cout << "video\n"; { using effector::ChainStage; diff --git a/tests/gui_tests.cpp b/tests/gui_tests.cpp index 020268a..2b18067 100644 --- a/tests/gui_tests.cpp +++ b/tests/gui_tests.cpp @@ -14,6 +14,8 @@ #include #include "cli/options.hpp" +#include "core/image.hpp" +#include "core/memory.hpp" #include "core/project.hpp" #include "core/registry.hpp" #include "core/version.hpp" @@ -514,6 +516,45 @@ void testVideo(Gtk::Window& window, const std::filesystem::path& dir) { stack->set_visible_child("chain"); } +// A project whose images will not fit is refused before any is decoded, and +// saving it then must not quietly drop them from the file. +void testMemoryGuard(Gtk::Window& window, const std::filesystem::path& dir) { + std::cout << "memory guard\n"; + std::filesystem::create_directories(dir); + effector::Project project; + for (int i = 0; i < 2; ++i) { + const auto source = dir / ("big" + std::to_string(i) + ".png"); + effector::Image(600, 600).save(source.string()); + project.sources.push_back(source.string()); + } + project.stages.push_back({effector::effectByName("scanlines"), {}}); + const auto path = dir / "big.json"; + effector::saveProject(path.string(), project); + + effector::limitAvailableMemory(std::size_t{1} << 20); // 2.1 MB of images, 1 MB free + window.activate_action("win.open-recent", + Glib::Variant::create(Glib::filename_to_uri(path.string()))); + pump(500); + auto* sources = named(window, "sources"); + auto lists = sources != nullptr ? all(*sources) : std::vector{}; + check(!lists.empty() && rowCount(*lists.front()) == 0, "images too large for the memory are not loaded"); + bool told = false; + for (auto* label : all(window)) { + const std::string text = label->get_text(); + if (label->is_drawable() && text.find("Loading these 2 images needs about") != std::string::npos && + text.find("stay in the project") != std::string::npos) { + told = true; + } + } + check(told, "and the window says why"); + + window.activate_action("win.save-project"); + pump(100); + check(effector::loadProject(path.string()).sources.size() == 2, + "saving the project keeps the images it could not load"); + effector::limitAvailableMemory(0); +} + } // namespace int main(int argc, char** argv) { @@ -542,6 +583,7 @@ int main(int argc, char** argv) { testHelpWindows(*window); testRecentProjects(*window, dataDir / "projects"); testVideo(*window, dataDir / "video"); + testMemoryGuard(*window, dataDir / "memory"); app->remove_window(*window); }); const int status = app->run(argc > 0 ? 1 : 0, argv);