Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for std::span (C++20) #1327

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

AbitTheGray
Copy link

Main goal of this Pull Request is to add support for converting glm::vec, glm::mat and glm::quat to <span>'s std::span but there are few more changes. The goal is to have a a single non-owning object to access underlying data (similar to std::string_view).

Changes

  • New Experimental glm/gtx/span.hpp
    • std::span for C++20 (tested with __cpp_lib_span)
    • std::mdspan for C++23 (tested with __cpp_lib_mdspan) - untested as GCC does not support it right now (see C++23 support list)
    • std::valarray to have something for older C++ versions, but it copies the underlying data
    • Tests (test/gtx/gtx_span.cpp) inspired by test/gtx/gtx_range.cpp
  • Update glm/gtx/range.hpp to use glm::type from glm/gtx/type_trait.hpp
    • Mark functions as constexpr and [[nodiscard]]
    • Use glm::type to get number of elements instead if calculating it just for ranges
    • Add support for quaternions (they may have worked before using the matrix fallback but it would be incorrect)
    • Expand tests (test/gtx/gtx_range.cpp) to include more types
  • Make glm::type from glm/gtx/type_trait.hpp easier to use
    • All specializations have all fields, including new elements (calculated as cols * rows for convenience)
      • glm::type<glm::vec<...>> did not have cols and rows
      • glm::type<glm::mat<...>> still has componenets=cols, which is the reason for elements
        • Watch out for glm::components<T>(T const&) from glm/gtx/range.hpp as it returns glm::type<T>::elements (which is previous behaviour)
    • Checks for const, volatile and & in type provided to glm::type<...> as those would use the fallback zeroes
      • glm::type<glm::vec3> returns correct values but glm::type<const glm::vec3> does not
        • Use can get the const there by using decltype like const glm::vec3 value{}; const auto components = glm::type<decltype(value)>::components; assert(components == 3);
  • CMake option GLM_ENABLE_CXX_23
    • Defines GLM_FORCE_CXX23 but otherwise works like GLM_ENABLE_CXX_20
    • Also added GLM_LANG_CXX23_FLAG and GLM_LANG_CXX23 in glm/detail/setup.hpp

I was also thinking about adding concepts but it would not work for older C++ versions. It may be something for future Pull Request as you can use it like void someFunc(glm::concept::vec auto v) where glm::vec3 and glm::i32vec3 would work but not glm::vec2.

Missing or problems

  • std::span is used as std::span<T> (with dynamic extent) while it may be std::span<T,N>
    • This is something I forgot about but is a good idea to add before merging this as it would allow libraries to have std::span<float, 3> as an argument and pass glm::vec3 but not glm::vec2
  • Untested with compilers other than GCC 14
    • I've just tested it with different GLM_ENABLE_CXX_?? values so hopefully C++ versions are covered
    • Did not try to run MSVC or CLang
  • There are still TODOs in std::span test (test/gtx/gtx_span.cpp) - there are no tests for std::valarray and std::mdspan
    • I will add those based on feedback on this MR (just did not want to waste more time in case there is a problem with my proposal to add the std::span support)
  • glm::type should probably handle the const, volatile and references correctly without the assertion, but it does not do it now so I feel like it should at least tell you
    • It could be hidden behind #ifndef in case someone depends on the unintuitive behavior
      • Right now it has only C++11 check (as static_assert and other used features did not exist before)

DISCLAIMER

This is my first Pull Request to project this big and I am proposing new functionality that, as far as I know, was not requested. There were some guesses from me about how some thing should be done.
I've tried to make it compatible for any compiler and C++ version but I have little experience there as well.

I hope there is no problem with me also changing the glm/gtx/range.hpp header (and its test) as I feel it is easier to read now.

No, this is not a "school project". It is something I found is missing in glm and took it as a good reason to learn more about glm's internals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant