a/util: Split history buf file up, factor out more generic iterator base

src/xrt/auxiliary/util/u_iterator_base.hpp

@@ -0,0 +1,360 @@
+// Copyright 2021-2022, Collabora, Ltd.
+//
+// SPDX-License-Identifier: BSL-1.0
+/*!
+ * @file
+ *
+ * @brief A template class to serve as the base of iterator and const_iterator
+ * types for things with "random access".
+ * @author Ryan Pavlik <ryan.pavlik@collabora.com>
+ * @ingroup aux_util
+ */
+
+#pragma once
+
+#include <stdexcept>
+#include <limits>
+#include <iterator>
+
+namespace xrt::auxiliary::util {
+/*!
+ * @brief Template for base class used by "random-access" iterators and const_iterators, providing all the functionality
+ * that is independent of element type and const-ness of the iterator.
+ *
+ * Using inheritance instead of composition here to more easily satisfy the C++ standard's requirements for
+ * iterators (e.g. that const iterators and iterators are comparable, etc.)
+ *
+ * All invalid instances will compare as equal, as required by the spec, but they are not all equivalent. You can freely
+ * go "past the end" (they will be invalid, so cannot dereference, but you can get them back to valid), but you can't go
+ * "past the beginning". That is, you can do `*(buf.end() - 1)` successfully if your buffer has at least one element,
+ * even though `buf.end()` is invalid.
+ *
+ * @tparam ContainerOrHelper Your container or some member thereof that provides a size() method. If it's a helper
+ * instead of the actual container, make it const.
+ *
+ */
+template <typename ContainerOrHelper> class RandomAccessIteratorBase
+{
+public:
+	using iterator_category = std::random_access_iterator_tag;
+	using difference_type = std::ptrdiff_t;
+
+
+	//! Is this iterator valid?
+	bool
+	valid() const noexcept;
+
+	//! Is this iterator valid?
+	explicit operator bool() const noexcept
+	{
+		return valid();
+	}
+
+	//! What is the index stored by this iterator?
+	size_t
+	index() const noexcept
+	{
+		return index_;
+	}
+
+	//! Is this iterator pointing "past the end" of the container?
+	bool
+	is_past_the_end() const noexcept
+	{
+		return container_ != nullptr && index_ >= container_->size();
+	}
+
+	/*!
+	 * @brief True if this iterator is "irrecoverably" invalid (aka, cleared/default constructed).
+	 *
+	 * Implies !valid() but is stronger. `buf.end().is_cleared()` is false.
+	 */
+	bool
+	is_cleared() const noexcept
+	{
+		return container_ == nullptr;
+	}
+
+	/*!
+	 * @brief Compute the difference between two iterators.
+	 *
+	 * - If both are cleared, the result is 0.
+	 * - Otherwise the result is the difference in index.
+	 *
+	 * @throws std::logic_error if exactly one of the iterators is cleared
+	 * @throws std::out_of_range if at least one of the iterators has an index larger than the maximum value of
+	 * std::ptrdiff_t.
+	 */
+	std::ptrdiff_t
+	operator-(const RandomAccessIteratorBase<ContainerOrHelper> &other) const;
+
+	/*!
+	 * @brief Increment by an arbitrary value.
+	 */
+	RandomAccessIteratorBase &
+	operator+=(std::ptrdiff_t n);
+
+	/*!
+	 * @brief Decrement by an arbitrary value.
+	 */
+	RandomAccessIteratorBase &
+	operator-=(std::ptrdiff_t n);
+
+	/*!
+	 * @brief Add some arbitrary amount to a copy of this iterator.
+	 */
+	RandomAccessIteratorBase
+	operator+(std::ptrdiff_t n) const;
+
+	/*!
+	 * @brief Subtract some arbitrary amount from a copy of this iterator.
+	 */
+	RandomAccessIteratorBase
+	operator-(std::ptrdiff_t n) const;
+
+	//! Factory function: construct the "begin" iterator
+	static RandomAccessIteratorBase
+	begin(ContainerOrHelper &container)
+	{
+		return RandomAccessIteratorBase(container, 0);
+	}
+
+	//! Factory function: construct the "past the end" iterator that can be decremented safely
+	static RandomAccessIteratorBase
+	end(ContainerOrHelper &container)
+	{
+		return RandomAccessIteratorBase(container, container.size());
+	}
+
+
+	/**
+	 * @brief Default constructor - initializes to "cleared" aka irrecoverably invalid
+	 */
+	RandomAccessIteratorBase() = default;
+
+	/**
+	 * @brief Constructor from a helper/container and index
+	 *
+	 * @param container The helper or container we will iterate through.
+	 * @param index An index - may be out of range.
+	 */
+	explicit RandomAccessIteratorBase(ContainerOrHelper &container, size_t index);
+
+
+	using const_iterator_base = std::conditional_t<std::is_const<ContainerOrHelper>::value,
+	                                               RandomAccessIteratorBase,
+	                                               RandomAccessIteratorBase<std::add_const_t<ContainerOrHelper>>>;
+
+	/**
+	 * @brief Get a const iterator base pointing to the same element as this element.
+	 *
+	 * @return const_iterator_base
+	 */
+	const_iterator_base
+	as_const() const
+	{
+		if (is_cleared()) {
+			return {};
+		}
+
+		return const_iterator_base{*container_, index_};
+	}
+
+protected:
+	//! for internal use
+	RandomAccessIteratorBase(ContainerOrHelper *container, size_t index) : container_(container), index_(index) {}
+
+	//! Increment an arbitrary amount
+	void
+	increment_n(std::size_t n);
+
+	//! Decrement an arbitrary amount
+	void
+	decrement_n(std::size_t n);
+
+	//! Access the container or helper
+	ContainerOrHelper *
+	container() const noexcept
+	{
+		return container_;
+	}
+
+private:
+	/**
+	 * @brief The container or helper we're associated with.
+	 *
+	 * If we were created knowing a container, this pointer is non-null.
+	 * Used to determine if an index is in bounds.
+	 * If this is null, the iterator is irrecoverably invalid.
+	 */
+	ContainerOrHelper *container_{nullptr};
+
+	//! This is the index in the container. May be out-of-range.
+	size_t index_{0};
+};
+
+/*!
+ * @brief Equality comparison operator for @ref RandomAccessIteratorBase
+ *
+ * @relates RandomAccessIteratorBase
+ */
+template <typename ContainerOrHelper>
+static inline bool
+operator==(RandomAccessIteratorBase<ContainerOrHelper> const &lhs,
+           RandomAccessIteratorBase<ContainerOrHelper> const &rhs) noexcept
+{
+	const bool lhs_valid = lhs.valid();
+	const bool rhs_valid = rhs.valid();
+	if (!lhs_valid && !rhs_valid) {
+		// all invalid iterators compare equal.
+		return true;
+	}
+	if (lhs_valid != rhs_valid) {
+		// valid is never equal to invalid.
+		return false;
+	}
+	// OK, so both are valid. Now, we look at the index
+	return lhs.index() == rhs.index();
+}
+
+/*!
+ * @overload
+ */
+template <typename ContainerOrHelper>
+static inline bool
+operator==(RandomAccessIteratorBase<const ContainerOrHelper> const &lhs,
+           RandomAccessIteratorBase<ContainerOrHelper> const &rhs) noexcept
+{
+	return lhs == rhs.as_const();
+}
+/*!
+ * @overload
+ */
+template <typename ContainerOrHelper>
+static inline bool
+operator==(RandomAccessIteratorBase<ContainerOrHelper> const &lhs,
+           RandomAccessIteratorBase<const ContainerOrHelper> const &rhs) noexcept
+{
+	return lhs.as_const() == rhs;
+}
+
+/*!
+ * @brief Inequality comparison operator for @ref RandomAccessIteratorBase
+ *
+ * @relates RandomAccessIteratorBase
+ */
+template <typename ContainerOrHelper>
+static inline bool
+operator!=(RandomAccessIteratorBase<ContainerOrHelper> const &lhs,
+           RandomAccessIteratorBase<ContainerOrHelper> const &rhs) noexcept
+{
+	return !(lhs == rhs);
+}
+
+/*!
+ * @overload
+ */
+template <typename ContainerOrHelper>
+static inline bool
+operator!=(RandomAccessIteratorBase<const ContainerOrHelper> const &lhs,
+           RandomAccessIteratorBase<ContainerOrHelper> const &rhs) noexcept
+{
+	return !(lhs == rhs.as_const());
+}
+
+/*!
+ * @overload
+ */
+template <typename ContainerOrHelper>
+static inline bool
+operator!=(RandomAccessIteratorBase<ContainerOrHelper> const &lhs,
+           RandomAccessIteratorBase<const ContainerOrHelper> const &rhs) noexcept
+{
+	return !(lhs.as_const() == rhs);
+}
+
+template <typename ContainerOrHelper>
+inline bool
+RandomAccessIteratorBase<ContainerOrHelper>::valid() const noexcept
+{
+	return container_ != nullptr && index_ < container_->size();
+}
+
+template <typename ContainerOrHelper>
+inline RandomAccessIteratorBase<ContainerOrHelper> &
+RandomAccessIteratorBase<ContainerOrHelper>::operator+=(std::ptrdiff_t n)
+{
+	if (n < 0) {
+		decrement_n(static_cast<size_t>(-1 * n));
+	} else {
+		increment_n(static_cast<size_t>(n));
+	}
+	return *this;
+}
+
+template <typename ContainerOrHelper>
+inline RandomAccessIteratorBase<ContainerOrHelper> &
+RandomAccessIteratorBase<ContainerOrHelper>::operator-=(std::ptrdiff_t n)
+{
+	if (n < 0) {
+		increment_n(static_cast<size_t>(-1 * n));
+	} else {
+		decrement_n(static_cast<size_t>(n));
+	}
+	return *this;
+}
+
+template <typename ContainerOrHelper>
+inline void
+RandomAccessIteratorBase<ContainerOrHelper>::increment_n(std::size_t n)
+{
+	// being cleared is permanent
+	if (is_cleared())
+		return;
+	index_ += n;
+}
+
+template <typename ContainerOrHelper>
+inline void
+RandomAccessIteratorBase<ContainerOrHelper>::decrement_n(std::size_t n)
+{
+	// being cleared is permanent
+	if (is_cleared())
+		return;
+	if (n > index_) {
+		// would move backward past the beginning which you can't recover from. So, clear it.
+		*this = {};
+		return;
+	}
+	index_ -= n;
+}
+
+template <typename ContainerOrHelper>
+inline std::ptrdiff_t
+RandomAccessIteratorBase<ContainerOrHelper>::operator-(const RandomAccessIteratorBase<ContainerOrHelper> &other) const
+{
+	const bool self_cleared = is_cleared();
+	const bool other_cleared = other.is_cleared();
+	if (self_cleared && other_cleared) {
+		// If both cleared, they're at the same place.
+		return 0;
+	}
+	if (self_cleared || other_cleared) {
+		// If only one is cleared, we can't do this.
+		throw std::logic_error(
+		    "Tried to find the difference between a cleared iterator and a non-cleared iterator.");
+	}
+	constexpr size_t max_ptrdiff = static_cast<size_t>(std::numeric_limits<std::ptrdiff_t>::max());
+	if (index_ > max_ptrdiff || other.index_ > max_ptrdiff) {
+		throw std::out_of_range("An index exceeded the maximum value of the signed version of the index type.");
+	}
+	// Otherwise subtract the index values.
+	return static_cast<std::ptrdiff_t>(index_) - static_cast<std::ptrdiff_t>(other.index_);
+}
+
+template <typename ContainerOrHelper>
+inline RandomAccessIteratorBase<ContainerOrHelper>::RandomAccessIteratorBase(ContainerOrHelper &container, size_t index)
+    : container_(&container), index_(index)
+{}
+
+} // namespace xrt::auxiliary::util

src/xrt/auxiliary/util/u_template_historybuf_const_iterator.inl

@@ -0,0 +1,257 @@
+// Copyright 2021-2022, Collabora, Ltd.
+//
+// SPDX-License-Identifier: BSL-1.0
+/*!
+ * @file
+ * @brief const_iterator details for ring buffer
+ * @author Ryan Pavlik <ryan.pavlik@collabora.com>
+ * @ingroup aux_util
+ */
+
+#include <stddef.h>
+#include <type_traits>
+
+namespace xrt::auxiliary::util {
+template <typename T, size_t MaxSize> class HistoryBuffer;
+
+namespace detail {
+	/**
+	 * @brief Class template for const_iterator for HistoryBuffer
+	 *
+	 * @tparam T Container element type - must match HistoryBuffer
+	 * @tparam MaxSize Maximum number of elements - must match HistoryBuffer
+	 */
+	template <typename T, size_t MaxSize>
+	class HistoryBufConstIterator : public RandomAccessIteratorBase<const RingBufferHelper<MaxSize>>
+	{
+		using base = RandomAccessIteratorBase<const RingBufferHelper<MaxSize>>;
+		friend class HistoryBuffer<T, MaxSize>;
+		friend class HistoryBufIterator<T, MaxSize>;
+
+	public:
+		using container_type = const HistoryBuffer<T, MaxSize>;
+		using typename base::difference_type;
+		using typename base::iterator_category;
+		using value_type = const T;
+		using pointer = const T *;
+		using reference = const T &;
+
+		//! Default-construct an (invalid) iterator.
+		HistoryBufConstIterator() = default;
+
+		// copy and move as you wish
+		HistoryBufConstIterator(HistoryBufConstIterator const &) = default;
+		HistoryBufConstIterator(HistoryBufConstIterator &&) = default;
+		HistoryBufConstIterator &
+		operator=(HistoryBufConstIterator const &) = default;
+		HistoryBufConstIterator &
+		operator=(HistoryBufConstIterator &&) = default;
+
+		//! Implicit conversion from a non-const iterator
+		HistoryBufConstIterator(const HistoryBufIterator<T, MaxSize> &other);
+
+		//! Is this iterator valid?
+		bool
+		valid() const noexcept
+		{
+			return container_ != nullptr && base::valid();
+		}
+
+		//! Is this iterator valid?
+		explicit operator bool() const noexcept
+		{
+			return valid();
+		}
+
+		//! Access the container: for internal use
+		container_type *
+		container() const noexcept
+		{
+			return container_;
+		}
+
+		//! Dereference operator: throws std::out_of_range if invalid
+		reference
+		operator*() const;
+
+		//! Smart pointer operator: returns nullptr if invalid
+		pointer
+		operator->() const noexcept;
+
+		//! Pre-increment: Advance, then return self.
+		HistoryBufConstIterator &
+		operator++();
+
+		//! Post-increment: return a copy of initial state after incrementing self
+		HistoryBufConstIterator
+		operator++(int);
+
+		//! Pre-decrement: Subtract, then return self.
+		HistoryBufConstIterator &
+		operator--();
+
+		//! Post-decrement: return a copy of initial state after decrementing self
+		HistoryBufConstIterator
+		operator--(int);
+
+		// Use the base class implementation of subtracting one iterator from another
+		using base::operator-;
+
+		//! Increment by an arbitrary amount.
+		HistoryBufConstIterator &
+		operator+=(std::ptrdiff_t n) noexcept;
+
+		//! Decrement by an arbitrary amount.
+		HistoryBufConstIterator &
+		operator-=(std::ptrdiff_t n) noexcept;
+
+		//! Increment a copy of the iterator by an arbitrary amount.
+		HistoryBufConstIterator
+		operator+(std::ptrdiff_t n) const noexcept;
+
+		//! Decrement a copy of the iterator by an arbitrary amount.
+		HistoryBufConstIterator
+		operator-(std::ptrdiff_t n) const noexcept;
+
+	private:
+		//! Factory for a "begin" iterator from a container and its helper: mostly for internal use.
+		static HistoryBufConstIterator
+		begin(container_type &container, const RingBufferHelper<MaxSize> &helper)
+		{
+			return {&container, std::move(base::begin(helper))};
+		}
+
+		//! Construct the "past the end" iterator that can be decremented safely
+		static HistoryBufConstIterator
+		end(container_type &container, const RingBufferHelper<MaxSize> &helper)
+		{
+			return {&container, std::move(base::end(helper))};
+		}
+
+		// for use internally
+		HistoryBufConstIterator(container_type *container, base &&iter_base)
+		    : base(std::move(iter_base)), container_(container)
+		{}
+		container_type *container_{nullptr};
+	};
+
+	template <typename T, size_t MaxSize>
+	inline typename HistoryBufConstIterator<T, MaxSize>::reference
+	HistoryBufConstIterator<T, MaxSize>::operator*() const
+	{
+		auto ptr = container_->get_at_index(base::index());
+		if (ptr == nullptr) {
+			throw std::out_of_range("Iterator index out of range");
+		}
+		return *ptr;
+	}
+
+	template <typename T, size_t MaxSize>
+	inline typename HistoryBufConstIterator<T, MaxSize>::pointer
+	HistoryBufConstIterator<T, MaxSize>::operator->() const noexcept
+	{
+		return container_->get_at_index(base::index());
+	}
+
+	template <typename T, size_t MaxSize>
+	inline HistoryBufConstIterator<T, MaxSize> &
+	HistoryBufConstIterator<T, MaxSize>::operator++()
+	{
+		this->increment_n(1);
+		return *this;
+	}
+
+	template <typename T, size_t MaxSize>
+	inline HistoryBufConstIterator<T, MaxSize>
+	HistoryBufConstIterator<T, MaxSize>::operator++(int)
+	{
+		HistoryBufConstIterator tmp = *this;
+		this->increment_n(1);
+		return tmp;
+	}
+
+	template <typename T, size_t MaxSize>
+	inline HistoryBufConstIterator<T, MaxSize> &
+	HistoryBufConstIterator<T, MaxSize>::operator--()
+	{
+		this->decrement_n(1);
+		return *this;
+	}
+
+	template <typename T, size_t MaxSize>
+	inline HistoryBufConstIterator<T, MaxSize>
+	HistoryBufConstIterator<T, MaxSize>::operator--(int)
+	{
+		HistoryBufConstIterator tmp = *this;
+		this->decrement_n(1);
+		return tmp;
+	}
+
+	template <typename T, size_t MaxSize>
+	inline HistoryBufConstIterator<T, MaxSize> &
+	HistoryBufConstIterator<T, MaxSize>::operator+=(std::ptrdiff_t n) noexcept
+	{
+		static_cast<base &>(*this) += n;
+		return *this;
+	}
+
+	template <typename T, size_t MaxSize>
+	inline HistoryBufConstIterator<T, MaxSize> &
+	HistoryBufConstIterator<T, MaxSize>::operator-=(std::ptrdiff_t n) noexcept
+	{
+		static_cast<base &>(*this) -= n;
+		return *this;
+	}
+
+	template <typename T, size_t MaxSize>
+	inline HistoryBufConstIterator<T, MaxSize>
+	HistoryBufConstIterator<T, MaxSize>::operator+(std::ptrdiff_t n) const noexcept
+	{
+		HistoryBufConstIterator ret(*this);
+		ret += n;
+		return ret;
+	}
+
+	template <typename T, size_t MaxSize>
+	inline HistoryBufConstIterator<T, MaxSize>
+	HistoryBufConstIterator<T, MaxSize>::operator-(std::ptrdiff_t n) const noexcept
+	{
+		HistoryBufConstIterator ret(*this);
+		ret -= n;
+		return ret;
+	}
+} // namespace detail
+
+
+template <typename T, size_t MaxSize>
+inline typename HistoryBuffer<T, MaxSize>::const_iterator
+HistoryBuffer<T, MaxSize>::cbegin() const noexcept
+{
+	static_assert(std::is_same<typename std::iterator_traits<const_iterator>::iterator_category,
+	                           std::random_access_iterator_tag>::value,
+	              "Iterator should be random access");
+	return const_iterator::begin(*this, helper_);
+}
+
+template <typename T, size_t MaxSize>
+inline typename HistoryBuffer<T, MaxSize>::const_iterator
+HistoryBuffer<T, MaxSize>::cend() const noexcept
+{
+	return const_iterator::end(*this, helper_);
+}
+
+template <typename T, size_t MaxSize>
+inline typename HistoryBuffer<T, MaxSize>::const_iterator
+HistoryBuffer<T, MaxSize>::begin() const noexcept
+{
+	return cbegin();
+}
+
+template <typename T, size_t MaxSize>
+inline typename HistoryBuffer<T, MaxSize>::const_iterator
+HistoryBuffer<T, MaxSize>::end() const noexcept
+{
+	return cend();
+}
+
+} // namespace xrt::auxiliary::util

