From 8aa95bdfc107c4e5e3bf99d86f6a6ddf6273d88f Mon Sep 17 00:00:00 2001 From: Michal Date: Mon, 12 Oct 2026 21:55:52 +0200 Subject: [PATCH 1/4] feat: implement sprite renderer component --- .github/workflows/ci.yml | 2 +- CLAUDE.md | 10 +- README.md | 98 +++++++++++- cmake/Dependencies.cmake | 3 +- include/parser/Parser.hpp | 14 +- include/scene/Scene.hpp | 1 + include/scene/components/BlinkComponent.hpp | 4 +- include/scene/components/Component.hpp | 8 +- .../components/ComponentBuildContext.hpp | 24 +++ .../scene/components/ComponentRegistry.hpp | 7 +- .../components/MarkerEmitterComponent.hpp | 6 +- include/scene/components/SpriteRenderer.hpp | 55 +++++++ protoFiles/neuronide.proto | 4 +- src/CMakeLists.txt | 12 +- src/parser/Parser.cpp | 21 ++- src/renderer/Renderer.cpp | 8 +- src/scene/CMakeLists.txt | 12 +- src/scene/Scene.cpp | 8 + src/scene/components/BlinkComponent.cpp | 5 +- src/scene/components/ComponentRegistry.cpp | 5 +- .../components/MarkerEmitterComponent.cpp | 3 +- src/scene/components/SpriteRenderer.cpp | 90 +++++++++++ tests/CMakeLists.txt | 3 +- tests/component_tests/sprite_scene_test.cpp | 109 +++++++++++++ tests/registration_tests/CMakeLists.txt | 7 + .../registration_tests/registration_test.cpp | 52 ++++++ tests/unit_tests/RendererTest.cpp | 30 ++++ .../component_build_context_test.cpp | 37 +++++ tests/unit_tests/sprite_renderer_test.cpp | 148 ++++++++++++++++++ tests/utils/SdlTestUtils.hpp | 85 ++++++++++ tests/utils/TempDir.hpp | 45 ++++++ 31 files changed, 877 insertions(+), 39 deletions(-) create mode 100644 include/scene/components/ComponentBuildContext.hpp create mode 100644 include/scene/components/SpriteRenderer.hpp create mode 100644 src/scene/components/SpriteRenderer.cpp create mode 100644 tests/component_tests/sprite_scene_test.cpp create mode 100644 tests/registration_tests/CMakeLists.txt create mode 100644 tests/registration_tests/registration_test.cpp create mode 100644 tests/unit_tests/component_build_context_test.cpp create mode 100644 tests/unit_tests/sprite_renderer_test.cpp create mode 100644 tests/utils/SdlTestUtils.hpp create mode 100644 tests/utils/TempDir.hpp diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9801b1f..84d30d0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,7 +7,7 @@ on: branches: [ "main", "master" ] env: - BUILD_DEPS: cmake ninja-build libsdl2-dev protobuf-compiler + BUILD_DEPS: cmake ninja-build libsdl2-dev libsdl2-image-dev protobuf-compiler jobs: build-and-test: diff --git a/CLAUDE.md b/CLAUDE.md index d056b2a..dde7d9c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -10,8 +10,10 @@ You are in the **runtime** repo. Read the system overview below first. - Layout: `include/` headers, `src/` impl, `src/parser/Parser.cpp` turns a `.neuroz`/`.pb` file into a `Scene`, `scene/components/` component classes, `tests/{unit,component}_tests/`. - New component: subclass `Component`, add a static creator, and - `REGISTER_COMPONENT(NeuronIDE::Component::k, Creator)` in its `.cpp`. The proto oneof - entry must exist first — which means a backend change too. + `REGISTER_COMPONENT(NeuronIDE::Component::k, Creator)` in its `.cpp`, then add it to + `tests/registration_tests` (static self-registration is silently dropped unless executables + link the `runtime_core` target). The proto oneof entry must exist first — which means a + backend change too. - Test scenes: write `.pbtxt`, then `protoc --encode=NeuronIDE.Scene protoFiles/neuronide.proto < x.pbtxt > x.pb`. - Read README §3 (architecture: Renderer / LSLReader / DataWriter threads) and §5 (what goes in @@ -96,7 +98,9 @@ launcher#1. ### Known cross-repo gaps (as of 2026-10-09 — fix them or update this list) -- Proto copies have diverged: backend lacks `MarkerEmitterComponent` (backend#23). +- Proto copies have diverged: backend lacks `MarkerEmitterComponent` (backend#23), and the + runtime reserved `SpriteRenderer.texture_path` (field 1) in favour of `img_path`; the backend + and launcher copies must follow. - Experiment flow graph is not in the proto, so it isn't saved, exported or run (epic frontend#59). - Scene object color and LSL marker are not persisted (frontend#56, backend#24). - Media files (images, audio, scripts) can't be uploaded, so the runtime can't find them (backend#25). diff --git a/README.md b/README.md index 959f8e6..6ecbf54 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,7 @@ EEG samples and the stimulus markers must share a **common clock domain** so tha | Experiment file format | Protocol Buffers (proto3) — see `protoFiles/neuronide.proto` | | Device config format | JSON (`config.json`) parsed with [nlohmann/json](https://github.com/nlohmann/json) | | EEG acquisition | [LSL — Lab Streaming Layer](https://github.com/sccn/liblsl) (`liblsl`) | -| Rendering / windowing | SDL2 (+ SDL2_image), with vsync | +| Rendering / windowing | SDL2 + SDL2_image, with vsync (SDL2_ttf planned, see [Components](#components)) | | Inter-thread queues | [moodycamel ConcurrentQueue](https://github.com/cameron314/concurrentqueue) (lock-free) | | Python scripting (planned)| pybind11 (`ScriptComponent`) | | Testing | GoogleTest + CTest | @@ -138,7 +138,28 @@ classDiagram Components are built from protobuf messages by a **`ComponentRegistry`** (a singleton mapping a proto component type id → factory function). New component types self-register via the `REGISTER_COMPONENT(typeId, creatorFunc)` macro, so -the parser does not need to know about concrete component classes. +the parser does not need to know about concrete component classes. Each creator +also receives a `ComponentBuildContext`, which resolves asset paths: a relative +path in the `.neuroz` is taken relative to the folder holding the `.neuroz`, and an +absolute one is used as-is. + +Self-registration runs from static initializers that nothing else references, so a +plain static link silently drops them. Executables therefore link the `runtime_core` +target, which pulls in the whole archive. `tests/registration_tests` guards this, and +every new component type gets a line there. + +After construction, a component goes through two hooks, both before the first frame: + +1. `onSceneReady(Scene&)`, called by `Parser` once every object exists, to resolve + and subscribe to events. +2. `onRenderTargetReady(const std::shared_ptr&)`, called by the + `Renderer` constructor once the window exists, to create GPU resources such as + textures. Doing it there, not lazily on the first `render()`, keeps uploads from + delaying the first stimulus frame and its marker. SDL frees a renderer's textures + along with it, so a component holding textures keeps the target alive. + +Anything that can fail on bad input (a missing image, for instance) fails during +parsing, before the window opens. ### Core data structures @@ -359,7 +380,7 @@ correct when `SDL_RenderPresent` waits for the refresh. | `Scene` / `SceneObject` | Implemented | component containers | | `Component` (base) | Implemented | abstract `update` / `render` | | `ComponentRegistry` | Implemented | proto-type → factory, macro-based self-registration | -| `specifiic components` | **Planned** | defined in `neuronide.proto`, not yet implemented in C++ | +| Scene components | **Partial** | 3 of the 9 MVP components have a C++ class, see [Components](#components) | | `Renderer` | Implemented | SDL + vsync, marker timestamping | | `LSLReader` | Implemented | LSL inlet → `eegQueue`, clock-synced (see §4); driven by `DeviceConfig`, enabled channels only, re-resolves lost streams, reports `AcquisitionState` | | `ConfigParser` | Implemented | `config.json` → `DeviceConfig` (1:1 mapping, major-version checked, see §5), nlohmann/json | @@ -371,6 +392,72 @@ correct when `SDL_RenderPresent` waits for the refresh. The class diagram in older docs is partly aspirational; the table above reflects the actual code. +### Components + +The MVP set of components an experiment is built from, and where each one stands. +Status as of **2026-10-10**, checked against `main`, open PRs and `neuronide.proto`. +Update the row in the same PR that changes it. + +| Component | What it does | Proto (`neuronide.proto`) | C++ (`scene/components/`) | Status | Tracking | +| --- | --- | --- | --- | --- | --- | +| Transform | Position, size, rotation. Exactly one per object. x/y is the top-left corner in window pixels, rotation is degrees clockwise about the centre. | `SceneObject.transform`, a field rather than a `Component` | `SceneObject::Transform` | **Partial**: parsed onto the object and drawn by SpriteRenderer, but not required (an object without one sits at all zeros; a sprite on it is rejected at parse time) | — | +| SpriteRenderer | Draws an image: visual stimuli, photos, icons. | `SpriteRenderer` (`renderer = 1`), `img_path`; field 1 (`texture_path`) is reserved | `SpriteRenderer`, registered | **Implemented**: any SDL2_image format (PNG, JPEG, …), stretched over the Transform and rotated. Decoded at parse time, so a missing or unreadable file stops the run before the window opens; uploaded to the GPU before the first frame | [#17][rt-17], [backend#25][be-25] | +| TextRenderer | Draws text in a chosen font and size: instructions, letters, numbers. | `TextRenderer` (`text = 2`) | — | **Planned**: needs SDL2_ttf, which is not a dependency yet. The message has no text color | [#18][rt-18], [backend#25][be-25] | +| BlinkComponent | Flickers the object at a set frequency in Hz. The basis for SSVEP. | `BlinkComponent` (`blinker = 3`) | `BlinkComponent`, registered and built by the parser | **In review**: `update()` is an empty TODO on `main`, so nothing blinks yet. PR [#34][rt-34] implements it; changes requested | [#16][rt-16] | +| MarkerEmitter | Emits an LSL marker on a scene event, ideally when an object appears or disappears. | `MarkerEmitterComponent` (`marker_emitter = 5`). Missing from the backend's copy | `MarkerEmitterComponent`, registered | **Implemented, nothing to bind to**: bindings resolve and push markers (tested against a test-only event source), but no production component exposes an event and objects have no appear/disappear events, so any binding in a real `.neuroz` fails at parse time | [backend#23][be-23] | +| Lifetime | Keeps the object visible for N ms, then hides it. | — | — | **Not started** | — | +| ResponseListener | While its object is active, records which key was pressed and the reaction time. | — | — | **Not started** | — | +| ScriptComponent | Custom logic in Python, outside the standard components. | `ScriptComponent` (`script = 4`) | — | **Planned**: needs pybind11, which is not a dependency yet | [backend#25][be-25] | +| AudioSource | Plays a sound. Possibly post-MVP. | — | — | **Not started** | — | + +- **Implemented**: registered class, behaviour complete, covered by tests. +- **In review**: the implementation is in an open PR. +- **Partial**: exists, with the gaps listed in its row. +- **Planned**: the proto message exists, the C++ class does not. The parser **drops such + components silently** (`ComponentRegistry::build` returns `nullptr` and `Parser` skips + it), so a scene that uses one loads without error and the component does nothing. +- **Not started**: neither a proto message nor a class. Adding the message comes first and + is a cross-team change (backend proto and dataclasses, launcher copy, frontend editor; see + `CLAUDE.md`). + +#### Open questions before the remaining components + +- **Event sources.** `MarkerEmitter` binds to events that a component exposes through + `getEvent()`, and nothing exposes one yet. The obvious first sources are an object + appearing or disappearing, a Lifetime timeout and a ResponseListener response. +- **Who owns visibility.** Blink, Lifetime and scripts all write `SceneObject::isVisible`. + If appear/disappear events follow that flag, a 15 Hz blinker emits 30 markers a second, + and the 0 Hz branch in PR [#34][rt-34] overrides a script that hides the object. Keeping + "the object is shown in this trial" (Lifetime, scripts; drives the events) separate from + "flicker phase" (Blink; affects drawing only) avoids both. +- **Frame-locked timing.** The screen only changes at vsync, and markers are stamped there. + Flicker phase and Lifetime durations should come from the frame being presented (frame + count × refresh period), not from summing `Context::timestamp`. That value is a loop + delta measured at the top of the loop, so it jitters, and a frame near an on/off edge + can land on either side. Frequencies above half the refresh rate can't be shown at all + and should be rejected. Dropped frames should be detected and logged. +- **Input and reaction time.** The render loop polls SDL events itself and handles only + `SDL_QUIT`, and `Context` carries no input. ResponseListener needs key events passed + in, stamped with when the key was pressed, not with the vsync time the Renderer gives + markers. With one poll per frame, reaction-time resolution is one frame (~16.7 ms at + 60 Hz). One option is to record the response as a marker (e.g. `response:`) with its + own timestamp and compute reaction time offline against the onset marker. +- **Media files.** Sprite, Text (fonts), Script and Audio all reference files by path. The + runtime resolves relative paths against the `.neuroz` folder (see + [Component model](#component-model)), but assets can't be uploaded or exported yet + ([backend#25][be-25]). Until they can, the launcher has to place them next to the + `.neuroz` by hand. +- **Transform units.** `Transform` is in window pixels, so a stimulus's physical size depends + on the lab monitor, which comes from `config.json`. Decide whether to stay with pixels or + move to normalized or visual-angle units before more renderers land. + +[rt-16]: https://github.com/KN-Neuron/Neuron-IDE-runtime/issues/16 +[rt-17]: https://github.com/KN-Neuron/Neuron-IDE-runtime/issues/17 +[rt-18]: https://github.com/KN-Neuron/Neuron-IDE-runtime/issues/18 +[rt-34]: https://github.com/KN-Neuron/Neuron-IDE-runtime/pull/34 +[be-23]: https://github.com/KN-Neuron/Neuron-IDE-backend/issues/23 +[be-25]: https://github.com/KN-Neuron/Neuron-IDE-backend/issues/25 + ## 8. Repository layout ``` @@ -393,7 +480,8 @@ Neuron-IDE-runtime/ # the C++ runtime (git repo) ├── src/ # one CMake subdirectory (static lib) per module └── tests/ ├── unit_tests/ # CTest label: "unit" - └── component_tests/ # CTest label: "component" + ├── component_tests/ # CTest label: "component" + └── registration_tests/ # separate binary: every component is linked in (label "component") ``` Each `src//` builds a static library; `runtime_core` links them together @@ -407,7 +495,7 @@ All commands are run from the `Neuron-IDE-runtime/` directory. ```bash sudo apt update -sudo apt install cmake ninja-build clang-format clang-tidy-20 libsdl2-dev protobuf-compiler gcovr +sudo apt install cmake ninja-build clang-format clang-tidy-20 libsdl2-dev libsdl2-image-dev protobuf-compiler gcovr # GTest, LSL, concurrentqueue and nlohmann/json are fetched automatically by CMake. ``` diff --git a/cmake/Dependencies.cmake b/cmake/Dependencies.cmake index d9f2c02..ca38e3b 100644 --- a/cmake/Dependencies.cmake +++ b/cmake/Dependencies.cmake @@ -52,8 +52,9 @@ FetchContent_MakeAvailable(googletest liblsl concurrentqueue nlohmann_json) set(CMAKE_C_FLAGS "${BACKUP_C_FLAGS}") set(CMAKE_CXX_FLAGS "${BACKUP_CXX_FLAGS}") -# 5. SDL2 (System installed) +# 5. SDL2 + SDL2_image (System installed) find_package(SDL2 REQUIRED) +find_package(SDL2_image REQUIRED) # 6. Protobuf (System installed) find_package(Protobuf REQUIRED) diff --git a/include/parser/Parser.hpp b/include/parser/Parser.hpp index d4e8660..920d06f 100644 --- a/include/parser/Parser.hpp +++ b/include/parser/Parser.hpp @@ -5,6 +5,8 @@ #include #include +#include "scene/components/ComponentBuildContext.hpp" + class Scene; class SceneObject; class Component; @@ -18,13 +20,19 @@ class Parser { public: Parser() = default; + // Relative asset paths in the scene (e.g. sprite images) resolve against the folder holding + // filePath. static std::shared_ptr parse(const std::string& filePath); - static std::shared_ptr parseStream(std::istream& stream); + // Relative asset paths resolve against context.assetRoot: the working directory by default. + static std::shared_ptr parseStream(std::istream& stream, + const ComponentBuildContext& context = {}); private: - static std::shared_ptr buildSceneObject(const NeuronIDE::SceneObject& protoObj); + static std::shared_ptr buildSceneObject(const NeuronIDE::SceneObject& protoObj, + const ComponentBuildContext& context); static std::unique_ptr buildComponent(const NeuronIDE::Component& protoComp, - const std::shared_ptr& owner); + const std::shared_ptr& owner, + const ComponentBuildContext& context); }; #endif // PARSER_HPP \ No newline at end of file diff --git a/include/scene/Scene.hpp b/include/scene/Scene.hpp index 358f526..60c781f 100644 --- a/include/scene/Scene.hpp +++ b/include/scene/Scene.hpp @@ -28,6 +28,7 @@ class Scene { void render(SDL_Renderer* renderer); void onSceneReady(); + void onRenderTargetReady(const std::shared_ptr& target); Delegate& resolveEvents(std::string_view objectName, std::string_view eventName); }; diff --git a/include/scene/components/BlinkComponent.hpp b/include/scene/components/BlinkComponent.hpp index dd68a4a..a72a6d5 100644 --- a/include/scene/components/BlinkComponent.hpp +++ b/include/scene/components/BlinkComponent.hpp @@ -8,6 +8,7 @@ namespace NeuronIDE { class Component; } +struct ComponentBuildContext; class BlinkComponent : public Component { public: @@ -19,7 +20,8 @@ class BlinkComponent : public Component { void render(SDL_Renderer* renderer) override; static std::unique_ptr createBlinker(const NeuronIDE::Component& protoComp, - const std::shared_ptr& owner); + const std::shared_ptr& owner, + const ComponentBuildContext& context); private: double blinkFrequencyHz = 0.0; diff --git a/include/scene/components/Component.hpp b/include/scene/components/Component.hpp index 0f17992..360a9f7 100644 --- a/include/scene/components/Component.hpp +++ b/include/scene/components/Component.hpp @@ -24,7 +24,13 @@ class Component { virtual void update(const Context& context) = 0; virtual void render(SDL_Renderer* renderer) = 0; - virtual void onSceneReady(Scene& scene) { (void)scene; } + virtual void onSceneReady(Scene& scene) { (void)scene; } + + // Called once the render target this scene is drawn to exists, before the first frame: the + // place to create GPU resources such as textures. SDL frees a renderer's textures along with + // it, so a component holding any keeps `target` alive for as long as it does. + virtual void onRenderTargetReady(const std::shared_ptr& target) { (void)target; } + virtual Delegate* getEvent(std::string_view eventName) { (void)eventName; return nullptr; diff --git a/include/scene/components/ComponentBuildContext.hpp b/include/scene/components/ComponentBuildContext.hpp new file mode 100644 index 0000000..4d58358 --- /dev/null +++ b/include/scene/components/ComponentBuildContext.hpp @@ -0,0 +1,24 @@ +#ifndef COMPONENTBUILDCONTEXT_HPP +#define COMPONENTBUILDCONTEXT_HPP + +#include + +// What a component creator may need beyond its own proto message and owner. Parser builds one per +// scene and hands it to every creator. +struct ComponentBuildContext { + // Folder that relative asset paths in the experiment file resolve against: the folder holding + // the .neuroz file. Empty for a scene parsed from a bare stream, so relative paths resolve + // against the working directory. + std::filesystem::path assetRoot; + + // An absolute path is returned as-is and a relative one is taken relative to assetRoot. An + // empty path stays empty, so the caller can still reject it. + [[nodiscard]] std::filesystem::path resolveAsset(const std::filesystem::path& path) const { + if (path.empty() || path.is_absolute()) { + return path; + } + return assetRoot / path; + } +}; + +#endif // COMPONENTBUILDCONTEXT_HPP diff --git a/include/scene/components/ComponentRegistry.hpp b/include/scene/components/ComponentRegistry.hpp index 1ef388e..35b72e8 100644 --- a/include/scene/components/ComponentRegistry.hpp +++ b/include/scene/components/ComponentRegistry.hpp @@ -10,9 +10,11 @@ namespace NeuronIDE { class Component; } class SceneObject; +struct ComponentBuildContext; using ComponentCreatorFunc = std::function( - const NeuronIDE::Component&, const std::shared_ptr&)>; + const NeuronIDE::Component&, const std::shared_ptr&, + const ComponentBuildContext&)>; class ComponentRegistry { public: @@ -31,7 +33,8 @@ class ComponentRegistry { void registerCreator(int typeId, ComponentCreatorFunc creator); std::unique_ptr build(const NeuronIDE::Component& protoComp, - const std::shared_ptr& owner); + const std::shared_ptr& owner, + const ComponentBuildContext& context); private: ComponentRegistry() = default; diff --git a/include/scene/components/MarkerEmitterComponent.hpp b/include/scene/components/MarkerEmitterComponent.hpp index 95c11ea..88707d8 100644 --- a/include/scene/components/MarkerEmitterComponent.hpp +++ b/include/scene/components/MarkerEmitterComponent.hpp @@ -10,6 +10,7 @@ #include "scene/components/Component.hpp" class Scene; +struct ComponentBuildContext; namespace NeuronIDE { class Component; @@ -36,8 +37,9 @@ class MarkerEmitterComponent : public Component { void render(SDL_Renderer* renderer) override; void onSceneReady(Scene& scene) override; - static std::unique_ptr createMarkerEmitter( - const NeuronIDE::Component& protoComp, const std::shared_ptr& owner); + static std::unique_ptr createMarkerEmitter(const NeuronIDE::Component& protoComp, + const std::shared_ptr& owner, + const ComponentBuildContext& context); private: struct ActiveSubscription { diff --git a/include/scene/components/SpriteRenderer.hpp b/include/scene/components/SpriteRenderer.hpp new file mode 100644 index 0000000..2accbab --- /dev/null +++ b/include/scene/components/SpriteRenderer.hpp @@ -0,0 +1,55 @@ +#ifndef SPRITERENDERER_HPP +#define SPRITERENDERER_HPP + +#include +#include + +#include "scene/components/Component.hpp" + +struct ComponentBuildContext; +struct SDL_Surface; +struct SDL_Texture; + +namespace NeuronIDE { +class Component; +} + +// Draws an image file into its owner's Transform: x/y is the top-left corner in window pixels, +// width/height the drawn size, and rotation is in degrees clockwise about the rectangle's centre. +// +// The image is decoded on construction, so a missing or unreadable file fails while the scene is +// parsed, before any window opens. It becomes a texture in onRenderTargetReady(), before the +// first frame, and the decoded pixels are released then. +class SpriteRenderer : public Component { + public: + // Throws std::invalid_argument for a null owner, an empty path, or an owner without a + // positive width and height; std::runtime_error when the image cannot be loaded. + SpriteRenderer(const std::shared_ptr& owner, std::filesystem::path path); + + void update(const Context& context) override; + void render(SDL_Renderer* renderer) override; + + // Uploads the image to `target`. A sprite belongs to one render target: a second call throws + // std::logic_error. + void onRenderTargetReady(const std::shared_ptr& target) override; + + static std::unique_ptr createSprite(const NeuronIDE::Component& protoComp, + const std::shared_ptr& owner, + const ComponentBuildContext& context); + + private: + struct SurfaceDeleter { + void operator()(SDL_Surface* surface) const noexcept; + }; + struct TextureDeleter { + // The renderer the texture belongs to, kept alive until the texture is destroyed. + std::shared_ptr target; + void operator()(SDL_Texture* texture) const noexcept; + }; + + std::filesystem::path imagePath; + std::unique_ptr image; + std::unique_ptr texture; +}; + +#endif // SPRITERENDERER_HPP diff --git a/protoFiles/neuronide.proto b/protoFiles/neuronide.proto index 0623fb6..10d580c 100644 --- a/protoFiles/neuronide.proto +++ b/protoFiles/neuronide.proto @@ -3,7 +3,9 @@ syntax = "proto3"; package NeuronIDE; message SpriteRenderer { - string texture_path = 1; + reserved 1; + reserved "texture_path"; + // Image file. A relative path resolves against the folder holding the .neuroz file. string img_path = 2; } diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index c134744..8ff558c 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -12,11 +12,11 @@ add_subdirectory(renderer) add_subdirectory(lslreader) add_subdirectory(datawriter) -add_library(runtime_core STATIC +add_library(runtime_core_archive STATIC Runtime.cpp ) -target_include_directories(runtime_core PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/../include) -target_link_libraries(runtime_core PUBLIC +target_include_directories(runtime_core_archive PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/../include) +target_link_libraries(runtime_core_archive PUBLIC scene parser config @@ -25,5 +25,11 @@ target_link_libraries(runtime_core PUBLIC datawriter ) +# Components register themselves from static initializers (REGISTER_COMPONENT) in object files +# nothing else references. A plain static link drops those objects, and the parser then skips +# every component of that type without a word. Link `runtime_core`, never the archive itself. +add_library(runtime_core INTERFACE) +target_link_libraries(runtime_core INTERFACE "$") + add_executable(NeuronIDE main.cpp) target_link_libraries(NeuronIDE PRIVATE runtime_core) \ No newline at end of file diff --git a/src/parser/Parser.cpp b/src/parser/Parser.cpp index 43413f6..acbeaf5 100644 --- a/src/parser/Parser.cpp +++ b/src/parser/Parser.cpp @@ -1,5 +1,6 @@ #include "parser/Parser.hpp" +#include #include #include #include @@ -16,15 +17,19 @@ std::shared_ptr Parser::parse(const std::string& filePath) { throw std::runtime_error("Parser: cannot open file: " + filePath); } + const ComponentBuildContext context{.assetRoot = + std::filesystem::absolute(filePath).parent_path()}; + try { - return parseStream(file); + return parseStream(file, context); } catch (const std::runtime_error& e) { throw std::runtime_error(std::string("Parser: failed to parse file ") + filePath + " - " + e.what()); } } -std::shared_ptr Parser::parseStream(std::istream& stream) { +std::shared_ptr Parser::parseStream(std::istream& stream, + const ComponentBuildContext& context) { NeuronIDE::Scene protoScene; if (!protoScene.ParseFromIstream(&stream)) { @@ -39,7 +44,7 @@ std::shared_ptr Parser::parseStream(std::istream& stream) { scene->setExperimentName(protoScene.project_name()); for (const auto& protoObj : protoScene.scene_objects()) { - auto obj = buildSceneObject(protoObj); + auto obj = buildSceneObject(protoObj, context); scene->addObject(std::move(obj)); } @@ -48,7 +53,8 @@ std::shared_ptr Parser::parseStream(std::istream& stream) { return scene; } -std::shared_ptr Parser::buildSceneObject(const NeuronIDE::SceneObject& protoObj) { +std::shared_ptr Parser::buildSceneObject(const NeuronIDE::SceneObject& protoObj, + const ComponentBuildContext& context) { if (protoObj.name().empty()) { throw std::invalid_argument("SceneObject name cannot be empty"); } @@ -71,7 +77,7 @@ std::shared_ptr Parser::buildSceneObject(const NeuronIDE::SceneObje } seenComponentTypes.insert(typeId); - auto comp = buildComponent(protoComp, obj); + auto comp = buildComponent(protoComp, obj, context); if (comp) { obj->addComponent(std::move(comp)); } @@ -81,6 +87,7 @@ std::shared_ptr Parser::buildSceneObject(const NeuronIDE::SceneObje } std::unique_ptr Parser::buildComponent(const NeuronIDE::Component& protoComp, - const std::shared_ptr& owner) { - return ComponentRegistry::instance().build(protoComp, owner); + const std::shared_ptr& owner, + const ComponentBuildContext& context) { + return ComponentRegistry::instance().build(protoComp, owner, context); } \ No newline at end of file diff --git a/src/renderer/Renderer.cpp b/src/renderer/Renderer.cpp index 142e969..ed1dc4b 100644 --- a/src/renderer/Renderer.cpp +++ b/src/renderer/Renderer.cpp @@ -16,7 +16,13 @@ Renderer::Renderer(const std::shared_ptr& scene, std::shared_ptrsdlRenderer) { + scene->onRenderTargetReady(this->sdlRenderer); + } +} void Renderer::render(const std::stop_token& stoken) { std::vector currentFrameMarkers; diff --git a/src/scene/CMakeLists.txt b/src/scene/CMakeLists.txt index 212cb07..2fa86e1 100644 --- a/src/scene/CMakeLists.txt +++ b/src/scene/CMakeLists.txt @@ -4,6 +4,14 @@ add_library(scene OBJECT components/ComponentRegistry.cpp components/BlinkComponent.cpp components/MarkerEmitterComponent.cpp + components/SpriteRenderer.cpp ) -target_include_directories(scene PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/../../include) -target_link_libraries(scene PUBLIC neuronide_proto) \ No newline at end of file +target_include_directories(scene PUBLIC + ${CMAKE_CURRENT_SOURCE_DIR}/../../include + ${SDL2_INCLUDE_DIRS} +) +target_link_libraries(scene PUBLIC + neuronide_proto + ${SDL2_LIBRARIES} + SDL2_image::SDL2_image +) \ No newline at end of file diff --git a/src/scene/Scene.cpp b/src/scene/Scene.cpp index b6a67e6..ff0fc19 100644 --- a/src/scene/Scene.cpp +++ b/src/scene/Scene.cpp @@ -28,6 +28,14 @@ void Scene::onSceneReady() { } } +void Scene::onRenderTargetReady(const std::shared_ptr& target) { + for (const auto& obj : objects) { + for (const auto& comp : obj->components) { + comp->onRenderTargetReady(target); + } + } +} + Delegate& Scene::resolveEvents(std::string_view objectName, std::string_view eventName) { for (const auto& obj : objects) { if (obj->name != objectName) { diff --git a/src/scene/components/BlinkComponent.cpp b/src/scene/components/BlinkComponent.cpp index 7911f0f..dd7da80 100644 --- a/src/scene/components/BlinkComponent.cpp +++ b/src/scene/components/BlinkComponent.cpp @@ -5,8 +5,9 @@ void BlinkComponent::setFrequency(double freq) { blinkFrequencyHz = freq; } -std::unique_ptr BlinkComponent::createBlinker( - const NeuronIDE::Component& protoComp, const std::shared_ptr& owner) { +std::unique_ptr BlinkComponent::createBlinker(const NeuronIDE::Component& protoComp, + const std::shared_ptr& owner, + const ComponentBuildContext& /*context*/) { return std::make_unique(owner, protoComp.blinker().blink_frequency_hz()); } diff --git a/src/scene/components/ComponentRegistry.cpp b/src/scene/components/ComponentRegistry.cpp index f8c67c8..77b02ee 100644 --- a/src/scene/components/ComponentRegistry.cpp +++ b/src/scene/components/ComponentRegistry.cpp @@ -14,14 +14,15 @@ void ComponentRegistry::registerCreator(int typeId, ComponentCreatorFunc creator } std::unique_ptr ComponentRegistry::build(const NeuronIDE::Component& protoComp, - const std::shared_ptr& owner) { + const std::shared_ptr& owner, + const ComponentBuildContext& context) { auto activeCase = protoComp.component_type_case(); int typeId = static_cast(activeCase); auto iter = creators.find(typeId); if (iter != creators.end()) { - return iter->second(protoComp, owner); + return iter->second(protoComp, owner, context); } return nullptr; diff --git a/src/scene/components/MarkerEmitterComponent.cpp b/src/scene/components/MarkerEmitterComponent.cpp index 9066439..2bf7776 100644 --- a/src/scene/components/MarkerEmitterComponent.cpp +++ b/src/scene/components/MarkerEmitterComponent.cpp @@ -25,7 +25,8 @@ void MarkerEmitterComponent::onSceneReady(Scene& scene) { } std::unique_ptr MarkerEmitterComponent::createMarkerEmitter( - const NeuronIDE::Component& protoComp, const std::shared_ptr& owner) { + const NeuronIDE::Component& protoComp, const std::shared_ptr& owner, + const ComponentBuildContext& /*context*/) { std::vector bindings; for (const auto& protoBinding : protoComp.marker_emitter().bindings()) { bindings.push_back({protoBinding.target_object(), protoBinding.target_event(), diff --git a/src/scene/components/SpriteRenderer.cpp b/src/scene/components/SpriteRenderer.cpp new file mode 100644 index 0000000..bde58c7 --- /dev/null +++ b/src/scene/components/SpriteRenderer.cpp @@ -0,0 +1,90 @@ +#include "scene/components/SpriteRenderer.hpp" + +#include +#include + +#include +#include +#include + +#include "neuronide.pb.h" +#include "scene/SceneObject.hpp" +#include "scene/components/ComponentBuildContext.hpp" +#include "scene/components/ComponentRegistry.hpp" + +void SpriteRenderer::SurfaceDeleter::operator()(SDL_Surface* surface) const noexcept { + SDL_FreeSurface(surface); +} + +void SpriteRenderer::TextureDeleter::operator()(SDL_Texture* texture) const noexcept { + SDL_DestroyTexture(texture); +} + +SpriteRenderer::SpriteRenderer(const std::shared_ptr& owner, + std::filesystem::path path) + : Component(owner), imagePath(std::move(path)) { + if (!owner) { + throw std::invalid_argument("SpriteRenderer: owner must not be null"); + } + const std::string where = "SpriteRenderer on '" + owner->name + "'"; + + if (imagePath.empty()) { + throw std::invalid_argument(where + ": image path is empty"); + } + // Written so that a NaN size is rejected too. + const bool hasArea = owner->transform.width > 0.0 && owner->transform.height > 0.0; + if (!hasArea) { + throw std::invalid_argument(where + ": the object needs a positive width and height"); + } + + image.reset(IMG_Load(imagePath.string().c_str())); + if (!image) { + throw std::runtime_error(where + ": cannot load image '" + imagePath.string() + + "': " + IMG_GetError()); + } +} + +void SpriteRenderer::update(const Context& context) { (void)context; } + +void SpriteRenderer::render(SDL_Renderer* renderer) { + const auto ownerPtr = owner.lock(); + if (renderer == nullptr || !texture || !ownerPtr) { + return; + } + + const auto& transform = ownerPtr->transform; + const SDL_FRect destination{ + static_cast(transform.posX), static_cast(transform.posY), + static_cast(transform.width), static_cast(transform.height)}; + + // A null centre rotates about the centre of `destination`. + SDL_RenderCopyExF(renderer, texture.get(), nullptr, &destination, transform.rotation, nullptr, + SDL_FLIP_NONE); +} + +void SpriteRenderer::onRenderTargetReady(const std::shared_ptr& target) { + if (!target) { + throw std::invalid_argument("SpriteRenderer: render target must not be null"); + } + if (!image) { + throw std::logic_error("SpriteRenderer: '" + imagePath.string() + + "' is already uploaded to a render target"); + } + + SDL_Texture* created = SDL_CreateTextureFromSurface(target.get(), image.get()); + if (created == nullptr) { + throw std::runtime_error("SpriteRenderer: cannot create a texture from '" + + imagePath.string() + "': " + SDL_GetError()); + } + texture = std::unique_ptr(created, TextureDeleter{target}); + image.reset(); +} + +std::unique_ptr SpriteRenderer::createSprite(const NeuronIDE::Component& protoComp, + const std::shared_ptr& owner, + const ComponentBuildContext& context) { + return std::make_unique(owner, + context.resolveAsset(protoComp.renderer().img_path())); +} + +REGISTER_COMPONENT(NeuronIDE::Component::kRenderer, SpriteRenderer::createSprite) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 840c37d..b666412 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -1,4 +1,5 @@ include(GoogleTest) add_subdirectory(unit_tests) -add_subdirectory(component_tests) \ No newline at end of file +add_subdirectory(component_tests) +add_subdirectory(registration_tests) \ No newline at end of file diff --git a/tests/component_tests/sprite_scene_test.cpp b/tests/component_tests/sprite_scene_test.cpp new file mode 100644 index 0000000..dbf7682 --- /dev/null +++ b/tests/component_tests/sprite_scene_test.cpp @@ -0,0 +1,109 @@ +#include + +#include +#include +#include +#include + +#include "neuronide.pb.h" +#include "parser/Parser.hpp" +#include "scene/Scene.hpp" +#include "scene/SceneObject.hpp" +#include "scene/components/SpriteRenderer.hpp" +#include "utils/SdlTestUtils.hpp" +#include "utils/TempDir.hpp" + +// Sprites in a .neuroz parsed from disk, the way Runtime loads an experiment. + +namespace { +namespace fs = std::filesystem; + +constexpr double kSpriteSize = 10.0; +constexpr utils::Rgba kLeft{255, 0, 0, 255}; +constexpr utils::Rgba kRight{0, 0, 255, 255}; + +NeuronIDE::Scene sceneWithSprite(const std::string& imagePath, bool withTransform = true) { + NeuronIDE::Scene scene; + scene.set_project_name("Sprites"); + + auto* object = scene.add_scene_objects(); + object->set_name("Stimulus"); + object->set_is_visible(true); + if (withTransform) { + object->mutable_transform()->set_width(kSpriteSize); + object->mutable_transform()->set_height(kSpriteSize); + } + object->add_components()->mutable_renderer()->set_img_path(imagePath); + return scene; +} + +class SpriteSceneTest : public ::testing::Test { + protected: + const fs::path& dir() const { return tempDir.path(); } + fs::path experimentPath() const { return dir() / "experiment.neuroz"; } + + void writeExperiment(const NeuronIDE::Scene& scene) const { + std::ofstream out(experimentPath(), std::ios::binary | std::ios::trunc); + ASSERT_TRUE(out.is_open()); + ASSERT_TRUE(scene.SerializeToOstream(&out)); + } + + std::shared_ptr parseExperiment() const { + return Parser::parse(experimentPath().string()); + } + + private: + utils::TempDir tempDir; +}; + +bool hasSprite(const Scene& scene) { + const auto& components = scene.getObjects().at(0)->components; + return components.size() == 1 && + dynamic_cast(components.front().get()) != nullptr; +} +} // namespace + +TEST_F(SpriteSceneTest, ResolvesRelativeImagePathAgainstTheExperimentFolder) { + // Otherwise the path would resolve even if it were taken relative to the working directory. + ASSERT_NE(fs::current_path(), dir()); + + fs::create_directories(dir() / "stimuli"); + utils::writeTwoPixelPng(dir() / "stimuli" / "face.png", kLeft, kRight); + writeExperiment(sceneWithSprite("stimuli/face.png")); + + EXPECT_TRUE(hasSprite(*parseExperiment())); +} + +TEST_F(SpriteSceneTest, UsesAbsoluteImagePathAsIs) { + const utils::TempDir elsewhere; + const fs::path image = elsewhere.path() / "face.png"; + utils::writeTwoPixelPng(image, kLeft, kRight); + writeExperiment(sceneWithSprite(image.string())); + + EXPECT_TRUE(hasSprite(*parseExperiment())); +} + +TEST_F(SpriteSceneTest, MissingImageFailsTheParseAndNamesTheFile) { + writeExperiment(sceneWithSprite("stimuli/missing.png")); + const std::string expectedPath = (dir() / "stimuli" / "missing.png").string(); + + try { + parseExperiment(); + FAIL() << "expected the parse to fail"; + } catch (const std::runtime_error& error) { + EXPECT_NE(std::string(error.what()).find(expectedPath), std::string::npos) << error.what(); + } +} + +TEST_F(SpriteSceneTest, EmptyImagePathFailsTheParse) { + writeExperiment(sceneWithSprite("")); + + EXPECT_THROW(parseExperiment(), std::invalid_argument); +} + +TEST_F(SpriteSceneTest, SpriteOnObjectWithoutTransformFailsTheParse) { + utils::writeTwoPixelPng(dir() / "face.png", kLeft, kRight); + writeExperiment(sceneWithSprite("face.png", /*withTransform=*/false)); + + EXPECT_THROW(parseExperiment(), std::invalid_argument); +} diff --git a/tests/registration_tests/CMakeLists.txt b/tests/registration_tests/CMakeLists.txt new file mode 100644 index 0000000..966c2f4 --- /dev/null +++ b/tests/registration_tests/CMakeLists.txt @@ -0,0 +1,7 @@ +# A separate executable on purpose: it references no component class, so it sees exactly the +# components that linking runtime_core brings in through REGISTER_COMPONENT, as NeuronIDE does. +add_executable(registration_tests registration_test.cpp) +target_link_libraries(registration_tests PRIVATE gtest_main runtime_core) +target_include_directories(registration_tests PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/..) + +gtest_discover_tests(registration_tests PROPERTIES LABELS "component") diff --git a/tests/registration_tests/registration_test.cpp b/tests/registration_tests/registration_test.cpp new file mode 100644 index 0000000..dccc05d --- /dev/null +++ b/tests/registration_tests/registration_test.cpp @@ -0,0 +1,52 @@ +#include + +#include +#include + +#include "neuronide.pb.h" +#include "parser/Parser.hpp" +#include "scene/Scene.hpp" +#include "scene/SceneObject.hpp" +#include "utils/SdlTestUtils.hpp" +#include "utils/TempDir.hpp" + +// Do not include or name any component class in this file. A component found here was linked in +// only through its REGISTER_COMPONENT static initializer, exactly as in the NeuronIDE executable. + +namespace { +constexpr double kBlinkFrequencyHz = 10.0; +constexpr utils::Rgba kWhite{255, 255, 255, 255}; + +NeuronIDE::Component* addObjectWithComponent(NeuronIDE::Scene& scene, const std::string& name) { + auto* object = scene.add_scene_objects(); + object->set_name(name); + object->set_is_visible(true); + object->mutable_transform()->set_width(1.0); + object->mutable_transform()->set_height(1.0); + return object->add_components(); +} +} // namespace + +// One object per implemented component type. Add yours here when you register a new one. +TEST(ComponentRegistrationTest, EveryImplementedComponentIsLinkedIn) { + const utils::TempDir assets; + utils::writeTwoPixelPng(assets.path() / "sprite.png", kWhite, kWhite); + + NeuronIDE::Scene proto; + proto.set_project_name("Registration"); + addObjectWithComponent(proto, "Blinker") + ->mutable_blinker() + ->set_blink_frequency_hz(kBlinkFrequencyHz); + addObjectWithComponent(proto, "MarkerEmitter")->mutable_marker_emitter(); + addObjectWithComponent(proto, "Sprite")->mutable_renderer()->set_img_path("sprite.png"); + + std::stringstream stream; + ASSERT_TRUE(proto.SerializeToOstream(&stream)); + const auto scene = + Parser::parseStream(stream, ComponentBuildContext{.assetRoot = assets.path()}); + + for (const auto& object : scene->getObjects()) { + EXPECT_EQ(object->components.size(), 1U) + << object->name << ": its component type is not registered in this binary"; + } +} diff --git a/tests/unit_tests/RendererTest.cpp b/tests/unit_tests/RendererTest.cpp index 6f552c3..ebdbac0 100644 --- a/tests/unit_tests/RendererTest.cpp +++ b/tests/unit_tests/RendererTest.cpp @@ -68,6 +68,22 @@ class MarkerComponent : public Component { std::shared_ptr stopSource; }; +class RenderTargetSpy : public Component { + public: + RenderTargetSpy(const std::shared_ptr& owner, + std::weak_ptr* received) + : Component(owner), received(received) {} + + void onRenderTargetReady(const std::shared_ptr& target) override { + *received = target; + } + void update(const Context& context) override { (void)context; } + void render(SDL_Renderer* renderer) override { (void)renderer; } + + private: + std::weak_ptr* received; +}; + class RendererTest : public ::testing::Test { protected: void SetUp() override { ASSERT_EQ(SDL_Init(SDL_INIT_EVENTS), 0); } @@ -110,6 +126,20 @@ TEST_F(RendererTest, RenderLoop_WhenComponentAdded_CallsUpdateExactlyOnceBeforeS EXPECT_EQ(*renders, 1); } +TEST_F(RendererTest, Constructor_HandsRenderTargetToComponentsBeforeFirstFrame) { + auto scene = std::make_shared(); + auto obj = std::make_shared("obj"); + std::weak_ptr received; + + obj->addComponent(std::make_unique(obj, &received)); + scene->addObject(obj); + + const auto target = makeSoftwareRenderer(); + const Renderer renderer(scene, target, std::make_shared>()); + + EXPECT_EQ(received.lock(), target); +} + TEST_F(RendererTest, RenderLoop_QueuesMarkersFromComponents) { auto scene = std::make_shared(); auto obj = std::make_shared("obj"); diff --git a/tests/unit_tests/component_build_context_test.cpp b/tests/unit_tests/component_build_context_test.cpp new file mode 100644 index 0000000..81eec90 --- /dev/null +++ b/tests/unit_tests/component_build_context_test.cpp @@ -0,0 +1,37 @@ +#include + +#include + +#include "scene/components/ComponentBuildContext.hpp" + +namespace { +namespace fs = std::filesystem; + +// temp_directory_path() is absolute on every platform, unlike a literal such as "/data". +fs::path experimentFolder() { return fs::temp_directory_path() / "experiment"; } +} // namespace + +TEST(ComponentBuildContextTest, ResolvesRelativePathAgainstAssetRoot) { + const ComponentBuildContext context{.assetRoot = experimentFolder()}; + + EXPECT_EQ(context.resolveAsset("stimuli/face.png"), experimentFolder() / "stimuli/face.png"); +} + +TEST(ComponentBuildContextTest, KeepsAbsolutePathAsIs) { + const ComponentBuildContext context{.assetRoot = experimentFolder()}; + const fs::path absolute = fs::temp_directory_path() / "elsewhere" / "face.png"; + + EXPECT_EQ(context.resolveAsset(absolute), absolute); +} + +TEST(ComponentBuildContextTest, KeepsEmptyPathEmpty) { + const ComponentBuildContext context{.assetRoot = experimentFolder()}; + + EXPECT_TRUE(context.resolveAsset(fs::path{}).empty()); +} + +TEST(ComponentBuildContextTest, WithoutAssetRootLeavesRelativePathForWorkingDirectory) { + const ComponentBuildContext context; + + EXPECT_EQ(context.resolveAsset("face.png"), fs::path("face.png")); +} diff --git a/tests/unit_tests/sprite_renderer_test.cpp b/tests/unit_tests/sprite_renderer_test.cpp new file mode 100644 index 0000000..4956352 --- /dev/null +++ b/tests/unit_tests/sprite_renderer_test.cpp @@ -0,0 +1,148 @@ +#include + +#include +#include +#include +#include + +#include "scene/SceneObject.hpp" +#include "scene/components/SpriteRenderer.hpp" +#include "utils/SdlTestUtils.hpp" +#include "utils/TempDir.hpp" + +namespace { +constexpr int kCanvasSize = 40; +constexpr double kSpriteX = 10.0; +constexpr double kSpriteY = 20.0; +constexpr double kSpriteWidth = 20.0; +constexpr double kSpriteHeight = 4.0; +constexpr double kQuarterTurn = 90.0; + +constexpr utils::Rgba kBackground{0, 0, 0, 255}; +constexpr utils::Rgba kLeft{255, 0, 0, 255}; +constexpr utils::Rgba kRight{0, 0, 255, 255}; + +// Probe points sit well inside one half of the sprite, away from edges where rounding could go +// either way. Unturned, the sprite covers x 10..30 and y 20..24, its left half x 10..20. +constexpr int kLeftHalfX = 12; +constexpr int kRightHalfX = 27; +constexpr int kRowY = 21; +// Turned a quarter clockwise about its centre (20, 22), it covers x 18..22 and y 12..32, with +// what was its left half now on top. +constexpr int kColumnX = 20; +constexpr int kTopY = 14; +constexpr int kBottomY = 29; +constexpr int kOutside = 5; + +class SpriteRendererTest : public ::testing::Test { + protected: + void SetUp() override { + image = tempDir.path() / "sprite.png"; + utils::writeTwoPixelPng(image, kLeft, kRight); + + object = std::make_shared("Stimulus"); + object->setTransform({kSpriteX, kSpriteY, kSpriteWidth, kSpriteHeight, 0.0}); + + renderTarget = utils::makeSoftwareTarget(kCanvasSize, kCanvasSize); + utils::clear(renderTarget.get(), kBackground); + } + + const std::filesystem::path& dir() const { return tempDir.path(); } + const std::filesystem::path& imagePath() const { return image; } + const std::shared_ptr& owner() const { return object; } + const std::shared_ptr& target() const { return renderTarget; } + void releaseTarget() { renderTarget.reset(); } + + utils::Rgba pixelAt(int column, int row) const { + return utils::readPixel(renderTarget.get(), column, row); + } + + private: + utils::TempDir tempDir; + std::filesystem::path image; + std::shared_ptr object; + std::shared_ptr renderTarget; +}; +} // namespace + +TEST_F(SpriteRendererTest, StretchesImageOverTransform) { + SpriteRenderer sprite(owner(), imagePath()); + sprite.onRenderTargetReady(target()); + sprite.render(target().get()); + + EXPECT_EQ(pixelAt(kLeftHalfX, kRowY), kLeft); + EXPECT_EQ(pixelAt(kRightHalfX, kRowY), kRight); + EXPECT_EQ(pixelAt(kColumnX, kTopY), kBackground); + EXPECT_EQ(pixelAt(kOutside, kOutside), kBackground); +} + +TEST_F(SpriteRendererTest, RotatesClockwiseAboutCentre) { + owner()->transform.rotation = kQuarterTurn; + + SpriteRenderer sprite(owner(), imagePath()); + sprite.onRenderTargetReady(target()); + sprite.render(target().get()); + + EXPECT_EQ(pixelAt(kColumnX, kTopY), kLeft); + EXPECT_EQ(pixelAt(kColumnX, kBottomY), kRight); + EXPECT_EQ(pixelAt(kLeftHalfX, kRowY), kBackground); +} + +TEST_F(SpriteRendererTest, DrawsNothingBeforeRenderTargetIsReady) { + SpriteRenderer sprite(owner(), imagePath()); + sprite.render(target().get()); + + EXPECT_EQ(pixelAt(kLeftHalfX, kRowY), kBackground); +} + +TEST_F(SpriteRendererTest, ThrowsWhenImageIsMissing) { + EXPECT_THROW(SpriteRenderer(owner(), dir() / "missing.png"), std::runtime_error); +} + +TEST_F(SpriteRendererTest, ThrowsWhenFileIsNotAnImage) { + const auto notAnImage = dir() / "notes.png"; + std::ofstream(notAnImage) << "not an image"; + + EXPECT_THROW(SpriteRenderer(owner(), notAnImage), std::runtime_error); +} + +TEST_F(SpriteRendererTest, ThrowsWhenImagePathIsEmpty) { + EXPECT_THROW(SpriteRenderer(owner(), std::filesystem::path{}), std::invalid_argument); +} + +TEST_F(SpriteRendererTest, ThrowsWhenOwnerIsNull) { + EXPECT_THROW(SpriteRenderer(nullptr, imagePath()), std::invalid_argument); +} + +TEST_F(SpriteRendererTest, ThrowsWhenRenderTargetIsNull) { + SpriteRenderer sprite(owner(), imagePath()); + + EXPECT_THROW(sprite.onRenderTargetReady(nullptr), std::invalid_argument); +} + +TEST_F(SpriteRendererTest, ThrowsWhenOwnerHasNoArea) { + owner()->transform.width = 0.0; + + EXPECT_THROW(SpriteRenderer(owner(), imagePath()), std::invalid_argument); +} + +TEST_F(SpriteRendererTest, RejectsASecondRenderTarget) { + SpriteRenderer sprite(owner(), imagePath()); + sprite.onRenderTargetReady(target()); + + EXPECT_THROW(sprite.onRenderTargetReady(target()), std::logic_error); +} + +// SDL frees a renderer's textures along with it, so destroying the texture afterwards would be a +// use-after-free. The texture therefore keeps its renderer alive. +TEST_F(SpriteRendererTest, KeepsRenderTargetAliveWhileTextureExists) { + auto sprite = std::make_unique(owner(), imagePath()); + sprite->onRenderTargetReady(target()); + + const std::weak_ptr weakTarget = target(); + releaseTarget(); + EXPECT_FALSE(weakTarget.expired()); + + sprite.reset(); + EXPECT_TRUE(weakTarget.expired()); +} diff --git a/tests/utils/SdlTestUtils.hpp b/tests/utils/SdlTestUtils.hpp new file mode 100644 index 0000000..d792937 --- /dev/null +++ b/tests/utils/SdlTestUtils.hpp @@ -0,0 +1,85 @@ +#ifndef SDL_TEST_UTILS_HPP +#define SDL_TEST_UTILS_HPP + +#include +#include + +#include +#include +#include +#include +#include + +namespace utils { + +// One pixel as {r, g, b, a}; ints rather than Uint8 so gtest prints them as numbers. +using Rgba = std::array; + +inline constexpr int kSurfaceDepth = 32; + +inline std::runtime_error sdlFailure(const std::string& what) { + return std::runtime_error(what + ": " + SDL_GetError()); +} + +// A software render target that draws into a CPU surface, so tests can render without a window +// and read the pixels back. +inline std::shared_ptr makeSoftwareTarget(int width, int height) { + std::shared_ptr surface( + SDL_CreateRGBSurfaceWithFormat(0, width, height, kSurfaceDepth, SDL_PIXELFORMAT_RGBA32), + &SDL_FreeSurface); + if (!surface) { + throw sdlFailure("SDL_CreateRGBSurfaceWithFormat"); + } + SDL_Renderer* renderer = SDL_CreateSoftwareRenderer(surface.get()); + if (renderer == nullptr) { + throw sdlFailure("SDL_CreateSoftwareRenderer"); + } + return {renderer, [surface](SDL_Renderer* target) { SDL_DestroyRenderer(target); }}; +} + +inline void clear(SDL_Renderer* renderer, const Rgba& color) { + SDL_SetRenderDrawColor(renderer, static_cast(color[0]), static_cast(color[1]), + static_cast(color[2]), static_cast(color[3])); + SDL_RenderClear(renderer); +} + +inline Rgba readPixel(SDL_Renderer* renderer, int column, int row) { + const SDL_Rect pixel{column, row, 1, 1}; + std::array bytes{}; + // RGBA32 is r, g, b, a in memory order on any endianness. + if (SDL_RenderReadPixels(renderer, &pixel, SDL_PIXELFORMAT_RGBA32, bytes.data(), + static_cast(bytes.size())) != 0) { + throw sdlFailure("SDL_RenderReadPixels"); + } + return {bytes[0], bytes[1], bytes[2], bytes[3]}; +} + +// Writes a 2x1 PNG whose left pixel is `left` and right pixel is `right`. Stretched over a +// sprite, the two halves show how the image was placed and which way it was turned. +inline void writeTwoPixelPng(const std::filesystem::path& path, const Rgba& left, + const Rgba& right) { + const std::unique_ptr surface( + SDL_CreateRGBSurfaceWithFormat(0, 2, 1, kSurfaceDepth, SDL_PIXELFORMAT_RGBA32), + &SDL_FreeSurface); + if (!surface) { + throw sdlFailure("SDL_CreateRGBSurfaceWithFormat"); + } + + const auto fill = [&surface](int column, const Rgba& color) { + const SDL_Rect pixel{column, 0, 1, 1}; + SDL_FillRect( + surface.get(), &pixel, + SDL_MapRGBA(surface->format, static_cast(color[0]), static_cast(color[1]), + static_cast(color[2]), static_cast(color[3]))); + }; + fill(0, left); + fill(1, right); + + if (IMG_SavePNG(surface.get(), path.string().c_str()) != 0) { + throw sdlFailure("IMG_SavePNG " + path.string()); + } +} + +} // namespace utils + +#endif // SDL_TEST_UTILS_HPP diff --git a/tests/utils/TempDir.hpp b/tests/utils/TempDir.hpp new file mode 100644 index 0000000..e9317dd --- /dev/null +++ b/tests/utils/TempDir.hpp @@ -0,0 +1,45 @@ +#ifndef TEMP_DIR_HPP +#define TEMP_DIR_HPP + +#include + +#include +#include +#include +#include + +namespace utils { + +// A fresh directory under the system temp folder, removed with everything in it on destruction. +// The name includes the running test, so tests in parallel processes never share one. +class TempDir { + public: + TempDir() { + const auto* info = ::testing::UnitTest::GetInstance()->current_test_info(); + const std::string testName = + info != nullptr ? std::string(info->test_suite_name()) + "_" + info->name() : "none"; + root = std::filesystem::temp_directory_path() / + ("neuronide_" + testName + "_" + + std::to_string(std::chrono::steady_clock::now().time_since_epoch().count())); + std::filesystem::create_directories(root); + } + + ~TempDir() { + std::error_code errorCode; + std::filesystem::remove_all(root, errorCode); + } + + TempDir(const TempDir&) = delete; + TempDir& operator=(const TempDir&) = delete; + TempDir(TempDir&&) = delete; + TempDir& operator=(TempDir&&) = delete; + + [[nodiscard]] const std::filesystem::path& path() const { return root; } + + private: + std::filesystem::path root; +}; + +} // namespace utils + +#endif // TEMP_DIR_HPP From c9ea060e6d9a112704f7c0fd0ad8c53f7130221b Mon Sep 17 00:00:00 2001 From: Michal Date: Mon, 19 Oct 2026 18:23:33 +0200 Subject: [PATCH 2/4] feat: share sprite images per scene and harden their upload Images are now decoded once per scene into an ImageLibrary owned by Scene, however many sprites show the same file. The library uploads every image when the render target is ready and keeps the decoded pixels, so textures can be rebuilt after the GPU loses them. --- CLAUDE.md | 4 +- README.md | 26 ++-- include/Runtime.hpp | 19 +-- include/parser/Parser.hpp | 8 +- include/renderer/Renderer.hpp | 6 + include/scene/Image.hpp | 45 +++++++ include/scene/ImageLibrary.hpp | 30 +++++ include/scene/Scene.hpp | 8 +- include/scene/components/BlinkComponent.hpp | 2 +- include/scene/components/Component.hpp | 7 +- .../components/ComponentBuildContext.hpp | 31 ++++- .../scene/components/ComponentRegistry.hpp | 2 +- .../components/MarkerEmitterComponent.hpp | 2 +- include/scene/components/SpriteRenderer.hpp | 40 ++----- protoFiles/neuronide.proto | 4 +- src/Runtime.cpp | 31 ++--- src/parser/Parser.cpp | 9 +- src/renderer/Renderer.cpp | 29 +++-- src/scene/CMakeLists.txt | 2 + src/scene/Image.cpp | 37 ++++++ src/scene/ImageLibrary.cpp | 20 ++++ src/scene/Scene.cpp | 1 + src/scene/components/SpriteRenderer.cpp | 74 +++++------- tests/component_tests/sprite_scene_test.cpp | 33 +++-- .../registration_tests/registration_test.cpp | 3 +- tests/unit_tests/RendererTest.cpp | 91 ++++++++++---- .../component_build_context_test.cpp | 25 ++-- tests/unit_tests/image_library_test.cpp | 113 ++++++++++++++++++ tests/unit_tests/runtime_test.cpp | 65 +++++----- tests/unit_tests/sprite_renderer_test.cpp | 99 +++++++-------- tests/unit_tests/temp_dir_test.cpp | 22 ++++ tests/utils/TempDir.hpp | 14 ++- 32 files changed, 638 insertions(+), 264 deletions(-) create mode 100644 include/scene/Image.hpp create mode 100644 include/scene/ImageLibrary.hpp create mode 100644 src/scene/Image.cpp create mode 100644 src/scene/ImageLibrary.cpp create mode 100644 tests/unit_tests/image_library_test.cpp create mode 100644 tests/unit_tests/temp_dir_test.cpp diff --git a/CLAUDE.md b/CLAUDE.md index dde7d9c..13bd724 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -98,9 +98,7 @@ launcher#1. ### Known cross-repo gaps (as of 2026-10-09 — fix them or update this list) -- Proto copies have diverged: backend lacks `MarkerEmitterComponent` (backend#23), and the - runtime reserved `SpriteRenderer.texture_path` (field 1) in favour of `img_path`; the backend - and launcher copies must follow. +- Proto copies have diverged: backend lacks `MarkerEmitterComponent` (backend#23). - Experiment flow graph is not in the proto, so it isn't saved, exported or run (epic frontend#59). - Scene object color and LSL marker are not persisted (frontend#56, backend#24). - Media files (images, audio, scripts) can't be uploaded, so the runtime can't find them (backend#25). diff --git a/README.md b/README.md index 6ecbf54..ed2b072 100644 --- a/README.md +++ b/README.md @@ -153,13 +153,21 @@ After construction, a component goes through two hooks, both before the first fr 1. `onSceneReady(Scene&)`, called by `Parser` once every object exists, to resolve and subscribe to events. 2. `onRenderTargetReady(const std::shared_ptr&)`, called by the - `Renderer` constructor once the window exists, to create GPU resources such as - textures. Doing it there, not lazily on the first `render()`, keeps uploads from - delaying the first stimulus frame and its marker. SDL frees a renderer's textures - along with it, so a component holding textures keeps the target alive. + `Renderer` constructor once the render target exists, to create GPU resources such + as textures, and again after `SDL_RENDER_DEVICE_RESET`, which loses every texture. + Doing it there, not lazily on the first `render()`, keeps uploads from delaying the + first stimulus frame and its marker. SDL frees a renderer's textures along with it, + so a component holding textures keeps the target alive. + +Images are decoded during parsing into the scene's `ImageLibrary`, once per file however +many sprites show it, and the library uploads them just before the components' hook +runs. The decoded pixels are kept, so a reset can rebuild the textures. Anything that can fail on bad input (a missing image, for instance) fails during -parsing, before the window opens. +parsing, before the window opens. The window is created hidden and shown only once the +scene is on the GPU, so a failed upload (an image larger than the GPU's texture limit) +doesn't flash it at the participant either. A sprite that fails to draw stops the run +before its frame is presented and marked. ### Core data structures @@ -367,8 +375,8 @@ bad config never flashes a window at the participant. `run()` then: `run()` is single-shot and must be called on the thread that constructed the `Runtime` (SDL handles window events only there); both are enforced with -`std::logic_error`. The default render target opens the window on -`display.index` and **refuses to run without vsync** — SDL silently drops +`std::logic_error`. The default render target opens the window (hidden until the +scene is uploaded) on `display.index` and **refuses to run without vsync** — SDL silently drops `PRESENTVSYNC` when the driver can't honour it, and marker timestamps are only correct when `SDL_RenderPresent` waits for the refresh. @@ -400,8 +408,8 @@ Update the row in the same PR that changes it. | Component | What it does | Proto (`neuronide.proto`) | C++ (`scene/components/`) | Status | Tracking | | --- | --- | --- | --- | --- | --- | -| Transform | Position, size, rotation. Exactly one per object. x/y is the top-left corner in window pixels, rotation is degrees clockwise about the centre. | `SceneObject.transform`, a field rather than a `Component` | `SceneObject::Transform` | **Partial**: parsed onto the object and drawn by SpriteRenderer, but not required (an object without one sits at all zeros; a sprite on it is rejected at parse time) | — | -| SpriteRenderer | Draws an image: visual stimuli, photos, icons. | `SpriteRenderer` (`renderer = 1`), `img_path`; field 1 (`texture_path`) is reserved | `SpriteRenderer`, registered | **Implemented**: any SDL2_image format (PNG, JPEG, …), stretched over the Transform and rotated. Decoded at parse time, so a missing or unreadable file stops the run before the window opens; uploaded to the GPU before the first frame | [#17][rt-17], [backend#25][be-25] | +| Transform | Position, size, rotation. Exactly one per object. x/y is the top-left corner in window pixels, rotation is degrees clockwise about the centre. | `SceneObject.transform`, a field rather than a `Component` | `SceneObject::Transform` | **Partial**: parsed onto the object and drawn by SpriteRenderer, but not required (an object without one sits at all zeros; a sprite with an image on it is rejected at parse time, as is a non-finite value) | — | +| SpriteRenderer | Draws an image: visual stimuli, photos, icons. | `SpriteRenderer` (`renderer = 1`), `img_path`. The runtime does not read `texture_path` | `SpriteRenderer`, registered | **Implemented**: any SDL2_image format (PNG, JPEG, …), stretched over the Transform and rotated. Decoded at parse time, so a missing or unreadable file stops the run before the window opens; uploaded to the GPU before the first frame. Objects showing the same file share one decoded image. An empty `img_path` draws nothing | [#17][rt-17], [backend#25][be-25] | | TextRenderer | Draws text in a chosen font and size: instructions, letters, numbers. | `TextRenderer` (`text = 2`) | — | **Planned**: needs SDL2_ttf, which is not a dependency yet. The message has no text color | [#18][rt-18], [backend#25][be-25] | | BlinkComponent | Flickers the object at a set frequency in Hz. The basis for SSVEP. | `BlinkComponent` (`blinker = 3`) | `BlinkComponent`, registered and built by the parser | **In review**: `update()` is an empty TODO on `main`, so nothing blinks yet. PR [#34][rt-34] implements it; changes requested | [#16][rt-16] | | MarkerEmitter | Emits an LSL marker on a scene event, ideally when an object appears or disappears. | `MarkerEmitterComponent` (`marker_emitter = 5`). Missing from the backend's copy | `MarkerEmitterComponent`, registered | **Implemented, nothing to bind to**: bindings resolve and push markers (tested against a test-only event source), but no production component exposes an event and objects have no appear/disappear events, so any binding in a real `.neuroz` fails at parse time | [backend#23][be-23] | diff --git a/include/Runtime.hpp b/include/Runtime.hpp index d181041..4d5503e 100644 --- a/include/Runtime.hpp +++ b/include/Runtime.hpp @@ -39,8 +39,9 @@ class Runtime { // Parses and validates both input files and builds every worker before the // render target is created, so a bad configuration fails without opening a - // window. The constructing thread becomes the only thread allowed to call - // run(). + // window. The render target's window, if it has one, is shown only after + // the scene has been uploaded to it. The constructing thread becomes the + // only thread allowed to call run(). explicit Runtime(const RuntimePaths& paths); Runtime(const RuntimePaths& paths, const RenderTargetFactory& renderTargetFactory, std::chrono::milliseconds acquisitionTimeout = kDefaultAcquisitionTimeout); @@ -77,10 +78,10 @@ class Runtime { // returned. const std::string& outputPath() const noexcept { return outputFilePath; } - // Opens a window on display.index (fullscreen or display.width x height) - // with an accelerated renderer, and throws std::runtime_error unless that - // renderer actually presents in sync with the display refresh. Marker - // timestamps are only correct when it does. + // Creates a hidden window on display.index (fullscreen or display.width x + // height) with an accelerated renderer, and throws std::runtime_error + // unless that renderer actually presents in sync with the display refresh. + // Marker timestamps are only correct when it does. static RenderTargetFactory defaultRenderTargetFactory(); private: @@ -106,11 +107,11 @@ class Runtime { std::thread::id ownerThread; bool hasRun = false; + // Declared before every member holding SDL objects, so SDL_Quit runs only + // after they are gone. Textures need no ordering against sdlRenderer: + // each one keeps its SDL_Renderer alive (see Image). SdlSession sdlSession; - // Declared before `scene` on purpose: members are destroyed in reverse - // order, so the scene - and any SDL_Texture its components hold - is - // released while the SDL_Renderer that owns those textures is still alive. std::shared_ptr sdlRenderer; std::shared_ptr scene; diff --git a/include/parser/Parser.hpp b/include/parser/Parser.hpp index 920d06f..4596153 100644 --- a/include/parser/Parser.hpp +++ b/include/parser/Parser.hpp @@ -1,15 +1,15 @@ #ifndef PARSER_HPP #define PARSER_HPP +#include #include #include #include -#include "scene/components/ComponentBuildContext.hpp" - class Scene; class SceneObject; class Component; +class ComponentBuildContext; namespace NeuronIDE { class SceneObject; @@ -23,9 +23,9 @@ class Parser { // Relative asset paths in the scene (e.g. sprite images) resolve against the folder holding // filePath. static std::shared_ptr parse(const std::string& filePath); - // Relative asset paths resolve against context.assetRoot: the working directory by default. + // Relative asset paths resolve against assetRoot: the working directory by default. static std::shared_ptr parseStream(std::istream& stream, - const ComponentBuildContext& context = {}); + const std::filesystem::path& assetRoot = {}); private: static std::shared_ptr buildSceneObject(const NeuronIDE::SceneObject& protoObj, diff --git a/include/renderer/Renderer.hpp b/include/renderer/Renderer.hpp index 3f551b0..783ea1c 100644 --- a/include/renderer/Renderer.hpp +++ b/include/renderer/Renderer.hpp @@ -24,7 +24,13 @@ class Renderer { void render(const std::stop_token& stoken); + // Handles every pending window event and returns true when the operator asked to close the + // window. Call it whenever events are polled outside render(), so a device reset is not lost. + bool pollEvents(); + private: + void prepareScene(); + struct SDLWindowDeleter { void operator()(SDL_Window* window) const; }; diff --git a/include/scene/Image.hpp b/include/scene/Image.hpp new file mode 100644 index 0000000..2e27040 --- /dev/null +++ b/include/scene/Image.hpp @@ -0,0 +1,45 @@ +#ifndef IMAGE_HPP +#define IMAGE_HPP + +#include +#include + +struct SDL_Surface; +struct SDL_Texture; +class SDL_Renderer; + +// An image file decoded into memory, plus its texture once uploaded to a render target. The +// decoded pixels are kept after the upload so the texture can be rebuilt when the render target +// loses it (SDL_RENDER_DEVICE_RESET). +class Image { + public: + // Throws std::runtime_error when the file cannot be opened or decoded. + explicit Image(std::filesystem::path path); + + [[nodiscard]] const std::filesystem::path& path() const noexcept { return filePath; } + + // Creates the texture on `target`, replacing any earlier one. Throws std::invalid_argument for + // a null target and std::runtime_error when SDL cannot create the texture, e.g. when the image + // is larger than the GPU's maximum texture size. + void upload(const std::shared_ptr& target); + + // Null until upload() has succeeded. + [[nodiscard]] SDL_Texture* texture() const noexcept { return gpuTexture.get(); } + + private: + struct SurfaceDeleter { + void operator()(SDL_Surface* surface) const noexcept; + }; + struct TextureDeleter { + // SDL frees a renderer's textures along with it, so destroying a texture after its + // renderer would be a use-after-free. The texture keeps its renderer alive instead. + std::shared_ptr target; + void operator()(SDL_Texture* texture) const noexcept; + }; + + std::filesystem::path filePath; + std::unique_ptr pixels; + std::unique_ptr gpuTexture; +}; + +#endif // IMAGE_HPP diff --git a/include/scene/ImageLibrary.hpp b/include/scene/ImageLibrary.hpp new file mode 100644 index 0000000..8e2f8f4 --- /dev/null +++ b/include/scene/ImageLibrary.hpp @@ -0,0 +1,30 @@ +#ifndef IMAGELIBRARY_HPP +#define IMAGELIBRARY_HPP + +#include +#include +#include +#include + +class Image; +class SDL_Renderer; + +// The images one scene shows. Each file is decoded once and uploaded once, however many sprites +// show it: an SSVEP grid of 40 objects sharing one checkerboard holds one copy, not 40. +class ImageLibrary { + public: + // The image already loaded from `path`, or a newly loaded one. Paths are compared after + // lexical normalisation, so "stimuli/./face.png" and "stimuli/face.png" are the same file. + // Throws std::runtime_error when the file cannot be loaded. + std::shared_ptr load(const std::filesystem::path& path); + + // (Re)creates every image's texture on `target`. See Image::upload() for what it throws. + void upload(const std::shared_ptr& target); + + [[nodiscard]] std::size_t size() const noexcept { return images.size(); } + + private: + std::map> images; +}; + +#endif // IMAGELIBRARY_HPP diff --git a/include/scene/Scene.hpp b/include/scene/Scene.hpp index 60c781f..70190a6 100644 --- a/include/scene/Scene.hpp +++ b/include/scene/Scene.hpp @@ -6,6 +6,8 @@ #include #include +#include "scene/ImageLibrary.hpp" + class Delegate; class SceneObject; class SDL_Renderer; @@ -15,6 +17,7 @@ class Scene { private: std::string experimentName; std::vector> objects; + ImageLibrary imageLibrary; public: void setExperimentName(const std::string& name) { experimentName = name; } @@ -23,11 +26,14 @@ class Scene { const std::string& getExperimentName() const { return experimentName; } const std::vector>& getObjects() const { return objects; } + ImageLibrary& images() { return imageLibrary; } void update(const Context& ctx); void render(SDL_Renderer* renderer); - void onSceneReady(); + void onSceneReady(); + // Uploads the scene's images to `target`, then hands it to every component. Runs before the + // first frame and again whenever the target has lost its textures. void onRenderTargetReady(const std::shared_ptr& target); Delegate& resolveEvents(std::string_view objectName, std::string_view eventName); }; diff --git a/include/scene/components/BlinkComponent.hpp b/include/scene/components/BlinkComponent.hpp index a72a6d5..795b939 100644 --- a/include/scene/components/BlinkComponent.hpp +++ b/include/scene/components/BlinkComponent.hpp @@ -8,7 +8,7 @@ namespace NeuronIDE { class Component; } -struct ComponentBuildContext; +class ComponentBuildContext; class BlinkComponent : public Component { public: diff --git a/include/scene/components/Component.hpp b/include/scene/components/Component.hpp index 360a9f7..6a1808a 100644 --- a/include/scene/components/Component.hpp +++ b/include/scene/components/Component.hpp @@ -26,9 +26,10 @@ class Component { virtual void onSceneReady(Scene& scene) { (void)scene; } - // Called once the render target this scene is drawn to exists, before the first frame: the - // place to create GPU resources such as textures. SDL frees a renderer's textures along with - // it, so a component holding any keeps `target` alive for as long as it does. + // Called once the render target this scene is drawn to exists, before the first frame, and + // again whenever the target has lost its textures (SDL_RENDER_DEVICE_RESET): the place to + // (re)create GPU resources. SDL frees a renderer's textures along with it, so a component + // holding any keeps `target` alive for as long as it does. virtual void onRenderTargetReady(const std::shared_ptr& target) { (void)target; } virtual Delegate* getEvent(std::string_view eventName) { diff --git a/include/scene/components/ComponentBuildContext.hpp b/include/scene/components/ComponentBuildContext.hpp index 4d58358..903e5e7 100644 --- a/include/scene/components/ComponentBuildContext.hpp +++ b/include/scene/components/ComponentBuildContext.hpp @@ -2,23 +2,42 @@ #define COMPONENTBUILDCONTEXT_HPP #include +#include +#include + +#include "scene/ImageLibrary.hpp" + +class Image; // What a component creator may need beyond its own proto message and owner. Parser builds one per // scene and hands it to every creator. -struct ComponentBuildContext { - // Folder that relative asset paths in the experiment file resolve against: the folder holding - // the .neuroz file. Empty for a scene parsed from a bare stream, so relative paths resolve - // against the working directory. - std::filesystem::path assetRoot; +class ComponentBuildContext { + public: + // `assetRoot` is the folder that relative asset paths in the experiment file resolve against: + // the folder holding the .neuroz file, or empty for a scene parsed from a bare stream, so + // relative paths resolve against the working directory. `images` is the scene's library and + // must outlive the context. + ComponentBuildContext(std::filesystem::path assetRoot, ImageLibrary& images) + : assetRoot(std::move(assetRoot)), images(&images) {} // An absolute path is returned as-is and a relative one is taken relative to assetRoot. An - // empty path stays empty, so the caller can still reject it. + // empty path stays empty. [[nodiscard]] std::filesystem::path resolveAsset(const std::filesystem::path& path) const { if (path.empty() || path.is_absolute()) { return path; } return assetRoot / path; } + + // The image at `path`, resolved as above, shared with every other component of the scene that + // shows the same file. Throws std::runtime_error when the file cannot be loaded. + [[nodiscard]] std::shared_ptr loadImage(const std::filesystem::path& path) const { + return images->load(resolveAsset(path)); + } + + private: + std::filesystem::path assetRoot; + ImageLibrary* images; }; #endif // COMPONENTBUILDCONTEXT_HPP diff --git a/include/scene/components/ComponentRegistry.hpp b/include/scene/components/ComponentRegistry.hpp index 35b72e8..12cf824 100644 --- a/include/scene/components/ComponentRegistry.hpp +++ b/include/scene/components/ComponentRegistry.hpp @@ -10,7 +10,7 @@ namespace NeuronIDE { class Component; } class SceneObject; -struct ComponentBuildContext; +class ComponentBuildContext; using ComponentCreatorFunc = std::function( const NeuronIDE::Component&, const std::shared_ptr&, diff --git a/include/scene/components/MarkerEmitterComponent.hpp b/include/scene/components/MarkerEmitterComponent.hpp index 88707d8..3f7ed6d 100644 --- a/include/scene/components/MarkerEmitterComponent.hpp +++ b/include/scene/components/MarkerEmitterComponent.hpp @@ -10,7 +10,7 @@ #include "scene/components/Component.hpp" class Scene; -struct ComponentBuildContext; +class ComponentBuildContext; namespace NeuronIDE { class Component; diff --git a/include/scene/components/SpriteRenderer.hpp b/include/scene/components/SpriteRenderer.hpp index 2accbab..c133a99 100644 --- a/include/scene/components/SpriteRenderer.hpp +++ b/include/scene/components/SpriteRenderer.hpp @@ -1,55 +1,41 @@ #ifndef SPRITERENDERER_HPP #define SPRITERENDERER_HPP -#include #include #include "scene/components/Component.hpp" -struct ComponentBuildContext; -struct SDL_Surface; -struct SDL_Texture; +class ComponentBuildContext; +class Image; namespace NeuronIDE { class Component; } -// Draws an image file into its owner's Transform: x/y is the top-left corner in window pixels, +// Draws an image into its owner's Transform: x/y is the top-left corner in window pixels, // width/height the drawn size, and rotation is in degrees clockwise about the rectangle's centre. // -// The image is decoded on construction, so a missing or unreadable file fails while the scene is -// parsed, before any window opens. It becomes a texture in onRenderTargetReady(), before the -// first frame, and the decoded pixels are released then. +// The image is decoded while the scene is parsed, so a missing or unreadable file fails before any +// window opens. The scene's ImageLibrary uploads it to the GPU before the first frame. A sprite +// without an image (an empty img_path) draws nothing. class SpriteRenderer : public Component { public: - // Throws std::invalid_argument for a null owner, an empty path, or an owner without a - // positive width and height; std::runtime_error when the image cannot be loaded. - SpriteRenderer(const std::shared_ptr& owner, std::filesystem::path path); + // `image` may be null. Throws std::invalid_argument for a null owner and, when there is an + // image, for an owner whose transform is not finite or has no positive width and height. + SpriteRenderer(const std::shared_ptr& owner, std::shared_ptr image); void update(const Context& context) override; - void render(SDL_Renderer* renderer) override; - // Uploads the image to `target`. A sprite belongs to one render target: a second call throws - // std::logic_error. - void onRenderTargetReady(const std::shared_ptr& target) override; + // Draws nothing while the image has no texture yet. Throws std::runtime_error when SDL fails + // to draw it, so a frame without its stimulus is never presented and marked as shown. + void render(SDL_Renderer* renderer) override; static std::unique_ptr createSprite(const NeuronIDE::Component& protoComp, const std::shared_ptr& owner, const ComponentBuildContext& context); private: - struct SurfaceDeleter { - void operator()(SDL_Surface* surface) const noexcept; - }; - struct TextureDeleter { - // The renderer the texture belongs to, kept alive until the texture is destroyed. - std::shared_ptr target; - void operator()(SDL_Texture* texture) const noexcept; - }; - - std::filesystem::path imagePath; - std::unique_ptr image; - std::unique_ptr texture; + std::shared_ptr image; }; #endif // SPRITERENDERER_HPP diff --git a/protoFiles/neuronide.proto b/protoFiles/neuronide.proto index 10d580c..0623fb6 100644 --- a/protoFiles/neuronide.proto +++ b/protoFiles/neuronide.proto @@ -3,9 +3,7 @@ syntax = "proto3"; package NeuronIDE; message SpriteRenderer { - reserved 1; - reserved "texture_path"; - // Image file. A relative path resolves against the folder holding the .neuroz file. + string texture_path = 1; string img_path = 2; } diff --git a/src/Runtime.cpp b/src/Runtime.cpp index 10e4936..efddf39 100644 --- a/src/Runtime.cpp +++ b/src/Runtime.cpp @@ -73,19 +73,6 @@ std::string describe(const std::exception_ptr& error) { return "unknown error"; } } - -// Handles every pending window event, so the window stays responsive while -// nothing is being rendered yet, and reports whether the operator closed it. -bool quitRequested() { - bool quit = false; - SDL_Event event{}; - while (SDL_PollEvent(&event) == 1) { - if (event.type == SDL_QUIT) { - quit = true; - } - } - return quit; -} } // namespace Runtime::SdlSession::SdlSession() { @@ -111,9 +98,11 @@ Runtime::RenderTargetFactory Runtime::defaultRenderTargetFactory() { " display(s) are connected"); } - // Desktop fullscreen keeps the monitor's native mode: no mode switch, - // and the compositor can hand the window the display directly. - Uint32 windowFlags = SDL_WINDOW_SHOWN; + // Hidden until Runtime has uploaded the scene, so a failed upload + // never flashes a window at the participant. Desktop fullscreen keeps + // the monitor's native mode: no mode switch, and the compositor can + // hand the window the display directly. + Uint32 windowFlags = SDL_WINDOW_HIDDEN; if (display.fullscreen) { windowFlags |= SDL_WINDOW_FULLSCREEN_DESKTOP; } @@ -181,7 +170,8 @@ Runtime::Runtime(const RuntimePaths& paths, const RenderTargetFactory& renderTar } // Everything that can still reject the configuration runs before the window - // opens, so a bad config fails without flashing a window at the participant. + // is created, and the window is shown only once the scene is on the GPU, so + // bad input fails without flashing a window at the participant. auto formatStrategy = DataFormatStrategyFactory::create(config.output.format); outputExtension = formatStrategy->fileExtension(); @@ -194,6 +184,11 @@ Runtime::Runtime(const RuntimePaths& paths, const RenderTargetFactory& renderTar } renderer = std::make_unique(scene, sdlRenderer, markerQueue); + + // Null for a render target without a window, such as a software renderer. + if (SDL_Window* window = SDL_RenderGetWindow(sdlRenderer.get())) { + SDL_ShowWindow(window); + } } Runtime::~Runtime() { shutdown(); } @@ -223,7 +218,7 @@ bool Runtime::awaitAcquisition() { const auto deadline = std::chrono::steady_clock::now() + acquisitionTimeout; while (!token.stop_requested()) { - if (quitRequested()) { + if (renderer->pollEvents()) { return false; } diff --git a/src/parser/Parser.cpp b/src/parser/Parser.cpp index acbeaf5..e419c16 100644 --- a/src/parser/Parser.cpp +++ b/src/parser/Parser.cpp @@ -9,6 +9,7 @@ #include "scene/Scene.hpp" #include "scene/SceneObject.hpp" #include "scene/components/Component.hpp" +#include "scene/components/ComponentBuildContext.hpp" #include "scene/components/ComponentRegistry.hpp" std::shared_ptr Parser::parse(const std::string& filePath) { @@ -17,11 +18,8 @@ std::shared_ptr Parser::parse(const std::string& filePath) { throw std::runtime_error("Parser: cannot open file: " + filePath); } - const ComponentBuildContext context{.assetRoot = - std::filesystem::absolute(filePath).parent_path()}; - try { - return parseStream(file, context); + return parseStream(file, std::filesystem::absolute(filePath).parent_path()); } catch (const std::runtime_error& e) { throw std::runtime_error(std::string("Parser: failed to parse file ") + filePath + " - " + e.what()); @@ -29,7 +27,7 @@ std::shared_ptr Parser::parse(const std::string& filePath) { } std::shared_ptr Parser::parseStream(std::istream& stream, - const ComponentBuildContext& context) { + const std::filesystem::path& assetRoot) { NeuronIDE::Scene protoScene; if (!protoScene.ParseFromIstream(&stream)) { @@ -42,6 +40,7 @@ std::shared_ptr Parser::parseStream(std::istream& stream, auto scene = std::make_shared<::Scene>(); scene->setExperimentName(protoScene.project_name()); + const ComponentBuildContext context(assetRoot, scene->images()); for (const auto& protoObj : protoScene.scene_objects()) { auto obj = buildSceneObject(protoObj, context); diff --git a/src/renderer/Renderer.cpp b/src/renderer/Renderer.cpp index ed1dc4b..31476c8 100644 --- a/src/renderer/Renderer.cpp +++ b/src/renderer/Renderer.cpp @@ -19,9 +19,27 @@ Renderer::Renderer(const std::shared_ptr& scene, std::shared_ptrsdlRenderer) { - scene->onRenderTargetReady(this->sdlRenderer); + prepareScene(); +} + +void Renderer::prepareScene() { + if (auto scene = currentScene.lock(); scene && sdlRenderer) { + scene->onRenderTargetReady(sdlRenderer); + } +} + +bool Renderer::pollEvents() { + bool quit = false; + SDL_Event event{}; + while (SDL_PollEvent(&event) == 1) { + if (event.type == SDL_QUIT) { + quit = true; + } else if (event.type == SDL_RENDER_DEVICE_RESET) { + // Every texture is gone (e.g. Direct3D after a lock screen or UAC prompt). + prepareScene(); + } } + return quit; } void Renderer::render(const std::stop_token& stoken) { @@ -36,11 +54,8 @@ void Renderer::render(const std::stop_token& stoken) { currentFrameMarkers.clear(); Context ctx{deltaTime, ¤tFrameMarkers}; - SDL_Event event; - while (SDL_PollEvent(&event) == 1) { - if (event.type == SDL_QUIT) { - return; - } + if (pollEvents()) { + return; } if (auto scene = currentScene.lock()) { diff --git a/src/scene/CMakeLists.txt b/src/scene/CMakeLists.txt index 2fa86e1..0686d42 100644 --- a/src/scene/CMakeLists.txt +++ b/src/scene/CMakeLists.txt @@ -1,6 +1,8 @@ add_library(scene OBJECT SceneObject.cpp Scene.cpp + Image.cpp + ImageLibrary.cpp components/ComponentRegistry.cpp components/BlinkComponent.cpp components/MarkerEmitterComponent.cpp diff --git a/src/scene/Image.cpp b/src/scene/Image.cpp new file mode 100644 index 0000000..2b0287c --- /dev/null +++ b/src/scene/Image.cpp @@ -0,0 +1,37 @@ +#include "scene/Image.hpp" + +#include +#include + +#include +#include +#include + +void Image::SurfaceDeleter::operator()(SDL_Surface* surface) const noexcept { + SDL_FreeSurface(surface); +} + +void Image::TextureDeleter::operator()(SDL_Texture* texture) const noexcept { + SDL_DestroyTexture(texture); +} + +Image::Image(std::filesystem::path path) : filePath(std::move(path)) { + pixels.reset(IMG_Load(filePath.string().c_str())); + if (!pixels) { + throw std::runtime_error("cannot load image '" + filePath.string() + + "': " + IMG_GetError()); + } +} + +void Image::upload(const std::shared_ptr& target) { + if (!target) { + throw std::invalid_argument("Image: render target must not be null"); + } + + SDL_Texture* created = SDL_CreateTextureFromSurface(target.get(), pixels.get()); + if (created == nullptr) { + throw std::runtime_error("cannot create a texture from '" + filePath.string() + + "': " + SDL_GetError()); + } + gpuTexture = std::unique_ptr(created, TextureDeleter{target}); +} diff --git a/src/scene/ImageLibrary.cpp b/src/scene/ImageLibrary.cpp new file mode 100644 index 0000000..ae34fe2 --- /dev/null +++ b/src/scene/ImageLibrary.cpp @@ -0,0 +1,20 @@ +#include "scene/ImageLibrary.hpp" + +#include "scene/Image.hpp" + +std::shared_ptr ImageLibrary::load(const std::filesystem::path& path) { + const std::filesystem::path key = path.lexically_normal(); + if (const auto found = images.find(key); found != images.end()) { + return found->second; + } + + auto image = std::make_shared(key); + images.emplace(key, image); + return image; +} + +void ImageLibrary::upload(const std::shared_ptr& target) { + for (const auto& [path, image] : images) { + image->upload(target); + } +} diff --git a/src/scene/Scene.cpp b/src/scene/Scene.cpp index ff0fc19..01cb34a 100644 --- a/src/scene/Scene.cpp +++ b/src/scene/Scene.cpp @@ -29,6 +29,7 @@ void Scene::onSceneReady() { } void Scene::onRenderTargetReady(const std::shared_ptr& target) { + imageLibrary.upload(target); for (const auto& obj : objects) { for (const auto& comp : obj->components) { comp->onRenderTargetReady(target); diff --git a/src/scene/components/SpriteRenderer.cpp b/src/scene/components/SpriteRenderer.cpp index bde58c7..f87c85b 100644 --- a/src/scene/components/SpriteRenderer.cpp +++ b/src/scene/components/SpriteRenderer.cpp @@ -1,46 +1,50 @@ #include "scene/components/SpriteRenderer.hpp" #include -#include +#include +#include #include #include #include #include "neuronide.pb.h" +#include "scene/Image.hpp" #include "scene/SceneObject.hpp" #include "scene/components/ComponentBuildContext.hpp" #include "scene/components/ComponentRegistry.hpp" -void SpriteRenderer::SurfaceDeleter::operator()(SDL_Surface* surface) const noexcept { - SDL_FreeSurface(surface); +namespace { +// SDL draws in float, and converting a double outside float range is undefined. Also false for +// NaN and infinity. +bool fitsInFloat(double value) { + return std::abs(value) <= static_cast(std::numeric_limits::max()); } -void SpriteRenderer::TextureDeleter::operator()(SDL_Texture* texture) const noexcept { - SDL_DestroyTexture(texture); -} +std::string describe(const SceneObject& owner) { return "SpriteRenderer on '" + owner.name + "'"; } +} // namespace SpriteRenderer::SpriteRenderer(const std::shared_ptr& owner, - std::filesystem::path path) - : Component(owner), imagePath(std::move(path)) { + std::shared_ptr image) + : Component(owner), image(std::move(image)) { if (!owner) { throw std::invalid_argument("SpriteRenderer: owner must not be null"); } - const std::string where = "SpriteRenderer on '" + owner->name + "'"; - - if (imagePath.empty()) { - throw std::invalid_argument(where + ": image path is empty"); - } - // Written so that a NaN size is rejected too. - const bool hasArea = owner->transform.width > 0.0 && owner->transform.height > 0.0; - if (!hasArea) { - throw std::invalid_argument(where + ": the object needs a positive width and height"); + if (!this->image) { + return; } - image.reset(IMG_Load(imagePath.string().c_str())); - if (!image) { - throw std::runtime_error(where + ": cannot load image '" + imagePath.string() + - "': " + IMG_GetError()); + const auto& transform = owner->transform; + for (const double value : + {transform.posX, transform.posY, transform.width, transform.height, transform.rotation}) { + if (!fitsInFloat(value)) { + throw std::invalid_argument(describe(*owner) + + ": the transform has a non-finite or out-of-range value"); + } + } + if (transform.width <= 0.0 || transform.height <= 0.0) { + throw std::invalid_argument(describe(*owner) + + ": the object needs a positive width and height"); } } @@ -48,7 +52,7 @@ void SpriteRenderer::update(const Context& context) { (void)context; } void SpriteRenderer::render(SDL_Renderer* renderer) { const auto ownerPtr = owner.lock(); - if (renderer == nullptr || !texture || !ownerPtr) { + if (renderer == nullptr || !image || image->texture() == nullptr || !ownerPtr) { return; } @@ -58,33 +62,19 @@ void SpriteRenderer::render(SDL_Renderer* renderer) { static_cast(transform.width), static_cast(transform.height)}; // A null centre rotates about the centre of `destination`. - SDL_RenderCopyExF(renderer, texture.get(), nullptr, &destination, transform.rotation, nullptr, - SDL_FLIP_NONE); -} - -void SpriteRenderer::onRenderTargetReady(const std::shared_ptr& target) { - if (!target) { - throw std::invalid_argument("SpriteRenderer: render target must not be null"); - } - if (!image) { - throw std::logic_error("SpriteRenderer: '" + imagePath.string() + - "' is already uploaded to a render target"); - } - - SDL_Texture* created = SDL_CreateTextureFromSurface(target.get(), image.get()); - if (created == nullptr) { - throw std::runtime_error("SpriteRenderer: cannot create a texture from '" + - imagePath.string() + "': " + SDL_GetError()); + if (SDL_RenderCopyExF(renderer, image->texture(), nullptr, &destination, transform.rotation, + nullptr, SDL_FLIP_NONE) != 0) { + throw std::runtime_error(describe(*ownerPtr) + ": cannot draw '" + image->path().string() + + "': " + SDL_GetError()); } - texture = std::unique_ptr(created, TextureDeleter{target}); - image.reset(); } std::unique_ptr SpriteRenderer::createSprite(const NeuronIDE::Component& protoComp, const std::shared_ptr& owner, const ComponentBuildContext& context) { + const std::string& path = protoComp.renderer().img_path(); return std::make_unique(owner, - context.resolveAsset(protoComp.renderer().img_path())); + path.empty() ? nullptr : context.loadImage(path)); } REGISTER_COMPONENT(NeuronIDE::Component::kRenderer, SpriteRenderer::createSprite) diff --git a/tests/component_tests/sprite_scene_test.cpp b/tests/component_tests/sprite_scene_test.cpp index dbf7682..f8ad769 100644 --- a/tests/component_tests/sprite_scene_test.cpp +++ b/tests/component_tests/sprite_scene_test.cpp @@ -22,18 +22,21 @@ constexpr double kSpriteSize = 10.0; constexpr utils::Rgba kLeft{255, 0, 0, 255}; constexpr utils::Rgba kRight{0, 0, 255, 255}; -NeuronIDE::Scene sceneWithSprite(const std::string& imagePath, bool withTransform = true) { - NeuronIDE::Scene scene; - scene.set_project_name("Sprites"); - +void addSprite(NeuronIDE::Scene& scene, const std::string& imagePath, bool withTransform = true) { auto* object = scene.add_scene_objects(); - object->set_name("Stimulus"); + object->set_name("Stimulus" + std::to_string(scene.scene_objects_size())); object->set_is_visible(true); if (withTransform) { object->mutable_transform()->set_width(kSpriteSize); object->mutable_transform()->set_height(kSpriteSize); } object->add_components()->mutable_renderer()->set_img_path(imagePath); +} + +NeuronIDE::Scene sceneWithSprite(const std::string& imagePath, bool withTransform = true) { + NeuronIDE::Scene scene; + scene.set_project_name("Sprites"); + addSprite(scene, imagePath, withTransform); return scene; } @@ -95,10 +98,24 @@ TEST_F(SpriteSceneTest, MissingImageFailsTheParseAndNamesTheFile) { } } -TEST_F(SpriteSceneTest, EmptyImagePathFailsTheParse) { - writeExperiment(sceneWithSprite("")); +// Only an image that is named but cannot be loaded is an error. +TEST_F(SpriteSceneTest, EmptyImagePathGivesASpriteWithoutImage) { + writeExperiment(sceneWithSprite("", /*withTransform=*/false)); - EXPECT_THROW(parseExperiment(), std::invalid_argument); + const auto scene = parseExperiment(); + EXPECT_TRUE(hasSprite(*scene)); + EXPECT_EQ(scene->images().size(), 0U); +} + +TEST_F(SpriteSceneTest, ObjectsShowingTheSameFileShareOneImage) { + utils::writeTwoPixelPng(dir() / "face.png", kLeft, kRight); + NeuronIDE::Scene proto; + proto.set_project_name("Sprites"); + addSprite(proto, "face.png"); + addSprite(proto, "./face.png"); + writeExperiment(proto); + + EXPECT_EQ(parseExperiment()->images().size(), 1U); } TEST_F(SpriteSceneTest, SpriteOnObjectWithoutTransformFailsTheParse) { diff --git a/tests/registration_tests/registration_test.cpp b/tests/registration_tests/registration_test.cpp index dccc05d..29f01a0 100644 --- a/tests/registration_tests/registration_test.cpp +++ b/tests/registration_tests/registration_test.cpp @@ -42,8 +42,7 @@ TEST(ComponentRegistrationTest, EveryImplementedComponentIsLinkedIn) { std::stringstream stream; ASSERT_TRUE(proto.SerializeToOstream(&stream)); - const auto scene = - Parser::parseStream(stream, ComponentBuildContext{.assetRoot = assets.path()}); + const auto scene = Parser::parseStream(stream, assets.path()); for (const auto& object : scene->getObjects()) { EXPECT_EQ(object->components.size(), 1U) diff --git a/tests/unit_tests/RendererTest.cpp b/tests/unit_tests/RendererTest.cpp index ebdbac0..e1da652 100644 --- a/tests/unit_tests/RendererTest.cpp +++ b/tests/unit_tests/RendererTest.cpp @@ -3,8 +3,10 @@ #include #include #include +#include #include #include +#include #include "data_structures/Context.hpp" #include "data_structures/Marker.hpp" @@ -12,13 +14,11 @@ #include "scene/Scene.hpp" #include "scene/SceneObject.hpp" #include "scene/components/Component.hpp" +#include "utils/SdlTestUtils.hpp" namespace { -constexpr int kDummySurfaceWidth = 10; -constexpr int kDummySurfaceHeight = 10; -constexpr int kDummySurfaceDepth = 32; -constexpr uint32_t kDummySurfaceFlags = 0; -constexpr auto kRenderSpinWait = std::chrono::milliseconds(30); +constexpr int kCanvasSize = 10; +constexpr auto kRenderSpinWait = std::chrono::milliseconds(30); class CustomComponent : public Component { public: @@ -68,20 +68,33 @@ class MarkerComponent : public Component { std::shared_ptr stopSource; }; +// Fails to draw, like a sprite whose texture SDL cannot copy. +class FailingDrawComponent : public Component { + public: + explicit FailingDrawComponent(const std::shared_ptr& owner) : Component(owner) {} + + void update(const Context& context) override { (void)context; } + void render(SDL_Renderer* renderer) override { + (void)renderer; + throw std::runtime_error("cannot draw"); + } +}; + +// Records every render target it is handed. class RenderTargetSpy : public Component { public: - RenderTargetSpy(const std::shared_ptr& owner, - std::weak_ptr* received) + RenderTargetSpy(const std::shared_ptr& owner, + std::vector>* received) : Component(owner), received(received) {} void onRenderTargetReady(const std::shared_ptr& target) override { - *received = target; + received->push_back(target); } void update(const Context& context) override { (void)context; } void render(SDL_Renderer* renderer) override { (void)renderer; } private: - std::weak_ptr* received; + std::vector>* received; }; class RendererTest : public ::testing::Test { @@ -90,18 +103,7 @@ class RendererTest : public ::testing::Test { void TearDown() override { SDL_Quit(); } static std::shared_ptr makeSoftwareRenderer() { - SDL_Surface* surface = SDL_CreateRGBSurfaceWithFormat( - kDummySurfaceFlags, kDummySurfaceWidth, kDummySurfaceHeight, kDummySurfaceDepth, - SDL_PIXELFORMAT_RGBA32); - SDL_Renderer* sdlRenderer = SDL_CreateSoftwareRenderer(surface); - return std::shared_ptr(sdlRenderer, [surface](SDL_Renderer* renderer) { - if (renderer != nullptr) { - SDL_DestroyRenderer(renderer); - } - if (surface != nullptr) { - SDL_FreeSurface(surface); - } - }); + return utils::makeSoftwareTarget(kCanvasSize, kCanvasSize); } }; } // namespace @@ -127,9 +129,9 @@ TEST_F(RendererTest, RenderLoop_WhenComponentAdded_CallsUpdateExactlyOnceBeforeS } TEST_F(RendererTest, Constructor_HandsRenderTargetToComponentsBeforeFirstFrame) { - auto scene = std::make_shared(); - auto obj = std::make_shared("obj"); - std::weak_ptr received; + auto scene = std::make_shared(); + auto obj = std::make_shared("obj"); + std::vector> received; obj->addComponent(std::make_unique(obj, &received)); scene->addObject(obj); @@ -137,7 +139,46 @@ TEST_F(RendererTest, Constructor_HandsRenderTargetToComponentsBeforeFirstFrame) const auto target = makeSoftwareRenderer(); const Renderer renderer(scene, target, std::make_shared>()); - EXPECT_EQ(received.lock(), target); + ASSERT_EQ(received.size(), 1U); + EXPECT_EQ(received.front().lock(), target); +} + +TEST_F(RendererTest, PollEvents_OnDeviceReset_HandsRenderTargetToComponentsAgain) { + auto scene = std::make_shared(); + auto obj = std::make_shared("obj"); + std::vector> received; + + obj->addComponent(std::make_unique(obj, &received)); + scene->addObject(obj); + + const auto target = makeSoftwareRenderer(); + Renderer renderer(scene, target, std::make_shared>()); + + SDL_Event reset{}; + reset.type = SDL_RENDER_DEVICE_RESET; + ASSERT_EQ(SDL_PushEvent(&reset), 1); + + EXPECT_FALSE(renderer.pollEvents()); + ASSERT_EQ(received.size(), 2U); + EXPECT_EQ(received.back().lock(), target); +} + +TEST_F(RendererTest, RenderLoop_WhenDrawingFails_ThrowsWithoutQueueingTheFrameMarkers) { + auto scene = std::make_shared(); + auto obj = std::make_shared("obj"); + auto stop_source = std::make_shared(); + + obj->addComponent(std::make_unique(obj, stop_source)); + obj->addComponent(std::make_unique(obj)); + scene->addObject(obj); + + auto markerQueue = std::make_shared>(); + Renderer renderer(scene, makeSoftwareRenderer(), markerQueue); + + EXPECT_THROW(renderer.render(stop_source->get_token()), std::runtime_error); + + Marker marker; + EXPECT_FALSE(markerQueue->try_dequeue(marker)); } TEST_F(RendererTest, RenderLoop_QueuesMarkersFromComponents) { diff --git a/tests/unit_tests/component_build_context_test.cpp b/tests/unit_tests/component_build_context_test.cpp index 81eec90..54d78ac 100644 --- a/tests/unit_tests/component_build_context_test.cpp +++ b/tests/unit_tests/component_build_context_test.cpp @@ -2,6 +2,7 @@ #include +#include "scene/ImageLibrary.hpp" #include "scene/components/ComponentBuildContext.hpp" namespace { @@ -9,29 +10,37 @@ namespace fs = std::filesystem; // temp_directory_path() is absolute on every platform, unlike a literal such as "/data". fs::path experimentFolder() { return fs::temp_directory_path() / "experiment"; } + +class ComponentBuildContextTest : public ::testing::Test { + protected: + ImageLibrary& images() { return library; } + + private: + ImageLibrary library; +}; } // namespace -TEST(ComponentBuildContextTest, ResolvesRelativePathAgainstAssetRoot) { - const ComponentBuildContext context{.assetRoot = experimentFolder()}; +TEST_F(ComponentBuildContextTest, ResolvesRelativePathAgainstAssetRoot) { + const ComponentBuildContext context(experimentFolder(), images()); EXPECT_EQ(context.resolveAsset("stimuli/face.png"), experimentFolder() / "stimuli/face.png"); } -TEST(ComponentBuildContextTest, KeepsAbsolutePathAsIs) { - const ComponentBuildContext context{.assetRoot = experimentFolder()}; +TEST_F(ComponentBuildContextTest, KeepsAbsolutePathAsIs) { + const ComponentBuildContext context(experimentFolder(), images()); const fs::path absolute = fs::temp_directory_path() / "elsewhere" / "face.png"; EXPECT_EQ(context.resolveAsset(absolute), absolute); } -TEST(ComponentBuildContextTest, KeepsEmptyPathEmpty) { - const ComponentBuildContext context{.assetRoot = experimentFolder()}; +TEST_F(ComponentBuildContextTest, KeepsEmptyPathEmpty) { + const ComponentBuildContext context(experimentFolder(), images()); EXPECT_TRUE(context.resolveAsset(fs::path{}).empty()); } -TEST(ComponentBuildContextTest, WithoutAssetRootLeavesRelativePathForWorkingDirectory) { - const ComponentBuildContext context; +TEST_F(ComponentBuildContextTest, WithoutAssetRootLeavesRelativePathForWorkingDirectory) { + const ComponentBuildContext context({}, images()); EXPECT_EQ(context.resolveAsset("face.png"), fs::path("face.png")); } diff --git a/tests/unit_tests/image_library_test.cpp b/tests/unit_tests/image_library_test.cpp new file mode 100644 index 0000000..3304c52 --- /dev/null +++ b/tests/unit_tests/image_library_test.cpp @@ -0,0 +1,113 @@ +#include + +#include +#include +#include +#include + +#include "scene/Image.hpp" +#include "scene/ImageLibrary.hpp" +#include "scene/SceneObject.hpp" +#include "scene/components/SpriteRenderer.hpp" +#include "utils/SdlTestUtils.hpp" +#include "utils/TempDir.hpp" + +namespace { +namespace fs = std::filesystem; + +constexpr int kCanvasSize = 4; +constexpr utils::Rgba kBackground{0, 0, 0, 255}; +constexpr utils::Rgba kWhite{255, 255, 255, 255}; + +class ImageLibraryTest : public ::testing::Test { + protected: + void SetUp() override { + utils::writeTwoPixelPng(dir() / "a.png", kWhite, kWhite); + utils::writeTwoPixelPng(dir() / "b.png", kWhite, kWhite); + } + + const fs::path& dir() const { return tempDir.path(); } + + private: + utils::TempDir tempDir; +}; +} // namespace + +TEST_F(ImageLibraryTest, LoadsEachFileOnce) { + ImageLibrary library; + + EXPECT_EQ(library.load(dir() / "a.png"), library.load(dir() / "a.png")); + EXPECT_EQ(library.load(dir() / "." / "a.png"), library.load(dir() / "a.png")); + EXPECT_EQ(library.size(), 1U); +} + +TEST_F(ImageLibraryTest, KeepsDifferentFilesApart) { + ImageLibrary library; + + EXPECT_NE(library.load(dir() / "a.png"), library.load(dir() / "b.png")); + EXPECT_EQ(library.size(), 2U); +} + +TEST_F(ImageLibraryTest, MissingFileThrowsAndIsNotRemembered) { + ImageLibrary library; + + EXPECT_THROW(library.load(dir() / "missing.png"), std::runtime_error); + EXPECT_THROW(library.load(dir() / "missing.png"), std::runtime_error); + EXPECT_EQ(library.size(), 0U); +} + +TEST_F(ImageLibraryTest, FileThatIsNotAnImageThrows) { + std::ofstream(dir() / "notes.png") << "not an image"; + ImageLibrary library; + + EXPECT_THROW(library.load(dir() / "notes.png"), std::runtime_error); +} + +TEST_F(ImageLibraryTest, UploadGivesEveryImageATexture) { + ImageLibrary library; + const auto first = library.load(dir() / "a.png"); + const auto second = library.load(dir() / "b.png"); + ASSERT_EQ(first->texture(), nullptr); + + library.upload(utils::makeSoftwareTarget(kCanvasSize, kCanvasSize)); + + EXPECT_NE(first->texture(), nullptr); + EXPECT_NE(second->texture(), nullptr); +} + +TEST_F(ImageLibraryTest, UploadToNullTargetThrows) { + Image image(dir() / "a.png"); + + EXPECT_THROW(image.upload(nullptr), std::invalid_argument); +} + +// After a device reset the scene uploads again, so a second upload must leave a drawable texture. +TEST_F(ImageLibraryTest, ImageCanBeUploadedAgain) { + const auto target = utils::makeSoftwareTarget(kCanvasSize, kCanvasSize); + const auto image = std::make_shared(dir() / "a.png"); + const auto owner = std::make_shared("Stimulus"); + owner->setTransform({0.0, 0.0, kCanvasSize, kCanvasSize, 0.0}); + SpriteRenderer sprite(owner, image); + + image->upload(target); + image->upload(target); + utils::clear(target.get(), kBackground); + sprite.render(target.get()); + + EXPECT_EQ(utils::readPixel(target.get(), 1, 1), kWhite); +} + +// SDL frees a renderer's textures along with it, so destroying the texture afterwards would be a +// use-after-free. The texture therefore keeps its renderer alive. +TEST_F(ImageLibraryTest, TextureKeepsRenderTargetAlive) { + auto image = std::make_unique(dir() / "a.png"); + auto target = utils::makeSoftwareTarget(kCanvasSize, kCanvasSize); + image->upload(target); + + const std::weak_ptr weakTarget = target; + target.reset(); + EXPECT_FALSE(weakTarget.expired()); + + image.reset(); + EXPECT_TRUE(weakTarget.expired()); +} diff --git a/tests/unit_tests/runtime_test.cpp b/tests/unit_tests/runtime_test.cpp index 99a7c53..ed6099f 100644 --- a/tests/unit_tests/runtime_test.cpp +++ b/tests/unit_tests/runtime_test.cpp @@ -21,6 +21,8 @@ #include "lsl_cpp.h" #include "utils/ParserTestUtils.hpp" +#include "utils/SdlTestUtils.hpp" +#include "utils/TempDir.hpp" // These tests drive a real LSLReader against a local LSL outlet over loopback, // like the LSLReader tests do. @@ -29,7 +31,6 @@ namespace { namespace fs = std::filesystem; constexpr int kSurfaceSize = 10; -constexpr int kSurfaceDepth = 32; constexpr int kStreamChannels = 1; constexpr int kWrongChannels = 2; constexpr double kSampleRateHz = 250.0; @@ -79,17 +80,7 @@ std::string makeConfigJson(const std::string& streamName, const ConfigOptions& o } std::shared_ptr createSoftwareRenderer() { - SDL_Surface* surface = SDL_CreateRGBSurfaceWithFormat(0, kSurfaceSize, kSurfaceSize, - kSurfaceDepth, SDL_PIXELFORMAT_RGBA32); - SDL_Renderer* renderer = SDL_CreateSoftwareRenderer(surface); - return {renderer, [surface](SDL_Renderer* target) { - if (target != nullptr) { - SDL_DestroyRenderer(target); - } - if (surface != nullptr) { - SDL_FreeSurface(surface); - } - }}; + return utils::makeSoftwareTarget(kSurfaceSize, kSurfaceSize); } // A software renderer backed by an in-memory surface: no window, GPU, or @@ -213,28 +204,18 @@ class RuntimeFixture : public ::testing::Test { // succeed without a display. The software renderer is unaffected by it. setenv("SDL_VIDEODRIVER", "dummy", 1); - tempDir = fs::temp_directory_path() / - ("neuronide_runtime_" + - std::to_string(std::chrono::steady_clock::now().time_since_epoch().count())); - fs::create_directories(tempDir); - streamName = currentStreamName(); - writeText(tempDir / "config.json", makeConfigJson(streamName)); - writeExperimentFile(tempDir / "experiment.pb", utils::buildSimpleScene()); - } - - void TearDown() override { - std::error_code errorCode; - fs::remove_all(tempDir, errorCode); + writeText(dir() / "config.json", makeConfigJson(streamName)); + writeExperimentFile(dir() / "experiment.pb", utils::buildSimpleScene()); } - const fs::path& dir() const { return tempDir; } + const fs::path& dir() const { return tempDir.path(); } const std::string& stream() const { return streamName; } RuntimePaths paths() const { - return RuntimePaths{.config = (tempDir / "config.json").string(), - .experiment = (tempDir / "experiment.pb").string(), - .outputDir = tempDir.string()}; + return RuntimePaths{.config = (dir() / "config.json").string(), + .experiment = (dir() / "experiment.pb").string(), + .outputDir = dir().string()}; } std::unique_ptr makeRuntime( @@ -257,8 +238,8 @@ class RuntimeFixture : public ::testing::Test { } private: - fs::path tempDir; - std::string streamName; + utils::TempDir tempDir; + std::string streamName; }; } // namespace @@ -341,6 +322,30 @@ TEST_F(RuntimeFixture, PassesTheConfiguredDisplayToTheRenderTargetFactory) { EXPECT_EQ(received.width, kConfiguredWidth); } +TEST_F(RuntimeFixture, ShowsTheRenderTargetsWindowOnceConstructed) { + SDL_Window* window = nullptr; + const auto factory = [&window](const std::string&, + const DisplayConfig&) -> std::shared_ptr { + window = SDL_CreateWindow("test", 0, 0, kSurfaceSize, kSurfaceSize, SDL_WINDOW_HIDDEN); + if (window == nullptr) { + throw utils::sdlFailure("SDL_CreateWindow"); + } + SDL_Renderer* target = SDL_CreateRenderer(window, -1, SDL_RENDERER_SOFTWARE); + if (target == nullptr) { + throw utils::sdlFailure("SDL_CreateRenderer"); + } + return {target, [created = window](SDL_Renderer* renderer) { + SDL_DestroyRenderer(renderer); + SDL_DestroyWindow(created); + }}; + }; + + const Runtime runtime(paths(), factory); + + ASSERT_NE(window, nullptr); + EXPECT_NE(SDL_GetWindowFlags(window) & SDL_WINDOW_SHOWN, 0U); +} + // --- Acquisition gate ------------------------------------------------------ TEST_F(RuntimeFixture, RunThrowsWithoutRecordingWhenNoStreamAppears) { diff --git a/tests/unit_tests/sprite_renderer_test.cpp b/tests/unit_tests/sprite_renderer_test.cpp index 4956352..ba4e4ac 100644 --- a/tests/unit_tests/sprite_renderer_test.cpp +++ b/tests/unit_tests/sprite_renderer_test.cpp @@ -1,10 +1,11 @@ #include #include -#include +#include #include #include +#include "scene/Image.hpp" #include "scene/SceneObject.hpp" #include "scene/components/SpriteRenderer.hpp" #include "utils/SdlTestUtils.hpp" @@ -17,6 +18,7 @@ constexpr double kSpriteY = 20.0; constexpr double kSpriteWidth = 20.0; constexpr double kSpriteHeight = 4.0; constexpr double kQuarterTurn = 90.0; +constexpr double kBeyondFloat = 1e39; constexpr utils::Rgba kBackground{0, 0, 0, 255}; constexpr utils::Rgba kLeft{255, 0, 0, 255}; @@ -37,8 +39,9 @@ constexpr int kOutside = 5; class SpriteRendererTest : public ::testing::Test { protected: void SetUp() override { - image = tempDir.path() / "sprite.png"; - utils::writeTwoPixelPng(image, kLeft, kRight); + const auto path = tempDir.path() / "sprite.png"; + utils::writeTwoPixelPng(path, kLeft, kRight); + loadedImage = std::make_shared(path); object = std::make_shared("Stimulus"); object->setTransform({kSpriteX, kSpriteY, kSpriteWidth, kSpriteHeight, 0.0}); @@ -47,27 +50,39 @@ class SpriteRendererTest : public ::testing::Test { utils::clear(renderTarget.get(), kBackground); } - const std::filesystem::path& dir() const { return tempDir.path(); } - const std::filesystem::path& imagePath() const { return image; } + const std::shared_ptr& image() const { return loadedImage; } const std::shared_ptr& owner() const { return object; } const std::shared_ptr& target() const { return renderTarget; } - void releaseTarget() { renderTarget.reset(); } utils::Rgba pixelAt(int column, int row) const { return utils::readPixel(renderTarget.get(), column, row); } + // Sets one transform field to `value` and reports whether a sprite on the owner is rejected. + bool rejectsTransformValue(double SceneObject::Transform::*field, double value) const { + const double original = object->transform.*field; + object->transform.*field = value; + bool rejected = false; + try { + const SpriteRenderer sprite(object, loadedImage); + } catch (const std::invalid_argument&) { + rejected = true; + } + object->transform.*field = original; + return rejected; + } + private: utils::TempDir tempDir; - std::filesystem::path image; + std::shared_ptr loadedImage; std::shared_ptr object; std::shared_ptr renderTarget; }; } // namespace TEST_F(SpriteRendererTest, StretchesImageOverTransform) { - SpriteRenderer sprite(owner(), imagePath()); - sprite.onRenderTargetReady(target()); + SpriteRenderer sprite(owner(), image()); + image()->upload(target()); sprite.render(target().get()); EXPECT_EQ(pixelAt(kLeftHalfX, kRowY), kLeft); @@ -79,8 +94,8 @@ TEST_F(SpriteRendererTest, StretchesImageOverTransform) { TEST_F(SpriteRendererTest, RotatesClockwiseAboutCentre) { owner()->transform.rotation = kQuarterTurn; - SpriteRenderer sprite(owner(), imagePath()); - sprite.onRenderTargetReady(target()); + SpriteRenderer sprite(owner(), image()); + image()->upload(target()); sprite.render(target().get()); EXPECT_EQ(pixelAt(kColumnX, kTopY), kLeft); @@ -88,61 +103,47 @@ TEST_F(SpriteRendererTest, RotatesClockwiseAboutCentre) { EXPECT_EQ(pixelAt(kLeftHalfX, kRowY), kBackground); } -TEST_F(SpriteRendererTest, DrawsNothingBeforeRenderTargetIsReady) { - SpriteRenderer sprite(owner(), imagePath()); +TEST_F(SpriteRendererTest, DrawsNothingBeforeImageIsUploaded) { + SpriteRenderer sprite(owner(), image()); sprite.render(target().get()); EXPECT_EQ(pixelAt(kLeftHalfX, kRowY), kBackground); } -TEST_F(SpriteRendererTest, ThrowsWhenImageIsMissing) { - EXPECT_THROW(SpriteRenderer(owner(), dir() / "missing.png"), std::runtime_error); -} +TEST_F(SpriteRendererTest, WithoutImageDrawsNothingAndNeedsNoTransform) { + owner()->transform.width = 0.0; -TEST_F(SpriteRendererTest, ThrowsWhenFileIsNotAnImage) { - const auto notAnImage = dir() / "notes.png"; - std::ofstream(notAnImage) << "not an image"; + SpriteRenderer sprite(owner(), nullptr); + sprite.render(target().get()); - EXPECT_THROW(SpriteRenderer(owner(), notAnImage), std::runtime_error); + EXPECT_EQ(pixelAt(kLeftHalfX, kRowY), kBackground); } -TEST_F(SpriteRendererTest, ThrowsWhenImagePathIsEmpty) { - EXPECT_THROW(SpriteRenderer(owner(), std::filesystem::path{}), std::invalid_argument); -} +TEST_F(SpriteRendererTest, ThrowsWhenDrawingFails) { + SpriteRenderer sprite(owner(), image()); + image()->upload(target()); -TEST_F(SpriteRendererTest, ThrowsWhenOwnerIsNull) { - EXPECT_THROW(SpriteRenderer(nullptr, imagePath()), std::invalid_argument); + // SDL refuses to copy a texture onto a renderer it was not created on. + const auto otherTarget = utils::makeSoftwareTarget(kCanvasSize, kCanvasSize); + EXPECT_THROW(sprite.render(otherTarget.get()), std::runtime_error); } -TEST_F(SpriteRendererTest, ThrowsWhenRenderTargetIsNull) { - SpriteRenderer sprite(owner(), imagePath()); - - EXPECT_THROW(sprite.onRenderTargetReady(nullptr), std::invalid_argument); +TEST_F(SpriteRendererTest, ThrowsWhenOwnerIsNull) { + EXPECT_THROW(SpriteRenderer(nullptr, image()), std::invalid_argument); } TEST_F(SpriteRendererTest, ThrowsWhenOwnerHasNoArea) { owner()->transform.width = 0.0; - EXPECT_THROW(SpriteRenderer(owner(), imagePath()), std::invalid_argument); -} - -TEST_F(SpriteRendererTest, RejectsASecondRenderTarget) { - SpriteRenderer sprite(owner(), imagePath()); - sprite.onRenderTargetReady(target()); - - EXPECT_THROW(sprite.onRenderTargetReady(target()), std::logic_error); + EXPECT_THROW(SpriteRenderer(owner(), image()), std::invalid_argument); } -// SDL frees a renderer's textures along with it, so destroying the texture afterwards would be a -// use-after-free. The texture therefore keeps its renderer alive. -TEST_F(SpriteRendererTest, KeepsRenderTargetAliveWhileTextureExists) { - auto sprite = std::make_unique(owner(), imagePath()); - sprite->onRenderTargetReady(target()); - - const std::weak_ptr weakTarget = target(); - releaseTarget(); - EXPECT_FALSE(weakTarget.expired()); - - sprite.reset(); - EXPECT_TRUE(weakTarget.expired()); +TEST_F(SpriteRendererTest, ThrowsWhenTransformIsNotFinite) { + using Transform = SceneObject::Transform; + for (const auto field : {&Transform::posX, &Transform::posY, &Transform::width, + &Transform::height, &Transform::rotation}) { + EXPECT_TRUE(rejectsTransformValue(field, std::numeric_limits::infinity())); + EXPECT_TRUE(rejectsTransformValue(field, std::numeric_limits::quiet_NaN())); + EXPECT_TRUE(rejectsTransformValue(field, kBeyondFloat)); + } } diff --git a/tests/unit_tests/temp_dir_test.cpp b/tests/unit_tests/temp_dir_test.cpp new file mode 100644 index 0000000..5b0583a --- /dev/null +++ b/tests/unit_tests/temp_dir_test.cpp @@ -0,0 +1,22 @@ +#include + +#include + +#include "utils/TempDir.hpp" + +namespace { +class TempDirTest : public ::testing::TestWithParam {}; +} // namespace + +// The suite and test names of a parameterised test contain '/'. +TEST_P(TempDirTest, IsADirectChildOfTheSystemTempFolder) { + std::filesystem::path root; + { + const utils::TempDir dir; + root = dir.path(); + EXPECT_EQ(root.parent_path(), std::filesystem::temp_directory_path()); + } + EXPECT_FALSE(std::filesystem::exists(root)); +} + +INSTANTIATE_TEST_SUITE_P(Names, TempDirTest, ::testing::Values(0)); diff --git a/tests/utils/TempDir.hpp b/tests/utils/TempDir.hpp index e9317dd..d8d5da2 100644 --- a/tests/utils/TempDir.hpp +++ b/tests/utils/TempDir.hpp @@ -3,6 +3,8 @@ #include +#include +#include #include #include #include @@ -15,9 +17,17 @@ namespace utils { class TempDir { public: TempDir() { - const auto* info = ::testing::UnitTest::GetInstance()->current_test_info(); - const std::string testName = + const auto* info = ::testing::UnitTest::GetInstance()->current_test_info(); + std::string testName = info != nullptr ? std::string(info->test_suite_name()) + "_" + info->name() : "none"; + // Parameterised and typed tests have '/' in their names, which would nest directories + // that remove_all(root) leaves behind. + std::replace_if( + testName.begin(), testName.end(), + [](unsigned char character) { + return std::isalnum(character) == 0 && character != '-' && character != '_'; + }, + '_'); root = std::filesystem::temp_directory_path() / ("neuronide_" + testName + "_" + std::to_string(std::chrono::steady_clock::now().time_since_epoch().count())); From 2559b4f9bfef7c999dfc396d2c7b0606c5e7cb7d Mon Sep 17 00:00:00 2001 From: Michal Date: Mon, 19 Oct 2026 18:42:26 +0200 Subject: [PATCH 3/4] fix: use utf8 encoding for string paths --- include/Utf8Path.hpp | 24 +++++++++++++++++++++ src/scene/Image.cpp | 8 ++++--- src/scene/components/SpriteRenderer.cpp | 9 ++++---- tests/component_tests/sprite_scene_test.cpp | 12 +++++++++++ tests/utils/SdlTestUtils.hpp | 6 ++++-- 5 files changed, 50 insertions(+), 9 deletions(-) create mode 100644 include/Utf8Path.hpp diff --git a/include/Utf8Path.hpp b/include/Utf8Path.hpp new file mode 100644 index 0000000..c579cb2 --- /dev/null +++ b/include/Utf8Path.hpp @@ -0,0 +1,24 @@ +#ifndef UTF8PATH_HPP +#define UTF8PATH_HPP + +#include +#include +#include + +// On Windows, std::filesystem converts plain std::string with the system code page (e.g. cp1250), +// while protobuf strings and SDL file names are UTF-8, so "twarz_ż.png" would arrive garbled. +// These convert through UTF-8 explicitly. On Linux both are a plain byte copy. +namespace utf8 { + +inline std::filesystem::path toPath(std::string_view text) { + return {std::u8string(text.begin(), text.end())}; +} + +inline std::string toString(const std::filesystem::path& path) { + const std::u8string text = path.u8string(); + return {text.begin(), text.end()}; +} + +} // namespace utf8 + +#endif // UTF8PATH_HPP diff --git a/src/scene/Image.cpp b/src/scene/Image.cpp index 2b0287c..f004935 100644 --- a/src/scene/Image.cpp +++ b/src/scene/Image.cpp @@ -7,6 +7,8 @@ #include #include +#include "Utf8Path.hpp" + void Image::SurfaceDeleter::operator()(SDL_Surface* surface) const noexcept { SDL_FreeSurface(surface); } @@ -16,9 +18,9 @@ void Image::TextureDeleter::operator()(SDL_Texture* texture) const noexcept { } Image::Image(std::filesystem::path path) : filePath(std::move(path)) { - pixels.reset(IMG_Load(filePath.string().c_str())); + pixels.reset(IMG_Load(utf8::toString(filePath).c_str())); if (!pixels) { - throw std::runtime_error("cannot load image '" + filePath.string() + + throw std::runtime_error("cannot load image '" + utf8::toString(filePath) + "': " + IMG_GetError()); } } @@ -30,7 +32,7 @@ void Image::upload(const std::shared_ptr& target) { SDL_Texture* created = SDL_CreateTextureFromSurface(target.get(), pixels.get()); if (created == nullptr) { - throw std::runtime_error("cannot create a texture from '" + filePath.string() + + throw std::runtime_error("cannot create a texture from '" + utf8::toString(filePath) + "': " + SDL_GetError()); } gpuTexture = std::unique_ptr(created, TextureDeleter{target}); diff --git a/src/scene/components/SpriteRenderer.cpp b/src/scene/components/SpriteRenderer.cpp index f87c85b..4832ca6 100644 --- a/src/scene/components/SpriteRenderer.cpp +++ b/src/scene/components/SpriteRenderer.cpp @@ -8,6 +8,7 @@ #include #include +#include "Utf8Path.hpp" #include "neuronide.pb.h" #include "scene/Image.hpp" #include "scene/SceneObject.hpp" @@ -64,8 +65,8 @@ void SpriteRenderer::render(SDL_Renderer* renderer) { // A null centre rotates about the centre of `destination`. if (SDL_RenderCopyExF(renderer, image->texture(), nullptr, &destination, transform.rotation, nullptr, SDL_FLIP_NONE) != 0) { - throw std::runtime_error(describe(*ownerPtr) + ": cannot draw '" + image->path().string() + - "': " + SDL_GetError()); + throw std::runtime_error(describe(*ownerPtr) + ": cannot draw '" + + utf8::toString(image->path()) + "': " + SDL_GetError()); } } @@ -73,8 +74,8 @@ std::unique_ptr SpriteRenderer::createSprite(const NeuronIDE::Compone const std::shared_ptr& owner, const ComponentBuildContext& context) { const std::string& path = protoComp.renderer().img_path(); - return std::make_unique(owner, - path.empty() ? nullptr : context.loadImage(path)); + return std::make_unique( + owner, path.empty() ? nullptr : context.loadImage(utf8::toPath(path))); } REGISTER_COMPONENT(NeuronIDE::Component::kRenderer, SpriteRenderer::createSprite) diff --git a/tests/component_tests/sprite_scene_test.cpp b/tests/component_tests/sprite_scene_test.cpp index f8ad769..4d90afe 100644 --- a/tests/component_tests/sprite_scene_test.cpp +++ b/tests/component_tests/sprite_scene_test.cpp @@ -118,6 +118,18 @@ TEST_F(SpriteSceneTest, ObjectsShowingTheSameFileShareOneImage) { EXPECT_EQ(parseExperiment()->images().size(), 1U); } +// Proto strings are UTF-8, and stimulus names need not be ASCII. +TEST_F(SpriteSceneTest, LoadsImageWithNonAsciiPath) { + const std::u8string imagePath = u8"bodźce/twarz_ż.png"; + fs::create_directories(dir() / fs::path(u8"bodźce")); + utils::writeTwoPixelPng(dir() / fs::path(imagePath), kLeft, kRight); + writeExperiment(sceneWithSprite(std::string(imagePath.begin(), imagePath.end()))); + + const auto scene = parseExperiment(); + EXPECT_TRUE(hasSprite(*scene)); + EXPECT_EQ(scene->images().size(), 1U); +} + TEST_F(SpriteSceneTest, SpriteOnObjectWithoutTransformFailsTheParse) { utils::writeTwoPixelPng(dir() / "face.png", kLeft, kRight); writeExperiment(sceneWithSprite("face.png", /*withTransform=*/false)); diff --git a/tests/utils/SdlTestUtils.hpp b/tests/utils/SdlTestUtils.hpp index d792937..258ec71 100644 --- a/tests/utils/SdlTestUtils.hpp +++ b/tests/utils/SdlTestUtils.hpp @@ -10,6 +10,8 @@ #include #include +#include "Utf8Path.hpp" + namespace utils { // One pixel as {r, g, b, a}; ints rather than Uint8 so gtest prints them as numbers. @@ -75,8 +77,8 @@ inline void writeTwoPixelPng(const std::filesystem::path& path, const Rgba& left fill(0, left); fill(1, right); - if (IMG_SavePNG(surface.get(), path.string().c_str()) != 0) { - throw sdlFailure("IMG_SavePNG " + path.string()); + if (IMG_SavePNG(surface.get(), utf8::toString(path).c_str()) != 0) { + throw sdlFailure("IMG_SavePNG " + utf8::toString(path)); } } From 22ee8745694815167a024ed4c693d02e3dacd2aa Mon Sep 17 00:00:00 2001 From: Michal Date: Tue, 20 Oct 2026 10:19:17 +0200 Subject: [PATCH 4/4] chore: mark missing log messages with TODO(quill) for future quill logger update --- src/renderer/Renderer.cpp | 2 ++ src/scene/Image.cpp | 2 ++ src/scene/ImageLibrary.cpp | 2 ++ src/scene/components/SpriteRenderer.cpp | 6 ++++++ 4 files changed, 12 insertions(+) diff --git a/src/renderer/Renderer.cpp b/src/renderer/Renderer.cpp index 31476c8..6c0685a 100644 --- a/src/renderer/Renderer.cpp +++ b/src/renderer/Renderer.cpp @@ -36,6 +36,8 @@ bool Renderer::pollEvents() { quit = true; } else if (event.type == SDL_RENDER_DEVICE_RESET) { // Every texture is gone (e.g. Direct3D after a lock screen or UAC prompt). + // TODO(quill): warning with lsl::local_clock(): frames around a reset may be dropped, + // and whoever analyses the recording needs to know when. prepareScene(); } } diff --git a/src/scene/Image.cpp b/src/scene/Image.cpp index f004935..edcb9d1 100644 --- a/src/scene/Image.cpp +++ b/src/scene/Image.cpp @@ -23,6 +23,8 @@ Image::Image(std::filesystem::path path) : filePath(std::move(path)) { throw std::runtime_error("cannot load image '" + utf8::toString(filePath) + "': " + IMG_GetError()); } + // TODO(quill): debug, the path and decoded size (pixels->w x pixels->h). The pixels stay in + // memory for the whole run, so large images are worth seeing. } void Image::upload(const std::shared_ptr& target) { diff --git a/src/scene/ImageLibrary.cpp b/src/scene/ImageLibrary.cpp index ae34fe2..d5e472d 100644 --- a/src/scene/ImageLibrary.cpp +++ b/src/scene/ImageLibrary.cpp @@ -5,6 +5,7 @@ std::shared_ptr ImageLibrary::load(const std::filesystem::path& path) { const std::filesystem::path key = path.lexically_normal(); if (const auto found = images.find(key); found != images.end()) { + // TODO(quill): debug, "'' is already loaded and is shared". return found->second; } @@ -17,4 +18,5 @@ void ImageLibrary::upload(const std::shared_ptr& target) { for (const auto& [path, image] : images) { image->upload(target); } + // TODO(quill): info, how many images were uploaded. } diff --git a/src/scene/components/SpriteRenderer.cpp b/src/scene/components/SpriteRenderer.cpp index 4832ca6..3bf928e 100644 --- a/src/scene/components/SpriteRenderer.cpp +++ b/src/scene/components/SpriteRenderer.cpp @@ -32,6 +32,7 @@ SpriteRenderer::SpriteRenderer(const std::shared_ptr& owner, throw std::invalid_argument("SpriteRenderer: owner must not be null"); } if (!this->image) { + // TODO(quill): warning, "SpriteRenderer on '' has no image and draws nothing". return; } @@ -47,6 +48,7 @@ SpriteRenderer::SpriteRenderer(const std::shared_ptr& owner, throw std::invalid_argument(describe(*owner) + ": the object needs a positive width and height"); } + // TODO(quill): debug, the image path and the transform it is drawn into. } void SpriteRenderer::update(const Context& context) { (void)context; } @@ -54,6 +56,8 @@ void SpriteRenderer::update(const Context& context) { (void)context; } void SpriteRenderer::render(SDL_Renderer* renderer) { const auto ownerPtr = owner.lock(); if (renderer == nullptr || !image || image->texture() == nullptr || !ownerPtr) { + // TODO(quill): warning when there is an image but no texture, or the owner is gone: the + // stimulus is missing from the screen. Once per sprite, not every frame. return; } @@ -74,6 +78,8 @@ std::unique_ptr SpriteRenderer::createSprite(const NeuronIDE::Compone const std::shared_ptr& owner, const ComponentBuildContext& context) { const std::string& path = protoComp.renderer().img_path(); + // TODO(quill): warning when img_path is empty but texture_path is set: the exporter filled the + // field the runtime does not read. return std::make_unique( owner, path.empty() ? nullptr : context.loadImage(utf8::toPath(path))); }