From bb8d31328579035a2274326fa0a656b693eba9eb Mon Sep 17 00:00:00 2001 From: Adam Kewley Date: Mon, 15 Jul 2024 16:24:36 +0200 Subject: [PATCH 01/13] Centralize transparent string hashing to TransparentStringViewHasher --- src/oscar/CMakeLists.txt | 1 + src/oscar/Graphics/GraphicsImplementation.cpp | 13 +---- src/oscar/Platform/AppSettings.cpp | 13 +---- src/oscar/Utils.h | 1 + src/oscar/Utils/StringName.cpp | 21 ++----- src/oscar/Utils/TransparentStringViewHasher.h | 23 ++++++++ tests/testoscar/CMakeLists.txt | 3 +- .../Utils/TestTransparentStringViewHasher.cpp | 57 +++++++++++++++++++ 8 files changed, 94 insertions(+), 38 deletions(-) create mode 100644 src/oscar/Utils/TransparentStringViewHasher.h create mode 100644 tests/testoscar/Utils/TestTransparentStringViewHasher.cpp diff --git a/src/oscar/CMakeLists.txt b/src/oscar/CMakeLists.txt index ed057e0f52..9114f1e47a 100644 --- a/src/oscar/CMakeLists.txt +++ b/src/oscar/CMakeLists.txt @@ -402,6 +402,7 @@ add_library(oscar STATIC Utils/TemporaryFile.cpp Utils/TemporaryFile.h Utils/TemporaryFileParameters.h + Utils/TransparentStringViewHasher.h Utils/Typelist.h Utils/UID.cpp Utils/UID.h diff --git a/src/oscar/Graphics/GraphicsImplementation.cpp b/src/oscar/Graphics/GraphicsImplementation.cpp index 62017b338b..3c5f7b0a9f 100644 --- a/src/oscar/Graphics/GraphicsImplementation.cpp +++ b/src/oscar/Graphics/GraphicsImplementation.cpp @@ -75,6 +75,7 @@ #include #include #include +#include #include #include @@ -523,21 +524,11 @@ namespace o << "ShadeElement(name = " << name << ", location = " << se.location << ", shader_type = " << se.shader_type << ", size = " << se.size << ')'; } - // see: ankerl/unordered_dense documentation for heterogeneous lookups - struct transparent_string_hash final { - using is_transparent = void; - using is_avalanching = void; - - [[nodiscard]] auto operator()(std::string_view str) const -> uint64_t { - return ankerl::unordered_dense::hash{}(str); - } - }; - template using FastStringHashtable = ankerl::unordered_dense::map< std::string, Value, - transparent_string_hash, + TransparentStringViewHasher, std::equal_to<> >; } diff --git a/src/oscar/Platform/AppSettings.cpp b/src/oscar/Platform/AppSettings.cpp index 12185eb61c..f70be09d3d 100644 --- a/src/oscar/Platform/AppSettings.cpp +++ b/src/oscar/Platform/AppSettings.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -98,20 +99,10 @@ R"(# configuration options auto end() const { return hashmap_.end(); } private: - // see: ankerl/unordered_dense documentation for heterogeneous lookups - struct transparent_string_hash final { - using is_transparent = void; - using is_avalanching = void; - - [[nodiscard]] auto operator()(std::string_view str) const -> uint64_t { - return ankerl::unordered_dense::hash{}(str); - } - }; - using Storage = ankerl::unordered_dense::map< std::string, AppSettingsLookupValue, - transparent_string_hash, + TransparentStringViewHasher, std::equal_to<> >; Storage hashmap_; diff --git a/src/oscar/Utils.h b/src/oscar/Utils.h index 44e381c9a3..a35320c141 100644 --- a/src/oscar/Utils.h +++ b/src/oscar/Utils.h @@ -37,6 +37,7 @@ #include #include #include +#include #include #include #include diff --git a/src/oscar/Utils/StringName.cpp b/src/oscar/Utils/StringName.cpp index 6ce7bff799..5014fabb4a 100644 --- a/src/oscar/Utils/StringName.cpp +++ b/src/oscar/Utils/StringName.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include @@ -41,28 +42,18 @@ namespace { return *ptr_; } - private: - std::unique_ptr ptr_; - }; - struct StringNameLutHasher final { - using is_transparent = void; - using is_avalanching = void; - - [[nodiscard]] auto operator()(std::string_view str) const noexcept -> uint64_t + operator std::string_view () const { - return ankerl::unordered_dense::hash{}(str); - } - - [[nodiscard]] auto operator()(const StringNameDataPtr& ptr) const noexcept -> uint64_t - { - return ankerl::unordered_dense::hash{}(ptr->value()); + return ptr_->value(); } + private: + std::unique_ptr ptr_; }; using StringNameLookup = ankerl::unordered_dense::set< StringNameDataPtr, - StringNameLutHasher, + TransparentStringViewHasher, std::equal_to<> >; diff --git a/src/oscar/Utils/TransparentStringViewHasher.h b/src/oscar/Utils/TransparentStringViewHasher.h new file mode 100644 index 0000000000..830c3ea8c6 --- /dev/null +++ b/src/oscar/Utils/TransparentStringViewHasher.h @@ -0,0 +1,23 @@ +#pragma once + +#include +#include + +namespace osc +{ + // a `std::hash`-like object that can transparently hash any object that is + // implicitly convertible to a `std::string_view` + struct TransparentStringViewHasher final { + + // important: this is how associative containers check that the hasher + // doesn't need to create keys at runtime (e.g. using a `std::string_view` + // to `.find` an `std::unordered_map` won't require + // constructing a `std::string`) + using is_transparent = void; + + size_t operator()(std::string_view sv) const noexcept + { + return std::hash{}(sv); + } + }; +} diff --git a/tests/testoscar/CMakeLists.txt b/tests/testoscar/CMakeLists.txt index 01f6cc68f0..643955dfe7 100644 --- a/tests/testoscar/CMakeLists.txt +++ b/tests/testoscar/CMakeLists.txt @@ -96,6 +96,7 @@ add_executable(testoscar Utils/TestStringHelpers.cpp Utils/TestStringName.cpp Utils/TestTemporaryFile.cpp + Utils/TestTransparentStringViewHasher.cpp Utils/TestTypelist.cpp Variant/TestVariant.cpp @@ -104,7 +105,7 @@ add_executable(testoscar TestingHelpers.cpp TestingHelpers.h testoscar.cpp # entry point - ) +) configure_file( diff --git a/tests/testoscar/Utils/TestTransparentStringViewHasher.cpp b/tests/testoscar/Utils/TestTransparentStringViewHasher.cpp new file mode 100644 index 0000000000..867789a3cc --- /dev/null +++ b/tests/testoscar/Utils/TestTransparentStringViewHasher.cpp @@ -0,0 +1,57 @@ +#include + +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +using namespace osc; +namespace rgs = std::ranges; + +namespace +{ + using TransparentMap = std::unordered_map>; +} + +TEST(TransparentStringViewHasher, can_construct_std_unordered_map_that_uses_transparent_string_hasher) +{ + [[maybe_unused]] TransparentMap map; // this should work +} + +TEST(TransparentStringViewHasher, transparent_unordered_map_enables_std_string_view_lookups) +{ + TransparentMap map; + [[maybe_unused]] const auto it = map.find(std::string_view{"i don't need to be converted into a std::string :)"}); +} + +TEST(TransparentStringViewHasher, transparent_unordered_map_enables_CStringView_lookups) +{ + TransparentMap map; + [[maybe_unused]] const auto it = map.find(CStringView{"i don't need to be converted into a std::string :)"}); +} + +TEST(TransparentStringViewHasher, transparent_unordered_map_enables_StringName_lookups) +{ + TransparentMap map; + [[maybe_unused]] const auto it = map.find(StringName{"i don't need to be converted into a std::string :)"}); +} + +TEST(TransparentStringViewHasher, produces_same_hash_for_all_of_OSCs_string_types) +{ + for (const char* str : {"", "some string", "why not try three?"}) { + const auto hashes = std::to_array({ + TransparentStringViewHasher{}(str), + TransparentStringViewHasher{}(std::string_view{str}), + TransparentStringViewHasher{}(CStringView{str}), + TransparentStringViewHasher{}(std::string{str}), + TransparentStringViewHasher{}(StringName{str}), + }); + ASSERT_TRUE(rgs::equal_range(hashes, hashes.front())); + } +} From c0decf30835b3f4b341724a81124870452916032 Mon Sep 17 00:00:00 2001 From: Adam Kewley Date: Tue, 16 Jul 2024 07:10:52 +0200 Subject: [PATCH 02/13] Drop std::unordered_map for is_transparent tests (Ubuntu20 doesn't support it) --- src/oscar/CMakeLists.txt | 2 +- src/oscar/Graphics/GraphicsImplementation.cpp | 4 +- src/oscar/Platform/AppSettings.cpp | 4 +- src/oscar/Utils.h | 2 +- src/oscar/Utils/StringName.cpp | 16 +++-- src/oscar/Utils/TransparentStringHasher.h | 29 ++++++++++ src/oscar/Utils/TransparentStringViewHasher.h | 23 -------- tests/testoscar/CMakeLists.txt | 2 +- .../Utils/TestTransparentStringHasher.cpp | 58 +++++++++++++++++++ .../Utils/TestTransparentStringViewHasher.cpp | 57 ------------------ 10 files changed, 104 insertions(+), 93 deletions(-) create mode 100644 src/oscar/Utils/TransparentStringHasher.h delete mode 100644 src/oscar/Utils/TransparentStringViewHasher.h create mode 100644 tests/testoscar/Utils/TestTransparentStringHasher.cpp delete mode 100644 tests/testoscar/Utils/TestTransparentStringViewHasher.cpp diff --git a/src/oscar/CMakeLists.txt b/src/oscar/CMakeLists.txt index 9114f1e47a..6066031baa 100644 --- a/src/oscar/CMakeLists.txt +++ b/src/oscar/CMakeLists.txt @@ -402,7 +402,7 @@ add_library(oscar STATIC Utils/TemporaryFile.cpp Utils/TemporaryFile.h Utils/TemporaryFileParameters.h - Utils/TransparentStringViewHasher.h + Utils/TransparentStringHasher.h Utils/Typelist.h Utils/UID.cpp Utils/UID.h diff --git a/src/oscar/Graphics/GraphicsImplementation.cpp b/src/oscar/Graphics/GraphicsImplementation.cpp index 3c5f7b0a9f..2d68e4ccb5 100644 --- a/src/oscar/Graphics/GraphicsImplementation.cpp +++ b/src/oscar/Graphics/GraphicsImplementation.cpp @@ -75,7 +75,7 @@ #include #include #include -#include +#include #include #include @@ -528,7 +528,7 @@ namespace using FastStringHashtable = ankerl::unordered_dense::map< std::string, Value, - TransparentStringViewHasher, + TransparentStringHasher, std::equal_to<> >; } diff --git a/src/oscar/Platform/AppSettings.cpp b/src/oscar/Platform/AppSettings.cpp index f70be09d3d..4b50f81d2e 100644 --- a/src/oscar/Platform/AppSettings.cpp +++ b/src/oscar/Platform/AppSettings.cpp @@ -7,7 +7,7 @@ #include #include #include -#include +#include #include #include @@ -102,7 +102,7 @@ R"(# configuration options using Storage = ankerl::unordered_dense::map< std::string, AppSettingsLookupValue, - TransparentStringViewHasher, + TransparentStringHasher, std::equal_to<> >; Storage hashmap_; diff --git a/src/oscar/Utils.h b/src/oscar/Utils.h index a35320c141..ab630e6f85 100644 --- a/src/oscar/Utils.h +++ b/src/oscar/Utils.h @@ -37,7 +37,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/src/oscar/Utils/StringName.cpp b/src/oscar/Utils/StringName.cpp index 5014fabb4a..675b5a31de 100644 --- a/src/oscar/Utils/StringName.cpp +++ b/src/oscar/Utils/StringName.cpp @@ -3,7 +3,7 @@ #include #include -#include +#include #include #include @@ -42,18 +42,22 @@ namespace { return *ptr_; } + private: + std::unique_ptr ptr_; + }; - operator std::string_view () const + struct TransparentHasher : public TransparentStringHasher { + using TransparentStringHasher::operator(); + + size_t operator()(const StringNameDataPtr& ptr) const noexcept { - return ptr_->value(); + return ptr->hash(); } - private: - std::unique_ptr ptr_; }; using StringNameLookup = ankerl::unordered_dense::set< StringNameDataPtr, - TransparentStringViewHasher, + TransparentHasher, std::equal_to<> >; diff --git a/src/oscar/Utils/TransparentStringHasher.h b/src/oscar/Utils/TransparentStringHasher.h new file mode 100644 index 0000000000..40f7bfc67a --- /dev/null +++ b/src/oscar/Utils/TransparentStringHasher.h @@ -0,0 +1,29 @@ +#pragma once + +#include + +#include +#include + +namespace osc +{ + // a `std::hash`-like object that can transparently hash any object that is + // string-like (i.e. implicitly converts into a `std::string_view`) + struct TransparentStringHasher { + + using is_transparent = void; // C++20, `std::unordered_map` uses this + + size_t operator()(std::string_view sv) const noexcept + { + // if something implicitly converts into a `std::string_view` then it's + // eligible for transparent hashing + return std::hash{}(sv); + } + + size_t operator()(const StringName& sn) const noexcept + { + // special case: `StringName`s are pre-hashed + return std::hash{}(sn); + } + }; +} diff --git a/src/oscar/Utils/TransparentStringViewHasher.h b/src/oscar/Utils/TransparentStringViewHasher.h deleted file mode 100644 index 830c3ea8c6..0000000000 --- a/src/oscar/Utils/TransparentStringViewHasher.h +++ /dev/null @@ -1,23 +0,0 @@ -#pragma once - -#include -#include - -namespace osc -{ - // a `std::hash`-like object that can transparently hash any object that is - // implicitly convertible to a `std::string_view` - struct TransparentStringViewHasher final { - - // important: this is how associative containers check that the hasher - // doesn't need to create keys at runtime (e.g. using a `std::string_view` - // to `.find` an `std::unordered_map` won't require - // constructing a `std::string`) - using is_transparent = void; - - size_t operator()(std::string_view sv) const noexcept - { - return std::hash{}(sv); - } - }; -} diff --git a/tests/testoscar/CMakeLists.txt b/tests/testoscar/CMakeLists.txt index 643955dfe7..5f9cfdcc6f 100644 --- a/tests/testoscar/CMakeLists.txt +++ b/tests/testoscar/CMakeLists.txt @@ -96,7 +96,7 @@ add_executable(testoscar Utils/TestStringHelpers.cpp Utils/TestStringName.cpp Utils/TestTemporaryFile.cpp - Utils/TestTransparentStringViewHasher.cpp + Utils/TestTransparentStringHasher.cpp Utils/TestTypelist.cpp Variant/TestVariant.cpp diff --git a/tests/testoscar/Utils/TestTransparentStringHasher.cpp b/tests/testoscar/Utils/TestTransparentStringHasher.cpp new file mode 100644 index 0000000000..c358d48379 --- /dev/null +++ b/tests/testoscar/Utils/TestTransparentStringHasher.cpp @@ -0,0 +1,58 @@ +#include + +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +using namespace osc; +namespace rgs = std::ranges; + +namespace +{ + // TODO: change to `std::unordered_map` after upgrading from Ubuntu20 (doesn't support C++20's transparent hashing) + using TransparentMap = ankerl::unordered_dense::map>; +} + +TEST(TransparentStringHasher, can_construct_unordered_map_that_uses_transparent_string_hasher) +{ + [[maybe_unused]] TransparentMap map; // this should work +} + +TEST(TransparentStringHasher, transparent_unordered_map_enables_std_string_view_lookups) +{ + TransparentMap map; + [[maybe_unused]] const auto it = map.find(std::string_view{"i don't need to be converted into a std::string :)"}); +} + +TEST(TransparentStringHasher, transparent_unordered_map_enables_CStringView_lookups) +{ + TransparentMap map; + [[maybe_unused]] const auto it = map.find(CStringView{"i don't need to be converted into a std::string :)"}); +} + +TEST(TransparentStringHasher, transparent_unordered_map_enables_StringName_lookups) +{ + TransparentMap map; + [[maybe_unused]] const auto it = map.find(StringName{"i don't need to be converted into a std::string :)"}); +} + +TEST(TransparentStringHasher, produces_same_hash_for_all_of_OSCs_string_types) +{ + for (const char* str : {"", "some string", "why not try three?"}) { + const auto hashes = std::to_array({ + TransparentStringHasher{}(str), + TransparentStringHasher{}(std::string_view{str}), + TransparentStringHasher{}(CStringView{str}), + TransparentStringHasher{}(std::string{str}), + TransparentStringHasher{}(StringName{str}), + }); + ASSERT_TRUE(rgs::equal_range(hashes, hashes.front())); + } +} diff --git a/tests/testoscar/Utils/TestTransparentStringViewHasher.cpp b/tests/testoscar/Utils/TestTransparentStringViewHasher.cpp deleted file mode 100644 index 867789a3cc..0000000000 --- a/tests/testoscar/Utils/TestTransparentStringViewHasher.cpp +++ /dev/null @@ -1,57 +0,0 @@ -#include - -#include -#include -#include - -#include -#include -#include -#include -#include -#include - -using namespace osc; -namespace rgs = std::ranges; - -namespace -{ - using TransparentMap = std::unordered_map>; -} - -TEST(TransparentStringViewHasher, can_construct_std_unordered_map_that_uses_transparent_string_hasher) -{ - [[maybe_unused]] TransparentMap map; // this should work -} - -TEST(TransparentStringViewHasher, transparent_unordered_map_enables_std_string_view_lookups) -{ - TransparentMap map; - [[maybe_unused]] const auto it = map.find(std::string_view{"i don't need to be converted into a std::string :)"}); -} - -TEST(TransparentStringViewHasher, transparent_unordered_map_enables_CStringView_lookups) -{ - TransparentMap map; - [[maybe_unused]] const auto it = map.find(CStringView{"i don't need to be converted into a std::string :)"}); -} - -TEST(TransparentStringViewHasher, transparent_unordered_map_enables_StringName_lookups) -{ - TransparentMap map; - [[maybe_unused]] const auto it = map.find(StringName{"i don't need to be converted into a std::string :)"}); -} - -TEST(TransparentStringViewHasher, produces_same_hash_for_all_of_OSCs_string_types) -{ - for (const char* str : {"", "some string", "why not try three?"}) { - const auto hashes = std::to_array({ - TransparentStringViewHasher{}(str), - TransparentStringViewHasher{}(std::string_view{str}), - TransparentStringViewHasher{}(CStringView{str}), - TransparentStringViewHasher{}(std::string{str}), - TransparentStringViewHasher{}(StringName{str}), - }); - ASSERT_TRUE(rgs::equal_range(hashes, hashes.front())); - } -} From c41dbf92ae3132cab2ae0a95d0f3730f65d44c65 Mon Sep 17 00:00:00 2001 From: Adam Kewley Date: Tue, 16 Jul 2024 07:23:14 +0200 Subject: [PATCH 03/13] Drop benchmarks/ (unused) --- .gitmodules | 3 - CHANGELOG.md | 2 + CMakeLists.txt | 9 -- benchmarks/BenchOpenSimCreator/CMakeLists.txt | 32 ------- .../Utils/BenchOpenSimHelpers.cpp | 86 ------------------- benchmarks/CMakeLists.txt | 3 - scripts/build_emscripten.sh | 2 +- third_party/benchmark | 1 - 8 files changed, 3 insertions(+), 135 deletions(-) delete mode 100644 benchmarks/BenchOpenSimCreator/CMakeLists.txt delete mode 100644 benchmarks/BenchOpenSimCreator/Utils/BenchOpenSimHelpers.cpp delete mode 100644 benchmarks/CMakeLists.txt delete mode 160000 third_party/benchmark diff --git a/.gitmodules b/.gitmodules index b5f1f6c40a..281b27a0ca 100644 --- a/.gitmodules +++ b/.gitmodules @@ -31,9 +31,6 @@ [submodule "third_party/lunasvg"] path = third_party/lunasvg url = https://github.com/ComputationalBiomechanicsLab/lunasvg -[submodule "third_party/benchmark"] - path = third_party/benchmark - url = https://github.com/ComputationalBiomechanicsLab/benchmark [submodule "third_party/unordered_dense"] path = third_party/unordered_dense url = https://github.com/ComputationalBiomechanicsLab/unordered_dense diff --git a/CHANGELOG.md b/CHANGELOG.md index b608a5f202..3980b8ac40 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,8 @@ on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Internal: Added barebones `TemporaryFile` class (handy for testing IO-related code) - Internal: `ImGuizmo` was mostly refactored into an internal concern, with OSC using an easier-to-integrate wrapper class +- Internal: `benchmarks/` were dropped (unused: microbenchmarks are better-suited to + `opensim-core`) ## [0.5.12] - 2024/04/29 diff --git a/CMakeLists.txt b/CMakeLists.txt index cea212e2ed..7f97ffb502 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -14,11 +14,6 @@ set( CACHE BOOL "enable/disable building the documentation (requires that sphinx-build is available on the PATH)" ) -set( - OSC_BUILD_BENCHMARKS ON - CACHE BOOL - "enable/disable building the benchmark suite (handy for development)" -) set( OSC_EMSCRIPTEN OFF CACHE BOOL @@ -68,7 +63,3 @@ add_subdirectory(tests) if(${OSC_BUILD_DOCS}) add_subdirectory(docs) endif() - -if(${OSC_BUILD_BENCHMARKS}) - add_subdirectory(benchmarks) -endif() diff --git a/benchmarks/BenchOpenSimCreator/CMakeLists.txt b/benchmarks/BenchOpenSimCreator/CMakeLists.txt deleted file mode 100644 index 35bda0a79c..0000000000 --- a/benchmarks/BenchOpenSimCreator/CMakeLists.txt +++ /dev/null @@ -1,32 +0,0 @@ -find_package(benchmark REQUIRED CONFIG) - -# BenchOpenSimCreator: main exe that links to `oscar` and benchmarks parts of the APIs -add_executable(BenchOpenSimCreator - - Utils/BenchOpenSimHelpers.cpp -) - -target_link_libraries(BenchOpenSimCreator PUBLIC - # set compile options - oscar_compiler_configuration - - # link to the to-be-tested library - OpenSimCreator - - # link to testing library - benchmark::benchmark - benchmark::benchmark_main -) - -# for development on Windows, copy all runtime dlls to the exe directory -# (because Windows doesn't have an RPATH) -# -# see: https://cmake.org/cmake/help/latest/manual/cmake-generator-expressions.7.html?highlight=runtime#genex:TARGET_RUNTIME_DLLS -if (WIN32) - add_custom_command( - TARGET BenchOpenSimCreator - PRE_BUILD - COMMAND ${CMAKE_COMMAND} -E copy_if_different $ $ - COMMAND_EXPAND_LISTS - ) -endif() diff --git a/benchmarks/BenchOpenSimCreator/Utils/BenchOpenSimHelpers.cpp b/benchmarks/BenchOpenSimCreator/Utils/BenchOpenSimHelpers.cpp deleted file mode 100644 index a23867db9f..0000000000 --- a/benchmarks/BenchOpenSimCreator/Utils/BenchOpenSimHelpers.cpp +++ /dev/null @@ -1,86 +0,0 @@ -#include - -#include -#include -#include - -#include - -struct NestedComponentChain { - std::unique_ptr root; - OpenSim::Component const* deepestChild; -}; - -static NestedComponentChain GenerateNestedComponentChain() -{ - auto root = std::make_unique(); - root->setName("rootName"); - - auto firstChild = std::make_unique(); - firstChild->setName("firstChild"); - - auto secondChild = std::make_unique(); - secondChild->setName("secondChild"); - - auto lastChild = std::make_unique(); - lastChild->setName("lastChild"); - - OpenSim::Component* lastChildPtr = lastChild.get(); - - secondChild->addComponent(lastChild.release()); - firstChild->addComponent(secondChild.release()); - root->addComponent(firstChild.release()); - - return NestedComponentChain{std::move(root), lastChildPtr}; -} - -static void BM_OpenSimGetAbsolutePathString(benchmark::State& state) -{ - NestedComponentChain c = GenerateNestedComponentChain(); - for ([[maybe_unused]] auto _ : state) - { - benchmark::DoNotOptimize(c.deepestChild->getAbsolutePathString()); - } -} -BENCHMARK(BM_OpenSimGetAbsolutePathString); - -static void BM_OscGetAbsolutePathString(benchmark::State& state) -{ - NestedComponentChain c = GenerateNestedComponentChain(); - for ([[maybe_unused]] auto _ : state) - { - benchmark::DoNotOptimize(osc::GetAbsolutePathString(*c.deepestChild)); - } -} -BENCHMARK(BM_OscGetAbsolutePathString); - -static void BM_OscGetAbsolutePathStringAssigning(benchmark::State& state) -{ - NestedComponentChain c = GenerateNestedComponentChain(); - std::string out; - for ([[maybe_unused]] auto _ : state) - { - osc::GetAbsolutePathString(*c.deepestChild, out); - } -} -BENCHMARK(BM_OscGetAbsolutePathStringAssigning); - -static void BM_OpenSimGetAbsolutePath(benchmark::State& state) -{ - NestedComponentChain c = GenerateNestedComponentChain(); - for ([[maybe_unused]] auto _ : state) - { - benchmark::DoNotOptimize(c.deepestChild->getAbsolutePath()); - } -} -BENCHMARK(BM_OpenSimGetAbsolutePath); - -static void BM_OscGetAbsolutePath(benchmark::State& state) -{ - NestedComponentChain c = GenerateNestedComponentChain(); - for ([[maybe_unused]] auto _ : state) - { - benchmark::DoNotOptimize(osc::GetAbsolutePath(*c.deepestChild)); - } -} -BENCHMARK(BM_OscGetAbsolutePath); diff --git a/benchmarks/CMakeLists.txt b/benchmarks/CMakeLists.txt deleted file mode 100644 index ab71fbbfdf..0000000000 --- a/benchmarks/CMakeLists.txt +++ /dev/null @@ -1,3 +0,0 @@ -if(${OSC_BUILD_OPENSIMCREATOR}) - add_subdirectory(BenchOpenSimCreator) -endif() diff --git a/scripts/build_emscripten.sh b/scripts/build_emscripten.sh index ba040d6fa0..61ece11324 100755 --- a/scripts/build_emscripten.sh +++ b/scripts/build_emscripten.sh @@ -25,7 +25,7 @@ source ./emsdk/emsdk_env.sh # - this is a custom build for now because I can't build all dependencies # with emsdk yet -CXXFLAGS="-fexceptions --use-port=sdl2" emcmake cmake -S third_party/ -B osc-deps-build -DOSCDEPS_GET_BENCHMARK=OFF -DOSCDEPS_GET_SDL=OFF -DOSCDEPS_GET_GLEW=OFF -DOSCDEPS_GET_NATIVEFILEDIALOG=OFF -DOSCDEPS_GET_OPENSIM=OFF -DCMAKE_INSTALL_PREFIX=${PWD}/osc-deps-install -DCMAKE_INSTALL_LIBDIR=${PWD}/osc-deps-install/lib +CXXFLAGS="-fexceptions --use-port=sdl2" emcmake cmake -S third_party/ -B osc-deps-build -DOSCDEPS_GET_SDL=OFF -DOSCDEPS_GET_GLEW=OFF -DOSCDEPS_GET_NATIVEFILEDIALOG=OFF -DOSCDEPS_GET_OPENSIM=OFF -DCMAKE_INSTALL_PREFIX=${PWD}/osc-deps-install -DCMAKE_INSTALL_LIBDIR=${PWD}/osc-deps-install/lib emmake cmake --build osc-deps-build -j$(nproc) -v LDFLAGS="-fexceptions -sNO_DISABLE_EXCEPTION_CATCHING=1 -sUSE_WEBGL2=1 -sMIN_WEBGL_VERSION=2 -sFULL_ES2=1 -sFULL_ES3=1 -sUSE_SDL=2" CXXFLAGS="-fexceptions --use-port=sdl2" emcmake cmake -S . -B osc-build -DOSC_BUILD_OPENSIMCREATOR=OFF -DOSC_DISCOVER_TESTS=OFF -DOSC_EMSCRIPTEN=ON -DCMAKE_PREFIX_PATH=${PWD}/osc-deps-install -DCMAKE_FIND_ROOT_PATH_MODE_PACKAGE=BOTH diff --git a/third_party/benchmark b/third_party/benchmark deleted file mode 160000 index a4cf155615..0000000000 --- a/third_party/benchmark +++ /dev/null @@ -1 +0,0 @@ -Subproject commit a4cf155615c63e019ae549e31703bf367df5b471 From a001ec2d56b3d5a6ec9ecef11f338a38015882bb Mon Sep 17 00:00:00 2001 From: Adam Kewley Date: Tue, 16 Jul 2024 07:24:43 +0200 Subject: [PATCH 04/13] Remove benchmarks from third_party/CMakeLists.txt --- third_party/CMakeLists.txt | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/third_party/CMakeLists.txt b/third_party/CMakeLists.txt index 71f0842ac0..1636397f74 100644 --- a/third_party/CMakeLists.txt +++ b/third_party/CMakeLists.txt @@ -4,7 +4,6 @@ project(osc-dependencies) # -------------- gather user-facing build cache vars ---------------- # set(OSCDEPS_GET_GOOGLETEST ON CACHE BOOL "enable getting googletest") -set(OSCDEPS_GET_BENCHMARK ON CACHE BOOL "enable getting benchmark") set(OSCDEPS_GET_LUNASVG ON CACHE BOOL "enable getting lunasvg") set(OSCDEPS_GET_SDL ON CACHE BOOL "enable getting SDL") set(OSCDEPS_GET_TOMLPLUSPLUS ON CACHE BOOL "enable gettting tomlplusplus") @@ -60,19 +59,6 @@ if(${OSCDEPS_GET_GOOGLETEST}) ) endif() -if(${OSCDEPS_GET_BENCHMARK}) - ExternalProject_Add(benchmark - DEPENDS googletest - SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/benchmark - BUILD_ALWAYS ${OSCDEPS_BUILD_ALWAYS} - CMAKE_CACHE_ARGS - ${OSCDEPS_DEPENDENCY_CMAKE_ARGS} - -DGOOGLETEST_PATH:STRING=${CMAKE_CURRENT_SOURCE_DIR}/googletest - -DBENCHMARK_ENABLE_TESTING:BOOL=OFF - -DBENCHMARK_INSTALL_DOCS:BOOL=OFF - ) -endif() - if(${OSCDEPS_GET_LUNASVG}) ExternalProject_Add(lunasvg SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/lunasvg From ac7a2cd5da92e729eb95742a603352b42ba74f70 Mon Sep 17 00:00:00 2001 From: Adam Kewley Date: Tue, 16 Jul 2024 07:36:20 +0200 Subject: [PATCH 05/13] Change base build type to 'Release' --- scripts/build_emscripten.sh | 13 +++++++++++-- scripts/build_windows.py | 2 +- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/scripts/build_emscripten.sh b/scripts/build_emscripten.sh index 61ece11324..76a01ecd8d 100755 --- a/scripts/build_emscripten.sh +++ b/scripts/build_emscripten.sh @@ -8,6 +8,15 @@ set -xeuo pipefail +# "base" build type to use when build types haven't been specified +OSC_BASE_BUILD_TYPE=${OSC_BASE_BUILD_TYPE:-Release} + +# build type for all of OSC's dependencies +OSC_DEPS_BUILD_TYPE=${OSC_DEPS_BUILD_TYPE:-`echo ${OSC_BASE_BUILD_TYPE}`} + +# build type for OSC +OSC_BUILD_TYPE=${OSC_BUILD_TYPE-`echo ${OSC_BASE_BUILD_TYPE}`} + # install emsdk if [ ! -d emsdk ]; then git clone https://github.com/emscripten-core/emsdk.git @@ -25,10 +34,10 @@ source ./emsdk/emsdk_env.sh # - this is a custom build for now because I can't build all dependencies # with emsdk yet -CXXFLAGS="-fexceptions --use-port=sdl2" emcmake cmake -S third_party/ -B osc-deps-build -DOSCDEPS_GET_SDL=OFF -DOSCDEPS_GET_GLEW=OFF -DOSCDEPS_GET_NATIVEFILEDIALOG=OFF -DOSCDEPS_GET_OPENSIM=OFF -DCMAKE_INSTALL_PREFIX=${PWD}/osc-deps-install -DCMAKE_INSTALL_LIBDIR=${PWD}/osc-deps-install/lib +CXXFLAGS="-fexceptions --use-port=sdl2" emcmake cmake -S third_party/ -B osc-deps-build -DOSCDEPS_GET_SDL=OFF -DOSCDEPS_GET_GLEW=OFF -DOSCDEPS_GET_NATIVEFILEDIALOG=OFF -DOSCDEPS_GET_OPENSIM=OFF -DCMAKE_INSTALL_PREFIX=${PWD}/osc-deps-install -DCMAKE_INSTALL_LIBDIR=${PWD}/osc-deps-install/lib -DCMAKE_BUILD_TYPE=${OSC_DEPS_BUILD_TYPE} emmake cmake --build osc-deps-build -j$(nproc) -v -LDFLAGS="-fexceptions -sNO_DISABLE_EXCEPTION_CATCHING=1 -sUSE_WEBGL2=1 -sMIN_WEBGL_VERSION=2 -sFULL_ES2=1 -sFULL_ES3=1 -sUSE_SDL=2" CXXFLAGS="-fexceptions --use-port=sdl2" emcmake cmake -S . -B osc-build -DOSC_BUILD_OPENSIMCREATOR=OFF -DOSC_DISCOVER_TESTS=OFF -DOSC_EMSCRIPTEN=ON -DCMAKE_PREFIX_PATH=${PWD}/osc-deps-install -DCMAKE_FIND_ROOT_PATH_MODE_PACKAGE=BOTH +LDFLAGS="-fexceptions -sNO_DISABLE_EXCEPTION_CATCHING=1 -sUSE_WEBGL2=1 -sMIN_WEBGL_VERSION=2 -sFULL_ES2=1 -sFULL_ES3=1 -sUSE_SDL=2" CXXFLAGS="-fexceptions --use-port=sdl2" emcmake cmake -S . -B osc-build -DOSC_BUILD_OPENSIMCREATOR=OFF -DOSC_DISCOVER_TESTS=OFF -DOSC_EMSCRIPTEN=ON -DCMAKE_PREFIX_PATH=${PWD}/osc-deps-install -DCMAKE_FIND_ROOT_PATH_MODE_PACKAGE=BOTH -DCMAKE_BUILD_TYPE=-DCMAKE_BUILD_TYPE=${OSC_BUILD_TYPE} emmake cmake --build osc-build --target testoscar -v -j$(nproc) emmake cmake --build osc-build --target hellotriangle -v -j$(nproc) diff --git a/scripts/build_windows.py b/scripts/build_windows.py index 83ae337e47..3378f87571 100644 --- a/scripts/build_windows.py +++ b/scripts/build_windows.py @@ -30,7 +30,7 @@ class BuildConfiguration: def __init__( self, *, - base_build_type="RelWithDebInfo", + base_build_type="Release", build_dir=os.curdir, build_concurrency=multiprocessing.cpu_count(), build_target="package", From 4c269921d23f2c0c163ea22663a3f6fbff818bfe Mon Sep 17 00:00:00 2001 From: Adam Kewley Date: Tue, 16 Jul 2024 10:05:01 +0200 Subject: [PATCH 06/13] Fix bad equal_range use in TransparentStringHasher test --- tests/testoscar/Utils/TestTransparentStringHasher.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/testoscar/Utils/TestTransparentStringHasher.cpp b/tests/testoscar/Utils/TestTransparentStringHasher.cpp index c358d48379..1bae9c244e 100644 --- a/tests/testoscar/Utils/TestTransparentStringHasher.cpp +++ b/tests/testoscar/Utils/TestTransparentStringHasher.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include @@ -53,6 +54,6 @@ TEST(TransparentStringHasher, produces_same_hash_for_all_of_OSCs_string_types) TransparentStringHasher{}(std::string{str}), TransparentStringHasher{}(StringName{str}), }); - ASSERT_TRUE(rgs::equal_range(hashes, hashes.front())); + ASSERT_TRUE(rgs::adjacent_find(hashes, rgs::not_equal_to{}) == hashes.end()); } } From d07985d009458e60a93f97056ef8388980a0cf7d Mon Sep 17 00:00:00 2001 From: Adam Kewley Date: Wed, 17 Jul 2024 20:19:48 +0200 Subject: [PATCH 07/13] Fix all gizmo transformations to actually use the correct maths ;) --- CHANGELOG.md | 3 + .../UI/MeshImporter/MeshImporterTab.cpp | 32 +- .../UI/Shared/ModelSelectionGizmo.cpp | 282 ++++++++---------- src/oscar/Maths/Qua.h | 8 +- src/oscar/UI/oscimgui.cpp | 33 +- src/oscar/UI/oscimgui.h | 18 +- 6 files changed, 167 insertions(+), 209 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3980b8ac40..c5f75e2d3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Selecting an `OpenSim::Joint` that has `OpenSim::PhysicalOffsetFrame`s for both its parent and child frames now shows a 3D manipulation gizmo that lets you move the joint center without moving anything else in the model (#159) +- Manipulating a child offset frame of a `OpenSim::Joint` should now work correctly for both + translation and rotation, allowing you visually place the parent of the child offset frame + (e.g. a body) that's moved by the joint - The UI now remembers which panels (e.g. Log, Properties) you had open between boots (#17) - The mesh warping tab now has a `Visualization Options` menu, which lets users toggle a few basic visualization options (e.g. grid lines, #892) diff --git a/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp b/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp index 2467c13e05..77c521cbad 100644 --- a/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp +++ b/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp @@ -203,7 +203,7 @@ class osc::mi::MeshImporterTab::Impl final : public IMeshImporterUILayerHost { updateFromImGuiKeyboardState(); } - if (!m_Maybe3DViewerModal && m_Shared->isRenderHovered() && !m_GizmoState.is_using()) + if (!m_Maybe3DViewerModal && m_Shared->isRenderHovered() && !m_Gizmo.is_using()) { ui::update_polar_camera_from_mouse_inputs(m_Shared->updCamera(), m_Shared->get3DSceneDims()); } @@ -828,7 +828,7 @@ class osc::mi::MeshImporterTab::Impl final : public IMeshImporterUILayerHost { tryAddingStationAtMousePosToHoveredElement(); return true; } - else if (m_GizmoState.handle_keyboard_inputs()) + else if (m_Gizmo.handle_keyboard_inputs()) { return true; } @@ -1826,9 +1826,9 @@ class osc::mi::MeshImporterTab::Impl final : public IMeshImporterUILayerHost { ui::same_line(); { - ui::GizmoOperation op = m_GizmoState.operation(); + ui::GizmoOperation op = m_Gizmo.operation(); if (ui::draw_gizmo_op_selector(op)) { - m_GizmoState.set_operation(op); + m_Gizmo.set_operation(op); } } @@ -1838,9 +1838,9 @@ class osc::mi::MeshImporterTab::Impl final : public IMeshImporterUILayerHost { // local/global dropdown { - ui::GizmoMode mode = m_GizmoState.mode(); + ui::GizmoMode mode = m_Gizmo.mode(); if (ui::draw_gizmo_mode_selector(mode)) { - m_GizmoState.set_mode(mode); + m_Gizmo.set_mode(mode); } } ui::same_line(); @@ -2067,7 +2067,7 @@ class osc::mi::MeshImporterTab::Impl final : public IMeshImporterUILayerHost { // // this is so that we can at least *show* the manipulation axes, and // because the user might start manipulating during this frame - if (not m_GizmoState.is_using()) { + if (not m_Gizmo.is_using()) { auto it = m_Shared->getCurrentSelection().begin(); auto end = m_Shared->getCurrentSelection().end(); @@ -2107,14 +2107,14 @@ class osc::mi::MeshImporterTab::Impl final : public IMeshImporterUILayerHost { Rect sceneRect = m_Shared->get3DSceneRect(); const Mat4 oldModelMatrix = m_GizmoModelMatrix; - const auto userManipulation = m_GizmoState.draw( + const auto userManipulation = m_Gizmo.draw( m_GizmoModelMatrix, m_Shared->getCamera().view_matrix(), m_Shared->getCamera().projection_matrix(aspect_ratio_of(sceneRect)), sceneRect ); - if (m_GizmoState.was_using() and not m_GizmoState.is_using()) { + if (m_Gizmo.was_using() and not m_Gizmo.is_using()) { // the user stopped editing, so save it and re-render m_Shared->commitCurrentModelGraph("manipulated selection"); App::upd().request_redraw(); @@ -2126,14 +2126,12 @@ class osc::mi::MeshImporterTab::Impl final : public IMeshImporterUILayerHost { for (UID id : m_Shared->getCurrentSelection()) { MIObject& el = m_Shared->updModelGraph().updByID(id); - switch (m_GizmoState.operation()) { + switch (m_Gizmo.operation()) { case ui::GizmoOperation::Rotate: el.applyRotation(m_Shared->getModelGraph(), userManipulation->rotation, m_GizmoModelMatrix[3]); break; case ui::GizmoOperation::Translate: { - // transform local-space position into ground, which is what `applyTransform` expects - const Vec3 worldTranslation = transform_point(oldModelMatrix, userManipulation->position); - el.applyTranslation(m_Shared->getModelGraph(), worldTranslation); + el.applyTranslation(m_Shared->getModelGraph(), userManipulation->position); break; } case ui::GizmoOperation::Scale: @@ -2153,7 +2151,7 @@ class osc::mi::MeshImporterTab::Impl final : public IMeshImporterUILayerHost { return m_MaybeHover; } - if (m_GizmoState.is_using()) + if (m_Gizmo.is_using()) { return MeshImporterHover{}; } @@ -2172,7 +2170,7 @@ class osc::mi::MeshImporterTab::Impl final : public IMeshImporterUILayerHost { const bool lcClicked = ui::is_mouse_released_without_dragging(ImGuiMouseButton_Left); const bool shiftDown = ui::is_shift_down(); const bool altDown = ui::is_alt_down(); - const bool isUsingGizmo = m_GizmoState.is_using(); + const bool isUsingGizmo = m_Gizmo.is_using(); if (!m_MaybeHover && lcClicked && !isUsingGizmo && !shiftDown) { @@ -2238,7 +2236,7 @@ class osc::mi::MeshImporterTab::Impl final : public IMeshImporterUILayerHost { // draw 3D scene (effectively, as an ui::Image) m_Shared->drawScene(MIObjects); - if (m_Shared->isRenderHovered() && ui::is_mouse_released_without_dragging(ImGuiMouseButton_Right) && !m_GizmoState.is_using()) + if (m_Shared->isRenderHovered() && ui::is_mouse_released_without_dragging(ImGuiMouseButton_Right) && !m_Gizmo.is_using()) { m_MaybeOpenedContextMenu = m_MaybeHover; ui::open_popup("##maincontextmenu"); @@ -2435,7 +2433,7 @@ class osc::mi::MeshImporterTab::Impl final : public IMeshImporterUILayerHost { std::shared_ptr m_Maybe3DViewerModal; // Gizmo state - ui::Gizmo m_GizmoState; + ui::Gizmo m_Gizmo; Mat4 m_GizmoModelMatrix = identity(); // manager for active modal popups (importer popups, etc.) diff --git a/src/OpenSimCreator/UI/Shared/ModelSelectionGizmo.cpp b/src/OpenSimCreator/UI/Shared/ModelSelectionGizmo.cpp index 597bbfce26..f80fc938ef 100644 --- a/src/OpenSimCreator/UI/Shared/ModelSelectionGizmo.cpp +++ b/src/OpenSimCreator/UI/Shared/ModelSelectionGizmo.cpp @@ -30,6 +30,7 @@ #include #include #include +#include #include #include @@ -83,14 +84,9 @@ namespace return implGetCurrentTransformInGround(); } - void onApplyTranslation(const Vec3& translationInGround) + void onApplyTransform(const ui::GizmoTransform& transformInGround) { - implOnApplyTranslation(translationInGround); - } - - void onApplyRotation(const Eulers& eulersInLocalSpace) - { - implOnApplyRotation(eulersInLocalSpace); + implOnApplyTransform(transformInGround); } void onSave() @@ -100,8 +96,7 @@ namespace private: virtual SupportedManipulationOpFlags implGetSupportedManipulationOps() const = 0; virtual Mat4 implGetCurrentTransformInGround() const = 0; - virtual void implOnApplyTranslation(const Vec3&) {} // default to noop - virtual void implOnApplyRotation(const Eulers&) {} // default to noop + virtual void implOnApplyTransform(const ui::GizmoTransform&) = 0; virtual void implOnSave() = 0; }; @@ -160,23 +155,13 @@ namespace } // perform runtime lookup for `AssociatedComponent` and forward into concrete implementation - void implOnApplyTranslation(const Vec3& translationInGround) final - { - const AssociatedComponent* maybeSelected = findSelection(); - if (not maybeSelected) { - return; // selection of that type does not exist in the model - } - implOnApplyTranslation(*maybeSelected, translationInGround); - } - - // perform runtime lookup for `AssociatedComponent` and forward into concrete implementation - void implOnApplyRotation(const Eulers& eulersInLocalSpace) final + void implOnApplyTransform(const ui::GizmoTransform& transformInGround) final { const AssociatedComponent* maybeSelected = findSelection(); if (not maybeSelected) { return; // selection of that type does not exist in the model } - implOnApplyRotation(*maybeSelected, eulersInLocalSpace); + implOnApplyTransform(*maybeSelected, transformInGround); } void implOnSave() final @@ -197,8 +182,7 @@ namespace // inheritors must implement concrete manipulation methods virtual Mat4 implGetCurrentTransformInGround(const AssociatedComponent&) const = 0; - virtual void implOnApplyTranslation(const AssociatedComponent&, const Vec3& translationInGround) = 0; - virtual void implOnApplyRotation(const AssociatedComponent&, [[maybe_unused]] const Eulers& eulersInLocalSpace) {} + virtual void implOnApplyTransform(const AssociatedComponent&, const ui::GizmoTransform& transformInGround) = 0; std::shared_ptr m_Model; OpenSim::ComponentPath m_ComponentAbsPath; @@ -233,13 +217,15 @@ namespace return transformInGround; } - void implOnApplyTranslation( + void implOnApplyTransform( const OpenSim::Station& station, - const Vec3& translationInGround) final + const ui::GizmoTransform& transformInGround) final { + // ignores `scale` and `position` + const SimTK::Rotation parentToGroundRotation = station.getParentFrame().getRotationInGround(getState()); const SimTK::InverseRotation& groundToParentRotation = parentToGroundRotation.invert(); - const Vec3 translationInParent = ToVec3(groundToParentRotation * ToSimTKVec3(translationInGround)); + const Vec3 translationInParent = ToVec3(groundToParentRotation * ToSimTKVec3(transformInGround.position)); ActionTranslateStation(getUndoableModel(), station, translationInParent); } @@ -275,13 +261,13 @@ namespace return transformInGround; } - void implOnApplyTranslation( + void implOnApplyTransform( const OpenSim::PathPoint& pathPoint, - const Vec3& translationInGround) final + const ui::GizmoTransform& transformInGround) final { const SimTK::Rotation parentToGroundRotation = pathPoint.getParentFrame().getRotationInGround(getState()); const SimTK::InverseRotation& groundToParentRotation = parentToGroundRotation.invert(); - const Vec3 translationInParent = ToVec3(groundToParentRotation * ToSimTKVec3(translationInGround)); + const Vec3 translationInParent = ToVec3(groundToParentRotation * ToSimTKVec3(transformInGround.position)); ActionTranslatePathPoint(getUndoableModel(), pathPoint, translationInParent); } @@ -330,55 +316,66 @@ namespace // if the POF that's being edited is the child frame of a joint then // its offset/orientation is constrained to be in the same location/orientation // as the joint's parent frame (plus coordinate transforms) - auto t = pof.getTransformInGround(getState()); - t.updR() = NegateRotation(t.R()); - t.updP() = pof.getParentFrame().getPositionInGround(getState()); - return ToMat4x4(t); + return ToMat4x4(pof.getParentFrame().getTransformInGround(getState())); } else { return ToMat4x4(pof.getTransformInGround(getState())); } } - void implOnApplyTranslation( + void implOnApplyTransform( const OpenSim::PhysicalOffsetFrame& pof, - const Vec3& translationInGround) final + const ui::GizmoTransform& transformInGround) final { - SimTK::Rotation parentToGroundRotation = pof.getParentFrame().getRotationInGround(getState()); if (m_IsChildFrameOfJoint) { - parentToGroundRotation = NegateRotation(parentToGroundRotation); + // the difference here is that the child frame's translation/rotation in ground + // is dictated by joints, but the user is manipulating stuff "as-if" they were + // editing the parent frame + // + // M_n * M_pofg * M_p^-1 * v_parent = M_pofg * X^-1 * v_parent + // + // - M_n user-enacted transformation in ground + // - M_pofg pof-to-ground transform + // - M_p pof-to-parent transform + // - v_parent a point, expressed in the pof's parent + + const SimTK::Transform M_n = {ToSimTKRotation(transformInGround.rotation), ToSimTKVec3(transformInGround.position)}; + const SimTK::Transform M_pofg = pof.getTransformInGround(getState()); + const SimTK::Transform M_p = pof.findTransformBetween(getState(), pof.getParentFrame()); + const SimTK::Transform X = (M_pofg.invert() * M_n * M_pofg * M_p.invert()).invert(); + + ActionTransformPofV2( + getUndoableModel(), + pof, + ToVec3(X.p()), + ToVec3(X.R().convertRotationToBodyFixedXYZ()) + ); + } + else { + // this might disgust you to hear, but the easiest way to figure this out is by + // getting out a pen and paper and solving the following for X: + // + // M_n * M_g * M_p * v_pof = M_g * X * v_pof + // + // where: + // + // - M_n user-enacted transformation in ground + // - M_g parent-to-ground transform + // - M_p pof-to-parent transform + // - v_pof a point, expressed in the pof + + const SimTK::Transform M_n = {ToSimTKRotation(transformInGround.rotation), ToSimTKVec3(transformInGround.position)}; + const SimTK::Transform M_g = pof.getParentFrame().getTransformInGround(getState()); + const SimTK::Transform M_p = pof.findTransformBetween(getState(), pof.getParentFrame()); + const SimTK::Transform X = M_g.invert() * M_n * M_g * M_p; + + ActionTransformPofV2( + getUndoableModel(), + pof, + ToVec3(X.p()), + ToVec3(X.R().convertRotationToBodyFixedXYZ()) + ); } - const SimTK::InverseRotation& groundToParentRotation = parentToGroundRotation.invert(); - const SimTK::Vec3 parentTranslation = groundToParentRotation * ToSimTKVec3(translationInGround); - const SimTK::Vec3& eulersInPofFrame = pof.get_orientation(); - - ActionTransformPof( - getUndoableModel(), - pof, - ToVec3(parentTranslation), - ToVec3(eulersInPofFrame) - ); - } - - void implOnApplyRotation( - const OpenSim::PhysicalOffsetFrame& pof, - const Eulers& eulersInLocalSpace) final - { - const OpenSim::Frame& parent = pof.getParentFrame(); - const SimTK::State& state = getState(); - - const Quat deltaRotationInGround = to_worldspace_rotation_quat(m_IsChildFrameOfJoint ? -eulersInLocalSpace : eulersInLocalSpace); - const Quat oldRotationInGround = ToQuat(pof.getRotationInGround(state)); - const Quat parentRotationInGround = ToQuat(parent.getRotationInGround(state)); - const Quat newRotationInGround = normalize(deltaRotationInGround * oldRotationInGround); - const Quat newRotationInParent = inverse(parentRotationInGround) * newRotationInGround; - - ActionTransformPof( - getUndoableModel(), - pof, - Vec3{}, // no translation delta - extract_eulers_xyz(newRotationInParent) - ); } bool m_IsChildFrameOfJoint = false; @@ -409,41 +406,30 @@ namespace return ToMat4x4(wrapToGround); } - void implOnApplyTranslation( + void implOnApplyTransform( const OpenSim::WrapObject& wrapObj, - const Vec3& translationInGround) final + const ui::GizmoTransform& transformInGround) final { - const SimTK::Rotation frameToGroundRotation = wrapObj.getFrame().getTransformInGround(getState()).R(); - const SimTK::InverseRotation& groundToFrameRotation = frameToGroundRotation.invert(); - const Vec3 translationInPofFrame = ToVec3(groundToFrameRotation * ToSimTKVec3(translationInGround)); - - ActionTransformWrapObject( - getUndoableModel(), - wrapObj, - translationInPofFrame, - ToVec3(wrapObj.get_xyz_body_rotation()) - ); - } - - void implOnApplyRotation( - const OpenSim::WrapObject& wrapObj, - const Eulers& eulersInLocalSpace) final - { - const OpenSim::Frame& parent = wrapObj.getFrame(); - const SimTK::State& state = getState(); - - const Quat deltaRotationInGround = to_worldspace_rotation_quat(eulersInLocalSpace); - const Quat oldRotationInGround = ToQuat(parent.getTransformInGround(state).R() * wrapObj.getTransform().R()); - const Quat newRotationInGround = normalize(deltaRotationInGround * oldRotationInGround); + // solve for X: + // + // M_n * M_g * M_w * v = M_g * X * v + // + // where: + // + // - M_n user-enacted transform in ground + // - M_g parent-frame-to-ground transform + // - M_w wrap object local transform - const Quat parentRotationInGround = ToQuat(parent.getRotationInGround(state)); - const Quat newRotationInParent = inverse(parentRotationInGround) * newRotationInGround; + const SimTK::Transform M_n = {ToSimTKRotation(transformInGround.rotation), ToSimTKVec3(transformInGround.position)}; + const SimTK::Transform M_g = wrapObj.getFrame().getTransformInGround(getState()); + const SimTK::Transform M_w = wrapObj.getTransform(); + const SimTK::Transform X = M_g.invert() * M_n * M_g * M_w; ActionTransformWrapObject( getUndoableModel(), wrapObj, - Vec3{}, // no translation - extract_eulers_xyz(newRotationInParent) + ToVec3(X.p() - M_w.p()), + ToVec3(X.R().convertRotationToBodyFixedXYZ()) ); } }; @@ -473,41 +459,30 @@ namespace return ToMat4x4(wrapToGround); } - void implOnApplyTranslation( + void implOnApplyTransform( const OpenSim::ContactGeometry& contactGeom, - const Vec3& deltaTranslationInGround) final + const ui::GizmoTransform& transformInGround) final { - const SimTK::Rotation frameToGroundRotation = contactGeom.getFrame().getTransformInGround(getState()).R(); - const SimTK::InverseRotation& groundToFrameRotation = frameToGroundRotation.invert(); - const Vec3 translationInPofFrame = ToVec3(groundToFrameRotation * ToSimTKVec3(deltaTranslationInGround)); - - ActionTransformContactGeometry( - getUndoableModel(), - contactGeom, - translationInPofFrame, - ToVec3(contactGeom.get_orientation()) - ); - } - - void implOnApplyRotation( - const OpenSim::ContactGeometry& contactGeom, - const Eulers& deltaEulerRadiansInGround) final - { - const OpenSim::Frame& parent = contactGeom.getFrame(); - const SimTK::State& state = getState(); - - const Quat deltaRotationInGround = to_worldspace_rotation_quat(deltaEulerRadiansInGround); - const Quat oldRotationInGround = ToQuat(parent.getTransformInGround(state).R() * contactGeom.getTransform().R()); - const Quat newRotationInGround = normalize(deltaRotationInGround * oldRotationInGround); + // solve for X: + // + // M_n * M_g * M_w * v = M_g * X * v + // + // where: + // + // - M_n user-enacted transform in ground + // - M_g parent-frame-to-ground transform + // - M_w contact geometry local transform - const Quat parentRotationInGround = ToQuat(parent.getRotationInGround(state)); - const Quat newRotationInParent = inverse(parentRotationInGround) * newRotationInGround; + const SimTK::Transform M_n = {ToSimTKRotation(transformInGround.rotation), ToSimTKVec3(transformInGround.position)}; + const SimTK::Transform M_g = contactGeom.getFrame().getTransformInGround(getState()); + const SimTK::Transform M_w = contactGeom.getTransform(); + const SimTK::Transform X = M_g.invert() * M_n * M_g * M_w; ActionTransformContactGeometry( getUndoableModel(), contactGeom, - Vec3{}, // no translation - extract_eulers_xyz(newRotationInParent) + ToVec3(X.p() - M_w.p()), + ToVec3(X.R().convertRotationToBodyFixedXYZ()) ); } }; @@ -552,20 +527,9 @@ namespace return ToMat4x4(joint.getParentFrame().getTransformInGround(getState())); } - void implOnApplyTranslation(const OpenSim::Joint& joint, const Vec3& deltaTranslationInGround) final - { - applyTransform(joint, deltaTranslationInGround, Eulers{}); - } - - void implOnApplyRotation([[maybe_unused]] const OpenSim::Joint& joint, [[maybe_unused]] const Eulers& deltaEulersInLocalSpace) final - { - applyTransform(joint, Vec3{}, deltaEulersInLocalSpace); - } - - void applyTransform( + void implOnApplyTransform( const OpenSim::Joint& joint, - const Vec3& deltaTranslationInGround, - const Eulers& deltaEulersInLocalSpace) + const ui::GizmoTransform& transformInGround) final { // in order to move a joint center without every child also moving around, we need to: // @@ -573,9 +537,8 @@ namespace // STEP 2) figure out what transform the child offset frame would need to have in // order for its parent (confusing, eh) to not move // - // this ended up being a headfuck, but I figured out that the easiest way to tackle this - // is to carefully track+name each frame definition and trust in god by using linear - // algebra to figure out the rest. So, given: + // the easiest way to tackle this is to carefully track+name each frame definition + // and trust in god by using linear algebra to figure out the rest. So, given: // // - M_cpof1 joint child offset frame to its parent transform (1: BEFORE) // - M_j joint child-to-parent transform @@ -609,22 +572,20 @@ namespace const auto& childPOF = dynamic_cast(joint.getChildFrame()); // get BEFORE transforms - const auto M_j = childPOF.findTransformBetween(getState(), parentPOF); - const auto M_ppof1 = parentPOF.findTransformBetween(getState(), parentPOF.getParentFrame()); - const auto M_cpof1 = childPOF.findTransformBetween(getState(), childPOF.getParentFrame()); + const SimTK::Transform M_j = childPOF.findTransformBetween(getState(), parentPOF); + const SimTK::Transform M_ppof1 = parentPOF.findTransformBetween(getState(), parentPOF.getParentFrame()); + const SimTK::Transform M_cpof1 = childPOF.findTransformBetween(getState(), childPOF.getParentFrame()); // STEP 1) move the parent offset frame (as normal) { - const SimTK::Rotation R = ToSimTKRotation(deltaEulersInLocalSpace); - const auto& M_p = M_ppof1; - - const SimTK::Rotation X_r = M_p.R() * R; - const SimTK::Vec3 X_p = M_p.p() + ToSimTKVec3(deltaTranslationInGround); - const SimTK::Transform X{X_r, X_p}; - ActionTransformPof( + // M_n * M_g * M_ppof1 * v = M_g * X * v + const SimTK::Transform M_n{ToSimTKRotation(transformInGround.rotation), ToSimTKVec3(transformInGround.position)}; + const SimTK::Transform M_g = parentPOF.getParentFrame().getTransformInGround(getState()); + const SimTK::Transform X = M_g.invert() * M_n * M_g * M_ppof1; + ActionTransformPofV2( getUndoableModel(), parentPOF, - ToVec3(X.p() - M_ppof1.p()), + ToVec3(X.p()), ToVec3(X.R().convertRotationToBodyFixedXYZ()) ); } @@ -633,7 +594,7 @@ namespace // order for its parent (confusing, eh) to not move // get AFTER transforms - const auto M_ppof2 = parentPOF.findTransformBetween(getState(), parentPOF.getParentFrame()); + const SimTK::Transform M_ppof2 = parentPOF.findTransformBetween(getState(), parentPOF.getParentFrame()); // caclulate `M_cpof2` (i.e. the desired new child transform) const SimTK::Transform M_cpof2 = (M_j.invert() * M_ppof2.invert() * M_ppof1 * M_j * M_cpof1.invert()).invert(); @@ -682,24 +643,17 @@ namespace } // draw the manipulator - const Mat4 oldModelMatrix = manipulator.getCurrentTransformInGround(); - Mat4 modelMatrix = oldModelMatrix; - const auto userEdit = gizmo.draw( + Mat4 modelMatrix = manipulator.getCurrentTransformInGround(); + const auto userEditInGround = gizmo.draw( modelMatrix, camera.view_matrix(), camera.projection_matrix(aspect_ratio_of(screenRect)), screenRect ); - if (userEdit) { + + if (userEditInGround) { // propagate user edit to the model via the `ISelectionManipulator` interface - if (gizmo.operation() == ui::GizmoOperation::Translate) { - // `ISelectionManipulator` expects translations in ground - const Vec3 groundTranslation = transform_point(oldModelMatrix, userEdit->position); - manipulator.onApplyTranslation(groundTranslation); - } - else if (gizmo.operation() == ui::GizmoOperation::Rotate) { - manipulator.onApplyRotation(userEdit->rotation); - } + manipulator.onApplyTransform(*userEditInGround); } // once the user stops using the manipulator, save the changes diff --git a/src/oscar/Maths/Qua.h b/src/oscar/Maths/Qua.h index f8618060ba..d07302279e 100644 --- a/src/oscar/Maths/Qua.h +++ b/src/oscar/Maths/Qua.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include #include #include @@ -83,7 +84,7 @@ namespace osc } // constructs a `Qua` from euler angles (pitch, yaw, roll), in radians. - constexpr explicit Qua(const Vec<3, T>& euler_angles) + explicit Qua(const Vec<3, T>& euler_angles) { Vec<3, T> c = cos(euler_angles * T(0.5)); Vec<3, T> s = sin(euler_angles * T(0.5)); @@ -94,6 +95,11 @@ namespace osc this->z = c.x * c.y * s.z - s.x * s.y * c.z; } + // constructs a `Qua` from euler angles + explicit Qua(const Eulers& euler_angles) : + Qua(Vec3{euler_angles.x.count(), euler_angles.y.count(), euler_angles.z.count()}) + {} + // constructs a `Qua` by decomposing an orthogonal matrix explicit Qua(const Mat<3, 3, T>& m) { diff --git a/src/oscar/UI/oscimgui.cpp b/src/oscar/UI/oscimgui.cpp index 0787bfb060..d1882f3cd5 100644 --- a/src/oscar/UI/oscimgui.cpp +++ b/src/oscar/UI/oscimgui.cpp @@ -156,7 +156,6 @@ std::optional osc::ui::Gizmo::draw( ImGuizmo::AllowAxisFlip(false); // user's didn't like this feature in UX sessions // use rotation from the parent, translation from station - Mat4 old_model_matrix = model_matrix; Mat4 delta_matrix; // ensure style matches OSC requirements @@ -170,6 +169,7 @@ std::optional osc::ui::Gizmo::draw( style.ScaleLineCircleSize = 8.0f; } + const Vec3 original_translation = model_matrix[3]; const bool gizmo_was_manipulated_by_user = ImGuizmo::Manipulate( value_ptr(view_matrix), value_ptr(projection_matrix), @@ -187,33 +187,24 @@ std::optional osc::ui::Gizmo::draw( } // else: figure out the local-space transform - // decompose the overall transformation into component parts - // - // note: ImGuizmo returns: - // - // - scale in model-space - // - rotation in model-space - // - translation in world-space (!!) + // decompose the additional transformation into component parts Vec3 world_translation{}; - Vec3 local_rotation_degrees{}; - Vec3 local_scale{}; + Vec3 world_rotation_in_degrees{}; + Vec3 world_scale{}; ImGuizmo::DecomposeMatrixToComponents( value_ptr(delta_matrix), value_ptr(world_translation), - value_ptr(local_rotation_degrees), - value_ptr(local_scale) + value_ptr(world_rotation_in_degrees), + value_ptr(world_scale) ); - Eulers local_rotation = Vec<3, Degrees>(local_rotation_degrees); + const Eulers world_eulers = Vec<3, Degrees>(world_rotation_in_degrees); - return GizmoTransform{ - .scale = local_scale, - - // convert the euler angles to a quaternion rotation - .rotation = local_rotation, - - // put the rotation into model-space - .position = transform_point(inverse(old_model_matrix), world_translation), + const GizmoTransform rv = { + .scale = world_scale, + .rotation = world_eulers, + .position = world_translation, }; + return rv; } bool osc::ui::Gizmo::is_using() const diff --git a/src/oscar/UI/oscimgui.h b/src/oscar/UI/oscimgui.h index 34fab94078..f1840a5e12 100644 --- a/src/oscar/UI/oscimgui.h +++ b/src/oscar/UI/oscimgui.h @@ -834,7 +834,10 @@ namespace osc::ui ImGui::ShowDemoWindow(); } - // a single user-enacted manipulation performed with a `Gizmo` + // a the "difference" added by a user-enacted manipulation with a `Gizmo` + // + // this can be left-multiplied by the original model matrix to apply the + // user's transformation struct GizmoTransform final { Vec3 scale = {1.0f, 1.0f, 1.0f}; Eulers rotation = {}; @@ -860,13 +863,16 @@ namespace osc::ui class Gizmo final { public: - // if the user manipulated the gizmo, returns a model-space transform based on what the - // user did in the UI + // if the user manipulated the gizmo, updates `model_matrix` to match the + // post-manipulation transform and returns a world-space transform that + // represents the "difference" added by the user's manipulation. I.e.: // - // i.e. the transform returned by this function (T) is defined in model-space and can be - // right-multiplied by `model_matrix` to produce a new model-to-world transform: + // transform_returned * model_matrix_before = model_matrix_after // - // new_model_matrix = model_matrix * to_mat4(T) + // note: a user-enacted rotation/scale may not happen at the origin, so if + // you're thinking "oh I'm only handling rotation/scaling, so I'll + // ignore the translational part of the transform" then you're in + // for a nasty surprise: T(origin)*R*S*T(-origin) std::optional draw( Mat4& model_matrix, // edited in-place const Mat4& view_matrix, From 9f3f382a0901018b8ebf5b2ad9c54f987305294d Mon Sep 17 00:00:00 2001 From: Adam Kewley Date: Thu, 18 Jul 2024 05:38:53 +0200 Subject: [PATCH 08/13] Rename Eulers -> EulerAngles and change gizmo API to use SimTK::Transform --- .../Documents/MeshImporter/MIObject.cpp | 2 +- .../Documents/MeshImporter/MIObject.h | 4 +- .../Documents/Model/UndoableModelActions.cpp | 8 ++-- .../Documents/Model/UndoableModelActions.h | 10 ++--- .../RendererGeometryShaderTab.cpp | 4 +- .../UI/MeshImporter/MeshImporterTab.cpp | 2 +- .../UI/Shared/ModelSelectionGizmo.cpp | 43 ++++++++----------- src/OpenSimCreator/Utils/SimTKHelpers.cpp | 6 +-- src/OpenSimCreator/Utils/SimTKHelpers.h | 8 ++-- src/oscar/CMakeLists.txt | 2 +- src/oscar/Maths.h | 2 +- src/oscar/Maths/EulerAngles.h | 10 +++++ src/oscar/Maths/Eulers.h | 10 ----- src/oscar/Maths/MathHelpers.h | 8 ++-- src/oscar/Maths/MathsImplementation.cpp | 8 ++-- src/oscar/Maths/Qua.h | 4 +- src/oscar/Maths/QuaternionFunctions.h | 2 +- src/oscar/Maths/TransformFunctions.h | 8 ++-- src/oscar/UI/ImGuiHelpers.cpp | 4 +- src/oscar/UI/ImGuiHelpers.h | 4 +- src/oscar/UI/MouseCapturingCamera.h | 8 ++-- src/oscar/UI/oscimgui.cpp | 4 +- src/oscar/UI/oscimgui.h | 4 +- src/oscar_demos/HittestTab.cpp | 2 +- .../LOGLCoordinateSystemsTab.cpp | 2 +- tests/testoscar/Graphics/TestMesh.cpp | 6 +-- 26 files changed, 83 insertions(+), 92 deletions(-) create mode 100644 src/oscar/Maths/EulerAngles.h delete mode 100644 src/oscar/Maths/Eulers.h diff --git a/src/OpenSimCreator/Documents/MeshImporter/MIObject.cpp b/src/OpenSimCreator/Documents/MeshImporter/MIObject.cpp index 50f509e5a4..312a7c28d3 100644 --- a/src/OpenSimCreator/Documents/MeshImporter/MIObject.cpp +++ b/src/OpenSimCreator/Documents/MeshImporter/MIObject.cpp @@ -8,7 +8,7 @@ namespace rgs = std::ranges; void osc::mi::MIObject::applyRotation( const IObjectFinder& lookup, - const Eulers& eulerAngles, + const EulerAngles& eulerAngles, const Vec3& rotationCenter) { Transform t = getXForm(lookup); diff --git a/src/OpenSimCreator/Documents/MeshImporter/MIObject.h b/src/OpenSimCreator/Documents/MeshImporter/MIObject.h index 5799e1d6c3..3721f62013 100644 --- a/src/OpenSimCreator/Documents/MeshImporter/MIObject.h +++ b/src/OpenSimCreator/Documents/MeshImporter/MIObject.h @@ -6,7 +6,7 @@ #include #include -#include +#include #include #include #include @@ -149,7 +149,7 @@ namespace osc::mi void applyRotation( const IObjectFinder& lookup, - const Eulers& eulerAngles, + const EulerAngles& eulerAngles, const Vec3& rotationCenter ); diff --git a/src/OpenSimCreator/Documents/Model/UndoableModelActions.cpp b/src/OpenSimCreator/Documents/Model/UndoableModelActions.cpp index bef7554fc5..0efe8c6240 100644 --- a/src/OpenSimCreator/Documents/Model/UndoableModelActions.cpp +++ b/src/OpenSimCreator/Documents/Model/UndoableModelActions.cpp @@ -1934,7 +1934,7 @@ bool osc::ActionTransformPof( UndoableModelStatePair& model, const OpenSim::PhysicalOffsetFrame& pof, const Vec3& deltaTranslationInParentFrame, - const Eulers& newPofEulers) + const EulerAngles& newPofEulers) { const OpenSim::ComponentPath pofPath = GetAbsolutePath(pof); const UID oldVersion = model.getModelVersion(); @@ -1973,7 +1973,7 @@ bool osc::ActionTransformPofV2( UndoableModelStatePair& model, const OpenSim::PhysicalOffsetFrame& pof, const Vec3& newTranslation, - const Eulers& newEulers) + const EulerAngles& newEulers) { const OpenSim::ComponentPath pofPath = GetAbsolutePath(pof); const UID oldVersion = model.getModelVersion(); @@ -2009,7 +2009,7 @@ bool osc::ActionTransformWrapObject( UndoableModelStatePair& model, const OpenSim::WrapObject& wo, const Vec3& deltaPosition, - const Eulers& newEulers) + const EulerAngles& newEulers) { const OpenSim::ComponentPath pofPath = GetAbsolutePath(wo); const UID oldVersion = model.getModelVersion(); @@ -2048,7 +2048,7 @@ bool osc::ActionTransformContactGeometry( UndoableModelStatePair& model, const OpenSim::ContactGeometry& contactGeom, const Vec3& deltaPosition, - const Eulers& newEulers) + const EulerAngles& newEulers) { const OpenSim::ComponentPath pofPath = GetAbsolutePath(contactGeom); const UID oldVersion = model.getModelVersion(); diff --git a/src/OpenSimCreator/Documents/Model/UndoableModelActions.h b/src/OpenSimCreator/Documents/Model/UndoableModelActions.h index 1f0c5b8087..8f637083ad 100644 --- a/src/OpenSimCreator/Documents/Model/UndoableModelActions.h +++ b/src/OpenSimCreator/Documents/Model/UndoableModelActions.h @@ -2,7 +2,7 @@ #include -#include +#include #include #include @@ -415,28 +415,28 @@ namespace osc UndoableModelStatePair&, const OpenSim::PhysicalOffsetFrame&, const Vec3& deltaTranslationInParentFrame, - const Eulers& newPofEulers + const EulerAngles& newPofEulers ); bool ActionTransformPofV2( UndoableModelStatePair&, const OpenSim::PhysicalOffsetFrame&, const Vec3& newTranslation, - const Eulers& newEulers + const EulerAngles& newEulers ); bool ActionTransformWrapObject( UndoableModelStatePair&, const OpenSim::WrapObject&, const Vec3& deltaPosition, - const Eulers& newEulers + const EulerAngles& newEulers ); bool ActionTransformContactGeometry( UndoableModelStatePair&, const OpenSim::ContactGeometry&, const Vec3& deltaPosition, - const Eulers& newEulers + const EulerAngles& newEulers ); bool ActionFitSphereToMesh( diff --git a/src/OpenSimCreator/UI/Experimental/RendererGeometryShaderTab.cpp b/src/OpenSimCreator/UI/Experimental/RendererGeometryShaderTab.cpp index 3b079f465c..8b66cef108 100644 --- a/src/OpenSimCreator/UI/Experimental/RendererGeometryShaderTab.cpp +++ b/src/OpenSimCreator/UI/Experimental/RendererGeometryShaderTab.cpp @@ -10,7 +10,7 @@ #include #include #include -#include +#include #include #include #include @@ -118,7 +118,7 @@ class osc::RendererGeometryShaderTab::Impl final { Mesh m_Mesh = LoadMeshViaSimTK(App::resource_filepath("geometry/hat_ribs_scap.vtp")); Camera m_SceneCamera; bool m_IsMouseCaptured = false; - Eulers m_CameraEulers = {}; + EulerAngles m_CameraEulers = {}; Color m_MeshColor = Color::white(); }; diff --git a/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp b/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp index 77c521cbad..f55b4ef510 100644 --- a/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp +++ b/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp @@ -903,7 +903,7 @@ class osc::mi::MeshImporterTab::Impl final : public IMeshImporterUILayerHost { // rotation editor if (e.canChangeRotation()) { - Eulers eulers = euler_angles(normalize(e.rotation(m_Shared->getModelGraph()))); + EulerAngles eulers = to_euler_angles(normalize(e.rotation(m_Shared->getModelGraph()))); if (ui::draw_angle3_input("Rotation", eulers, "%.6f")) { diff --git a/src/OpenSimCreator/UI/Shared/ModelSelectionGizmo.cpp b/src/OpenSimCreator/UI/Shared/ModelSelectionGizmo.cpp index f80fc938ef..30e975421c 100644 --- a/src/OpenSimCreator/UI/Shared/ModelSelectionGizmo.cpp +++ b/src/OpenSimCreator/UI/Shared/ModelSelectionGizmo.cpp @@ -13,7 +13,7 @@ #include #include #include -#include +#include #include #include #include @@ -84,7 +84,7 @@ namespace return implGetCurrentTransformInGround(); } - void onApplyTransform(const ui::GizmoTransform& transformInGround) + void onApplyTransform(const SimTK::Transform& transformInGround) { implOnApplyTransform(transformInGround); } @@ -96,7 +96,7 @@ namespace private: virtual SupportedManipulationOpFlags implGetSupportedManipulationOps() const = 0; virtual Mat4 implGetCurrentTransformInGround() const = 0; - virtual void implOnApplyTransform(const ui::GizmoTransform&) = 0; + virtual void implOnApplyTransform(const SimTK::Transform&) = 0; virtual void implOnSave() = 0; }; @@ -155,7 +155,7 @@ namespace } // perform runtime lookup for `AssociatedComponent` and forward into concrete implementation - void implOnApplyTransform(const ui::GizmoTransform& transformInGround) final + void implOnApplyTransform(const SimTK::Transform& transformInGround) final { const AssociatedComponent* maybeSelected = findSelection(); if (not maybeSelected) { @@ -182,7 +182,7 @@ namespace // inheritors must implement concrete manipulation methods virtual Mat4 implGetCurrentTransformInGround(const AssociatedComponent&) const = 0; - virtual void implOnApplyTransform(const AssociatedComponent&, const ui::GizmoTransform& transformInGround) = 0; + virtual void implOnApplyTransform(const AssociatedComponent&, const SimTK::Transform& transformInGround) = 0; std::shared_ptr m_Model; OpenSim::ComponentPath m_ComponentAbsPath; @@ -219,13 +219,13 @@ namespace void implOnApplyTransform( const OpenSim::Station& station, - const ui::GizmoTransform& transformInGround) final + const SimTK::Transform& transformInGround) final { - // ignores `scale` and `position` + // ignores `rotation` const SimTK::Rotation parentToGroundRotation = station.getParentFrame().getRotationInGround(getState()); const SimTK::InverseRotation& groundToParentRotation = parentToGroundRotation.invert(); - const Vec3 translationInParent = ToVec3(groundToParentRotation * ToSimTKVec3(transformInGround.position)); + const Vec3 translationInParent = ToVec3(groundToParentRotation * transformInGround.p()); ActionTranslateStation(getUndoableModel(), station, translationInParent); } @@ -263,11 +263,13 @@ namespace void implOnApplyTransform( const OpenSim::PathPoint& pathPoint, - const ui::GizmoTransform& transformInGround) final + const SimTK::Transform& transformInGround) final { + // ignores `rotation` + const SimTK::Rotation parentToGroundRotation = pathPoint.getParentFrame().getRotationInGround(getState()); const SimTK::InverseRotation& groundToParentRotation = parentToGroundRotation.invert(); - const Vec3 translationInParent = ToVec3(groundToParentRotation * ToSimTKVec3(transformInGround.position)); + const Vec3 translationInParent = ToVec3(groundToParentRotation * transformInGround.p()); ActionTranslatePathPoint(getUndoableModel(), pathPoint, translationInParent); } @@ -325,7 +327,7 @@ namespace void implOnApplyTransform( const OpenSim::PhysicalOffsetFrame& pof, - const ui::GizmoTransform& transformInGround) final + const SimTK::Transform& M_n) final { if (m_IsChildFrameOfJoint) { // the difference here is that the child frame's translation/rotation in ground @@ -339,11 +341,9 @@ namespace // - M_p pof-to-parent transform // - v_parent a point, expressed in the pof's parent - const SimTK::Transform M_n = {ToSimTKRotation(transformInGround.rotation), ToSimTKVec3(transformInGround.position)}; const SimTK::Transform M_pofg = pof.getTransformInGround(getState()); const SimTK::Transform M_p = pof.findTransformBetween(getState(), pof.getParentFrame()); const SimTK::Transform X = (M_pofg.invert() * M_n * M_pofg * M_p.invert()).invert(); - ActionTransformPofV2( getUndoableModel(), pof, @@ -364,11 +364,9 @@ namespace // - M_p pof-to-parent transform // - v_pof a point, expressed in the pof - const SimTK::Transform M_n = {ToSimTKRotation(transformInGround.rotation), ToSimTKVec3(transformInGround.position)}; const SimTK::Transform M_g = pof.getParentFrame().getTransformInGround(getState()); const SimTK::Transform M_p = pof.findTransformBetween(getState(), pof.getParentFrame()); const SimTK::Transform X = M_g.invert() * M_n * M_g * M_p; - ActionTransformPofV2( getUndoableModel(), pof, @@ -408,7 +406,7 @@ namespace void implOnApplyTransform( const OpenSim::WrapObject& wrapObj, - const ui::GizmoTransform& transformInGround) final + const SimTK::Transform& M_n) final { // solve for X: // @@ -420,11 +418,9 @@ namespace // - M_g parent-frame-to-ground transform // - M_w wrap object local transform - const SimTK::Transform M_n = {ToSimTKRotation(transformInGround.rotation), ToSimTKVec3(transformInGround.position)}; const SimTK::Transform M_g = wrapObj.getFrame().getTransformInGround(getState()); const SimTK::Transform M_w = wrapObj.getTransform(); const SimTK::Transform X = M_g.invert() * M_n * M_g * M_w; - ActionTransformWrapObject( getUndoableModel(), wrapObj, @@ -461,7 +457,7 @@ namespace void implOnApplyTransform( const OpenSim::ContactGeometry& contactGeom, - const ui::GizmoTransform& transformInGround) final + const SimTK::Transform& M_n) final { // solve for X: // @@ -473,11 +469,9 @@ namespace // - M_g parent-frame-to-ground transform // - M_w contact geometry local transform - const SimTK::Transform M_n = {ToSimTKRotation(transformInGround.rotation), ToSimTKVec3(transformInGround.position)}; const SimTK::Transform M_g = contactGeom.getFrame().getTransformInGround(getState()); const SimTK::Transform M_w = contactGeom.getTransform(); const SimTK::Transform X = M_g.invert() * M_n * M_g * M_w; - ActionTransformContactGeometry( getUndoableModel(), contactGeom, @@ -527,9 +521,7 @@ namespace return ToMat4x4(joint.getParentFrame().getTransformInGround(getState())); } - void implOnApplyTransform( - const OpenSim::Joint& joint, - const ui::GizmoTransform& transformInGround) final + void implOnApplyTransform(const OpenSim::Joint& joint, const SimTK::Transform& M_n) final { // in order to move a joint center without every child also moving around, we need to: // @@ -579,7 +571,6 @@ namespace // STEP 1) move the parent offset frame (as normal) { // M_n * M_g * M_ppof1 * v = M_g * X * v - const SimTK::Transform M_n{ToSimTKRotation(transformInGround.rotation), ToSimTKVec3(transformInGround.position)}; const SimTK::Transform M_g = parentPOF.getParentFrame().getTransformInGround(getState()); const SimTK::Transform X = M_g.invert() * M_n * M_g * M_ppof1; ActionTransformPofV2( @@ -653,7 +644,7 @@ namespace if (userEditInGround) { // propagate user edit to the model via the `ISelectionManipulator` interface - manipulator.onApplyTransform(*userEditInGround); + manipulator.onApplyTransform(SimTK::Transform{ToSimTKRotation(userEditInGround->rotation), ToSimTKVec3(userEditInGround->position)}); } // once the user stops using the manipulator, save the changes diff --git a/src/OpenSimCreator/Utils/SimTKHelpers.cpp b/src/OpenSimCreator/Utils/SimTKHelpers.cpp index f0d0023f96..c684f63804 100644 --- a/src/OpenSimCreator/Utils/SimTKHelpers.cpp +++ b/src/OpenSimCreator/Utils/SimTKHelpers.cpp @@ -23,7 +23,7 @@ SimTK::Vec3 osc::ToSimTKVec3(const Vec3& v) }; } -SimTK::Vec3 osc::ToSimTKVec3(const Eulers& v) +SimTK::Vec3 osc::ToSimTKVec3(const EulerAngles& v) { return { static_cast(v.x.count()), @@ -55,7 +55,7 @@ SimTK::Transform osc::ToSimTKTransform(const Transform& t) return SimTK::Transform{ToSimTKRotation(t.rotation), ToSimTKVec3(t.position)}; } -SimTK::Transform osc::ToSimTKTransform(const Eulers& eulers, const Vec3& translation) +SimTK::Transform osc::ToSimTKTransform(const EulerAngles& eulers, const Vec3& translation) { return SimTK::Transform{ToSimTKRotation(eulers), ToSimTKVec3(translation)}; } @@ -65,7 +65,7 @@ SimTK::Rotation osc::ToSimTKRotation(const Quat& q) return SimTK::Rotation{ToSimTKMat3(mat3_cast(q))}; } -SimTK::Rotation osc::ToSimTKRotation(const Eulers& eulers) +SimTK::Rotation osc::ToSimTKRotation(const EulerAngles& eulers) { return ToSimTKRotation(to_worldspace_rotation_quat(eulers)); } diff --git a/src/OpenSimCreator/Utils/SimTKHelpers.h b/src/OpenSimCreator/Utils/SimTKHelpers.h index 7a2adbd30c..dfe9d41f1f 100644 --- a/src/OpenSimCreator/Utils/SimTKHelpers.h +++ b/src/OpenSimCreator/Utils/SimTKHelpers.h @@ -4,7 +4,7 @@ #include #include #include -#include +#include #include #include #include @@ -21,13 +21,13 @@ namespace osc // converters: from osc types to SimTK, SimTK::Vec3 ToSimTKVec3(const Vec3&); inline SimTK::Vec3 ToSimTKVec3(const Vec4& v) { return ToSimTKVec3(Vec3{v}); } - SimTK::Vec3 ToSimTKVec3(const Eulers&); + SimTK::Vec3 ToSimTKVec3(const EulerAngles&); SimTK::Mat33 ToSimTKMat3(const Mat3&); SimTK::Inertia ToSimTKInertia(const Vec3&); SimTK::Transform ToSimTKTransform(const Transform&); - SimTK::Transform ToSimTKTransform(const Eulers&, const Vec3&); + SimTK::Transform ToSimTKTransform(const EulerAngles&, const Vec3&); SimTK::Rotation ToSimTKRotation(const Quat&); - SimTK::Rotation ToSimTKRotation(const Eulers&); + SimTK::Rotation ToSimTKRotation(const EulerAngles&); SimTK::Vec3 ToSimTKRGBVec3(const Color&); // converters: from SimTK types to osc diff --git a/src/oscar/CMakeLists.txt b/src/oscar/CMakeLists.txt index 6066031baa..fbf6595699 100644 --- a/src/oscar/CMakeLists.txt +++ b/src/oscar/CMakeLists.txt @@ -202,7 +202,7 @@ add_library(oscar STATIC Maths/Ellipsoid.h Maths/EllipsoidFunctions.h Maths/EulerPerspectiveCamera.h - Maths/Eulers.h + Maths/EulerAngles.h Maths/FrustumPlanes.h Maths/Functors.h Maths/GeometricFunctions.h diff --git a/src/oscar/Maths.h b/src/oscar/Maths.h index 16c296cc24..49b162a0e7 100644 --- a/src/oscar/Maths.h +++ b/src/oscar/Maths.h @@ -19,7 +19,7 @@ #include #include #include -#include +#include #include #include #include diff --git a/src/oscar/Maths/EulerAngles.h b/src/oscar/Maths/EulerAngles.h new file mode 100644 index 0000000000..6c611148ca --- /dev/null +++ b/src/oscar/Maths/EulerAngles.h @@ -0,0 +1,10 @@ +#pragma once + +#include +#include + +namespace osc +{ + // a sequence of X -> Y -> Z euler angles + using EulerAngles = Vec<3, Radians>; +} diff --git a/src/oscar/Maths/Eulers.h b/src/oscar/Maths/Eulers.h deleted file mode 100644 index 54d1bad413..0000000000 --- a/src/oscar/Maths/Eulers.h +++ /dev/null @@ -1,10 +0,0 @@ -#pragma once - -#include -#include - -namespace osc -{ - // a sequence of euler angles that describe a 3D rotation - using Eulers = Vec<3, Radians>; -} diff --git a/src/oscar/Maths/MathHelpers.h b/src/oscar/Maths/MathHelpers.h index ce5842287a..d7125ee8d1 100644 --- a/src/oscar/Maths/MathHelpers.h +++ b/src/oscar/Maths/MathHelpers.h @@ -3,7 +3,7 @@ #include #include #include -#include +#include #include #include #include @@ -135,17 +135,17 @@ namespace osc Mat4 mat4_transform_between_directions(const Vec3& dir1, const Vec3& dir2); // returns euler angles for performing an intrinsic, step-by-step, rotation about X, Y, and then Z - Eulers extract_eulers_xyz(const Quat&); + EulerAngles extract_eulers_xyz(const Quat&); Vec3 transform_point(const Mat4&, const Vec3&); // returns the a `Quat` equivalent to the given euler angles - Quat to_worldspace_rotation_quat(const Eulers&); + Quat to_worldspace_rotation_quat(const EulerAngles&); // applies a world-space rotation to the transform void apply_worldspace_rotation( Transform& application_target, - const Eulers& euler_angles, + const EulerAngles& euler_angles, const Vec3& rotation_center ); diff --git a/src/oscar/Maths/MathsImplementation.cpp b/src/oscar/Maths/MathsImplementation.cpp index 2ab6d300bf..203b9778f4 100644 --- a/src/oscar/Maths/MathsImplementation.cpp +++ b/src/oscar/Maths/MathsImplementation.cpp @@ -1011,7 +1011,7 @@ Mat4 osc::mat4_transform_between_directions(const Vec3& dir1, const Vec3& dir2) return rotate(identity(), theta, rotation_axis); } -Eulers osc::extract_eulers_xyz(const Quat& quaternion) +EulerAngles osc::extract_eulers_xyz(const Quat& quaternion) { return extract_eulers_xyz(mat4_cast(quaternion)); } @@ -1367,15 +1367,15 @@ Vec3 osc::transform_point(const Mat4& mat, const Vec3& point) return Vec3{mat * Vec4{point, 1.0f}}; } -Quat osc::to_worldspace_rotation_quat(const Eulers& eulers) +Quat osc::to_worldspace_rotation_quat(const EulerAngles& eulers) { - static_assert(std::is_same_v); + static_assert(std::is_same_v); return normalize(Quat{Vec3{eulers.x.count(), eulers.y.count(), eulers.z.count()}}); } void osc::apply_worldspace_rotation( Transform& application_target, - const Eulers& euler_angles, + const EulerAngles& euler_angles, const Vec3& rotation_center) { Quat q = to_worldspace_rotation_quat(euler_angles); diff --git a/src/oscar/Maths/Qua.h b/src/oscar/Maths/Qua.h index d07302279e..1c2f04131d 100644 --- a/src/oscar/Maths/Qua.h +++ b/src/oscar/Maths/Qua.h @@ -1,7 +1,7 @@ #pragma once #include -#include +#include #include #include #include @@ -96,7 +96,7 @@ namespace osc } // constructs a `Qua` from euler angles - explicit Qua(const Eulers& euler_angles) : + explicit Qua(const EulerAngles& euler_angles) : Qua(Vec3{euler_angles.x.count(), euler_angles.y.count(), euler_angles.z.count()}) {} diff --git a/src/oscar/Maths/QuaternionFunctions.h b/src/oscar/Maths/QuaternionFunctions.h index 9f00347d28..f633f0b565 100644 --- a/src/oscar/Maths/QuaternionFunctions.h +++ b/src/oscar/Maths/QuaternionFunctions.h @@ -251,7 +251,7 @@ namespace osc } template - Vec<3, RadiansT> euler_angles(const Qua& x) + Vec<3, RadiansT> to_euler_angles(const Qua& x) { return Vec<3, RadiansT>(pitch(x), yaw(x), roll(x)); } diff --git a/src/oscar/Maths/TransformFunctions.h b/src/oscar/Maths/TransformFunctions.h index 0e91af495b..91fbc7f64d 100644 --- a/src/oscar/Maths/TransformFunctions.h +++ b/src/oscar/Maths/TransformFunctions.h @@ -1,7 +1,7 @@ #pragma once #include -#include +#include #include #include #include @@ -143,7 +143,7 @@ namespace osc // rotation rotates around y', and the third rotation rotates around z'' // // see: https://en.wikipedia.org/wiki/Euler_angles#Conventions_by_intrinsic_rotations - inline Eulers extract_eulers_xyz(const Transform& transform) + inline EulerAngles extract_eulers_xyz(const Transform& transform) { return extract_eulers_xyz(mat4_cast(transform.rotation)); } @@ -155,9 +155,9 @@ namespace osc // to a moving body (the thing being rotated) // // see: https://en.wikipedia.org/wiki/Euler_angles#Conventions_by_extrinsic_rotations - inline Eulers extract_extrinsic_eulers_xyz(const Transform& transform) + inline EulerAngles extract_extrinsic_eulers_xyz(const Transform& transform) { - return euler_angles(transform.rotation); + return to_euler_angles(transform.rotation); } // returns the provided transform, but rotated such that the given axis, as expressed diff --git a/src/oscar/UI/ImGuiHelpers.cpp b/src/oscar/UI/ImGuiHelpers.cpp index 151733d231..1ce33662c6 100644 --- a/src/oscar/UI/ImGuiHelpers.cpp +++ b/src/oscar/UI/ImGuiHelpers.cpp @@ -5,7 +5,7 @@ #include #include #include -#include +#include #include #include #include @@ -340,7 +340,7 @@ bool osc::ui::update_polar_camera_from_all_inputs( return mouse_handled or keyboard_handled; } -void osc::ui::update_camera_from_all_inputs(Camera& camera, Eulers& eulers) +void osc::ui::update_camera_from_all_inputs(Camera& camera, EulerAngles& eulers) { const Vec3 front = camera.direction(); const Vec3 up = camera.upwards_direction(); diff --git a/src/oscar/UI/ImGuiHelpers.h b/src/oscar/UI/ImGuiHelpers.h index bc8f635c1a..d4d29ecb3b 100644 --- a/src/oscar/UI/ImGuiHelpers.h +++ b/src/oscar/UI/ImGuiHelpers.h @@ -2,7 +2,7 @@ #include #include -#include +#include #include #include #include @@ -52,7 +52,7 @@ namespace osc::ui void update_camera_from_all_inputs( Camera&, - Eulers& + EulerAngles& ); // returns the UI content region available in screenspace as a `Rect` diff --git a/src/oscar/UI/MouseCapturingCamera.h b/src/oscar/UI/MouseCapturingCamera.h index da9b49b0e0..673c60c9cb 100644 --- a/src/oscar/UI/MouseCapturingCamera.h +++ b/src/oscar/UI/MouseCapturingCamera.h @@ -2,7 +2,7 @@ #include #include -#include +#include #include #include @@ -52,11 +52,11 @@ namespace osc bool is_capturing_mouse() const { return mouse_captured_; } - const Eulers& eulers() const { return camera_eulers_; } - Eulers& eulers() { return camera_eulers_; } + const EulerAngles& eulers() const { return camera_eulers_; } + EulerAngles& eulers() { return camera_eulers_; } private: bool mouse_captured_ = false; - Eulers camera_eulers_ = {}; + EulerAngles camera_eulers_ = {}; }; } diff --git a/src/oscar/UI/oscimgui.cpp b/src/oscar/UI/oscimgui.cpp index d1882f3cd5..d1e2d22a54 100644 --- a/src/oscar/UI/oscimgui.cpp +++ b/src/oscar/UI/oscimgui.cpp @@ -1,6 +1,6 @@ #include "oscimgui.h" -#include +#include #include #include #include @@ -197,7 +197,7 @@ std::optional osc::ui::Gizmo::draw( value_ptr(world_rotation_in_degrees), value_ptr(world_scale) ); - const Eulers world_eulers = Vec<3, Degrees>(world_rotation_in_degrees); + const EulerAngles world_eulers = Vec<3, Degrees>(world_rotation_in_degrees); const GizmoTransform rv = { .scale = world_scale, diff --git a/src/oscar/UI/oscimgui.h b/src/oscar/UI/oscimgui.h index f1840a5e12..7c5ee6abf3 100644 --- a/src/oscar/UI/oscimgui.h +++ b/src/oscar/UI/oscimgui.h @@ -7,7 +7,7 @@ #include #include #include -#include +#include #include #include #include @@ -840,7 +840,7 @@ namespace osc::ui // user's transformation struct GizmoTransform final { Vec3 scale = {1.0f, 1.0f, 1.0f}; - Eulers rotation = {}; + EulerAngles rotation = {}; Vec3 position = {}; }; diff --git a/src/oscar_demos/HittestTab.cpp b/src/oscar_demos/HittestTab.cpp index 669a2b3c56..31e4e5a61d 100644 --- a/src/oscar_demos/HittestTab.cpp +++ b/src/oscar_demos/HittestTab.cpp @@ -262,7 +262,7 @@ class osc::HittestTab::Impl final : public StandardTabImpl { AABB scene_sphere_aabb_ = sphere_mesh_.bounds(); Sphere sphere_bounding_sphere_ = bounding_sphere_of(sphere_mesh_); bool is_mouse_captured_ = false; - Eulers camera_eulers{}; + EulerAngles camera_eulers{}; bool showing_aabbs_ = true; }; diff --git a/src/oscar_learnopengl/GettingStarted/LOGLCoordinateSystemsTab.cpp b/src/oscar_learnopengl/GettingStarted/LOGLCoordinateSystemsTab.cpp index e3c5456c77..75273a38a0 100644 --- a/src/oscar_learnopengl/GettingStarted/LOGLCoordinateSystemsTab.cpp +++ b/src/oscar_learnopengl/GettingStarted/LOGLCoordinateSystemsTab.cpp @@ -142,7 +142,7 @@ class osc::LOGLCoordinateSystemsTab::Impl final : public StandardTabImpl { const Vec3 camera_position = camera_.position(); ui::draw_text("camera pos = (%f, %f, %f)", camera_position.x, camera_position.y, camera_position.z); - const Eulers camera_eulers = camera_.eulers(); + const EulerAngles camera_eulers = camera_.eulers(); ui::draw_text("camera eulers = (%f, %f, %f)", camera_eulers.x.count(), camera_eulers.y.count(), camera_eulers.z.count()); ui::end_panel(); diff --git a/tests/testoscar/Graphics/TestMesh.cpp b/tests/testoscar/Graphics/TestMesh.cpp index 069275730d..83bcebff92 100644 --- a/tests/testoscar/Graphics/TestMesh.cpp +++ b/tests/testoscar/Graphics/TestMesh.cpp @@ -9,7 +9,7 @@ #include #include #include -#include +#include #include #include #include @@ -380,7 +380,7 @@ TEST(Mesh, TransformVertsWithTransformAppliesTransformToVerts) // create appropriate transform const Transform transform = { .scale = Vec3{0.25f}, - .rotation = to_worldspace_rotation_quat(Eulers{90_deg, 0_deg, 0_deg}), + .rotation = to_worldspace_rotation_quat(EulerAngles{90_deg, 0_deg, 0_deg}), .position = {1.0f, 0.25f, 0.125f}, }; @@ -417,7 +417,7 @@ TEST(Mesh, TransformVertsWithMat4AppliesTransformToVerts) { const Mat4 mat = mat4_cast(Transform{ .scale = Vec3{0.25f}, - .rotation = to_worldspace_rotation_quat(Eulers{90_deg, 0_deg, 0_deg}), + .rotation = to_worldspace_rotation_quat(EulerAngles{90_deg, 0_deg, 0_deg}), .position = {1.0f, 0.25f, 0.125f}, }); From 5cd658deece22af00125ba3f94eeb3bf56176026 Mon Sep 17 00:00:00 2001 From: Adam Kewley Date: Thu, 18 Jul 2024 13:37:23 +0200 Subject: [PATCH 09/13] Redesign Vec to have correct constraints and typecasting --- .../UI/Experimental/TPS2DTab.cpp | 2 +- .../UI/MeshImporter/MeshImporterTab.cpp | 2 +- .../UI/Shared/ModelSelectionGizmo.cpp | 12 +- src/OpenSimCreator/Utils/SimTKHelpers.cpp | 5 + src/OpenSimCreator/Utils/SimTKHelpers.h | 1 + src/oscar/CMakeLists.txt | 1 + src/oscar/Graphics/Unorm8.h | 9 + src/oscar/Maths.h | 1 + src/oscar/Maths/Angle.h | 15 +- src/oscar/Maths/EulerAngles.h | 9 +- src/oscar/Maths/MathsImplementation.cpp | 7 +- src/oscar/Maths/Qua.h | 10 +- src/oscar/Maths/Scalar.h | 33 +++ src/oscar/Maths/TrigonometricFunctions.h | 20 +- src/oscar/Maths/Vec.h | 21 +- src/oscar/Maths/Vec2.h | 182 +++++++++------- src/oscar/Maths/Vec3.h | 201 ++++++++++-------- src/oscar/Maths/Vec4.h | 173 ++++++++------- src/oscar/Maths/VecFunctions.h | 5 +- src/oscar/UI/oscimgui.cpp | 5 +- tests/testoscar/Maths/TestAngle.cpp | 8 + 21 files changed, 443 insertions(+), 279 deletions(-) create mode 100644 src/oscar/Maths/Scalar.h diff --git a/src/OpenSimCreator/UI/Experimental/TPS2DTab.cpp b/src/OpenSimCreator/UI/Experimental/TPS2DTab.cpp index 0b88b63d4b..0fdaaf44ad 100644 --- a/src/OpenSimCreator/UI/Experimental/TPS2DTab.cpp +++ b/src/OpenSimCreator/UI/Experimental/TPS2DTab.cpp @@ -292,7 +292,7 @@ namespace Mesh ApplyThinPlateWarpToMesh(const ThinPlateWarper2D& t, const Mesh& mesh) { Mesh rv = mesh; - rv.transform_vertices([&t](Vec3 v) { return Vec3{t.transform(v), v.z}; }); + rv.transform_vertices([&t](Vec3 v) { return Vec3{t.transform(Vec2{v}), v.z}; }); return rv; } } diff --git a/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp b/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp index f55b4ef510..069276a2c2 100644 --- a/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp +++ b/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp @@ -2128,7 +2128,7 @@ class osc::mi::MeshImporterTab::Impl final : public IMeshImporterUILayerHost { MIObject& el = m_Shared->updModelGraph().updByID(id); switch (m_Gizmo.operation()) { case ui::GizmoOperation::Rotate: - el.applyRotation(m_Shared->getModelGraph(), userManipulation->rotation, m_GizmoModelMatrix[3]); + el.applyRotation(m_Shared->getModelGraph(), userManipulation->rotation, Vec3{m_GizmoModelMatrix[3]}); break; case ui::GizmoOperation::Translate: { el.applyTranslation(m_Shared->getModelGraph(), userManipulation->position); diff --git a/src/OpenSimCreator/UI/Shared/ModelSelectionGizmo.cpp b/src/OpenSimCreator/UI/Shared/ModelSelectionGizmo.cpp index 30e975421c..4850faab59 100644 --- a/src/OpenSimCreator/UI/Shared/ModelSelectionGizmo.cpp +++ b/src/OpenSimCreator/UI/Shared/ModelSelectionGizmo.cpp @@ -348,7 +348,7 @@ namespace getUndoableModel(), pof, ToVec3(X.p()), - ToVec3(X.R().convertRotationToBodyFixedXYZ()) + ToEulerAngles(X.R()) ); } else { @@ -371,7 +371,7 @@ namespace getUndoableModel(), pof, ToVec3(X.p()), - ToVec3(X.R().convertRotationToBodyFixedXYZ()) + ToEulerAngles(X.R()) ); } } @@ -425,7 +425,7 @@ namespace getUndoableModel(), wrapObj, ToVec3(X.p() - M_w.p()), - ToVec3(X.R().convertRotationToBodyFixedXYZ()) + ToEulerAngles(X.R()) ); } }; @@ -476,7 +476,7 @@ namespace getUndoableModel(), contactGeom, ToVec3(X.p() - M_w.p()), - ToVec3(X.R().convertRotationToBodyFixedXYZ()) + ToEulerAngles(X.R()) ); } }; @@ -577,7 +577,7 @@ namespace getUndoableModel(), parentPOF, ToVec3(X.p()), - ToVec3(X.R().convertRotationToBodyFixedXYZ()) + ToEulerAngles(X.R()) ); } @@ -595,7 +595,7 @@ namespace getUndoableModel(), childPOF, ToVec3(M_cpof2.p()), - ToVec3(M_cpof2.R().convertRotationToBodyFixedXYZ()) + ToEulerAngles(M_cpof2.R()) ); } }; diff --git a/src/OpenSimCreator/Utils/SimTKHelpers.cpp b/src/OpenSimCreator/Utils/SimTKHelpers.cpp index c684f63804..0596bf1020 100644 --- a/src/OpenSimCreator/Utils/SimTKHelpers.cpp +++ b/src/OpenSimCreator/Utils/SimTKHelpers.cpp @@ -165,6 +165,11 @@ Quat osc::ToQuat(const SimTK::Rotation& r) }; } +EulerAngles osc::ToEulerAngles(const SimTK::Rotation& r) +{ + return EulerAngles{ToVec3(r.convertRotationToBodyFixedXYZ())}; +} + std::array osc::ToArray(const SimTK::Vec6& v) { return { diff --git a/src/OpenSimCreator/Utils/SimTKHelpers.h b/src/OpenSimCreator/Utils/SimTKHelpers.h index dfe9d41f1f..69b2d4165b 100644 --- a/src/OpenSimCreator/Utils/SimTKHelpers.h +++ b/src/OpenSimCreator/Utils/SimTKHelpers.h @@ -37,6 +37,7 @@ namespace osc Mat3 ToMat3(const SimTK::Mat33&); Mat4 mat4_cast(const SimTK::Rotation&); Quat ToQuat(const SimTK::Rotation&); + EulerAngles ToEulerAngles(const SimTK::Rotation&); std::array ToArray(const SimTK::Vec6&); Transform decompose_to_transform(const SimTK::Transform&); } diff --git a/src/oscar/CMakeLists.txt b/src/oscar/CMakeLists.txt index fbf6595699..3aa1c3a2bb 100644 --- a/src/oscar/CMakeLists.txt +++ b/src/oscar/CMakeLists.txt @@ -226,6 +226,7 @@ add_library(oscar STATIC Maths/RayCollision.h Maths/Rect.h Maths/RectFunctions.h + Maths/Scalar.h Maths/Sphere.h Maths/Tetrahedron.h Maths/Transform.h diff --git a/src/oscar/Graphics/Unorm8.h b/src/oscar/Graphics/Unorm8.h index 7534da90f4..33cc87ec04 100644 --- a/src/oscar/Graphics/Unorm8.h +++ b/src/oscar/Graphics/Unorm8.h @@ -1,5 +1,7 @@ #pragma once +#include + #include #include #include @@ -72,4 +74,11 @@ namespace osc uint8_t value_; }; + + // tag `Unorm8` as scalar-like, so that other parts of the codebase (e.g. + // vectors, matrices) accept it + template<> + struct IsScalar final { + static constexpr bool value = true; + }; } diff --git a/src/oscar/Maths.h b/src/oscar/Maths.h index 49b162a0e7..523159d96d 100644 --- a/src/oscar/Maths.h +++ b/src/oscar/Maths.h @@ -42,6 +42,7 @@ #include #include #include +#include #include #include #include diff --git a/src/oscar/Maths/Angle.h b/src/oscar/Maths/Angle.h index d1c5c2f77a..df97da4a79 100644 --- a/src/oscar/Maths/Angle.h +++ b/src/oscar/Maths/Angle.h @@ -1,5 +1,7 @@ #pragma once +#include + #include #include #include @@ -32,13 +34,13 @@ namespace osc value_{static_cast(value_)} {} - // implicitly constructs from an angle expressed in other units + // implicitly constructs from an angle with the same representation, but expressed in other units template constexpr Angle(const Angle& other) : value_{static_cast(other.count() * (Units2::radians_per_rep/Units::radians_per_rep))} {} - // explicitly constructs from an angle expressed of type `Rep2` expressed in other units + // explicitly constructs from an angle with representation `Rep2`, expressed in other units template Rep2, AngularUnitTraits Units2> explicit constexpr Angle(const Angle& other) : value_{static_cast(other.count() * (Units2::radians_per_rep/Units::radians_per_rep))} @@ -149,6 +151,13 @@ namespace osc { return o << angle.count() << ' ' << Units::unit_label; } + + // tag `Unorm8` as scalar-like, so that other parts of the codebase (e.g. + // vectors, matrices) accept it + template + struct IsScalar> final { + static constexpr bool value = true; + }; } // a specialization of `std::common_type` for `osc::Angle`s @@ -203,7 +212,7 @@ namespace osc namespace literals { constexpr Degrees operator""_deg(long double degrees) { return Degrees{degrees}; } - constexpr Degrees operator""_deg(unsigned long long int degrees) { return Degrees{degrees};} + constexpr Degrees operator""_deg(unsigned long long int degrees) { return Degrees{degrees}; } } // turns diff --git a/src/oscar/Maths/EulerAngles.h b/src/oscar/Maths/EulerAngles.h index 6c611148ca..e76f5a95e9 100644 --- a/src/oscar/Maths/EulerAngles.h +++ b/src/oscar/Maths/EulerAngles.h @@ -5,6 +5,11 @@ namespace osc { - // a sequence of X -> Y -> Z euler angles - using EulerAngles = Vec<3, Radians>; + // in OSC, Euler angles represent an intrinsic 3D rotation about + // the X, then Y, then Z axes + + template + using EulerAnglesIn = Vec<3, Units>; // useful for writing `EulerAnglesIn(vec)` + + using EulerAngles = EulerAnglesIn; } diff --git a/src/oscar/Maths/MathsImplementation.cpp b/src/oscar/Maths/MathsImplementation.cpp index 203b9778f4..daa94bdd1b 100644 --- a/src/oscar/Maths/MathsImplementation.cpp +++ b/src/oscar/Maths/MathsImplementation.cpp @@ -1158,8 +1158,8 @@ AABB osc::bounding_aabb_of(const Sphere& sphere) Line osc::transform_line(const Line& line, const Mat4& mat) { Line rv{}; - rv.direction = mat * Vec4{line.direction, 0.0f}; - rv.origin = mat * Vec4{line.origin, 1.0f}; + rv.direction = Vec3{mat * Vec4{line.direction, 0.0f}}; + rv.origin = Vec3{mat * Vec4{line.origin, 1.0f}}; return rv; } @@ -1369,8 +1369,7 @@ Vec3 osc::transform_point(const Mat4& mat, const Vec3& point) Quat osc::to_worldspace_rotation_quat(const EulerAngles& eulers) { - static_assert(std::is_same_v); - return normalize(Quat{Vec3{eulers.x.count(), eulers.y.count(), eulers.z.count()}}); + return normalize(Quat{eulers}); } void osc::apply_worldspace_rotation( diff --git a/src/oscar/Maths/Qua.h b/src/oscar/Maths/Qua.h index 1c2f04131d..3891f961ba 100644 --- a/src/oscar/Maths/Qua.h +++ b/src/oscar/Maths/Qua.h @@ -83,8 +83,9 @@ namespace osc *this = normalize(Qua::wxyz(real_part, t.x, t.y, t.z)); } - // constructs a `Qua` from euler angles (pitch, yaw, roll), in radians. - explicit Qua(const Vec<3, T>& euler_angles) + // constructs a `Qua` from Euler angles that are assumed to represent an + // intrinsic, step-by-step, rotation about X, Y, and then Z + explicit Qua(const EulerAngles& euler_angles) { Vec<3, T> c = cos(euler_angles * T(0.5)); Vec<3, T> s = sin(euler_angles * T(0.5)); @@ -95,11 +96,6 @@ namespace osc this->z = c.x * c.y * s.z - s.x * s.y * c.z; } - // constructs a `Qua` from euler angles - explicit Qua(const EulerAngles& euler_angles) : - Qua(Vec3{euler_angles.x.count(), euler_angles.y.count(), euler_angles.z.count()}) - {} - // constructs a `Qua` by decomposing an orthogonal matrix explicit Qua(const Mat<3, 3, T>& m) { diff --git a/src/oscar/Maths/Scalar.h b/src/oscar/Maths/Scalar.h new file mode 100644 index 0000000000..3c4401deab --- /dev/null +++ b/src/oscar/Maths/Scalar.h @@ -0,0 +1,33 @@ +#pragma once + +#include + +namespace osc +{ + // metaclass that has a `value` member equal to `true` if its type argument + // behaves like a scalar (which is checked when resolving overloads of matrices + // and vectors) + template + struct IsScalar final { + static constexpr bool value = false; + }; + + template + struct IsScalar final { + static constexpr bool value = true; + }; + + template + struct IsScalar final { + static constexpr bool value = true; + }; + + template + inline constexpr bool IsScalarV = IsScalar::value; + + template + concept Scalar = IsScalarV; + + template + concept ScalarOrBoolean = Scalar || std::same_as; +} diff --git a/src/oscar/Maths/TrigonometricFunctions.h b/src/oscar/Maths/TrigonometricFunctions.h index 7026135219..da15647a22 100644 --- a/src/oscar/Maths/TrigonometricFunctions.h +++ b/src/oscar/Maths/TrigonometricFunctions.h @@ -27,6 +27,12 @@ namespace osc return map(v, sin); } + template + Vec sin(const Vec>& v) + { + return map(v, sin); + } + template GenType cos(GenType v) { @@ -45,6 +51,12 @@ namespace osc return map(v, cos); } + template + Vec cos(const Vec>& v) + { + return map(v, cos); + } + template GenType tan(GenType v) { @@ -60,7 +72,13 @@ namespace osc template Vec tan(const Vec& v) { - return elementwise_map(v, tan); + return map(v, tan); + } + + template + Vec tan(const Vec>& v) + { + return map(v, tan); } template diff --git a/src/oscar/Maths/Vec.h b/src/oscar/Maths/Vec.h index ce46918426..0e4e0219f7 100644 --- a/src/oscar/Maths/Vec.h +++ b/src/oscar/Maths/Vec.h @@ -1,5 +1,6 @@ #pragma once +#include #include #include @@ -13,10 +14,10 @@ namespace osc { // specialized by `Vec2`, `Vec3`, etc. - template + template struct Vec; - template + template std::ostream& operator<<(std::ostream& out, const Vec& vec) { out << "Vec" << L << '('; @@ -29,7 +30,7 @@ namespace osc return out; } - template + template std::string to_string(const Vec& vec) { std::stringstream ss; @@ -39,30 +40,30 @@ namespace osc // when handled as a tuple-like object, a `Vec` decomposes into its elements - template + template constexpr const T& get(const Vec& vec) { return vec[I]; } - template + template constexpr T& get(Vec& vec) { return vec[I]; } - template + template constexpr T&& get(Vec&& vec) { return std::move(vec[I]); } - template + template constexpr const T&& get(const Vec&& vec) { return std::move(vec[I]); } } -template +template struct std::tuple_size> { static inline constexpr size_t value = L; }; -template +template struct std::tuple_element> { using type = T; }; -template +template struct std::hash> final { size_t operator()(const osc::Vec& vec) const { diff --git a/src/oscar/Maths/Vec2.h b/src/oscar/Maths/Vec2.h index 275870dc21..83d5c15be3 100644 --- a/src/oscar/Maths/Vec2.h +++ b/src/oscar/Maths/Vec2.h @@ -1,13 +1,15 @@ #pragma once +#include #include +#include #include #include namespace osc { - template + template struct Vec<2, T> { using value_type = T; using element_type = T; @@ -21,40 +23,47 @@ namespace osc using const_iterator = const T*; constexpr Vec() = default; - constexpr explicit Vec(T scalar) : - x{scalar}, - y{scalar} + explicit constexpr Vec(T value) : + x{value}, + y{value} {} constexpr Vec(T x_, T y_) : x{x_}, y{y_} {} template + requires std::constructible_from and std::constructible_from + explicit (not (std::convertible_to and std::convertible_to)) constexpr Vec(A x_, B y_) : x{static_cast(x_)}, y{static_cast(y_)} {} - template + template + requires std::constructible_from + explicit (not std::convertible_to) constexpr Vec(const Vec<2, U>& v) : x{static_cast(v.x)}, y{static_cast(v.y)} {} - template - constexpr Vec(const Vec<3, U>& v) : + template + requires std::constructible_from + explicit constexpr Vec(const Vec<3, U>& v) : x{static_cast(v.x)}, y{static_cast(v.y)} {} - template - constexpr Vec(const Vec<4, U>& v) : + template + requires std::constructible_from + explicit constexpr Vec(const Vec<4, U>& v) : x{static_cast(v.x)}, y{static_cast(v.y)} {} - template + template + requires std::assignable_from constexpr Vec& operator=(const Vec<2, U>& v) { - this->x = static_cast(v.x); - this->y = static_cast(v.y); + this->x = v.x; + this->y = v.y; return *this; } @@ -70,71 +79,80 @@ namespace osc friend constexpr bool operator==(const Vec<2, T>&, const Vec<2, T>&) = default; - template + template + requires (not std::same_as) constexpr Vec<2, T>& operator+=(U scalar) { - this->x += static_cast(scalar); - this->y += static_cast(scalar); + this->x += scalar; + this->y += scalar; return *this; } - template + template + requires (not std::same_as) constexpr Vec& operator+=(const Vec<2, U>& rhs) { - this->x += static_cast(rhs.x); - this->y += static_cast(rhs.y); + this->x += rhs.x; + this->y += rhs.y; return *this; } - template + template + requires (not std::same_as) constexpr Vec& operator-=(U scalar) { - this->x -= static_cast(scalar); - this->y -= static_cast(scalar); + this->x -= scalar; + this->y -= scalar; return *this; } - template + template + requires (not std::same_as) constexpr Vec& operator-=(const Vec<2, U>& rhs) { - this->x -= static_cast(rhs.x); - this->y -= static_cast(rhs.y); + this->x -= rhs.x; + this->y -= rhs.y; return *this; } - template + template + requires (not std::same_as) constexpr Vec& operator*=(U scalar) { - this->x *= static_cast(scalar); - this->y *= static_cast(scalar); + this->x *= scalar; + this->y *= scalar; return *this; } - template + template + requires (not std::same_as) constexpr Vec& operator*=(Vec<2, U> const& rhs) { - this->x *= static_cast(rhs.x); - this->y *= static_cast(rhs.y); + this->x *= rhs.x; + this->y *= rhs.y; return *this; } - template + template + requires (not std::same_as) constexpr Vec& operator/=(U scalar) { - this->x /= static_cast(scalar); - this->y /= static_cast(scalar); + this->x /= scalar; + this->y /= scalar; return *this; } - template + template + requires (not std::same_as) constexpr Vec& operator/=(const Vec<2, U>& rhs) { - this->x /= static_cast(rhs.x); - this->y /= static_cast(rhs.y); + this->x /= rhs.x; + this->y /= rhs.y; return *this; } constexpr Vec& operator++() + requires std::incrementable { ++this->x; ++this->y; @@ -142,6 +160,7 @@ namespace osc } constexpr Vec& operator--() + requires std::incrementable { --this->x; --this->y; @@ -149,6 +168,7 @@ namespace osc } constexpr Vec operator++(int) + requires std::incrementable { Vec copy{*this}; ++(*this); @@ -156,6 +176,7 @@ namespace osc } constexpr Vec operator--(int) + requires std::incrementable { Vec copy{*this}; --(*this); @@ -163,10 +184,11 @@ namespace osc } template - constexpr Vec with_element(size_type pos, U scalar) const + requires std::constructible_from + constexpr Vec with_element(size_type pos, U value) const { Vec copy{*this}; - copy[pos] = static_cast(scalar); + copy[pos] = static_cast(value); return copy; } @@ -174,98 +196,98 @@ namespace osc T y{}; }; - template + template constexpr Vec<2, T> operator+(const Vec<2, T>& vec) { - return vec; + return {+vec.x, +vec.y}; } - template + template constexpr Vec<2, T> operator-(const Vec<2, T>& vec) { - return Vec<2, T>{-vec.x, -vec.y}; + return {-vec.x, -vec.y}; } - template - constexpr Vec<2, T> operator+(const Vec<2, T>& vec, T scalar) + template + constexpr Vec<2, decltype(T{} + U{})> operator+(const Vec<2, T>& vec, U scalar) { - return Vec<2, T>{vec.x + scalar, vec.y + scalar}; + return {vec.x + scalar, vec.y + scalar}; } - template - constexpr Vec<2, T> operator+(T scalar, const Vec<2, T>& vec) + template + constexpr Vec<2, decltype(T{} + U{})> operator+(T scalar, const Vec<2, U>& vec) { - return Vec<2, T>{scalar + vec.x, scalar + vec.y}; + return {scalar + vec.x, scalar + vec.y}; } - template - constexpr Vec<2, T> operator+(const Vec<2, T>& lhs, const Vec<2, T>& rhs) + template + constexpr Vec<2, decltype(T{} + U{})> operator+(const Vec<2, T>& lhs, const Vec<2, U>& rhs) { - return Vec<2, T>{lhs.x + rhs.x, lhs.y + rhs.y}; + return {lhs.x + rhs.x, lhs.y + rhs.y}; } - template - constexpr Vec<2, T> operator-(const Vec<2, T>& vec, T scalar) + template + constexpr Vec<2, decltype(T{} - U{})> operator-(const Vec<2, T>& vec, U scalar) { - return Vec<2, T>{vec.x - scalar, vec.y - scalar}; + return {vec.x - scalar, vec.y - scalar}; } - template - constexpr Vec<2, T> operator-(T scalar, const Vec<2, T>& vec) + template + constexpr Vec<2, decltype(T{} - U{})> operator-(T scalar, const Vec<2, U>& vec) { - return Vec<2, T>{scalar - vec.x, scalar - vec.y}; + return {scalar - vec.x, scalar - vec.y}; } - template - constexpr Vec<2, T> operator-(const Vec<2, T>& lhs, const Vec<2, T>& rhs) + template + constexpr Vec<2, decltype(T{} - U{})> operator-(const Vec<2, T>& lhs, const Vec<2, U>& rhs) { - return Vec<2, T>{lhs.x - rhs.x, lhs.y - rhs.y}; + return {lhs.x - rhs.x, lhs.y - rhs.y}; } - template - constexpr Vec<2, T> operator*(const Vec<2, T>& vec, T scalar) + template + constexpr Vec<2, decltype(T{} * U{})> operator*(const Vec<2, T>& vec, U scalar) { - return Vec<2, T>{vec.x * scalar, vec.y * scalar}; + return {vec.x * scalar, vec.y * scalar}; } - template - constexpr Vec<2, T> operator*(T scalar, const Vec<2, T>& vec) + template + constexpr Vec<2, decltype(T{} * U{})> operator*(T scalar, const Vec<2, U>& vec) { - return Vec<2, T>{scalar * vec.x, scalar * vec.y}; + return {scalar * vec.x, scalar * vec.y}; } - template - constexpr Vec<2, T> operator*(const Vec<2, T>& lhs, const Vec<2, T>& rhs) + template + constexpr Vec<2, decltype(T{} * U{})> operator*(const Vec<2, T>& lhs, const Vec<2, U>& rhs) { - return Vec<2, T>{lhs.x * rhs.x, lhs.y * rhs.y}; + return {lhs.x * rhs.x, lhs.y * rhs.y}; } - template - constexpr Vec<2, T> operator/(const Vec<2, T>& vec, T scalar) + template + constexpr Vec<2, decltype(T{} / U{})> operator/(const Vec<2, T>& vec, U scalar) { - return Vec<2, T>{vec.x / scalar, vec.y / scalar}; + return {vec.x / scalar, vec.y / scalar}; } - template - constexpr Vec<2, T> operator/(T scalar, const Vec<2, T>& vec) + template + constexpr Vec<2, decltype(T{} / U{})> operator/(T scalar, const Vec<2, U>& vec) { - return Vec<2, T>{scalar / vec.x, scalar / vec.y}; + return {scalar / vec.x, scalar / vec.y}; } - template - constexpr Vec<2, T> operator/(const Vec<2, T>& lhs, const Vec<2, T>& rhs) + template + constexpr Vec<2, decltype(T{} / U{})> operator/(const Vec<2, T>& lhs, const Vec<2, U>& rhs) { - return Vec<2, T>{lhs.x / rhs.x, lhs.y / rhs.y}; + return {lhs.x / rhs.x, lhs.y / rhs.y}; } constexpr Vec<2, bool> operator&&(const Vec<2, bool>& lhs, const Vec<2, bool>& rhs) { - return Vec<2, bool>{lhs.x && rhs.x, lhs.y && rhs.y}; + return {lhs.x && rhs.x, lhs.y && rhs.y}; } constexpr Vec<2, bool> operator||(const Vec<2, bool>& lhs, const Vec<2, bool>& rhs) { - return Vec<2, bool>{lhs.x || rhs.x, lhs.y || rhs.y}; + return {lhs.x || rhs.x, lhs.y || rhs.y}; } using Vec2 = Vec<2, float>; diff --git a/src/oscar/Maths/Vec3.h b/src/oscar/Maths/Vec3.h index 159aa8b0d3..1689e294f3 100644 --- a/src/oscar/Maths/Vec3.h +++ b/src/oscar/Maths/Vec3.h @@ -1,13 +1,15 @@ #pragma once +#include #include +#include #include #include namespace osc { - template + template struct Vec<3, T> { using value_type = T; using element_type = T; @@ -21,10 +23,10 @@ namespace osc using const_iterator = const T*; constexpr Vec() = default; - constexpr explicit Vec(T scalar) : - x{scalar}, - y{scalar}, - z{scalar} + explicit constexpr Vec(T value) : + x{value}, + y{value}, + z{value} {} constexpr Vec(T x_, T y_, T z_) : x{x_}, @@ -32,42 +34,52 @@ namespace osc z{z_} {} template + requires std::constructible_from and std::constructible_from and std::constructible_from + explicit (not (std::convertible_to and std::convertible_to and std::convertible_to)) constexpr Vec(X x_, Y y_, Z z_) : x{static_cast(x_)}, y{static_cast(y_)}, z{static_cast(z_)} {} - template + template + requires std::constructible_from and std::constructible_from + explicit (not (std::convertible_to and std::convertible_to)) constexpr Vec(const Vec<2, A>& xy_, B z_) : x{static_cast(xy_.x)}, y{static_cast(xy_.y)}, z{static_cast(z_)} {} - template + template + requires std::constructible_from and std::constructible_from + explicit (not (std::convertible_to and std::convertible_to)) constexpr Vec(A x_, const Vec<2, B>& yz_) : x{static_cast(x_)}, y{static_cast(yz_.y)}, z{static_cast(yz_.z)} {} - template - constexpr Vec(const Vec<4, U>& v) : + template + requires std::constructible_from + explicit constexpr Vec(const Vec<4, U>& v) : x{static_cast(v.x)}, y{static_cast(v.y)}, z{static_cast(v.z)} {} - template + template + requires std::constructible_from + explicit (not (std::convertible_to)) constexpr Vec(const Vec<3, U>& v) : x{static_cast(v.x)}, y{static_cast(v.y)}, z{static_cast(v.z)} {} - template + template + requires std::assignable_from constexpr Vec& operator=(const Vec<3, U>& v) { - this->x = static_cast(v.x); - this->y = static_cast(v.y); - this->z = static_cast(v.z); + this->x = v.x; + this->y = v.y; + this->z = v.z; return *this; } @@ -83,79 +95,88 @@ namespace osc friend constexpr bool operator==(const Vec&, const Vec&) = default; - template + template + requires (not std::same_as) constexpr Vec& operator+=(U scalar) { - this->x += static_cast(scalar); - this->y += static_cast(scalar); - this->z += static_cast(scalar); + this->x += scalar; + this->y += scalar; + this->z += scalar; return *this; } - template + template + requires (not std::same_as) constexpr Vec& operator+=(const Vec<3, U>& rhs) { - this->x += static_cast(rhs.x); - this->y += static_cast(rhs.y); - this->z += static_cast(rhs.z); + this->x += rhs.x; + this->y += rhs.y; + this->z += rhs.z; return *this; } - template + template + requires (not std::same_as) constexpr Vec& operator-=(U scalar) { - this->x -= static_cast(scalar); - this->y -= static_cast(scalar); - this->z -= static_cast(scalar); + this->x -= scalar; + this->y -= scalar; + this->z -= scalar; return *this; } - template + template + requires (not std::same_as) constexpr Vec& operator-=(const Vec<3, U>& rhs) { - this->x -= static_cast(rhs.x); - this->y -= static_cast(rhs.y); - this->z -= static_cast(rhs.z); + this->x -= rhs.x; + this->y -= rhs.y; + this->z -= rhs.z; return *this; } - template + template + requires (not std::same_as) constexpr Vec& operator*=(U scalar) { - this->x *= static_cast(scalar); - this->y *= static_cast(scalar); - this->z *= static_cast(scalar); + this->x *= scalar; + this->y *= scalar; + this->z *= scalar; return *this; } - template + template + requires (not std::same_as) constexpr Vec& operator*=(const Vec<3, U>& rhs) { - this->x *= static_cast(rhs.x); - this->y *= static_cast(rhs.y); - this->z *= static_cast(rhs.z); + this->x *= rhs.x; + this->y *= rhs.y; + this->z *= rhs.z; return *this; } - template + template + requires (not std::same_as) constexpr Vec& operator/=(U scalar) { - this->x /= static_cast(scalar); - this->y /= static_cast(scalar); - this->z /= static_cast(scalar); + this->x /= scalar; + this->y /= scalar; + this->z /= scalar; return *this; } - template + template + requires (not std::same_as) constexpr Vec& operator/=(const Vec<3, U>& rhs) { - this->x /= static_cast(rhs.x); - this->y /= static_cast(rhs.y); - this->z /= static_cast(rhs.z); + this->x /= rhs.x; + this->y /= rhs.y; + this->z /= rhs.z; return *this; } constexpr Vec& operator++() + requires std::incrementable { ++this->x; ++this->y; @@ -164,6 +185,7 @@ namespace osc } constexpr Vec& operator--() + requires std::incrementable { --this->x; --this->y; @@ -172,6 +194,7 @@ namespace osc } constexpr Vec operator++(int) + requires std::incrementable { Vec copy{*this}; ++(*this); @@ -179,6 +202,7 @@ namespace osc } constexpr Vec operator--(int) + requires std::incrementable { Vec copy{*this}; --(*this); @@ -186,10 +210,11 @@ namespace osc } template - constexpr Vec with_element(size_type pos, U scalar) const + requires std::constructible_from + constexpr Vec with_element(size_type pos, U value) const { Vec copy{*this}; - copy[pos] = static_cast(scalar); + copy[pos] = static_cast(value); return copy; } @@ -198,91 +223,91 @@ namespace osc T z{}; }; - template + template constexpr Vec<3, T> operator+(const Vec<3, T>& vec) { - return vec; + return {+vec.x, +vec.y, +vec.z}; } - template + template constexpr Vec<3, T> operator-(const Vec<3, T>& vec) { - return Vec<3, T>{-vec.x, -vec.y, -vec.z}; + return {-vec.x, -vec.y, -vec.z}; } - template - constexpr Vec<3, T> operator+(const Vec<3, T>& vec, T scalar) + template + constexpr Vec<3, decltype(T{} + U{})> operator+(const Vec<3, T>& vec, U scalar) { - return Vec<3, T>{vec.x + scalar, vec.y + scalar, vec.z + scalar}; + return {vec.x + scalar, vec.y + scalar, vec.z + scalar}; } - template - constexpr Vec<3, T> operator+(T scalar, const Vec<3, T>& vec) + template + constexpr Vec<3, decltype(T{} + U{})> operator+(T scalar, const Vec<3, U>& vec) { - return Vec<3, T>{scalar + vec.x, scalar + vec.y, scalar + vec.z}; + return {scalar + vec.x, scalar + vec.y, scalar + vec.z}; } - template - constexpr Vec<3, T> operator+(const Vec<3, T>& lhs, const Vec<3, T>& rhs) + template + constexpr Vec<3, decltype(T{} + U{})> operator+(const Vec<3, T>& lhs, const Vec<3, U>& rhs) { - return Vec<3, T>{lhs.x + rhs.x, lhs.y + rhs.y, lhs.z + rhs.z}; + return {lhs.x + rhs.x, lhs.y + rhs.y, lhs.z + rhs.z}; } - template - constexpr Vec<3, T> operator-(const Vec<3, T>& vec, T scalar) + template + constexpr Vec<3, decltype(T{} - U{})> operator-(const Vec<3, T>& vec, U scalar) { - return Vec<3, T>{vec.x - scalar, vec.y - scalar, vec.z - scalar}; + return {vec.x - scalar, vec.y - scalar, vec.z - scalar}; } - template - constexpr Vec<3, T> operator-(T scalar, const Vec<3, T>& vec) + template + constexpr Vec<3, decltype(T{} - U{})> operator-(T scalar, const Vec<3, U>& vec) { - return Vec<3, T>{scalar - vec.x, scalar - vec.y, scalar - vec.z}; + return {scalar - vec.x, scalar - vec.y, scalar - vec.z}; } - template - constexpr Vec<3, T> operator-(const Vec<3, T>& lhs, const Vec<3, T>& rhs) + template + constexpr Vec<3, decltype(T{} - U{})> operator-(const Vec<3, T>& lhs, const Vec<3, U>& rhs) { - return Vec<3, T>{lhs.x - rhs.x, lhs.y - rhs.y, lhs.z - rhs.z}; + return {lhs.x - rhs.x, lhs.y - rhs.y, lhs.z - rhs.z}; } - template - constexpr Vec<3, T> operator*(const Vec<3, T>& vec, T scalar) + template + constexpr Vec<3, decltype(T{} * U{})> operator*(const Vec<3, T>& vec, U scalar) { - return Vec<3, T>{vec.x * scalar, vec.y * scalar, vec.z * scalar}; + return {vec.x * scalar, vec.y * scalar, vec.z * scalar}; } - template - constexpr Vec<3, T> operator*(T scalar, const Vec<3, T>& vec) + template + constexpr Vec<3, decltype(T{} * U{})> operator*(T scalar, const Vec<3, U>& vec) { - return Vec<3, T>{scalar * vec.x, scalar * vec.y, scalar * vec.z}; + return {scalar * vec.x, scalar * vec.y, scalar * vec.z}; } - template - constexpr Vec<3, T> operator*(const Vec<3, T>& lhs, const Vec<3, T>& rhs) + template + constexpr Vec<3, decltype(T{} * U{})> operator*(const Vec<3, T>& lhs, const Vec<3, U>& rhs) { - return Vec<3, T>{lhs.x * rhs.x, lhs.y * rhs.y, lhs.z * rhs.z}; + return {lhs.x * rhs.x, lhs.y * rhs.y, lhs.z * rhs.z}; } - template - constexpr Vec<3, T> operator/(const Vec<3, T>& vec, T scalar) + template + constexpr Vec<3, decltype(T{} / U{})> operator/(const Vec<3, T>& vec, U scalar) { - return Vec<3, T>{vec.x / scalar, vec.y / scalar, vec.z / scalar}; + return {vec.x / scalar, vec.y / scalar, vec.z / scalar}; } - template - constexpr Vec<3, T> operator/(T scalar, const Vec<3, T>& vec) + template + constexpr Vec<3, decltype(T{} / U{})> operator/(T scalar, const Vec<3, U>& vec) { - return Vec<3, T>{scalar / vec.x, scalar / vec.y, scalar / vec.z}; + return {scalar / vec.x, scalar / vec.y, scalar / vec.z}; } - template - constexpr Vec<3, T> operator/(const Vec<3, T>& lhs, const Vec<3, T>& rhs) + template + constexpr Vec<3, decltype(T{} / U{})> operator/(const Vec<3, T>& lhs, const Vec<3, U>& rhs) { - return Vec<3, T>{lhs.x / rhs.x, lhs.y / rhs.y, lhs.z / rhs.z}; + return {lhs.x / rhs.x, lhs.y / rhs.y, lhs.z / rhs.z}; } constexpr Vec<3, bool> operator&&(const Vec<3, bool>& lhs, const Vec<3, bool>& rhs) diff --git a/src/oscar/Maths/Vec4.h b/src/oscar/Maths/Vec4.h index b99858a490..0e8a185ef5 100644 --- a/src/oscar/Maths/Vec4.h +++ b/src/oscar/Maths/Vec4.h @@ -1,13 +1,15 @@ #pragma once +#include #include +#include #include #include namespace osc { - template + template struct Vec<4, T> { using value_type = T; using element_type = T; @@ -21,11 +23,11 @@ namespace osc using const_iterator = const T*; constexpr Vec() = default; - constexpr explicit Vec(T scalar) : - x{scalar}, - y{scalar}, - z{scalar}, - w{scalar} + constexpr explicit Vec(T value) : + x{value}, + y{value}, + z{value}, + w{value} {} constexpr Vec(T x_, T y_, T z_, T w_) : x{x_}, @@ -33,57 +35,72 @@ namespace osc z{z_}, w{w_} {} - template - constexpr Vec(X x_, Y y_, Z z_, W w_) : + template + requires std::constructible_from and std::constructible_from and std::constructible_from and std::constructible_from + explicit (not (std::convertible_to and std::convertible_to and std::convertible_to and std::convertible_to)) + constexpr Vec(A x_, B y_, C z_, D w_) : x{static_cast(x_)}, y{static_cast(y_)}, z{static_cast(z_)}, w{static_cast(w_)} {} - template + template + requires std::constructible_from and std::constructible_from and std::constructible_from + explicit (not (std::convertible_to and std::convertible_to and std::convertible_to)) constexpr Vec(const Vec<2, A>& xy_, B z_, C w_) : x{static_cast(xy_.x)}, y{static_cast(xy_.y)}, z{static_cast(z_)}, w{static_cast(w_)} {} - template + template + requires std::constructible_from and std::constructible_from and std::constructible_from + explicit (not (std::convertible_to and std::convertible_to and std::convertible_to)) constexpr Vec(A x_, const Vec<2, B>& yz_, C w_) : x{static_cast(x_)}, y{static_cast(yz_.x)}, z{static_cast(yz_.y)}, w{static_cast(w_)} {} - template + template + requires std::constructible_from and std::constructible_from and std::constructible_from + explicit (not (std::convertible_to and std::convertible_to and std::convertible_to)) constexpr Vec(A x_, B y_, const Vec<2, C>& zw_) : x{static_cast(x_)}, y{static_cast(y_)}, z{static_cast(zw_.x)}, w{static_cast(zw_.y)} {} - template + template + requires std::constructible_from and std::constructible_from + explicit (not (std::convertible_to and std::convertible_to)) constexpr Vec(const Vec<3, A>& xyz_, B w_) : x{static_cast(xyz_.x)}, y{static_cast(xyz_.y)}, z{static_cast(xyz_.z)}, w{static_cast(w_)} {} - template + template + requires std::constructible_from and std::constructible_from + explicit (not (std::convertible_to and std::convertible_to)) constexpr Vec(A x_, const Vec<3, B>& yzw_) : x{static_cast(x_)}, y{static_cast(yzw_.x)}, z{static_cast(yzw_.y)}, w{static_cast(yzw_.z)} {} - template + template + requires std::constructible_from and std::constructible_from + explicit (not (std::convertible_to and std::convertible_to)) constexpr Vec(const Vec<2, A>& xy_, const Vec<2, B>& wz_) : x{static_cast(xy_.x)}, y{static_cast(xy_.y)}, z{static_cast(wz_.x)}, w{static_cast(wz_.w)} {} - template - constexpr explicit Vec(const Vec<4, U>& v) : + template + requires std::constructible_from + explicit constexpr Vec(const Vec<4, A>& v) : x{static_cast(v.x)}, y{static_cast(v.y)}, z{static_cast(v.z)}, @@ -102,17 +119,19 @@ namespace osc constexpr friend bool operator==(const Vec&, const Vec&) = default; - template + template + requires std::assignable_from constexpr Vec& operator=(const Vec<4, U>& v) { - this->x = static_cast(v.x); - this->y = static_cast(v.y); - this->z = static_cast(v.z); - this->w = static_cast(v.w); + this->x = v.x; + this->y = v.y; + this->z = v.z; + this->w = v.w; return *this; } - template + template + requires (not std::same_as) constexpr Vec& operator+=(U scalar) { this->x += static_cast(scalar); @@ -122,7 +141,8 @@ namespace osc return *this; } - template + template + requires (not std::same_as) constexpr Vec& operator+=(const Vec<4, U>& rhs) { this->x += static_cast(rhs.x); @@ -132,7 +152,8 @@ namespace osc return *this; } - template + template + requires (not std::same_as) constexpr Vec& operator-=(U scalar) { this->x -= static_cast(scalar); @@ -142,7 +163,8 @@ namespace osc return *this; } - template + template + requires (not std::same_as) constexpr Vec& operator-=(const Vec<4, U>& rhs) { this->x -= static_cast(rhs.x); @@ -152,7 +174,8 @@ namespace osc return *this; } - template + template + requires (not std::same_as) constexpr Vec& operator*=(U scalar) { this->x *= static_cast(scalar); @@ -162,7 +185,8 @@ namespace osc return *this; } - template + template + requires (not std::same_as) constexpr Vec& operator*=(const Vec<4, U>& rhs) { this->x *= static_cast(rhs.x); @@ -172,7 +196,8 @@ namespace osc return *this; } - template + template + requires (not std::same_as) constexpr Vec& operator/=(U scalar) { this->x /= static_cast(scalar); @@ -182,7 +207,8 @@ namespace osc return *this; } - template + template + requires (not std::same_as) constexpr Vec& operator/=(const Vec<4, U>& rhs) { this->x /= static_cast(rhs.x); @@ -193,6 +219,7 @@ namespace osc } constexpr Vec& operator++() + requires std::incrementable { ++this->x; ++this->y; @@ -202,6 +229,7 @@ namespace osc } constexpr Vec& operator--() + requires std::incrementable { --this->x; --this->y; @@ -211,6 +239,7 @@ namespace osc } constexpr Vec operator++(int) + requires std::incrementable { Vec copy{*this}; ++copy; @@ -218,6 +247,7 @@ namespace osc } constexpr Vec operator--(int) + requires std::incrementable { Vec copy{*this}; --copy; @@ -225,10 +255,11 @@ namespace osc } template - constexpr Vec with_element(size_type pos, U scalar) const + requires std::constructible_from + constexpr Vec with_element(size_type pos, U value) const { Vec copy{*this}; - copy[pos] = static_cast(scalar); + copy[pos] = static_cast(value); return copy; } @@ -238,89 +269,89 @@ namespace osc T w{}; }; - template + template constexpr Vec<4, T> operator+(const Vec<4, T>& vec) { - return vec; + return {+vec.x, +vec.y, +vec.z, +vec.w}; } - template + template constexpr Vec<4, T> operator-(const Vec<4, T>& vec) { - return Vec<4, T>{-vec.x, -vec.y, -vec.z, -vec.w}; + return {-vec.x, -vec.y, -vec.z, -vec.w}; } - template - constexpr Vec<4, T> operator+(const Vec<4, T>& vec, T scalar) + template + constexpr Vec<4, decltype(T{} + U{})> operator+(const Vec<4, T>& vec, U scalar) { - return Vec<4, T>{vec.x + scalar, vec.y + scalar, vec.z + scalar, vec.w + scalar}; + return {vec.x + scalar, vec.y + scalar, vec.z + scalar, vec.w + scalar}; } - template - constexpr Vec<4, T> operator+(T scalar, const Vec<4, T>& vec) + template + constexpr Vec<4, decltype(T{} + U{})> operator+(T scalar, const Vec<4, U>& vec) { - return Vec<4, T>{scalar + vec.x, scalar + vec.y, scalar + vec.z, scalar + vec.w}; + return {scalar + vec.x, scalar + vec.y, scalar + vec.z, scalar + vec.w}; } - template - constexpr Vec<4, T> operator+(const Vec<4, T>& lhs, const Vec<4, T>& rhs) + template + constexpr Vec<4, decltype(T{} + U{})> operator+(const Vec<4, T>& lhs, const Vec<4, U>& rhs) { - return Vec<4, T>{lhs.x + rhs.x, lhs.y + rhs.y, lhs.z + rhs.z, lhs.w + rhs.w}; + return {lhs.x + rhs.x, lhs.y + rhs.y, lhs.z + rhs.z, lhs.w + rhs.w}; } - template - constexpr Vec<4, T> operator-(const Vec<4, T>& vec, T scalar) + template + constexpr Vec<4, decltype(T{} - U{})> operator-(const Vec<4, T>& vec, U scalar) { - return Vec<4, T>{vec.x - scalar, vec.y - scalar, vec.z - scalar, vec.w - scalar}; + return {vec.x - scalar, vec.y - scalar, vec.z - scalar, vec.w - scalar}; } - template - constexpr Vec<4, T> operator-(T scalar, const Vec<4, T>& vec) + template + constexpr Vec<4, decltype(T{} - U{})> operator-(T scalar, const Vec<4, U>& vec) { - return Vec<4, T>{scalar - vec.x, scalar - vec.y, scalar - vec.z, scalar - vec.w}; + return {scalar - vec.x, scalar - vec.y, scalar - vec.z, scalar - vec.w}; } - template - constexpr Vec<4, T> operator-(const Vec<4, T>& lhs, const Vec<4, T>& rhs) + template + constexpr Vec<4, decltype(T{} - U{})> operator-(const Vec<4, T>& lhs, const Vec<4, U>& rhs) { - return Vec<4, T>{lhs.x - rhs.x, lhs.y - rhs.y, lhs.z - rhs.z, lhs.w - rhs.w}; + return {lhs.x - rhs.x, lhs.y - rhs.y, lhs.z - rhs.z, lhs.w - rhs.w}; } - template - constexpr Vec<4, T> operator*(const Vec<4, T>& vec, T scalar) + template + constexpr Vec<4, decltype(T{} * U{})> operator*(const Vec<4, T>& vec, U scalar) { - return Vec<4, T>{vec.x * scalar, vec.y * scalar, vec.z * scalar, vec.w * scalar}; + return {vec.x * scalar, vec.y * scalar, vec.z * scalar, vec.w * scalar}; } - template - constexpr Vec<4, T> operator*(T scalar, const Vec<4, T>& vec) + template + constexpr Vec<4, decltype(T{} * U{})> operator*(T scalar, const Vec<4, U>& vec) { - return Vec<4, T>{scalar * vec.x, scalar * vec.y, scalar * vec.z, scalar * vec.w}; + return {scalar * vec.x, scalar * vec.y, scalar * vec.z, scalar * vec.w}; } - template - constexpr Vec<4, T> operator*(const Vec<4, T>& lhs, const Vec<4, T>& rhs) + template + constexpr Vec<4, decltype(T{} * U{})> operator*(const Vec<4, T>& lhs, const Vec<4, U>& rhs) { - return Vec<4, T>{lhs.x * rhs.x, lhs.y * rhs.y, lhs.z * rhs.z, lhs.w * rhs.w}; + return {lhs.x * rhs.x, lhs.y * rhs.y, lhs.z * rhs.z, lhs.w * rhs.w}; } - template - constexpr Vec<4, T> operator/(const Vec<4, T>& vec, T scalar) + template + constexpr Vec<4, decltype(T{} / U{})> operator/(const Vec<4, T>& vec, U scalar) { - return Vec<4, T>{vec.x / scalar, vec.y / scalar, vec.z / scalar, vec.w / scalar}; + return {vec.x / scalar, vec.y / scalar, vec.z / scalar, vec.w / scalar}; } - template - constexpr Vec<4, T> operator/(T scalar, const Vec<4, T>& vec) + template + constexpr Vec<4, decltype(T{} / U{})> operator/(T scalar, const Vec<4, U>& vec) { - return Vec<4, T>{scalar / vec.x, scalar / vec.y, scalar / vec.z, scalar / vec.w}; + return {scalar / vec.x, scalar / vec.y, scalar / vec.z, scalar / vec.w}; } - template - constexpr Vec<4, T> operator/(const Vec<4, T>& lhs, const Vec<4, T>& rhs) + template + constexpr Vec<4, decltype(T{} / U{})> operator/(const Vec<4, T>& lhs, const Vec<4, U>& rhs) { - return Vec<4, T>{lhs.x / rhs.x, lhs.y / rhs.y, lhs.z / rhs.z, lhs.w / rhs.w}; + return {lhs.x / rhs.x, lhs.y / rhs.y, lhs.z / rhs.z, lhs.w / rhs.w}; } constexpr Vec<4, bool> operator&&(const Vec<4, bool>& lhs, const Vec<4, bool>& rhs) diff --git a/src/oscar/Maths/VecFunctions.h b/src/oscar/Maths/VecFunctions.h index da8808d089..6f90142820 100644 --- a/src/oscar/Maths/VecFunctions.h +++ b/src/oscar/Maths/VecFunctions.h @@ -1,18 +1,19 @@ #pragma once +#include #include #include namespace osc { - template + template constexpr const T* value_ptr(const Vec& vec) { return vec.data(); } - template + template constexpr T* value_ptr(Vec& vec) { return vec.data(); diff --git a/src/oscar/UI/oscimgui.cpp b/src/oscar/UI/oscimgui.cpp index d1e2d22a54..6b94f7f0f6 100644 --- a/src/oscar/UI/oscimgui.cpp +++ b/src/oscar/UI/oscimgui.cpp @@ -169,7 +169,7 @@ std::optional osc::ui::Gizmo::draw( style.ScaleLineCircleSize = 8.0f; } - const Vec3 original_translation = model_matrix[3]; + const Vec3 original_translation = Vec3{model_matrix[3]}; const bool gizmo_was_manipulated_by_user = ImGuizmo::Manipulate( value_ptr(view_matrix), value_ptr(projection_matrix), @@ -197,11 +197,10 @@ std::optional osc::ui::Gizmo::draw( value_ptr(world_rotation_in_degrees), value_ptr(world_scale) ); - const EulerAngles world_eulers = Vec<3, Degrees>(world_rotation_in_degrees); const GizmoTransform rv = { .scale = world_scale, - .rotation = world_eulers, + .rotation = EulerAnglesIn{world_rotation_in_degrees}, .position = world_translation, }; return rv; diff --git a/tests/testoscar/Maths/TestAngle.cpp b/tests/testoscar/Maths/TestAngle.cpp index 12ccabd1e2..5dce70c02d 100644 --- a/tests/testoscar/Maths/TestAngle.cpp +++ b/tests/testoscar/Maths/TestAngle.cpp @@ -112,3 +112,11 @@ TEST(Angle, CanUseProjectedClampWithAngles) }; static_assert(clamp(S{-10_deg}, S{0_deg}, S{180_deg}, {}, &S::ang).ang == S{0_deg}.ang); } + +TEST(Angle, std_is_convertible_to_works_between_angles) +{ + static_assert(std::convertible_to); + static_assert(std::convertible_to); + static_assert(std::convertible_to); + // etc. - many parts of the codebase require that these are automatically convertible +} From 42491ff6c175dd68b4922bfb2fcc4f3f4418c411 Mon Sep 17 00:00:00 2001 From: Adam Kewley Date: Thu, 18 Jul 2024 13:39:51 +0200 Subject: [PATCH 10/13] Remove unused variable --- src/oscar/UI/oscimgui.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/oscar/UI/oscimgui.cpp b/src/oscar/UI/oscimgui.cpp index 6b94f7f0f6..1001c619aa 100644 --- a/src/oscar/UI/oscimgui.cpp +++ b/src/oscar/UI/oscimgui.cpp @@ -169,7 +169,6 @@ std::optional osc::ui::Gizmo::draw( style.ScaleLineCircleSize = 8.0f; } - const Vec3 original_translation = Vec3{model_matrix[3]}; const bool gizmo_was_manipulated_by_user = ImGuizmo::Manipulate( value_ptr(view_matrix), value_ptr(projection_matrix), From 8c108d7793915f593a29b3c8ea2020279226481f Mon Sep 17 00:00:00 2001 From: Adam Kewley Date: Thu, 18 Jul 2024 14:32:39 +0200 Subject: [PATCH 11/13] Clean up gizmo API design slightly --- .../UI/MeshImporter/MeshImporterTab.cpp | 16 ++------- .../UI/Shared/ModelEditorViewerPanel.cpp | 18 +++------- .../UI/Shared/ModelSelectionGizmo.h | 2 ++ src/oscar/UI/oscimgui.cpp | 33 ++++++++++++++++--- src/oscar/UI/oscimgui.h | 24 +++++++------- 5 files changed, 50 insertions(+), 43 deletions(-) diff --git a/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp b/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp index 069276a2c2..e793f90364 100644 --- a/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp +++ b/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp @@ -1825,24 +1825,14 @@ class osc::mi::MeshImporterTab::Impl final : public IMeshImporterUILayerHost { ui::same_line(); - { - ui::GizmoOperation op = m_Gizmo.operation(); - if (ui::draw_gizmo_op_selector(op)) { - m_Gizmo.set_operation(op); - } - } + ui::draw_gizmo_op_selector(m_Gizmo); ui::push_style_var(ImGuiStyleVar_ItemSpacing, {0.0f, 0.0f}); ui::same_line(); ui::pop_style_var(); // local/global dropdown - { - ui::GizmoMode mode = m_Gizmo.mode(); - if (ui::draw_gizmo_mode_selector(mode)) { - m_Gizmo.set_mode(mode); - } - } + ui::draw_gizmo_mode_selector(m_Gizmo); ui::same_line(); // scale factor @@ -2128,7 +2118,7 @@ class osc::mi::MeshImporterTab::Impl final : public IMeshImporterUILayerHost { MIObject& el = m_Shared->updModelGraph().updByID(id); switch (m_Gizmo.operation()) { case ui::GizmoOperation::Rotate: - el.applyRotation(m_Shared->getModelGraph(), userManipulation->rotation, Vec3{m_GizmoModelMatrix[3]}); + el.applyRotation(m_Shared->getModelGraph(), extract_eulers_xyz(userManipulation->rotation), Vec3{m_GizmoModelMatrix[3]}); break; case ui::GizmoOperation::Translate: { el.applyTranslation(m_Shared->getModelGraph(), userManipulation->position); diff --git a/src/OpenSimCreator/UI/Shared/ModelEditorViewerPanel.cpp b/src/OpenSimCreator/UI/Shared/ModelEditorViewerPanel.cpp index b57a27c117..2eccea9b5a 100644 --- a/src/OpenSimCreator/UI/Shared/ModelEditorViewerPanel.cpp +++ b/src/OpenSimCreator/UI/Shared/ModelEditorViewerPanel.cpp @@ -188,13 +188,8 @@ namespace ui::same_line(); // draw translate/rotate/scale selector - { - ui::GizmoOperation op = m_Gizmo.getOperation(); - if (ui::draw_gizmo_op_selector(op, true, true, false)) - { - m_Gizmo.setOperation(op); - edited = true; - } + if (ui::draw_gizmo_op_selector(m_Gizmo, true, true, false)) { + edited = true; } ui::push_style_var(ImGuiStyleVar_ItemSpacing, {0.0f, 0.0f}); @@ -202,13 +197,8 @@ namespace ui::pop_style_var(); // draw global/world selector - { - ui::GizmoMode mode = m_Gizmo.getMode(); - if (ui::draw_gizmo_mode_selector(mode)) - { - m_Gizmo.setMode(mode); - edited = true; - } + if (ui::draw_gizmo_mode_selector(m_Gizmo)) { + edited = true; } return edited; diff --git a/src/OpenSimCreator/UI/Shared/ModelSelectionGizmo.h b/src/OpenSimCreator/UI/Shared/ModelSelectionGizmo.h index f9875611cc..41c6f197b8 100644 --- a/src/OpenSimCreator/UI/Shared/ModelSelectionGizmo.h +++ b/src/OpenSimCreator/UI/Shared/ModelSelectionGizmo.h @@ -31,6 +31,8 @@ namespace osc ui::GizmoMode getMode() { return m_Gizmo.mode(); } void setMode(ui::GizmoMode mode) { m_Gizmo.set_mode(mode); } + operator ui::Gizmo& () { return m_Gizmo; } + private: std::shared_ptr m_Model; ui::Gizmo m_Gizmo; diff --git a/src/oscar/UI/oscimgui.cpp b/src/oscar/UI/oscimgui.cpp index 1001c619aa..df7ec28df6 100644 --- a/src/oscar/UI/oscimgui.cpp +++ b/src/oscar/UI/oscimgui.cpp @@ -46,6 +46,16 @@ namespace } } +bool osc::ui::draw_gizmo_mode_selector(Gizmo& gizmo) +{ + GizmoMode mode = gizmo.mode(); + if (draw_gizmo_mode_selector(mode)) { + gizmo.set_mode(mode); + return true; + } + return false; +} + bool osc::ui::draw_gizmo_mode_selector(GizmoMode& mode) { constexpr auto mode_labels = std::to_array({ "local", "global" }); @@ -68,7 +78,21 @@ bool osc::ui::draw_gizmo_mode_selector(GizmoMode& mode) } bool osc::ui::draw_gizmo_op_selector( - ui::GizmoOperation& op, + Gizmo& gizmo, + bool can_translate, + bool can_rotate, + bool can_scale) +{ + GizmoOperation op = gizmo.operation(); + if (draw_gizmo_op_selector(op, can_translate, can_rotate, can_scale)) { + gizmo.set_operation(op); + return true; + } + return false; +} + +bool osc::ui::draw_gizmo_op_selector( + GizmoOperation& op, bool can_translate, bool can_rotate, bool can_scale) @@ -132,7 +156,7 @@ bool osc::ui::draw_gizmo_op_selector( return rv; } -std::optional osc::ui::Gizmo::draw( +std::optional osc::ui::Gizmo::draw( Mat4& model_matrix, const Mat4& view_matrix, const Mat4& projection_matrix, @@ -197,12 +221,11 @@ std::optional osc::ui::Gizmo::draw( value_ptr(world_scale) ); - const GizmoTransform rv = { + return Transform{ .scale = world_scale, - .rotation = EulerAnglesIn{world_rotation_in_degrees}, + .rotation = to_worldspace_rotation_quat(EulerAnglesIn{world_rotation_in_degrees}), .position = world_translation, }; - return rv; } bool osc::ui::Gizmo::is_using() const diff --git a/src/oscar/UI/oscimgui.h b/src/oscar/UI/oscimgui.h index 7c5ee6abf3..a1469c546e 100644 --- a/src/oscar/UI/oscimgui.h +++ b/src/oscar/UI/oscimgui.h @@ -834,16 +834,6 @@ namespace osc::ui ImGui::ShowDemoWindow(); } - // a the "difference" added by a user-enacted manipulation with a `Gizmo` - // - // this can be left-multiplied by the original model matrix to apply the - // user's transformation - struct GizmoTransform final { - Vec3 scale = {1.0f, 1.0f, 1.0f}; - EulerAngles rotation = {}; - Vec3 position = {}; - }; - // an operation that a ui `Gizmo` shall perform enum class GizmoOperation { Translate, @@ -873,7 +863,7 @@ namespace osc::ui // you're thinking "oh I'm only handling rotation/scaling, so I'll // ignore the translational part of the transform" then you're in // for a nasty surprise: T(origin)*R*S*T(-origin) - std::optional draw( + std::optional draw( Mat4& model_matrix, // edited in-place const Mat4& view_matrix, const Mat4& projection_matrix, @@ -896,9 +886,21 @@ namespace osc::ui bool was_using_last_frame_ = false; }; + bool draw_gizmo_mode_selector( + Gizmo& + ); + bool draw_gizmo_mode_selector( GizmoMode& ); + + bool draw_gizmo_op_selector( + Gizmo&, + bool can_translate = true, + bool can_rotate = true, + bool can_scale = true + ); + bool draw_gizmo_op_selector( GizmoOperation&, bool can_translate = true, From 5858b11c122b40c85595b512b66065a1e0d712ef Mon Sep 17 00:00:00 2001 From: Adam Kewley Date: Thu, 18 Jul 2024 17:30:24 +0200 Subject: [PATCH 12/13] Add FunctionCurveViewerPopup --- CHANGELOG.md | 2 + src/OpenSimCreator/CMakeLists.txt | 4 +- .../UI/Shared/FunctionCurveViewerPopup.cpp | 167 ++++++++++++++++++ .../UI/Shared/FunctionCurveViewerPopup.h | 39 ++++ .../UI/Shared/ObjectPropertiesEditor.cpp | 80 ++++++++- src/oscar/Maths/ClosedInterval.h | 20 +++ tests/testoscar/Maths/TestClosedInterval.cpp | 22 +++ 7 files changed, 325 insertions(+), 9 deletions(-) create mode 100644 src/OpenSimCreator/UI/Shared/FunctionCurveViewerPopup.cpp create mode 100644 src/OpenSimCreator/UI/Shared/FunctionCurveViewerPopup.h diff --git a/CHANGELOG.md b/CHANGELOG.md index c5f75e2d3c..e9dad5aba4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [Upcoming Release] +- The model editor UI now has experimental support for viewing `OpenSim::Function` curves. This + is currently exposed as an eye icon in the property editor panel - Selecting an `OpenSim::Joint` that has `OpenSim::PhysicalOffsetFrame`s for both its parent and child frames now shows a 3D manipulation gizmo that lets you move the joint center without moving anything else in the model (#159) diff --git a/src/OpenSimCreator/CMakeLists.txt b/src/OpenSimCreator/CMakeLists.txt index a01f784d01..07ea10aae4 100644 --- a/src/OpenSimCreator/CMakeLists.txt +++ b/src/OpenSimCreator/CMakeLists.txt @@ -328,6 +328,8 @@ add_library(OpenSimCreator STATIC UI/Shared/ChooseComponentsEditorLayer.cpp UI/Shared/ChooseComponentsEditorLayer.h UI/Shared/ChooseComponentsEditorLayerParameters.h + UI/Shared/FunctionCurveViewerPopup.cpp + UI/Shared/FunctionCurveViewerPopup.h UI/Shared/GeometryPathEditorPopup.cpp UI/Shared/GeometryPathEditorPopup.h UI/Shared/ImportStationsFromCSVPopup.cpp @@ -409,7 +411,7 @@ add_library(OpenSimCreator STATIC Utils/SimTKHelpers.h Utils/TPS3D.cpp Utils/TPS3D.h - ) +) target_include_directories(OpenSimCreator PUBLIC diff --git a/src/OpenSimCreator/UI/Shared/FunctionCurveViewerPopup.cpp b/src/OpenSimCreator/UI/Shared/FunctionCurveViewerPopup.cpp new file mode 100644 index 0000000000..954eafd4a8 --- /dev/null +++ b/src/OpenSimCreator/UI/Shared/FunctionCurveViewerPopup.cpp @@ -0,0 +1,167 @@ +#include "FunctionCurveViewerPopup.h" + +#include + +#include +#include +#include +#include + +#include +#include +#include +#include + +using namespace osc; + +class osc::FunctionCurveViewerPopup::Impl final : public StandardPopup { +public: + Impl( + std::string_view popupName, + std::shared_ptr targetModel, + std::function functionGetter) : + + StandardPopup{popupName, {768.0f, 0.0f}, ImGuiWindowFlags_AlwaysAutoResize}, + m_TargetModel{std::move(targetModel)}, + m_FunctionGetter{std::move(functionGetter)} + {} +private: + struct FunctionParameters final { + friend bool operator==(const FunctionParameters& lhs, const FunctionParameters& rhs) = default; + + UID modelVerison; + UID stateVersion; + ClosedInterval inputRange = {-1.0f, 1.0f}; + int numPoints = 100; + }; + + void impl_draw_content() final + { + // update parameter state and check if replotting is necessary + m_LatestParameters.modelVerison = m_TargetModel->getModelVersion(); + m_LatestParameters.stateVersion = m_TargetModel->getStateVersion(); + if (m_LatestParameters != m_PlottedParameters) { + m_PlottedParameters = m_LatestParameters; + m_PlotPoints = generatePlotPoints(m_LatestParameters); + } + + drawTopEditors(); + drawPlot(); + drawBottomButtons(); + } + + void drawTopEditors() + { + ui::draw_float_input("min", &m_LatestParameters.inputRange.lower); + ui::draw_float_input("max", &m_LatestParameters.inputRange.upper); + if (ui::draw_int_input("num points", &m_LatestParameters.numPoints)) { + m_LatestParameters.numPoints = clamp(m_LatestParameters.numPoints, 0, 10000); // sanity + } + } + + void drawPlot() + { + if (m_PlotPoints.empty()) { + return; // don't try to plot null data etc. + } + + const float availableWidth = ui::get_content_region_avail().x; + const Vec2 plotDimensions{availableWidth}; + + if (ImPlot::BeginPlot("Plot", plotDimensions)) { + ImPlot::SetNextMarkerStyle(ImPlotMarker_Circle, 2.0f); + ImPlot::PlotLine( + "Plot", + &m_PlotPoints.front().x, + &m_PlotPoints.front().y, + static_cast(m_PlotPoints.size()), + 0, + 0, + sizeof(typename decltype(m_PlotPoints)::value_type) + ); + } + } + + void drawBottomButtons() + { + if (ui::draw_button("cancel")) { + request_close(); + } + } + + std::vector generatePlotPoints(const FunctionParameters& params) + { + std::vector rv; + + const OpenSim::Function* function = m_FunctionGetter(); + if (not function) { + m_Error = "could not get the function from the model (maybe the model was edited, or the function was deleted?)"; + return rv; + } + + rv.reserve(params.numPoints); + const double stepSize = params.inputRange.step_size(params.numPoints); + SimTK::Vector x(1); + try { + for (int step = 0; step < params.numPoints; ++step) { + x[0] = params.inputRange.lower + stepSize*step; + const double y = function->calcValue(x); + rv.push_back({x[0], y}); + } + } + catch (const std::exception& ex) { + m_Error = ex.what(); + rv = {}; + return rv; + } + + return rv; + } + + std::shared_ptr m_TargetModel; + std::function m_FunctionGetter; + FunctionParameters m_LatestParameters = { + .modelVerison = m_TargetModel->getModelVersion(), + .stateVersion = m_TargetModel->getStateVersion(), + }; + std::optional m_PlottedParameters; + std::vector m_PlotPoints; + std::optional m_Error; +}; + + +osc::FunctionCurveViewerPopup::FunctionCurveViewerPopup( + std::string_view popupName, + std::shared_ptr targetModel, + std::function functionGetter) : + + m_Impl{std::make_unique(popupName, std::move(targetModel), std::move(functionGetter))} +{} +osc::FunctionCurveViewerPopup::FunctionCurveViewerPopup(FunctionCurveViewerPopup&&) noexcept = default; +osc::FunctionCurveViewerPopup& osc::FunctionCurveViewerPopup::operator=(FunctionCurveViewerPopup&&) noexcept = default; +osc::FunctionCurveViewerPopup::~FunctionCurveViewerPopup() noexcept = default; + +bool osc::FunctionCurveViewerPopup::impl_is_open() const +{ + return m_Impl->is_open(); +} +void osc::FunctionCurveViewerPopup::impl_open() +{ + m_Impl->open(); +} +void osc::FunctionCurveViewerPopup::impl_close() +{ + m_Impl->close(); +} +bool osc::FunctionCurveViewerPopup::impl_begin_popup() +{ + return m_Impl->begin_popup(); +} +void osc::FunctionCurveViewerPopup::impl_on_draw() +{ + m_Impl->on_draw(); +} +void osc::FunctionCurveViewerPopup::impl_end_popup() +{ + m_Impl->end_popup(); +} diff --git a/src/OpenSimCreator/UI/Shared/FunctionCurveViewerPopup.h b/src/OpenSimCreator/UI/Shared/FunctionCurveViewerPopup.h new file mode 100644 index 0000000000..633aa2f470 --- /dev/null +++ b/src/OpenSimCreator/UI/Shared/FunctionCurveViewerPopup.h @@ -0,0 +1,39 @@ +#pragma once + +#include + +#include +#include +#include + +namespace OpenSim { class Function; } +namespace osc { class IConstModelStatePair; } + +namespace osc +{ + + class FunctionCurveViewerPopup final : public IPopup { + public: + FunctionCurveViewerPopup( + std::string_view popupName, + std::shared_ptr targetModel, + std::function functionGetter + ); + FunctionCurveViewerPopup(const FunctionCurveViewerPopup&) = delete; + FunctionCurveViewerPopup(FunctionCurveViewerPopup&&) noexcept; + FunctionCurveViewerPopup& operator=(const FunctionCurveViewerPopup&) = delete; + FunctionCurveViewerPopup& operator=(FunctionCurveViewerPopup&&) noexcept; + ~FunctionCurveViewerPopup() noexcept; + + private: + bool impl_is_open() const final; + void impl_open() final; + void impl_close() final; + bool impl_begin_popup() final; + void impl_on_draw() final; + void impl_end_popup() final; + + class Impl; + std::unique_ptr m_Impl; + }; +} diff --git a/src/OpenSimCreator/UI/Shared/ObjectPropertiesEditor.cpp b/src/OpenSimCreator/UI/Shared/ObjectPropertiesEditor.cpp index e3f570ef91..15ebe8c752 100644 --- a/src/OpenSimCreator/UI/Shared/ObjectPropertiesEditor.cpp +++ b/src/OpenSimCreator/UI/Shared/ObjectPropertiesEditor.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -11,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -373,9 +375,9 @@ namespace template ConcreteProperty> struct PropertyEditorTraits { - static bool IsCompatibleWith(const OpenSim::AbstractProperty& prop) + static const ConcreteProperty* TryGet(const OpenSim::AbstractProperty* prop) { - return dynamic_cast(&prop) != nullptr; + return dynamic_cast(prop); } }; @@ -390,7 +392,7 @@ namespace static bool IsCompatibleWith(const OpenSim::AbstractProperty& prop) { - return Traits::IsCompatibleWith(prop); + return Traits::TryGet(&prop) != nullptr; } explicit PropertyEditor(PropertyEditorArgs args) : @@ -401,14 +403,14 @@ namespace protected: const property_type* tryGetProperty() const { - return dynamic_cast(m_Args.propertyAccessor()); + return Traits::TryGet(m_Args.propertyAccessor()); } std::function getPropertyAccessor() const { return [accessor = this->m_Args.propertyAccessor]() { - return dynamic_cast(accessor()); + return Traits::TryGet(accessor()); }; } @@ -448,7 +450,7 @@ namespace private: bool implIsCompatibleWith(const OpenSim::AbstractProperty& prop) const final { - return Traits::IsCompatibleWith(prop); + return Traits::TryGet(&prop) != nullptr; } PropertyEditorArgs m_Args; @@ -1456,7 +1458,6 @@ namespace } ui::next_column(); - if (*m_ReturnValueHolder) { std::optional edit; @@ -1497,6 +1498,68 @@ namespace // shared between this property editor and a popup it may have spawned std::shared_ptr> m_ReturnValueHolder = std::make_shared>(); }; + + struct FunctionPropertyEditorTraits { + static const OpenSim::ObjectProperty* TryGet(const OpenSim::AbstractProperty* prop) + { + if (not prop) { + return nullptr; + } + if (not prop->isObjectProperty()) { + return nullptr; + } + if (prop->size() <= 0) { + return nullptr; + } + if (dynamic_cast(&prop->getValueAsObject(0)) == nullptr) { + return nullptr; + } + return static_cast*>(prop); + } + }; + + class FunctionPropertyEditor final : + public PropertyEditor, FunctionPropertyEditorTraits> { + public: + using PropertyEditor::PropertyEditor; + + private: + std::optional> implOnDraw() final + { + const property_type* maybeProp = tryGetProperty(); + if (!maybeProp) + { + return std::nullopt; + } + const property_type& prop = *maybeProp; + + ui::draw_separator(); + DrawPropertyName(prop); + ui::next_column(); + if (ui::draw_button(ICON_FA_EYE)) { + std::stringstream ss; + ss << "View " << prop.getName() << " (" << prop.getObjectClassName() << ')'; + pushPopup(std::make_unique( + std::move(ss).str(), + getModelPtr(), + [accessor = getPropertyAccessor()]() -> const OpenSim::Function* + { + const property_type* p = accessor(); + if (!p || p->isListProperty()) { + return nullptr; + } + return &p->getValue(); + } + )); + } + ui::draw_tooltip_if_item_hovered("View Function", ICON_FA_MAGIC " Experimental Feature " ICON_FA_MAGIC ": currently, plots the `OpenSim::Function`, but it doesn't know what the X or Y axes are, or what values might be reasonable for either. It also doesn't spawn a non-modal panel, which would be handy if you wanted to view multiple functions at the same time - I should work on that ;)"); + ui::same_line(); + ui::draw_text(prop.getObjectClassName()); + ui::next_column(); + + return std::nullopt; + } + }; } namespace @@ -1513,7 +1576,8 @@ namespace IntPropertyEditor, AppearancePropertyEditor, ContactParameterSetEditor, - AbstractGeometryPathPropertyEditor + AbstractGeometryPathPropertyEditor, + FunctionPropertyEditor >; // a type-erased entry in the runtime registry LUT diff --git a/src/oscar/Maths/ClosedInterval.h b/src/oscar/Maths/ClosedInterval.h index 53e37d9ef4..19195563e2 100644 --- a/src/oscar/Maths/ClosedInterval.h +++ b/src/oscar/Maths/ClosedInterval.h @@ -1,6 +1,9 @@ #pragma once +#include + #include +#include namespace osc { @@ -8,6 +11,23 @@ namespace osc template requires std::equality_comparable and std::totally_ordered struct ClosedInterval final { + friend bool operator==(const ClosedInterval&, const ClosedInterval&) = default; + + template + constexpr T step_size(U nsteps) const + { + if (nsteps <= 1) { + return upper - lower; // edge-case + } + + return (upper - lower) / (static_cast>(nsteps) - 1); + } + + constexpr T normalized_interpolant_at(T v) const + { + return (v - lower) / (upper - lower); + } + T lower; T upper; }; diff --git a/tests/testoscar/Maths/TestClosedInterval.cpp b/tests/testoscar/Maths/TestClosedInterval.cpp index a6bf948633..32dfc6b288 100644 --- a/tests/testoscar/Maths/TestClosedInterval.cpp +++ b/tests/testoscar/Maths/TestClosedInterval.cpp @@ -1,6 +1,7 @@ #include #include +#include #include @@ -21,3 +22,24 @@ TEST(ClosedInterval, TimestampsAreAllowed) using TP = std::chrono::system_clock::time_point; [[maybe_unused]] ClosedInterval r{TP{}, TP{} + std::chrono::seconds{1}}; } + +TEST(ClosedInterval, normalized_interpolant_at_returns_zero_if_equal_to_lower) +{ + static_assert(ClosedInterval{-3.0f, 7.0f}.normalized_interpolant_at(-3.0f) == 0.0f); +} + +TEST(ClosedInterval, normalized_interpolant_at_returns_1_if_equal_to_upper) +{ + static_assert(ClosedInterval{-3.0f, 7.0f}.normalized_interpolant_at(7.0f) == 1.0f); +} + +TEST(ClosedInterval, step_size_returns_expected_answers) +{ + static_assert(ClosedInterval{0.0f, 0.0f}.step_size(0) == 0.0f); + static_assert(ClosedInterval{0.0f, 0.0f}.step_size(1) == 0.0f); + + static_assert(ClosedInterval{0.0f, 1.0f}.step_size(0) == 1.0f); + static_assert(ClosedInterval{0.0f, 1.0f}.step_size(1) == 1.0f); + static_assert(ClosedInterval{0.0f, 1.0f}.step_size(2) == 1.0f); + static_assert(ClosedInterval{0.0f, 1.0f}.step_size(3) == 0.5f); +} From 8f8cc9a95c340443cc48b81659f3edaddeffe0da Mon Sep 17 00:00:00 2001 From: Adam Kewley Date: Fri, 19 Jul 2024 14:33:25 +0200 Subject: [PATCH 13/13] Fix (most of the) issues found by clang-tidy --- src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp | 1 - .../UI/Shared/FunctionCurveViewerPopup.cpp | 2 +- src/OpenSimCreator/UI/Shared/ModelSelectionGizmo.cpp | 9 ++------- src/OpenSimCreator/UI/Shared/ObjectPropertiesEditor.cpp | 2 +- src/oscar/Maths/ClosedInterval.h | 9 ++++++++- 5 files changed, 12 insertions(+), 11 deletions(-) diff --git a/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp b/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp index e793f90364..140ae0d60e 100644 --- a/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp +++ b/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp @@ -2096,7 +2096,6 @@ class osc::mi::MeshImporterTab::Impl final : public IMeshImporterUILayerHost { Rect sceneRect = m_Shared->get3DSceneRect(); - const Mat4 oldModelMatrix = m_GizmoModelMatrix; const auto userManipulation = m_Gizmo.draw( m_GizmoModelMatrix, m_Shared->getCamera().view_matrix(), diff --git a/src/OpenSimCreator/UI/Shared/FunctionCurveViewerPopup.cpp b/src/OpenSimCreator/UI/Shared/FunctionCurveViewerPopup.cpp index 954eafd4a8..ed98760a18 100644 --- a/src/OpenSimCreator/UI/Shared/FunctionCurveViewerPopup.cpp +++ b/src/OpenSimCreator/UI/Shared/FunctionCurveViewerPopup.cpp @@ -106,7 +106,7 @@ class osc::FunctionCurveViewerPopup::Impl final : public StandardPopup { for (int step = 0; step < params.numPoints; ++step) { x[0] = params.inputRange.lower + stepSize*step; const double y = function->calcValue(x); - rv.push_back({x[0], y}); + rv.emplace_back(x[0], y); } } catch (const std::exception& ex) { diff --git a/src/OpenSimCreator/UI/Shared/ModelSelectionGizmo.cpp b/src/OpenSimCreator/UI/Shared/ModelSelectionGizmo.cpp index 4850faab59..f73803498d 100644 --- a/src/OpenSimCreator/UI/Shared/ModelSelectionGizmo.cpp +++ b/src/OpenSimCreator/UI/Shared/ModelSelectionGizmo.cpp @@ -290,11 +290,6 @@ namespace return false; } - SimTK::Rotation NegateRotation(const SimTK::Rotation& r) - { - return SimTK::Rotation{-SimTK::Mat33{r}, true}; - } - // an `ISelectionManipulator` that manipulates an `OpenSim::PhysicalOffsetFrame` class PhysicalOffsetFrameManipulator final : public SelectionManipulator { public: @@ -341,7 +336,7 @@ namespace // - M_p pof-to-parent transform // - v_parent a point, expressed in the pof's parent - const SimTK::Transform M_pofg = pof.getTransformInGround(getState()); + const SimTK::Transform& M_pofg = pof.getTransformInGround(getState()); const SimTK::Transform M_p = pof.findTransformBetween(getState(), pof.getParentFrame()); const SimTK::Transform X = (M_pofg.invert() * M_n * M_pofg * M_p.invert()).invert(); ActionTransformPofV2( @@ -419,7 +414,7 @@ namespace // - M_w wrap object local transform const SimTK::Transform M_g = wrapObj.getFrame().getTransformInGround(getState()); - const SimTK::Transform M_w = wrapObj.getTransform(); + const SimTK::Transform& M_w = wrapObj.getTransform(); const SimTK::Transform X = M_g.invert() * M_n * M_g * M_w; ActionTransformWrapObject( getUndoableModel(), diff --git a/src/OpenSimCreator/UI/Shared/ObjectPropertiesEditor.cpp b/src/OpenSimCreator/UI/Shared/ObjectPropertiesEditor.cpp index 15ebe8c752..e43b39bdd2 100644 --- a/src/OpenSimCreator/UI/Shared/ObjectPropertiesEditor.cpp +++ b/src/OpenSimCreator/UI/Shared/ObjectPropertiesEditor.cpp @@ -1508,7 +1508,7 @@ namespace if (not prop->isObjectProperty()) { return nullptr; } - if (prop->size() <= 0) { + if (prop->empty()) { return nullptr; } if (dynamic_cast(&prop->getValueAsObject(0)) == nullptr) { diff --git a/src/oscar/Maths/ClosedInterval.h b/src/oscar/Maths/ClosedInterval.h index 19195563e2..9079512430 100644 --- a/src/oscar/Maths/ClosedInterval.h +++ b/src/oscar/Maths/ClosedInterval.h @@ -4,6 +4,7 @@ #include #include +#include namespace osc { @@ -11,6 +12,12 @@ namespace osc template requires std::equality_comparable and std::totally_ordered struct ClosedInterval final { + + constexpr ClosedInterval(T lower_, T upper_) : + lower{std::move(lower_)}, + upper{std::move(upper_)} + {} + friend bool operator==(const ClosedInterval&, const ClosedInterval&) = default; template @@ -20,7 +27,7 @@ namespace osc return upper - lower; // edge-case } - return (upper - lower) / (static_cast>(nsteps) - 1); + return (upper - lower) / static_cast((static_cast>(nsteps) - 1)); } constexpr T normalized_interpolant_at(T v) const