Define a column major iterator for code requiring that iteration order.

GCC and specifically GCC6 really don't like inheriting common code, requires redefining almost everything.

Source/engine/points_in_rectangle_range.hpp

@@ -5,35 +5,74 @@
 #include "point.hpp"
 #include "rectangle.hpp"
 
-namespace devilution {
+namespace devilution {
 
-class PointsInRectangleRange {
+class PointsInRectangleIteratorBase {
 public:
-	using const_iterator = class PointsInRectangleIterator {
-	public:
-		using iterator_category = std::random_access_iterator_tag;
-		using difference_type = int;
-		using value_type = Point;
-		using pointer = void;
-		using reference = value_type;
+	using iterator_category = std::random_access_iterator_tag;
+	using difference_type = int;
+	using value_type = Point;
+	using pointer = void;
+	using reference = value_type;
 
-		PointsInRectangleIterator() = default;
+protected:
+	PointsInRectangleIteratorBase(Point origin, int majorDimension, int majorIndex, int minorIndex)
+	    : origin(origin)
+	    , majorDimension(majorDimension)
+	    , majorIndex(majorIndex)
+	    , minorIndex(minorIndex)
+	{
+	}
 
-		PointsInRectangleIterator(Point origin, int majorDimension, int majorIndex, int minorIndex)
-		    : origin(origin)
-		    , majorDimension(majorDimension)
-		    , majorIndex(majorIndex)
-		    , minorIndex(minorIndex)
-		{
+	explicit PointsInRectangleIteratorBase(Point origin, int majorDimension, int index = 0)
+	    : PointsInRectangleIteratorBase(origin, majorDimension, index / majorDimension, index % majorDimension)
+	{
+	}
+
+	void Increment()
+	{
+		++minorIndex;
+		if (minorIndex >= majorDimension) {
+			++majorIndex;
+			minorIndex -= majorDimension;
 		}
+	}
 
-		explicit PointsInRectangleIterator(Point origin, int majorDimension, int index = 0)
-		    : PointsInRectangleIterator(origin, majorDimension, index / majorDimension, index % majorDimension)
-		{
+	void Decrement()
+	{
+		if (minorIndex <= 0) {
+			--majorIndex;
+			minorIndex += majorDimension;
+		}
+		--minorIndex;
+	}
+
+	void Offset(difference_type delta)
+	{
+		majorIndex += (minorIndex + delta) / majorDimension;
+		minorIndex = (minorIndex + delta) % majorDimension;
+		if (minorIndex < 0) {
+			minorIndex += majorDimension;
+			--majorIndex;
 		}
+	}
+
+	Point origin;
+
+	int majorDimension;
+
+	int majorIndex;
+	int minorIndex;
+};
+
+class PointsInRectangleRange {
+public:
+	using const_iterator = class PointsInRectangleIterator : public PointsInRectangleIteratorBase {
+	public:
+		PointsInRectangleIterator() = default;
 
 		PointsInRectangleIterator(Rectangle region, int index = 0)
-		    : PointsInRectangleIterator(region.position, region.size.width, index)
+		    : PointsInRectangleIteratorBase(region.position, region.size.width, index)
 		{
 		}
 
@@ -141,45 +180,178 @@ public:
 		{
 			return **(this + offset);
 		}
+	};
+
+	PointsInRectangleRange(Rectangle region)
+	    : region(region)
+	{
+	}
+
+	[[nodiscard]] const_iterator cbegin() const
+	{
+		return region;
+	}
+
+	[[nodiscard]] const_iterator begin() const
+	{
+		return cbegin();
+	}
+
+	[[nodiscard]] const_iterator cend() const
+	{
+		return { region, region.size.width * region.size.height };
+	}
+
+	[[nodiscard]] const_iterator end() const
+	{
+		return cend();
+	}
+
+	[[nodiscard]] auto crbegin() const
+	{
+		// explicit type needed for older GCC versions
+		return std::reverse_iterator<const_iterator>(cend());
+	}
+
+	[[nodiscard]] auto rbegin() const
+	{
+		return crbegin();
+	}
+
+	[[nodiscard]] auto crend() const
+	{
+		// explicit type needed for older GCC versions
+		return std::reverse_iterator<const_iterator>(cbegin());
+	}
+
+	[[nodiscard]] auto rend() const
+	{
+		return crend();
+	}
+
+protected:
+	Rectangle region;
+};
+
+class PointsInRectangleRangeColMajor {
+public:
+	using const_iterator = class PointsInRectangleIteratorColMajor : public PointsInRectangleIteratorBase {
+	public:
+		PointsInRectangleIteratorColMajor() = default;
+
+		PointsInRectangleIteratorColMajor(Rectangle region, int index = 0)
+		    : PointsInRectangleIteratorBase(region.position, region.size.height, index)
+		{
+		}
 
-	protected:
-		void Increment()
+		value_type operator*() const
 		{
-			++minorIndex;
-			if (minorIndex >= majorDimension) {
-				++majorIndex;
-				minorIndex -= majorDimension;
-			}
+			// Col-major iteration e.g. {0, 0}, {0, 1}, {0, 2}, {1, 0}, {1, 1}, ...
+			return origin + Displacement { majorIndex, minorIndex };
 		}
 
-		void Decrement()
+		// Equality comparable concepts
+		bool operator==(const PointsInRectangleIteratorColMajor &rhs) const
 		{
-			if (minorIndex <= 0) {
-				--majorIndex;
-				minorIndex += majorDimension;
-			}
-			--minorIndex;
+			return this->majorIndex == rhs.majorIndex && this->minorIndex == rhs.minorIndex;
 		}
 
-		void Offset(difference_type delta)
+		bool operator!=(const PointsInRectangleIteratorColMajor &rhs) const
 		{
-			majorIndex += (minorIndex + delta) / majorDimension;
-			minorIndex = (minorIndex + delta) % majorDimension;
-			if (minorIndex < 0) {
-				minorIndex += majorDimension;
-				--majorIndex;
-			}
+			return !(*this == rhs);
 		}
 
-		Point origin;
+		// Partially ordered concepts
+		bool operator>=(const PointsInRectangleIteratorColMajor &rhs) const
+		{
+			return this->majorIndex > rhs.majorIndex || (this->majorIndex == rhs.majorIndex && this->minorIndex >= rhs.minorIndex);
+		}
+
+		bool operator<(const PointsInRectangleIteratorColMajor &rhs) const
+		{
+			return !(*this >= rhs);
+		}
 
-		int majorDimension;
+		bool operator<=(const PointsInRectangleIteratorColMajor &rhs) const
+		{
+			return this->majorIndex < rhs.majorIndex || (this->majorIndex == rhs.majorIndex && this->minorIndex <= rhs.minorIndex);
+		}
 
-		int majorIndex;
-		int minorIndex;
+		bool operator>(const PointsInRectangleIteratorColMajor &rhs) const
+		{
+			return !(*this <= rhs);
+		}
+
+		difference_type operator-(const PointsInRectangleIteratorColMajor &rhs) const
+		{
+			return (this->majorIndex - rhs.majorIndex) * majorDimension + (this->minorIndex - rhs.minorIndex);
+		}
+
+		// Forward concepts
+		PointsInRectangleIteratorColMajor &operator++()
+		{
+			Increment();
+			return *this;
+		}
+
+		PointsInRectangleIteratorColMajor operator++(int)
+		{
+			auto copy = *this;
+			++(*this);
+			return copy;
+		}
+
+		// Bidirectional concepts
+		PointsInRectangleIteratorColMajor &operator--()
+		{
+			Decrement();
+			return *this;
+		}
+
+		PointsInRectangleIteratorColMajor operator--(int)
+		{
+			auto copy = *this;
+			--(*this);
+			return copy;
+		}
+
+		// Random access concepts
+		PointsInRectangleIteratorColMajor operator+(difference_type delta) const
+		{
+			auto copy = *this;
+			return copy += delta;
+		}
+
+		PointsInRectangleIteratorColMajor &operator+=(difference_type delta)
+		{
+			Offset(delta);
+			return *this;
+		}
+
+		friend PointsInRectangleIteratorColMajor operator+(difference_type delta, const PointsInRectangleIteratorColMajor &it)
+		{
+			return it + delta;
+		}
+
+		PointsInRectangleIteratorColMajor &operator-=(difference_type delta)
+		{
+			return *this += -delta;
+		}
+
+		PointsInRectangleIteratorColMajor operator-(difference_type delta) const
+		{
+			auto copy = *this;
+			return copy -= delta;
+		}
+
+		value_type operator[](difference_type offset) const
+		{
+			return **(this + offset);
+		}
 	};
 
-	PointsInRectangleRange(Rectangle region)
+	// gcc6 needs a defined constructor?
+	PointsInRectangleRangeColMajor(Rectangle region)
 	    : region(region)
 	{
 	}

test/drlg_common_test.cpp

@@ -40,6 +40,34 @@ TEST(DrlgTest, RectangleRangeIterator)
 	EXPECT_EQ(region[2][4], 3) << "Reverse iterators are required";
 	EXPECT_EQ(region[4][2], 7) << "Reverse iterators are required";
 	EXPECT_EQ(region[2][2], 9) << "Reverse iterators are required";
+
+	region = {};
+	counter = 0;
+
+	for (Point position : PointsInRectangleRangeColMajor { topLeftArea }) {
+		region[position.x][position.y] = ++counter;
+	}
+
+	EXPECT_EQ(counter, 9) << "Iterating over a 9 tile range should return exactly 9 points";
+	EXPECT_EQ(region[2][2], 9) << "Iterating over a 9 tile range should return exactly 9 points";
+
+	EXPECT_EQ(region[0][0], 1) << "col-major iterator must use col-major order (where x defines the column, y defines the row)";
+	EXPECT_EQ(region[0][1], 2) << "col-major iterator must use col-major order (where x defines the column, y defines the row)";
+	EXPECT_EQ(region[0][2], 3) << "col-major iterator must use col-major order (where x defines the column, y defines the row)";
+	EXPECT_EQ(region[2][0], 7) << "col-major iterator must use col-major order (where x defines the column, y defines the row)";
+
+	EXPECT_EQ(region[0][3], 0) << "Iterator should not return out of bounds points";
+	EXPECT_EQ(region[3][0], 0) << "Iterator should not return out of bounds points";
+
+	region = {};
+	counter = 0;
+
+	PointsInRectangleRangeColMajor colMajorRange { bottomRightArea };
+	std::for_each(colMajorRange.rbegin(), colMajorRange.rend(), [&region, &counter](Point position) { region[position.x][position.y] = ++counter; });
+	EXPECT_EQ(region[4][4], 1) << "Reverse iterators are required";
+	EXPECT_EQ(region[4][2], 3) << "Reverse iterators are required";
+	EXPECT_EQ(region[2][4], 7) << "Reverse iterators are required";
+	EXPECT_EQ(region[2][2], 9) << "Reverse iterators are required";
 }
 
 } // namespace devilution