src/xrt/auxiliary/util/u_template_historybuf_impl_helpers.hpp

@@ -0,0 +1,195 @@
+// Copyright 2021-2022, Collabora, Ltd.
+//
+// SPDX-License-Identifier: BSL-1.0
+/*!
+ * @file
+ * @brief All the "element-type-independent" code (helper objects, base classes) for a ringbuffer implementation on top
+ * of a fixed size array
+ * @author Ryan Pavlik <ryan.pavlik@collabora.com>
+ * @author Moses Turner <moses@collabora.com>
+ * @ingroup aux_util
+ */
+
+#pragma once
+
+#include <algorithm>
+#include <assert.h>
+#include <stdlib.h>
+
+
+//|  -4   |   -3   |  -2 | -1 | Top | Garbage |
+// OR
+//|  -4   |   -3   |  -2 | -1 | Top | -7 | -6 | -5 |
+
+namespace xrt::auxiliary::util::detail {
+/**
+ * @brief All the bookkeeping for adapting a fixed-size array to a ring buffer.
+ *
+ * This is all the guts of the ring buffer except for the actual buffer.
+ * We split it out to
+ * - reduce code size (this can be shared among multiple types)
+ * - separate concerns (keeping track of the indices separate from owning the buffer)
+ * - allow easier implementation of both const iterators and non-const iterators
+ *
+ * There are a few types of "index":
+ *
+ * - just "index": an index where the least-recently-added element still remaining is numbered 0, the next
+ *   oldest is 1, etc. (Chronological)
+ * - "age": Reverse chronological order: 0 means most-recently-added, 1 means the one before it, etc.
+ * - "inner" index: the index in the underlying array/buffer. It's called "inner" because the consumer of the
+ *   ring buffer should not ever deal with this index, it's an implementation detail.
+ */
+template <size_t MaxSize> class RingBufferHelper
+{
+public:
+	//! Get the inner index for a given age (if possible)
+	bool
+	age_to_inner_index(size_t age, size_t &out_inner_idx) const noexcept;
+
+	//! Get the inner index for a given index (if possible)
+	bool
+	index_to_inner_index(size_t index, size_t &out_inner_idx) const noexcept;
+
+	//! Is the buffer empty?
+	bool
+	empty() const noexcept
+	{
+		return length_ == 0;
+	}
+
+	//! How many elements are in the buffer?
+	size_t
+	size() const noexcept
+	{
+		return length_;
+	}
+
+	/*!
+	 * @brief Update internal state for pushing an element to the back, and return the inner index to store
+	 * the element at.
+	 *
+	 * This is the implementation of "push_back" excluding all the messy "actually dealing with the data"
+	 * part ;-)
+	 */
+	size_t
+	push_back_location() noexcept;
+
+	/*!
+	 * @brief Record the logical removal of the front element, if any.
+	 *
+	 * Does nothing if the buffer is empty. Does not actually modify the value stored in the backing array.
+	 */
+	void
+	pop_front() noexcept;
+
+	//! Get the inner index of the front (oldest) value, or MaxSize if empty.
+	size_t
+	front_inner_index() const noexcept;
+
+	//! Get the inner index of the back (newest) value, or MaxSize if empty.
+	size_t
+	back_inner_index() const noexcept;
+
+private:
+	//! The inner index containing the most recently added element, if any
+	size_t latest_inner_idx_ = 0;
+
+	//! The number of elements populated.
+	size_t length_ = 0;
+
+	/**
+	 * @brief Get the inner index of the front (oldest) value: assumes not empty!
+	 *
+	 * For internal use in this class only.
+	 *
+	 * @see front_inner_index() for the "safe" equivalent (that wraps this with error handling)
+	 */
+	size_t
+	front_impl_() const noexcept;
+};
+
+
+template <size_t MaxSize>
+inline size_t
+RingBufferHelper<MaxSize>::front_impl_() const noexcept
+{
+	assert(!empty());
+	// length will not exceed MaxSize, so this will not underflow
+	return (latest_inner_idx_ + MaxSize - length_ + 1) % MaxSize;
+}
+
+template <size_t MaxSize>
+inline bool
+RingBufferHelper<MaxSize>::age_to_inner_index(size_t age, size_t &out_inner_idx) const noexcept
+{
+
+	if (empty()) {
+		return false;
+	}
+	if (age >= length_) {
+		return false;
+	}
+	// latest_inner_idx_ is the same as (latest_inner_idx_ + MaxSize) % MaxSize so we add MaxSize to
+	// prevent underflow with unsigned values
+	out_inner_idx = (latest_inner_idx_ + MaxSize - age) % MaxSize;
+	return true;
+}
+
+template <size_t MaxSize>
+inline bool
+RingBufferHelper<MaxSize>::index_to_inner_index(size_t index, size_t &out_inner_idx) const noexcept
+{
+
+	if (empty()) {
+		return false;
+	}
+	if (index >= length_) {
+		return false;
+	}
+	// Just add to the front (oldest) index and take modulo MaxSize
+	out_inner_idx = (front_impl_() + index) % MaxSize;
+	return true;
+}
+
+template <size_t MaxSize>
+inline size_t
+RingBufferHelper<MaxSize>::push_back_location() noexcept
+{
+	// We always increment the latest inner index modulo MaxSize
+	latest_inner_idx_ = (latest_inner_idx_ + 1) % MaxSize;
+	// Length cannot exceed MaxSize. If it already was MaxSize, that just means we're overwriting something at
+	// latest_inner_idx_
+	length_ = std::min(length_ + 1, MaxSize);
+	return latest_inner_idx_;
+}
+
+template <size_t MaxSize>
+inline void
+RingBufferHelper<MaxSize>::pop_front() noexcept
+{
+	if (!empty()) {
+		length_--;
+	}
+}
+
+template <size_t MaxSize>
+inline size_t
+RingBufferHelper<MaxSize>::front_inner_index() const noexcept
+{
+	if (empty()) {
+		return MaxSize;
+	}
+	return front_impl_();
+}
+
+template <size_t MaxSize>
+inline size_t
+RingBufferHelper<MaxSize>::back_inner_index() const noexcept
+{
+	if (empty()) {
+		return MaxSize;
+	}
+	return latest_inner_idx_;
+}
+
+} // namespace xrt::auxiliary::util::detail

