From 92e3ea90a5f8ddc114c43d89805ffb024137b8b0 Mon Sep 17 00:00:00 2001 From: Michael Tao Date: Sat, 28 Mar 2026 12:10:05 -0400 Subject: [PATCH 1/2] feat: enhance scene graph with O(1) feature lookup, signals, FeatureIndex, and traversal API Add four incremental enhancements to the Object+AbstractFeature scene graph: 1. O(1) feature lookup via type_index map in Object (dynamic_cast fallback for base-class queries) 2. Feature signals using palacaze/sigslot: on_feature_added/removing, on_child_added/removing on Object, and on_modified on AbstractFeature 3. Scene-level FeatureIndex that subscribes to signals and maintains a type->objects map for efficient "give me all objects with X" queries 4. Traversal API: for_each_descendant, for_each_ancestor, find_descendant with pruning support via bool return values Adds sigslot v1.2.3 as a wrap-git subproject with custom meson.build. All 8 test suites pass (23,717 assertions). --- subprojects/.gitignore | 1 + subprojects/packagefiles/sigslot/meson.build | 11 + subprojects/sigslot.wrap | 7 + .../balsa/scene_graph/AbstractFeature.hpp | 11 +- .../balsa/scene_graph/FeatureIndex.hpp | 88 +++ .../include/balsa/scene_graph/Object.hpp | 154 +++++- visualization/meson.build | 4 +- .../src/scene_graph/FeatureIndex.cpp | 82 +++ visualization/src/scene_graph/Object.cpp | 58 +- visualization/tests/test_scene_graph.cpp | 501 ++++++++++++++++-- 10 files changed, 838 insertions(+), 79 deletions(-) create mode 100644 subprojects/packagefiles/sigslot/meson.build create mode 100644 subprojects/sigslot.wrap create mode 100644 visualization/include/balsa/scene_graph/FeatureIndex.hpp create mode 100644 visualization/src/scene_graph/FeatureIndex.cpp diff --git a/subprojects/.gitignore b/subprojects/.gitignore index 8f7a5ff..1891f96 100644 --- a/subprojects/.gitignore +++ b/subprojects/.gitignore @@ -1,5 +1,6 @@ */ !patches/ +!packagefiles/ # Auto-generated wrap-redirects from transitive deps lua.wrap diff --git a/subprojects/packagefiles/sigslot/meson.build b/subprojects/packagefiles/sigslot/meson.build new file mode 100644 index 0000000..63de5b9 --- /dev/null +++ b/subprojects/packagefiles/sigslot/meson.build @@ -0,0 +1,11 @@ +project('sigslot', 'cpp', + version: '1.2.3', + license: 'MIT', + default_options: ['cpp_std=c++14']) + +thread_dep = dependency('threads') + +sigslot_dep = declare_dependency( + include_directories: include_directories('include'), + dependencies: thread_dep, +) diff --git a/subprojects/sigslot.wrap b/subprojects/sigslot.wrap new file mode 100644 index 0000000..60557cf --- /dev/null +++ b/subprojects/sigslot.wrap @@ -0,0 +1,7 @@ +[wrap-git] +url = https://github.com/palacaze/sigslot.git +revision = v1.2.3 +patch_directory = sigslot + +[provide] +sigslot = sigslot_dep diff --git a/visualization/include/balsa/scene_graph/AbstractFeature.hpp b/visualization/include/balsa/scene_graph/AbstractFeature.hpp index febf16b..867e2d2 100644 --- a/visualization/include/balsa/scene_graph/AbstractFeature.hpp +++ b/visualization/include/balsa/scene_graph/AbstractFeature.hpp @@ -1,6 +1,8 @@ #if !defined(BALSA_SCENE_GRAPH_ABSTRACT_FEATURE_HPP) #define BALSA_SCENE_GRAPH_ABSTRACT_FEATURE_HPP +#include + namespace balsa::scene_graph { class Object; @@ -21,6 +23,13 @@ class AbstractFeature { Object &object() { return *_object; } const Object &object() const { return *_object; } + // Signal emitted when this feature's data has changed. + // Subscribers receive a reference to the modified feature. + sigslot::signal_st on_modified; + + // Call this from derived classes after mutating internal state. + void mark_modified() { on_modified(*this); } + protected: AbstractFeature() = default; @@ -34,6 +43,6 @@ class AbstractFeature { Object *_object = nullptr; }; -}// namespace balsa::scene_graph +} // namespace balsa::scene_graph #endif diff --git a/visualization/include/balsa/scene_graph/FeatureIndex.hpp b/visualization/include/balsa/scene_graph/FeatureIndex.hpp new file mode 100644 index 0000000..17c29f3 --- /dev/null +++ b/visualization/include/balsa/scene_graph/FeatureIndex.hpp @@ -0,0 +1,88 @@ +#pragma once + +#include +#include +#include +#include + +#include + +#include "AbstractFeature.hpp" +#include "Object.hpp" + +namespace balsa::scene_graph { + +// ── FeatureIndex ──────────────────────────────────────────────────── +// +// Maintains a scene-level index that maps feature types to the set of +// Objects that carry a feature of that type. Subscribes to Object +// signals (on_feature_added, on_feature_removing, on_child_added, +// on_child_removing) so the index stays up to date automatically. +// +// Usage: +// FeatureIndex index; +// index.track(root); // recursively subscribe to root + all descendants +// +// auto meshes = index.objects_with(); // O(1) lookup +// for (Object* obj : meshes) { ... } +// +// Inherits sigslot::observer_st so that all signal connections are +// automatically severed when the FeatureIndex is destroyed. + +class FeatureIndex : public sigslot::observer_st { + public: + FeatureIndex() = default; + ~FeatureIndex() override = default; + + // Non-copyable, non-movable (observer_st manages connections). + FeatureIndex(const FeatureIndex &) = delete; + FeatureIndex &operator=(const FeatureIndex &) = delete; + FeatureIndex(FeatureIndex &&) = delete; + FeatureIndex &operator=(FeatureIndex &&) = delete; + + // Recursively subscribe to an Object and all its current + // descendants. Indexes every feature already present. + void track(Object &root); + + // Stop tracking an Object (and its descendants). + void untrack(Object &root); + + // Return all Objects that currently carry a feature of type F. + template + auto objects_with() const -> std::span; + + // Return the count of Objects carrying feature type F. + template + auto count() const -> std::size_t; + + private: + void track_single(Object &obj); + void untrack_single(Object &obj); + + // Signal handlers. + void on_feature_added(Object &obj, AbstractFeature &feature); + void on_feature_removing(Object &obj, AbstractFeature &feature); + void on_child_added(Object &parent, Object &child); + void on_child_removing(Object &parent, Object &child); + + // type_index -> vector of Object pointers carrying that feature type. + std::unordered_map> _index; +}; + +// ── Template implementations ──────────────────────────────────────── + +template +auto FeatureIndex::objects_with() const -> std::span { + auto it = _index.find(std::type_index(typeid(F))); + if (it == _index.end()) return {}; + return it->second; +} + +template +auto FeatureIndex::count() const -> std::size_t { + auto it = _index.find(std::type_index(typeid(F))); + if (it == _index.end()) return 0; + return it->second.size(); +} + +} // namespace balsa::scene_graph diff --git a/visualization/include/balsa/scene_graph/Object.hpp b/visualization/include/balsa/scene_graph/Object.hpp index bee14be..6c5aa31 100644 --- a/visualization/include/balsa/scene_graph/Object.hpp +++ b/visualization/include/balsa/scene_graph/Object.hpp @@ -6,6 +6,8 @@ #include #include #include +#include +#include #include #include "AbstractFeature.hpp" @@ -101,7 +103,9 @@ class Object { // ── Hierarchy ─────────────────────────────────────────────────── Object *parent() const { return _parent; } - const std::vector> &children() const { return _children; } + const std::vector> &children() const { + return _children; + } std::size_t children_count() const { return _children.size(); } // Create a new child Object with the given name. @@ -115,24 +119,50 @@ class Object { // Returns nullptr if this Object has no parent. std::unique_ptr detach(); + // ── Traversal ─────────────────────────────────────────────────── + + // Visit every descendant (depth-first, pre-order). + // If fn returns bool, returning false prunes the subtree. + // If fn returns void, all descendants are visited. + template + void for_each_descendant(Fn &&fn); + + template + void for_each_descendant(Fn &&fn) const; + + // Walk the ancestor chain (parent, grandparent, ...). + // If fn returns bool, returning false stops traversal. + template + void for_each_ancestor(Fn &&fn) const; + + // Return the first descendant (DFS pre-order) satisfying pred, + // or nullptr if none. + template + auto find_descendant(Pred &&pred) -> Object *; + + template + auto find_descendant(Pred &&pred) const -> const Object *; + // ── Features ──────────────────────────────────────────────────── // Construct and attach a feature of type F. // F must derive from AbstractFeature. // Returns a reference to the newly created feature. - template + template F &emplace_feature(Args &&...args); // Find the first attached feature of type F (exact or derived). // Returns nullptr if no such feature is found. - template + template F *find_feature(); - template + template const F *find_feature() const; // Access all features (for advanced iteration). - const std::vector> &features() const { return _features; } + const std::vector> &features() const { + return _features; + } // Remove a feature by pointer. Returns true if the feature was found // and removed. The feature is destroyed. @@ -148,19 +178,36 @@ class Object { // Used for scene fixtures such as the default camera. bool permanent = false; + // ── Signals ───────────────────────────────────────────────────── + + // Emitted after a feature is constructed and attached. + sigslot::signal_st on_feature_added; + + // Emitted just before a feature is destroyed and removed. + sigslot::signal_st on_feature_removing; + + // Emitted after a child Object is added to this node. + sigslot::signal_st on_child_added; + + // Emitted just before a child Object is removed from this node. + sigslot::signal_st on_child_removing; + private: - Vec3f _translation;// default: {0, 0, 0} - Quaternionf _rotation{ 1, 0, 0, 0 };// identity quaternion (w=1) - Vec3f _scale;// default: {1, 1, 1} — set in ctor + Vec3f _translation; // default: {0, 0, 0} + Quaternionf _rotation{1, 0, 0, 0}; // identity quaternion (w=1) + Vec3f _scale; // default: {1, 1, 1} — set in ctor Object *_parent = nullptr; std::vector> _children; std::vector> _features; -}; + // O(1) lookup by exact type. Keyed by typeid(F) used at + // emplace_feature() time. Points into _features' unique_ptrs. + std::unordered_map _feature_map; +}; // ── Template implementations ──────────────────────────────────────── -template +template F &Object::emplace_feature(Args &&...args) { static_assert(std::is_base_of_v, "F must derive from AbstractFeature"); @@ -168,25 +215,106 @@ F &Object::emplace_feature(Args &&...args) { ptr->_object = this; F &ref = *ptr; _features.push_back(std::move(ptr)); + _feature_map[std::type_index(typeid(F))] = &ref; + on_feature_added(*this, ref); return ref; } -template +template F *Object::find_feature() { + // Fast path: exact type match via map. + if (auto it = _feature_map.find(std::type_index(typeid(F))); + it != _feature_map.end()) { + return static_cast(it->second); + } + // Slow path: base-class query via dynamic_cast scan. for (auto &f : _features) { if (auto *p = dynamic_cast(f.get())) return p; } return nullptr; } -template +template const F *Object::find_feature() const { + // Fast path: exact type match via map. + if (auto it = _feature_map.find(std::type_index(typeid(F))); + it != _feature_map.end()) { + return static_cast(it->second); + } + // Slow path: base-class query via dynamic_cast scan. for (auto &f : _features) { if (auto *p = dynamic_cast(f.get())) return p; } return nullptr; } -}// namespace balsa::scene_graph +// ── Traversal implementations ─────────────────────────────────────── + +namespace detail { + // Detect if Fn(Object&) returns bool. + template + constexpr bool returns_bool_v = + std::is_same_v, bool>; +} // namespace detail + +template +void Object::for_each_descendant(Fn &&fn) { + for (auto &child : _children) { + if (!child) continue; + if constexpr (detail::returns_bool_v) { + if (!fn(*child)) continue; // false -> prune subtree + } else { + fn(*child); + } + child->for_each_descendant(fn); + } +} + +template +void Object::for_each_descendant(Fn &&fn) const { + for (auto &child : _children) { + if (!child) continue; + if constexpr (detail::returns_bool_v) { + if (!fn(*child)) continue; + } else { + fn(*child); + } + child->for_each_descendant(fn); + } +} + +template +void Object::for_each_ancestor(Fn &&fn) const { + for (Object *p = _parent; p != nullptr; p = p->_parent) { + if constexpr (detail::returns_bool_v) { + if (!fn(*p)) return; + } else { + fn(*p); + } + } +} + +template +auto Object::find_descendant(Pred &&pred) -> Object * { + for (auto &child : _children) { + if (!child) continue; + if (pred(*child)) return child.get(); + if (auto *found = child->find_descendant(pred)) return found; + } + return nullptr; +} + +template +auto Object::find_descendant(Pred &&pred) const -> const Object * { + for (auto &child : _children) { + if (!child) continue; + if (pred(std::as_const(*child))) return child.get(); + if (auto *found = std::as_const(*child).find_descendant(pred)) + return found; + } + return nullptr; +} + +} // namespace balsa::scene_graph #endif diff --git a/visualization/meson.build b/visualization/meson.build index b8462f6..d7f0d56 100644 --- a/visualization/meson.build +++ b/visualization/meson.build @@ -35,9 +35,10 @@ endif colormap_proj = subproject('colormap_shaders') colormap_shaders_dep = colormap_proj.get_variable('colormap_shaders_dep') +sigslot_dep = dependency('sigslot', fallback: ['sigslot', 'sigslot_dep']) # Public deps: propagated to consumers via declare_dependency(). -visualization_public_deps = [vulkan_dep, shaderc_dep, dl_lib, colormap_shaders_dep] +visualization_public_deps = [vulkan_dep, shaderc_dep, dl_lib, colormap_shaders_dep, sigslot_dep] # Private deps: used only for building the library (not propagated). visualization_private_deps = [] @@ -58,6 +59,7 @@ visualization_sources = [ 'src/scene_graph/MeshData.cpp', 'src/scene_graph/BVHData.cpp', 'src/scene_graph/DrawableGroup.cpp', + 'src/scene_graph/FeatureIndex.cpp', ] if get_option('qt') diff --git a/visualization/src/scene_graph/FeatureIndex.cpp b/visualization/src/scene_graph/FeatureIndex.cpp new file mode 100644 index 0000000..07665bb --- /dev/null +++ b/visualization/src/scene_graph/FeatureIndex.cpp @@ -0,0 +1,82 @@ +#include "balsa/scene_graph/FeatureIndex.hpp" + +#include + +namespace balsa::scene_graph { + +// ── Public API ────────────────────────────────────────────────────── + +void FeatureIndex::track(Object &root) { + track_single(root); + for (auto &child_ptr : root.children()) { + if (child_ptr) track(*child_ptr); + } +} + +void FeatureIndex::untrack(Object &root) { + for (auto &child_ptr : root.children()) { + if (child_ptr) untrack(*child_ptr); + } + untrack_single(root); +} + +// ── Internals ─────────────────────────────────────────────────────── + +void FeatureIndex::track_single(Object &obj) { + // Index all features already present on this object. + for (auto &f : obj.features()) { + if (f) { _index[std::type_index(typeid(*f))].push_back(&obj); } + } + + // Subscribe to future feature add/remove. + obj.on_feature_added.connect(&FeatureIndex::on_feature_added, this); + obj.on_feature_removing.connect(&FeatureIndex::on_feature_removing, this); + + // Subscribe to child add/remove so we auto-track new children. + obj.on_child_added.connect(&FeatureIndex::on_child_added, this); + obj.on_child_removing.connect(&FeatureIndex::on_child_removing, this); +} + +void FeatureIndex::untrack_single(Object &obj) { + // Disconnect all signals from this object to us. + obj.on_feature_added.disconnect(this); + obj.on_feature_removing.disconnect(this); + obj.on_child_added.disconnect(this); + obj.on_child_removing.disconnect(this); + + // Remove this object from the index. + for (auto &f : obj.features()) { + if (!f) continue; + auto ti = std::type_index(typeid(*f)); + auto map_it = _index.find(ti); + if (map_it == _index.end()) continue; + auto &vec = map_it->second; + vec.erase(std::remove(vec.begin(), vec.end(), &obj), vec.end()); + if (vec.empty()) _index.erase(map_it); + } +} + +// ── Signal handlers ───────────────────────────────────────────────── + +void FeatureIndex::on_feature_added(Object &obj, AbstractFeature &feature) { + _index[std::type_index(typeid(feature))].push_back(&obj); +} + +void FeatureIndex::on_feature_removing(Object &obj, AbstractFeature &feature) { + auto ti = std::type_index(typeid(feature)); + auto map_it = _index.find(ti); + if (map_it == _index.end()) return; + auto &vec = map_it->second; + vec.erase(std::remove(vec.begin(), vec.end(), &obj), vec.end()); + if (vec.empty()) _index.erase(map_it); +} + +void FeatureIndex::on_child_added(Object & /*parent*/, Object &child) { + track(child); +} + +void FeatureIndex::on_child_removing(Object & /*parent*/, Object &child) { + untrack(child); +} + +} // namespace balsa::scene_graph diff --git a/visualization/src/scene_graph/Object.cpp b/visualization/src/scene_graph/Object.cpp index ceab8c8..d6e82e3 100644 --- a/visualization/src/scene_graph/Object.cpp +++ b/visualization/src/scene_graph/Object.cpp @@ -11,12 +11,11 @@ namespace balsa::scene_graph { namespace { constexpr float k_deg2rad = std::numbers::pi_v / 180.0f; constexpr float k_rad2deg = 180.0f / std::numbers::pi_v; -}// namespace +} // namespace // ── Constructor / Destructor ──────────────────────────────────────── -Object::Object(std::string name) - : name(std::move(name)) { +Object::Object(std::string name) : name(std::move(name)) { // _translation: Vec3f default-constructs to zero. // _rotation: explicitly initialized to identity quaternion {1,0,0,0}. // _scale: needs explicit init to {1,1,1}. @@ -30,16 +29,12 @@ Object::~Object() = default; // ── Move operations ───────────────────────────────────────────────── Object::Object(Object &&other) noexcept - : name(std::move(other.name)), - visible(other.visible), - selectable(other.selectable), - _translation(other._translation), - _rotation(other._rotation), - _scale(other._scale), - _parent(other._parent), + : name(std::move(other.name)), visible(other.visible), + selectable(other.selectable), _translation(other._translation), + _rotation(other._rotation), _scale(other._scale), _parent(other._parent), _children(std::move(other._children)), - _features(std::move(other._features)) { - + _features(std::move(other._features)), + _feature_map(std::move(other._feature_map)) { // Fix up children's parent pointers. for (auto &child : _children) { if (child) child->_parent = this; @@ -62,6 +57,7 @@ Object &Object::operator=(Object &&other) noexcept { _parent = other._parent; _children = std::move(other._children); _features = std::move(other._features); + _feature_map = std::move(other._feature_map); for (auto &child : _children) { if (child) child->_parent = this; @@ -141,9 +137,7 @@ void Object::set_from_transform(const AffineTransformf &xf) { } AffineTransformf Object::world_transform() const { - if (_parent) { - return _parent->world_transform() * local_transform(); - } + if (_parent) { return _parent->world_transform() * local_transform(); } return local_transform(); } @@ -153,6 +147,7 @@ Object &Object::add_child(std::string child_name) { auto child = std::make_unique(std::move(child_name)); child->_parent = this; _children.push_back(std::move(child)); + on_child_added(*this, *_children.back()); return *_children.back(); } @@ -163,8 +158,12 @@ Object &Object::add_child(std::unique_ptr child) { } // Detach from previous parent if needed. if (child->_parent) { + child->_parent->on_child_removing(*child->_parent, *child); auto &siblings = child->_parent->_children; - auto it = std::find_if(siblings.begin(), siblings.end(), [&](const auto &p) { return p.get() == child.get(); }); + auto it = + std::find_if(siblings.begin(), siblings.end(), [&](const auto &p) { + return p.get() == child.get(); + }); if (it != siblings.end()) { // Release ownership without destroying — we're taking it. it->release(); @@ -173,18 +172,23 @@ Object &Object::add_child(std::unique_ptr child) { } child->_parent = this; _children.push_back(std::move(child)); + on_child_added(*this, *_children.back()); return *_children.back(); } std::unique_ptr Object::detach() { if (!_parent) return nullptr; - if (permanent) return nullptr;// permanent objects cannot be detached + if (permanent) return nullptr; // permanent objects cannot be detached auto &siblings = _parent->_children; - auto it = std::find_if(siblings.begin(), siblings.end(), [this](const auto &p) { return p.get() == this; }); + auto it = std::find_if(siblings.begin(), + siblings.end(), + [this](const auto &p) { return p.get() == this; }); if (it == siblings.end()) return nullptr; + _parent->on_child_removing(*_parent, *this); + std::unique_ptr self = std::move(*it); siblings.erase(it); _parent = nullptr; @@ -195,10 +199,24 @@ std::unique_ptr Object::detach() { bool Object::remove_feature(AbstractFeature *feature) { if (!feature) return false; - auto it = std::find_if(_features.begin(), _features.end(), [feature](const auto &p) { return p.get() == feature; }); + auto it = + std::find_if(_features.begin(), + _features.end(), + [feature](const auto &p) { return p.get() == feature; }); if (it == _features.end()) return false; + + // Signal before destruction. + on_feature_removing(*this, *feature); + + // Remove from type map. Use typeid of the concrete object so it matches + // the key inserted by emplace_feature(). + auto map_it = _feature_map.find(std::type_index(typeid(*feature))); + if (map_it != _feature_map.end() && map_it->second == feature) { + _feature_map.erase(map_it); + } + _features.erase(it); return true; } -}// namespace balsa::scene_graph +} // namespace balsa::scene_graph diff --git a/visualization/tests/test_scene_graph.cpp b/visualization/tests/test_scene_graph.cpp index 3bb12f2..5a5828e 100644 --- a/visualization/tests/test_scene_graph.cpp +++ b/visualization/tests/test_scene_graph.cpp @@ -1,16 +1,17 @@ #define CATCH_CONFIG_MAIN #include -#include #include +#include +#include #include +#include #include #include #include #include - TEST_CASE("Object creation and hierarchy", "[scene_graph]") { using namespace balsa::scene_graph; @@ -98,7 +99,7 @@ TEST_CASE("MeshData geometry and version tracking", "[scene_graph]") { uint64_t v1 = md.version(); // Set triangle indices - std::vector tri_idx = { 0, 1, 2, 0, 2, 3 }; + std::vector tri_idx = {0, 1, 2, 0, 2, 3}; md.set_triangle_indices(tri_idx); CHECK(md.triangle_count() == 2); CHECK(md.has_triangle_indices()); @@ -106,7 +107,7 @@ TEST_CASE("MeshData geometry and version tracking", "[scene_graph]") { uint64_t v2 = md.version(); // Set edge indices - std::vector edge_idx = { 0, 1, 1, 2, 2, 3, 3, 0 }; + std::vector edge_idx = {0, 1, 1, 2, 2, 3, 3, 0}; md.set_edge_indices(edge_idx); CHECK(md.edge_count() == 4); CHECK(md.has_edge_indices()); @@ -135,7 +136,7 @@ TEST_CASE("MeshData auto-derives edges from triangles", "[scene_graph]") { // tri 1: (0, 2, 3) // // Expected unique edges: 0-1, 1-2, 0-2, 2-3, 0-3 = 5 edges - std::vector tri_idx = { 0, 1, 2, 0, 2, 3 }; + std::vector tri_idx = {0, 1, 2, 0, 2, 3}; md.set_triangle_indices(tri_idx); CHECK(md.has_triangle_indices()); @@ -148,9 +149,7 @@ TEST_CASE("MeshData auto-derives edges from triangles", "[scene_graph]") { // Verify edge indices are valid vertex references auto edges = md.edge_indices(); - for (std::size_t i = 0; i < edges.size(); ++i) { - CHECK(edges[i] < 4); - } + for (std::size_t i = 0; i < edges.size(); ++i) { CHECK(edges[i] < 4); } } TEST_CASE("MeshData explicit edges override auto-derived", "[scene_graph]") { @@ -159,19 +158,19 @@ TEST_CASE("MeshData explicit edges override auto-derived", "[scene_graph]") { MeshData md; // Set triangles first (auto-derives 5 edges) - std::vector tri_idx = { 0, 1, 2, 0, 2, 3 }; + std::vector tri_idx = {0, 1, 2, 0, 2, 3}; md.set_triangle_indices(tri_idx); CHECK(md.edge_count() == 5); // Explicitly set fewer edges — should override - std::vector edge_idx = { 0, 1, 2, 3 }; + std::vector edge_idx = {0, 1, 2, 3}; md.set_edge_indices(edge_idx); CHECK(md.edge_count() == 2); // Setting triangles again should NOT override explicit edges - std::vector tri_idx2 = { 0, 1, 2 }; + std::vector tri_idx2 = {0, 1, 2}; md.set_triangle_indices(tri_idx2); - CHECK(md.edge_count() == 2);// Still the explicit edges + CHECK(md.edge_count() == 2); // Still the explicit edges } TEST_CASE("MeshData topology from single triangle", "[scene_graph]") { @@ -179,12 +178,12 @@ TEST_CASE("MeshData topology from single triangle", "[scene_graph]") { MeshData md; - std::vector tri_idx = { 0, 1, 2 }; + std::vector tri_idx = {0, 1, 2}; md.set_triangle_indices(tri_idx); CHECK(md.has_topology()); CHECK(md.triangle_count() == 1); - CHECK(md.edge_count() == 3);// A single triangle has 3 edges + CHECK(md.edge_count() == 3); // A single triangle has 3 edges } TEST_CASE("MeshData no topology without triangles", "[scene_graph]") { @@ -194,12 +193,10 @@ TEST_CASE("MeshData no topology without triangles", "[scene_graph]") { // Only set positions and explicit edges — no topology std::vector positions(4); - for (auto &p : positions) { - p(0) = p(1) = p(2) = 0.0f; - } + for (auto &p : positions) { p(0) = p(1) = p(2) = 0.0f; } md.set_positions(positions); - std::vector edge_idx = { 0, 1, 1, 2 }; + std::vector edge_idx = {0, 1, 1, 2}; md.set_edge_indices(edge_idx); CHECK_FALSE(md.has_topology()); @@ -285,7 +282,8 @@ TEST_CASE("Object default TRS is identity", "[scene_graph][trs]") { for (int r = 0; r < 4; ++r) { for (int c = 0; c < 4; ++c) { float expected = (r == c) ? 1.0f : 0.0f; - CHECK(static_cast(m(r, c)) == Catch::Approx(expected).margin(1e-6f)); + CHECK(static_cast(m(r, c)) + == Catch::Approx(expected).margin(1e-6f)); } } } @@ -314,7 +312,8 @@ TEST_CASE("Object set/get translation", "[scene_graph][trs]") { for (int r = 0; r < 3; ++r) { for (int c = 0; c < 3; ++c) { float expected = (r == c) ? 1.0f : 0.0f; - CHECK(static_cast(m(r, c)) == Catch::Approx(expected).margin(1e-6f)); + CHECK(static_cast(m(r, c)) + == Catch::Approx(expected).margin(1e-6f)); } } } @@ -356,7 +355,8 @@ TEST_CASE("Object set_uniform_scale", "[scene_graph][trs]") { CHECK(static_cast(obj.scale_factors()(2)) == Catch::Approx(5.0f)); } -TEST_CASE("Object translate() adds to current translation", "[scene_graph][trs]") { +TEST_CASE("Object translate() adds to current translation", + "[scene_graph][trs]") { using namespace balsa::scene_graph; Object obj("accum"); @@ -410,9 +410,9 @@ TEST_CASE("Object euler angle round-trip", "[scene_graph][trs]") { Object obj("euler"); Vec3f euler_deg; - euler_deg(0) = 30.0f;// pitch (X) - euler_deg(1) = 45.0f;// yaw (Y) - euler_deg(2) = 60.0f;// roll (Z) + euler_deg(0) = 30.0f; // pitch (X) + euler_deg(1) = 45.0f; // yaw (Y) + euler_deg(2) = 60.0f; // roll (Z) obj.set_rotation_euler(euler_deg); Vec3f result = obj.rotation_euler(); @@ -421,7 +421,8 @@ TEST_CASE("Object euler angle round-trip", "[scene_graph][trs]") { CHECK(static_cast(result(2)) == Catch::Approx(60.0f).margin(0.01f)); } -TEST_CASE("Object euler angle zero is identity rotation", "[scene_graph][trs]") { +TEST_CASE("Object euler angle zero is identity rotation", + "[scene_graph][trs]") { using namespace balsa::scene_graph; Object obj("euler_zero"); @@ -433,10 +434,14 @@ TEST_CASE("Object euler angle zero is identity rotation", "[scene_graph][trs]") obj.set_rotation_euler(zero); // Should produce identity quaternion. - CHECK(static_cast(obj.rotation().w()) == Catch::Approx(1.0f).margin(1e-6f)); - CHECK(static_cast(obj.rotation().x()) == Catch::Approx(0.0f).margin(1e-6f)); - CHECK(static_cast(obj.rotation().y()) == Catch::Approx(0.0f).margin(1e-6f)); - CHECK(static_cast(obj.rotation().z()) == Catch::Approx(0.0f).margin(1e-6f)); + CHECK(static_cast(obj.rotation().w()) + == Catch::Approx(1.0f).margin(1e-6f)); + CHECK(static_cast(obj.rotation().x()) + == Catch::Approx(0.0f).margin(1e-6f)); + CHECK(static_cast(obj.rotation().y()) + == Catch::Approx(0.0f).margin(1e-6f)); + CHECK(static_cast(obj.rotation().z()) + == Catch::Approx(0.0f).margin(1e-6f)); } TEST_CASE("Object reset_transform restores identity", "[scene_graph][trs]") { @@ -511,23 +516,33 @@ TEST_CASE("Object set_from_transform round-trip", "[scene_graph][trs]") { dst.set_from_transform(xf); // Translation should match. - CHECK(static_cast(dst.translation()(0)) == Catch::Approx(3.0f).margin(1e-4f)); - CHECK(static_cast(dst.translation()(1)) == Catch::Approx(-1.0f).margin(1e-4f)); - CHECK(static_cast(dst.translation()(2)) == Catch::Approx(7.0f).margin(1e-4f)); + CHECK(static_cast(dst.translation()(0)) + == Catch::Approx(3.0f).margin(1e-4f)); + CHECK(static_cast(dst.translation()(1)) + == Catch::Approx(-1.0f).margin(1e-4f)); + CHECK(static_cast(dst.translation()(2)) + == Catch::Approx(7.0f).margin(1e-4f)); // Scale should match. - CHECK(static_cast(dst.scale_factors()(0)) == Catch::Approx(2.0f).margin(1e-4f)); - CHECK(static_cast(dst.scale_factors()(1)) == Catch::Approx(0.5f).margin(1e-4f)); - CHECK(static_cast(dst.scale_factors()(2)) == Catch::Approx(1.5f).margin(1e-4f)); + CHECK(static_cast(dst.scale_factors()(0)) + == Catch::Approx(2.0f).margin(1e-4f)); + CHECK(static_cast(dst.scale_factors()(1)) + == Catch::Approx(0.5f).margin(1e-4f)); + CHECK(static_cast(dst.scale_factors()(2)) + == Catch::Approx(1.5f).margin(1e-4f)); // Euler angles should match (check via re-extraction). Vec3f dst_euler = dst.rotation_euler(); - CHECK(static_cast(dst_euler(0)) == Catch::Approx(20.0f).margin(0.1f)); - CHECK(static_cast(dst_euler(1)) == Catch::Approx(35.0f).margin(0.1f)); - CHECK(static_cast(dst_euler(2)) == Catch::Approx(-10.0f).margin(0.1f)); + CHECK(static_cast(dst_euler(0)) + == Catch::Approx(20.0f).margin(0.1f)); + CHECK(static_cast(dst_euler(1)) + == Catch::Approx(35.0f).margin(0.1f)); + CHECK(static_cast(dst_euler(2)) + == Catch::Approx(-10.0f).margin(0.1f)); } -TEST_CASE("Object local_transform composes T*R*S correctly", "[scene_graph][trs]") { +TEST_CASE("Object local_transform composes T*R*S correctly", + "[scene_graph][trs]") { using namespace balsa::scene_graph; Object obj("trs"); @@ -566,7 +581,8 @@ TEST_CASE("Object local_transform composes T*R*S correctly", "[scene_graph][trs] CHECK(static_cast(m(2, 2)) == Catch::Approx(1.0f).margin(1e-5f)); } -TEST_CASE("Object world_transform composes parent chain", "[scene_graph][trs]") { +TEST_CASE("Object world_transform composes parent chain", + "[scene_graph][trs]") { using namespace balsa::scene_graph; Object root("root"); @@ -596,7 +612,8 @@ TEST_CASE("Object world_transform composes parent chain", "[scene_graph][trs]") CHECK(static_cast(child_world(2, 3)) == Catch::Approx(0.0f)); } -TEST_CASE("Object world_transform with rotation propagation", "[scene_graph][trs]") { +TEST_CASE("Object world_transform with rotation propagation", + "[scene_graph][trs]") { using namespace balsa::scene_graph; // Parent has 90° rotation around Z. @@ -616,9 +633,12 @@ TEST_CASE("Object world_transform with rotation propagation", "[scene_graph][trs // In world space, the child's translation should be rotated by // parent's 90°-Z: (1,0,0) → (0,1,0). auto child_world = child.world_transform().to_matrix(); - CHECK(static_cast(child_world(0, 3)) == Catch::Approx(0.0f).margin(1e-5f)); - CHECK(static_cast(child_world(1, 3)) == Catch::Approx(1.0f).margin(1e-5f)); - CHECK(static_cast(child_world(2, 3)) == Catch::Approx(0.0f).margin(1e-5f)); + CHECK(static_cast(child_world(0, 3)) + == Catch::Approx(0.0f).margin(1e-5f)); + CHECK(static_cast(child_world(1, 3)) + == Catch::Approx(1.0f).margin(1e-5f)); + CHECK(static_cast(child_world(2, 3)) + == Catch::Approx(0.0f).margin(1e-5f)); } TEST_CASE("Object selectability flag", "[scene_graph]") { @@ -674,3 +694,396 @@ TEST_CASE("Object rotate() post-multiplies quaternion", "[scene_graph][trs]") { CHECK(static_cast(m(0, 1)) == Catch::Approx(0.0f).margin(1e-5f)); CHECK(static_cast(m(1, 0)) == Catch::Approx(0.0f).margin(1e-5f)); } + +// ════════════════════════════════════════════════════════════════════ +// ── O(1) Feature Lookup Tests ────────────────────────────────────── +// ════════════════════════════════════════════════════════════════════ + +TEST_CASE("find_feature O(1) exact type lookup", "[scene_graph][feature_map]") { + using namespace balsa::scene_graph; + + Object obj("test"); + auto &cam = obj.emplace_feature(); + auto &light = obj.emplace_feature(); + + // Exact type queries should hit the map. + CHECK(obj.find_feature() == &cam); + CHECK(obj.find_feature() == &light); + + // Type not present returns nullptr. + CHECK(obj.find_feature() == nullptr); +} + +TEST_CASE("find_feature const overload", "[scene_graph][feature_map]") { + using namespace balsa::scene_graph; + + Object obj("test"); + obj.emplace_feature(); + + const Object &cobj = obj; + const Camera *cam = cobj.find_feature(); + REQUIRE(cam != nullptr); + CHECK(cobj.find_feature() == nullptr); +} + +TEST_CASE("remove_feature clears type map", "[scene_graph][feature_map]") { + using namespace balsa::scene_graph; + + Object obj("test"); + auto &cam = obj.emplace_feature(); + REQUIRE(obj.find_feature() == &cam); + + bool removed = obj.remove_feature(&cam); + CHECK(removed); + CHECK(obj.find_feature() == nullptr); + CHECK(obj.features().empty()); +} + +TEST_CASE("remove_feature nonexistent returns false", + "[scene_graph][feature_map]") { + using namespace balsa::scene_graph; + + Object obj("test"); + Object other("other"); + auto &cam = other.emplace_feature(); + + CHECK_FALSE(obj.remove_feature(&cam)); + CHECK_FALSE(obj.remove_feature(nullptr)); +} + +// ════════════════════════════════════════════════════════════════════ +// ── Signal Tests ─────────────────────────────────────────────────── +// ════════════════════════════════════════════════════════════════════ + +TEST_CASE("on_feature_added fires on emplace", "[scene_graph][signals]") { + using namespace balsa::scene_graph; + + Object obj("test"); + int count = 0; + AbstractFeature *received = nullptr; + + obj.on_feature_added.connect([&](Object &, AbstractFeature &f) { + ++count; + received = &f; + }); + + auto &cam = obj.emplace_feature(); + CHECK(count == 1); + CHECK(received == &cam); + + obj.emplace_feature(); + CHECK(count == 2); +} + +TEST_CASE("on_feature_removing fires on remove", "[scene_graph][signals]") { + using namespace balsa::scene_graph; + + Object obj("test"); + auto &cam = obj.emplace_feature(); + + int count = 0; + AbstractFeature *received = nullptr; + + obj.on_feature_removing.connect([&](Object &, AbstractFeature &f) { + ++count; + received = &f; + }); + + obj.remove_feature(&cam); + CHECK(count == 1); + CHECK(received == &cam); +} + +TEST_CASE("on_child_added fires on add_child", "[scene_graph][signals]") { + using namespace balsa::scene_graph; + + Object root("root"); + int count = 0; + std::string last_child_name; + + root.on_child_added.connect([&](Object &, Object &child) { + ++count; + last_child_name = child.name; + }); + + root.add_child("child1"); + CHECK(count == 1); + CHECK(last_child_name == "child1"); + + root.add_child("child2"); + CHECK(count == 2); + CHECK(last_child_name == "child2"); +} + +TEST_CASE("on_child_removing fires on detach", "[scene_graph][signals]") { + using namespace balsa::scene_graph; + + Object root("root"); + auto &child = root.add_child("child"); + + int count = 0; + root.on_child_removing.connect([&](Object &, Object &c) { + ++count; + CHECK(c.name == "child"); + }); + + auto detached = child.detach(); + CHECK(count == 1); + REQUIRE(detached != nullptr); +} + +TEST_CASE("mark_modified fires on_modified signal", "[scene_graph][signals]") { + using namespace balsa::scene_graph; + + Object obj("test"); + auto &cam = obj.emplace_feature(); + + int count = 0; + cam.on_modified.connect([&](AbstractFeature &f) { + ++count; + CHECK(&f == &cam); + }); + + cam.mark_modified(); + CHECK(count == 1); + + cam.mark_modified(); + CHECK(count == 2); +} + +// ════════════════════════════════════════════════════════════════════ +// ── FeatureIndex Tests ───────────────────────────────────────────── +// ════════════════════════════════════════════════════════════════════ + +TEST_CASE("FeatureIndex tracks existing features", + "[scene_graph][feature_index]") { + using namespace balsa::scene_graph; + + Object root("root"); + auto &c1 = root.add_child("cam_node"); + c1.emplace_feature(); + auto &c2 = root.add_child("mesh_node"); + c2.emplace_feature(); + + FeatureIndex index; + index.track(root); + + CHECK(index.count() == 1); + CHECK(index.count() == 1); + CHECK(index.count() == 0); + + auto cam_objects = index.objects_with(); + REQUIRE(cam_objects.size() == 1); + CHECK(cam_objects[0] == &c1); +} + +TEST_CASE("FeatureIndex auto-updates on feature add/remove", + "[scene_graph][feature_index]") { + using namespace balsa::scene_graph; + + Object root("root"); + auto &child = root.add_child("child"); + + FeatureIndex index; + index.track(root); + + CHECK(index.count() == 0); + + // Add a feature after tracking. + auto &cam = child.emplace_feature(); + CHECK(index.count() == 1); + + // Remove it. + child.remove_feature(&cam); + CHECK(index.count() == 0); +} + +TEST_CASE("FeatureIndex auto-tracks new children", + "[scene_graph][feature_index]") { + using namespace balsa::scene_graph; + + Object root("root"); + FeatureIndex index; + index.track(root); + + CHECK(index.count() == 0); + + // Add a child with a feature after tracking started. + auto &child = root.add_child("late_child"); + child.emplace_feature(); + + CHECK(index.count() == 1); +} + +TEST_CASE("FeatureIndex untracks on child removal", + "[scene_graph][feature_index]") { + using namespace balsa::scene_graph; + + Object root("root"); + auto &child = root.add_child("child"); + child.emplace_feature(); + + FeatureIndex index; + index.track(root); + CHECK(index.count() == 1); + + // Detach the child — its features should be removed from the index. + auto detached = child.detach(); + CHECK(index.count() == 0); +} + +TEST_CASE("FeatureIndex destroyed before Objects is safe", + "[scene_graph][feature_index]") { + using namespace balsa::scene_graph; + + Object root("root"); + root.add_child("child").emplace_feature(); + + { + FeatureIndex index; + index.track(root); + CHECK(index.count() == 1); + } + // index destroyed — observer disconnected. + // Adding more features should not crash. + root.add_child("child2").emplace_feature(); +} + +// ════════════════════════════════════════════════════════════════════ +// ── Traversal Tests ──────────────────────────────────────────────── +// ════════════════════════════════════════════════════════════════════ + +TEST_CASE("for_each_descendant visits all descendants", + "[scene_graph][traversal]") { + using namespace balsa::scene_graph; + + // root + // / \ . + // a b + // / + // c + Object root("root"); + root.add_child("a").add_child("c"); + root.add_child("b"); + + std::vector visited; + root.for_each_descendant([&](Object &obj) { visited.push_back(obj.name); }); + + REQUIRE(visited.size() == 3); + // DFS pre-order: a, c, b + CHECK(visited[0] == "a"); + CHECK(visited[1] == "c"); + CHECK(visited[2] == "b"); +} + +TEST_CASE("for_each_descendant const overload", "[scene_graph][traversal]") { + using namespace balsa::scene_graph; + + Object root("root"); + root.add_child("a"); + root.add_child("b"); + + const Object &croot = root; + int count = 0; + croot.for_each_descendant([&](const Object &) { ++count; }); + CHECK(count == 2); +} + +TEST_CASE("for_each_descendant with pruning", "[scene_graph][traversal]") { + using namespace balsa::scene_graph; + + // root + // / \ . + // a b + // / + // c + Object root("root"); + root.add_child("a").add_child("c"); + root.add_child("b"); + + std::vector visited; + root.for_each_descendant([&](Object &obj) -> bool { + visited.push_back(obj.name); + return obj.name != "a"; // prune subtree under "a" + }); + + REQUIRE(visited.size() == 2); + CHECK(visited[0] == "a"); + CHECK(visited[1] == "b"); + // "c" should NOT be visited because "a" was pruned. +} + +TEST_CASE("for_each_ancestor walks parent chain", "[scene_graph][traversal]") { + using namespace balsa::scene_graph; + + Object root("root"); + auto &a = root.add_child("a"); + auto &c = a.add_child("c"); + + std::vector ancestors; + c.for_each_ancestor( + [&](const Object &obj) { ancestors.push_back(obj.name); }); + + REQUIRE(ancestors.size() == 2); + CHECK(ancestors[0] == "a"); + CHECK(ancestors[1] == "root"); +} + +TEST_CASE("for_each_ancestor with early stop", "[scene_graph][traversal]") { + using namespace balsa::scene_graph; + + Object root("root"); + auto &a = root.add_child("a"); + auto &c = a.add_child("c"); + + std::vector ancestors; + c.for_each_ancestor([&](const Object &obj) -> bool { + ancestors.push_back(obj.name); + return false; // stop after first ancestor + }); + + REQUIRE(ancestors.size() == 1); + CHECK(ancestors[0] == "a"); +} + +TEST_CASE("find_descendant returns first match", "[scene_graph][traversal]") { + using namespace balsa::scene_graph; + + Object root("root"); + root.add_child("a").emplace_feature(); + root.add_child("b").emplace_feature(); + + Object *found = root.find_descendant( + [](Object &obj) { return obj.find_feature() != nullptr; }); + + REQUIRE(found != nullptr); + CHECK(found->name == "a"); // first in DFS order +} + +TEST_CASE("find_descendant returns nullptr if no match", + "[scene_graph][traversal]") { + using namespace balsa::scene_graph; + + Object root("root"); + root.add_child("a"); + root.add_child("b"); + + Object *found = root.find_descendant( + [](Object &obj) { return obj.find_feature() != nullptr; }); + + CHECK(found == nullptr); +} + +TEST_CASE("find_descendant const overload", "[scene_graph][traversal]") { + using namespace balsa::scene_graph; + + Object root("root"); + root.add_child("target").emplace_feature(); + + const Object &croot = root; + const Object *found = croot.find_descendant( + [](const Object &obj) { return obj.find_feature() != nullptr; }); + + REQUIRE(found != nullptr); + CHECK(found->name == "target"); +} From 49f9a2c0b58f6d59ef212e2e805a52f4fe0c325b Mon Sep 17 00:00:00 2001 From: Michael Tao Date: Sun, 29 Mar 2026 10:51:15 -0400 Subject: [PATCH 2/2] style: fix idiom compliance (include guards, trailing returns) --- .../balsa/scene_graph/AbstractFeature.hpp | 6 +- .../balsa/scene_graph/FeatureIndex.hpp | 21 +++--- .../include/balsa/scene_graph/Object.hpp | 70 +++++++++---------- .../src/scene_graph/FeatureIndex.cpp | 16 ++--- visualization/src/scene_graph/Object.cpp | 28 ++++---- 5 files changed, 72 insertions(+), 69 deletions(-) diff --git a/visualization/include/balsa/scene_graph/AbstractFeature.hpp b/visualization/include/balsa/scene_graph/AbstractFeature.hpp index 867e2d2..31b6656 100644 --- a/visualization/include/balsa/scene_graph/AbstractFeature.hpp +++ b/visualization/include/balsa/scene_graph/AbstractFeature.hpp @@ -20,15 +20,15 @@ class AbstractFeature { public: virtual ~AbstractFeature() = default; - Object &object() { return *_object; } - const Object &object() const { return *_object; } + auto object() -> Object & { return *_object; } + auto object() const -> const Object & { return *_object; } // Signal emitted when this feature's data has changed. // Subscribers receive a reference to the modified feature. sigslot::signal_st on_modified; // Call this from derived classes after mutating internal state. - void mark_modified() { on_modified(*this); } + auto mark_modified() -> void { on_modified(*this); } protected: AbstractFeature() = default; diff --git a/visualization/include/balsa/scene_graph/FeatureIndex.hpp b/visualization/include/balsa/scene_graph/FeatureIndex.hpp index 17c29f3..9a1798c 100644 --- a/visualization/include/balsa/scene_graph/FeatureIndex.hpp +++ b/visualization/include/balsa/scene_graph/FeatureIndex.hpp @@ -1,4 +1,5 @@ -#pragma once +#if !defined(BALSA_SCENE_GRAPH_FEATUREINDEX_HPP) +#define BALSA_SCENE_GRAPH_FEATUREINDEX_HPP #include #include @@ -42,10 +43,10 @@ class FeatureIndex : public sigslot::observer_st { // Recursively subscribe to an Object and all its current // descendants. Indexes every feature already present. - void track(Object &root); + auto track(Object &root) -> void; // Stop tracking an Object (and its descendants). - void untrack(Object &root); + auto untrack(Object &root) -> void; // Return all Objects that currently carry a feature of type F. template @@ -56,14 +57,14 @@ class FeatureIndex : public sigslot::observer_st { auto count() const -> std::size_t; private: - void track_single(Object &obj); - void untrack_single(Object &obj); + auto track_single(Object &obj) -> void; + auto untrack_single(Object &obj) -> void; // Signal handlers. - void on_feature_added(Object &obj, AbstractFeature &feature); - void on_feature_removing(Object &obj, AbstractFeature &feature); - void on_child_added(Object &parent, Object &child); - void on_child_removing(Object &parent, Object &child); + auto on_feature_added(Object &obj, AbstractFeature &feature) -> void; + auto on_feature_removing(Object &obj, AbstractFeature &feature) -> void; + auto on_child_added(Object &parent, Object &child) -> void; + auto on_child_removing(Object &parent, Object &child) -> void; // type_index -> vector of Object pointers carrying that feature type. std::unordered_map> _index; @@ -86,3 +87,5 @@ auto FeatureIndex::count() const -> std::size_t { } } // namespace balsa::scene_graph + +#endif diff --git a/visualization/include/balsa/scene_graph/Object.hpp b/visualization/include/balsa/scene_graph/Object.hpp index 6c5aa31..45e978d 100644 --- a/visualization/include/balsa/scene_graph/Object.hpp +++ b/visualization/include/balsa/scene_graph/Object.hpp @@ -52,72 +52,72 @@ class Object { // ── Transform (decomposed TRS) ───────────────────────────────── // Translation - const Vec3f &translation() const { return _translation; } - void set_translation(const Vec3f &t) { _translation = t; } + auto translation() const -> const Vec3f & { return _translation; } + auto set_translation(const Vec3f &t) -> void { _translation = t; } // Rotation (stored as unit quaternion) - const Quaternionf &rotation() const { return _rotation; } - void set_rotation(const Quaternionf &q) { _rotation = q; } + auto rotation() const -> const Quaternionf & { return _rotation; } + auto set_rotation(const Quaternionf &q) -> void { _rotation = q; } // Rotation as Euler angles in degrees (XYZ / pitch-yaw-roll). // Returns (pitch, yaw, roll) in degrees. - Vec3f rotation_euler() const; + auto rotation_euler() const -> Vec3f; // Set rotation from Euler angles in degrees (pitch, yaw, roll). - void set_rotation_euler(const Vec3f °rees); + auto set_rotation_euler(const Vec3f °rees) -> void; // Scale factors (per-axis) - const Vec3f &scale_factors() const { return _scale; } - void set_scale_factors(const Vec3f &s) { _scale = s; } + auto scale_factors() const -> const Vec3f & { return _scale; } + auto set_scale_factors(const Vec3f &s) -> void { _scale = s; } // ── Convenience mutators ──────────────────────────────────────── // Add to current translation. - void translate(const Vec3f &delta); + auto translate(const Vec3f &delta) -> void; // Post-multiply rotation by the given quaternion. - void rotate(const Quaternionf &q); + auto rotate(const Quaternionf &q) -> void; // Set uniform scale on all axes. - void set_uniform_scale(float s); + auto set_uniform_scale(float s) -> void; // Reset TRS to identity (translation=0, rotation=identity, scale=1). - void reset_transform(); + auto reset_transform() -> void; // ── Composed transforms ───────────────────────────────────────── // Compose T * R * S and return the resulting affine transform. // Computed on the fly — not cached. - AffineTransformf local_transform() const; + auto local_transform() const -> AffineTransformf; // Decompose an affine transform into T/R/S and store them. // Assumes the transform contains no shear (only translation, // rotation, and axis-aligned scale). If the linear block has // negative determinant, one scale factor will be negative. - void set_from_transform(const AffineTransformf &xf); + auto set_from_transform(const AffineTransformf &xf) -> void; // Accumulated world transform: product of all ancestors' local // transforms, from root down to (and including) this node. - AffineTransformf world_transform() const; + auto world_transform() const -> AffineTransformf; // ── Hierarchy ─────────────────────────────────────────────────── - Object *parent() const { return _parent; } - const std::vector> &children() const { + auto parent() const -> Object * { return _parent; } + auto children() const -> const std::vector> & { return _children; } - std::size_t children_count() const { return _children.size(); } + auto children_count() const -> std::size_t { return _children.size(); } // Create a new child Object with the given name. - Object &add_child(std::string name = ""); + auto add_child(std::string name = "") -> Object &; // Take ownership of an existing Object and add it as a child. // Reparents: removes the object from its previous parent (if any). - Object &add_child(std::unique_ptr child); + auto add_child(std::unique_ptr child) -> Object &; // Detach this Object from its parent, returning ownership. // Returns nullptr if this Object has no parent. - std::unique_ptr detach(); + auto detach() -> std::unique_ptr; // ── Traversal ─────────────────────────────────────────────────── @@ -125,15 +125,15 @@ class Object { // If fn returns bool, returning false prunes the subtree. // If fn returns void, all descendants are visited. template - void for_each_descendant(Fn &&fn); + auto for_each_descendant(Fn &&fn) -> void; template - void for_each_descendant(Fn &&fn) const; + auto for_each_descendant(Fn &&fn) const -> void; // Walk the ancestor chain (parent, grandparent, ...). // If fn returns bool, returning false stops traversal. template - void for_each_ancestor(Fn &&fn) const; + auto for_each_ancestor(Fn &&fn) const -> void; // Return the first descendant (DFS pre-order) satisfying pred, // or nullptr if none. @@ -149,24 +149,24 @@ class Object { // F must derive from AbstractFeature. // Returns a reference to the newly created feature. template - F &emplace_feature(Args &&...args); + auto emplace_feature(Args &&...args) -> F &; // Find the first attached feature of type F (exact or derived). // Returns nullptr if no such feature is found. template - F *find_feature(); + auto find_feature() -> F *; template - const F *find_feature() const; + auto find_feature() const -> const F *; // Access all features (for advanced iteration). - const std::vector> &features() const { + auto features() const -> const std::vector> & { return _features; } // Remove a feature by pointer. Returns true if the feature was found // and removed. The feature is destroyed. - bool remove_feature(AbstractFeature *feature); + auto remove_feature(AbstractFeature *feature) -> bool; // ── Identity ──────────────────────────────────────────────────── @@ -208,7 +208,7 @@ class Object { // ── Template implementations ──────────────────────────────────────── template -F &Object::emplace_feature(Args &&...args) { +auto Object::emplace_feature(Args &&...args) -> F & { static_assert(std::is_base_of_v, "F must derive from AbstractFeature"); auto ptr = std::make_unique(std::forward(args)...); @@ -221,7 +221,7 @@ F &Object::emplace_feature(Args &&...args) { } template -F *Object::find_feature() { +auto Object::find_feature() -> F * { // Fast path: exact type match via map. if (auto it = _feature_map.find(std::type_index(typeid(F))); it != _feature_map.end()) { @@ -235,7 +235,7 @@ F *Object::find_feature() { } template -const F *Object::find_feature() const { +auto Object::find_feature() const -> const F * { // Fast path: exact type match via map. if (auto it = _feature_map.find(std::type_index(typeid(F))); it != _feature_map.end()) { @@ -258,7 +258,7 @@ namespace detail { } // namespace detail template -void Object::for_each_descendant(Fn &&fn) { +auto Object::for_each_descendant(Fn &&fn) -> void { for (auto &child : _children) { if (!child) continue; if constexpr (detail::returns_bool_v) { @@ -271,7 +271,7 @@ void Object::for_each_descendant(Fn &&fn) { } template -void Object::for_each_descendant(Fn &&fn) const { +auto Object::for_each_descendant(Fn &&fn) const -> void { for (auto &child : _children) { if (!child) continue; if constexpr (detail::returns_bool_v) { @@ -284,7 +284,7 @@ void Object::for_each_descendant(Fn &&fn) const { } template -void Object::for_each_ancestor(Fn &&fn) const { +auto Object::for_each_ancestor(Fn &&fn) const -> void { for (Object *p = _parent; p != nullptr; p = p->_parent) { if constexpr (detail::returns_bool_v) { if (!fn(*p)) return; diff --git a/visualization/src/scene_graph/FeatureIndex.cpp b/visualization/src/scene_graph/FeatureIndex.cpp index 07665bb..a412f35 100644 --- a/visualization/src/scene_graph/FeatureIndex.cpp +++ b/visualization/src/scene_graph/FeatureIndex.cpp @@ -6,14 +6,14 @@ namespace balsa::scene_graph { // ── Public API ────────────────────────────────────────────────────── -void FeatureIndex::track(Object &root) { +auto FeatureIndex::track(Object &root) -> void { track_single(root); for (auto &child_ptr : root.children()) { if (child_ptr) track(*child_ptr); } } -void FeatureIndex::untrack(Object &root) { +auto FeatureIndex::untrack(Object &root) -> void { for (auto &child_ptr : root.children()) { if (child_ptr) untrack(*child_ptr); } @@ -22,7 +22,7 @@ void FeatureIndex::untrack(Object &root) { // ── Internals ─────────────────────────────────────────────────────── -void FeatureIndex::track_single(Object &obj) { +auto FeatureIndex::track_single(Object &obj) -> void { // Index all features already present on this object. for (auto &f : obj.features()) { if (f) { _index[std::type_index(typeid(*f))].push_back(&obj); } @@ -37,7 +37,7 @@ void FeatureIndex::track_single(Object &obj) { obj.on_child_removing.connect(&FeatureIndex::on_child_removing, this); } -void FeatureIndex::untrack_single(Object &obj) { +auto FeatureIndex::untrack_single(Object &obj) -> void { // Disconnect all signals from this object to us. obj.on_feature_added.disconnect(this); obj.on_feature_removing.disconnect(this); @@ -58,11 +58,11 @@ void FeatureIndex::untrack_single(Object &obj) { // ── Signal handlers ───────────────────────────────────────────────── -void FeatureIndex::on_feature_added(Object &obj, AbstractFeature &feature) { +auto FeatureIndex::on_feature_added(Object &obj, AbstractFeature &feature) -> void { _index[std::type_index(typeid(feature))].push_back(&obj); } -void FeatureIndex::on_feature_removing(Object &obj, AbstractFeature &feature) { +auto FeatureIndex::on_feature_removing(Object &obj, AbstractFeature &feature) -> void { auto ti = std::type_index(typeid(feature)); auto map_it = _index.find(ti); if (map_it == _index.end()) return; @@ -71,11 +71,11 @@ void FeatureIndex::on_feature_removing(Object &obj, AbstractFeature &feature) { if (vec.empty()) _index.erase(map_it); } -void FeatureIndex::on_child_added(Object & /*parent*/, Object &child) { +auto FeatureIndex::on_child_added(Object & /*parent*/, Object &child) -> void { track(child); } -void FeatureIndex::on_child_removing(Object & /*parent*/, Object &child) { +auto FeatureIndex::on_child_removing(Object & /*parent*/, Object &child) -> void { untrack(child); } diff --git a/visualization/src/scene_graph/Object.cpp b/visualization/src/scene_graph/Object.cpp index d6e82e3..ae6395c 100644 --- a/visualization/src/scene_graph/Object.cpp +++ b/visualization/src/scene_graph/Object.cpp @@ -46,7 +46,7 @@ Object::Object(Object &&other) noexcept other._parent = nullptr; } -Object &Object::operator=(Object &&other) noexcept { +auto Object::operator=(Object &&other) noexcept -> Object & { if (this != &other) { name = std::move(other.name); visible = other.visible; @@ -72,7 +72,7 @@ Object &Object::operator=(Object &&other) noexcept { // ── Euler angles ──────────────────────────────────────────────────── -Vec3f Object::rotation_euler() const { +auto Object::rotation_euler() const -> Vec3f { auto rad = ::zipper::transform::euler_angles(_rotation); Vec3f deg; deg(0) = static_cast(rad(0)) * k_rad2deg; @@ -81,7 +81,7 @@ Vec3f Object::rotation_euler() const { return deg; } -void Object::set_rotation_euler(const Vec3f °rees) { +auto Object::set_rotation_euler(const Vec3f °rees) -> void { Vec3f radians; radians(0) = degrees(0) * k_deg2rad; radians(1) = degrees(1) * k_deg2rad; @@ -91,22 +91,22 @@ void Object::set_rotation_euler(const Vec3f °rees) { // ── Convenience mutators ──────────────────────────────────────────── -void Object::translate(const Vec3f &delta) { +auto Object::translate(const Vec3f &delta) -> void { _translation = (_translation + delta).eval(); } -void Object::rotate(const Quaternionf &q) { +auto Object::rotate(const Quaternionf &q) -> void { // Post-multiply: new_rotation = q * old_rotation _rotation = (q * _rotation).eval(); } -void Object::set_uniform_scale(float s) { +auto Object::set_uniform_scale(float s) -> void { _scale(0) = s; _scale(1) = s; _scale(2) = s; } -void Object::reset_transform() { +auto Object::reset_transform() -> void { _translation(0) = 0; _translation(1) = 0; _translation(2) = 0; @@ -118,7 +118,7 @@ void Object::reset_transform() { // ── Composed transforms ───────────────────────────────────────────── -AffineTransformf Object::local_transform() const { +auto Object::local_transform() const -> AffineTransformf { // Compose T * R * S using zipper's transform composition. Translation3f t(_translation); Rotation3f r(::zipper::transform::to_rotation_matrix(_rotation)); @@ -126,7 +126,7 @@ AffineTransformf Object::local_transform() const { return t * r * s; } -void Object::set_from_transform(const AffineTransformf &xf) { +auto Object::set_from_transform(const AffineTransformf &xf) -> void { auto [t, r, s] = ::zipper::transform::trs_decompose(xf); for (int i = 0; i < 3; ++i) { @@ -136,14 +136,14 @@ void Object::set_from_transform(const AffineTransformf &xf) { _rotation = ::zipper::transform::to_quaternion(r.matrix()); } -AffineTransformf Object::world_transform() const { +auto Object::world_transform() const -> AffineTransformf { if (_parent) { return _parent->world_transform() * local_transform(); } return local_transform(); } // ── Hierarchy ─────────────────────────────────────────────────────── -Object &Object::add_child(std::string child_name) { +auto Object::add_child(std::string child_name) -> Object & { auto child = std::make_unique(std::move(child_name)); child->_parent = this; _children.push_back(std::move(child)); @@ -151,7 +151,7 @@ Object &Object::add_child(std::string child_name) { return *_children.back(); } -Object &Object::add_child(std::unique_ptr child) { +auto Object::add_child(std::unique_ptr child) -> Object & { if (!child) { // Create an empty child rather than storing a null. return add_child(std::string{}); @@ -176,7 +176,7 @@ Object &Object::add_child(std::unique_ptr child) { return *_children.back(); } -std::unique_ptr Object::detach() { +auto Object::detach() -> std::unique_ptr { if (!_parent) return nullptr; if (permanent) return nullptr; // permanent objects cannot be detached @@ -197,7 +197,7 @@ std::unique_ptr Object::detach() { // ── Feature removal ───────────────────────────────────────────────── -bool Object::remove_feature(AbstractFeature *feature) { +auto Object::remove_feature(AbstractFeature *feature) -> bool { if (!feature) return false; auto it = std::find_if(_features.begin(),