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

[REQUEST] Provide C++20 range API for iteration #1771

Open
BenFrantzDale opened this issue Feb 28, 2024 · 4 comments
Open

[REQUEST] Provide C++20 range API for iteration #1771

BenFrantzDale opened this issue Feb 28, 2024 · 4 comments

Comments

@BenFrantzDale
Copy link
Contributor

Is your feature request related to a problem? Please describe.

The current OpenVDB iterator API is non-standard and uses testing the truthiness of an iterator to see if it reaches the end of the range. This means that standard algorithms don't work with OpenVDB iterators.

In C++20's ranges library, they use sentinels to expend that traditional definition of iterators to cover things just like this. OpenVDB would just need to provide a sentinel type like

//! C++20-style end sentinel for an OpenVDB iterator, which is falsey at the end of its range.
template <typename VdbIter>
struct EndSentinel {
    [[nodiscard]] friend constexpr bool operator==(EndSentinel, const VdbIter& it) { return !static_cast<bool>(it); }
};

then we could use std::ranges::subrange to create a std::rangs-compatible view of things like node.beginValueOn(). That would let us do for (auto& v : node.valuesOn()) { or std::ranges::count_if(node.valuesAll(), isInteresting).

Describe the solution you'd like

  1. Provide an end-sentinel that tests the iterator for falsiness.
  2. Provide a function to turn an OpenVDB iterator into a std::ranges::subrange from the iterator to the end of range.
  3. Optionally: Provide an adaptor that views a range as its iterators rather than values, so we can do things like
auto bboxes = std::ranges::to<std::vector>(
    viewOfIterators(node.valuesAll()) 
    | std::views::transform([](const auto& it) { return it.getBoundingBox(); })
);

Possibly it makes more sense for 3 to be the default behavior, so the iterators iterate over "values" that have a .getBoundingBox() and .getValue() member function.

Describe alternatives you've considered

I've implemented this as free functions in my codebase and found it much easier to work with: Places we were doing raw loops could easily become algorithms, like

for (auto it = node.beginValueOn(); it; ++it) {
    if (pred(it)) {
        return true;
    }
}
return false;

became

return std::ranges::any_of(viewOfIterators(viewValueOn(node)), pred);

and with this suggestion, it could even be

return std::ranges::any_of(node.valuesOn(), pred);

Additional context

The iterator–sentinel pair notion isn't supported by the classic STL algorithms, so this is most useful with C++20, which is the same version that provides std::ranges::subrange.

@OpenVDB-DevTeam
Copy link
Contributor

OpenVDB-DevTeam commented Feb 29, 2024 via email

@Idclip
Copy link
Contributor

Idclip commented Feb 29, 2024

Unfortunately openvdb is stuck with C++17 (for now) because it is tied to the vfx platform

This isn't strictly true. We can support C++20 primitives as long as they are ifdef'ed. It will however complicate our CI, though we do already have some basic C++20 CI. I don't see why we couldn't support this suggestion.

@BenFrantzDale
Copy link
Contributor Author

For what it’s worth, a simple approximation of std::ranges::subrange is easy (I’m happy to provide it). That gets you to the point of using language-level range-based for loops without C++20.

@BenFrantzDale
Copy link
Contributor Author

The biggest question in my mind is if we move toward a more-standard ranges-like world, would it make sense to iterate over values or over proxy objects that provide the "extra" stuff that OpenVDB iterators provide, or iterate over a range of iterators (which is a little weird feeling, although it works and in some ways is clearer (less magical) than iterating over proxy objects).

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

No branches or pull requests

3 participants