src/xrt/auxiliary/util/u_template_historybuf_iterator.inl

@@ -0,0 +1,245 @@
+// Copyright 2021-2022, Collabora, Ltd.
+//
+// SPDX-License-Identifier: BSL-1.0
+/*!
+ * @file
+ * @brief iterator details for ring buffer
+ * @author Ryan Pavlik <ryan.pavlik@collabora.com>
+ * @ingroup aux_util
+ */
+
+#include <stddef.h>
+#include <type_traits>
+
+namespace xrt::auxiliary::util {
+template <typename T, size_t MaxSize> class HistoryBuffer;
+
+namespace detail {
+	/**
+	 * @brief Class template for iterator for HistoryBuffer
+	 *
+	 * @tparam T Container element type - must match HistoryBuffer
+	 * @tparam MaxSize Maximum number of elements - must match HistoryBuffer
+	 */
+	template <typename T, size_t MaxSize>
+	class HistoryBufIterator : public RandomAccessIteratorBase<const RingBufferHelper<MaxSize>>
+	{
+		using base = RandomAccessIteratorBase<const RingBufferHelper<MaxSize>>;
+		friend class HistoryBuffer<T, MaxSize>;
+
+	public:
+		using container_type = HistoryBuffer<T, MaxSize>;
+		using typename base::difference_type;
+		using typename base::iterator_category;
+		using value_type = T;
+		using pointer = T *;
+		using reference = T &;
+
+		//! Default-construct an (invalid) iterator.
+		HistoryBufIterator() = default;
+
+		// copy and move as you wish
+		HistoryBufIterator(HistoryBufIterator const &) = default;
+		HistoryBufIterator(HistoryBufIterator &&) = default;
+		HistoryBufIterator &
+		operator=(HistoryBufIterator const &) = default;
+		HistoryBufIterator &
+		operator=(HistoryBufIterator &&) = default;
+
+		//! Is this iterator valid?
+		bool
+		valid() const noexcept
+		{
+			return container_ != nullptr && base::valid();
+		}
+
+		//! Is this iterator valid?
+		explicit operator bool() const noexcept
+		{
+			return valid();
+		}
+
+		//! Access the container: for internal use
+		container_type *
+		container() const noexcept
+		{
+			return container_;
+		}
+
+		//! Dereference operator: throws std::out_of_range if invalid
+		reference
+		operator*() const;
+
+		//! Smart pointer operator: returns nullptr if invalid
+		pointer
+		operator->() const noexcept;
+
+		//! Pre-increment: Advance, then return self.
+		HistoryBufIterator &
+		operator++();
+
+		//! Post-increment: return a copy of initial state after incrementing self
+		HistoryBufIterator
+		operator++(int);
+
+		//! Pre-decrement: Subtract, then return self.
+		HistoryBufIterator &
+		operator--();
+
+		//! Post-decrement: return a copy of initial state after decrementing self
+		HistoryBufIterator
+		operator--(int);
+
+		// Use the base class implementation of subtracting one iterator from another
+		using base::operator-;
+
+		//! Increment by an arbitrary amount.
+		HistoryBufIterator &
+		operator+=(std::ptrdiff_t n) noexcept;
+
+		//! Decrement by an arbitrary amount.
+		HistoryBufIterator &
+		operator-=(std::ptrdiff_t n) noexcept;
+
+		//! Increment a copy of the iterator by an arbitrary amount.
+		HistoryBufIterator
+		operator+(std::ptrdiff_t n) const noexcept;
+
+		//! Decrement a copy of the iterator by an arbitrary amount.
+		HistoryBufIterator
+		operator-(std::ptrdiff_t n) const noexcept;
+
+	private:
+		//! Factory for a "begin" iterator from a container and its helper: mostly for internal use.
+		static HistoryBufIterator
+		begin(container_type &container, const RingBufferHelper<MaxSize> &helper)
+		{
+			return HistoryBufIterator{&container, std::move(base::begin(helper))};
+		}
+
+		//! Construct the "past the end" iterator that can be decremented safely
+		static HistoryBufIterator
+		end(container_type &container, const RingBufferHelper<MaxSize> &helper)
+		{
+			return HistoryBufIterator{&container, std::move(base::end(helper))};
+		}
+
+		// for use internally
+		HistoryBufIterator(container_type *container, base &&iter_base)
+		    : base(std::move(iter_base)), container_(container)
+		{}
+		container_type *container_{nullptr};
+	};
+
+	template <typename T, size_t MaxSize>
+	inline typename HistoryBufIterator<T, MaxSize>::reference
+	HistoryBufIterator<T, MaxSize>::operator*() const
+	{
+		auto ptr = container_->get_at_index(base::index());
+		if (ptr == nullptr) {
+			throw std::out_of_range("Iterator index out of range");
+		}
+		return *ptr;
+	}
+
+	template <typename T, size_t MaxSize>
+	inline typename HistoryBufIterator<T, MaxSize>::pointer
+	HistoryBufIterator<T, MaxSize>::operator->() const noexcept
+	{
+		return container_->get_at_index(base::index());
+	}
+
+	template <typename T, size_t MaxSize>
+	inline HistoryBufIterator<T, MaxSize> &
+	HistoryBufIterator<T, MaxSize>::operator++()
+	{
+		this->increment_n(1);
+		return *this;
+	}
+
+	template <typename T, size_t MaxSize>
+	inline HistoryBufIterator<T, MaxSize>
+	HistoryBufIterator<T, MaxSize>::operator++(int)
+	{
+		HistoryBufIterator tmp = *this;
+		this->increment_n(1);
+		return tmp;
+	}
+
+	template <typename T, size_t MaxSize>
+	inline HistoryBufIterator<T, MaxSize> &
+	HistoryBufIterator<T, MaxSize>::operator--()
+	{
+		this->decrement_n(1);
+		return *this;
+	}
+
+	template <typename T, size_t MaxSize>
+	inline HistoryBufIterator<T, MaxSize>
+	HistoryBufIterator<T, MaxSize>::operator--(int)
+	{
+		HistoryBufIterator tmp = *this;
+		this->decrement_n(1);
+		return tmp;
+	}
+
+	template <typename T, size_t MaxSize>
+	inline HistoryBufIterator<T, MaxSize> &
+	HistoryBufIterator<T, MaxSize>::operator+=(std::ptrdiff_t n) noexcept
+	{
+		static_cast<base &>(*this) += n;
+		return *this;
+	}
+
+	template <typename T, size_t MaxSize>
+	inline HistoryBufIterator<T, MaxSize> &
+	HistoryBufIterator<T, MaxSize>::operator-=(std::ptrdiff_t n) noexcept
+	{
+		static_cast<base &>(*this) -= n;
+		return *this;
+	}
+
+	template <typename T, size_t MaxSize>
+	inline HistoryBufIterator<T, MaxSize>
+	HistoryBufIterator<T, MaxSize>::operator+(std::ptrdiff_t n) const noexcept
+	{
+		HistoryBufIterator ret(*this);
+		ret += n;
+		return ret;
+	}
+
+	template <typename T, size_t MaxSize>
+	inline HistoryBufIterator<T, MaxSize>
+	HistoryBufIterator<T, MaxSize>::operator-(std::ptrdiff_t n) const noexcept
+	{
+		HistoryBufIterator ret(*this);
+		ret -= n;
+		return ret;
+	}
+
+	// Conversion constructor for const iterator
+	template <typename T, size_t MaxSize>
+	inline HistoryBufConstIterator<T, MaxSize>::HistoryBufConstIterator(const HistoryBufIterator<T, MaxSize> &other)
+	    : HistoryBufConstIterator(other.container(), base{other})
+	{}
+} // namespace detail
+
+
+template <typename T, size_t MaxSize>
+inline typename HistoryBuffer<T, MaxSize>::iterator
+HistoryBuffer<T, MaxSize>::begin() noexcept
+{
+	static_assert(std::is_same<typename std::iterator_traits<iterator>::iterator_category,
+	                           std::random_access_iterator_tag>::value,
+	              "Iterator should be random access");
+	return iterator::begin(*this, helper_);
+}
+
+template <typename T, size_t MaxSize>
+inline typename HistoryBuffer<T, MaxSize>::iterator
+HistoryBuffer<T, MaxSize>::end() noexcept
+{
+	return iterator::end(*this, helper_);
+}
+
+} // namespace xrt::auxiliary::util

tests/tests_history_buf.cpp

@@ -11,9 +11,9 @@
 #include <util/u_template_historybuf.hpp>
 using xrt::auxiliary::util::HistoryBuffer;
 
-template <size_t MaxSize>
+template <typename Container>
 static inline std::ostream &
-operator<<(std::ostream &os, const xrt::auxiliary::util::detail::RingBufferIteratorBase<MaxSize> &iter_base)
+operator<<(std::ostream &os, const xrt::auxiliary::util::RandomAccessIteratorBase<Container> &iter_base)
 {
 	os << "Iterator@[" << iter_base.index() << "]";
 	return os;
@@ -349,10 +349,10 @@ TEST_CASE("IteratorBase")
 	buffer.push_back(2);
 	buffer.push_back(4);
 	using namespace xrt::auxiliary::util;
-	using iterator = typename HistoryBuffer<int, 4>::iterator;
-	iterator default_constructed{};
-	iterator begin_constructed = buffer.begin();
-	iterator end_constructed = buffer.end();
+	using const_iterator = typename HistoryBuffer<int, 4>::const_iterator;
+	const_iterator default_constructed{};
+	const_iterator begin_constructed = buffer.begin();
+	const_iterator end_constructed = buffer.end();
 
 	SECTION("Check default constructed")
 	{
@@ -369,6 +369,7 @@ TEST_CASE("IteratorBase")
 	{
 		CHECK_FALSE(end_constructed.valid());
 		CHECK_FALSE(end_constructed.is_cleared());
-		CHECK((++end_constructed).is_cleared());
+		// if we go past the end, we can go backwards into validity.
+		CHECK_FALSE((++end_constructed).is_cleared());
 	}
 }