find_best_packing: store orders side-by-side in a single vector (#15)

* initial

* remove .cache/

* re-constify rect reference in all_inserted

* properly resize vector every time

* find_best_packing: put order vector on stack instead of TLS

* replace std::span with templated iterators

* remove unused type alias

* fix whitespace

* merge changes

* fix whitespace

* pointers instead of iterators + correctly set vector size

* remove unused typedef

* fix: resize before copy to ensure proper end() pointer

* restore example.cpp

* find_best_packing: return early if there are no subjects

* example: clarify raw arrays can be used

* find_best_packing_dont_sort: fix edge case of empty subjects

* example: clarify necessary STL container functions

* remove extraneous scope

* fix whitespace

* example: restore rectangle sizes

* relocate empty subject list check to find_best_packing_impl

* backtrack certain changes; use unique_ptr for storing rects

* example: document std::size() subject requirement

* use std::pair<iterator, iterator> as a makeshift std::span

* compile fix: missing <memory>

* find_best_packing_impl: use std::optional for best order iterator

* find_best_packing_impl: move both iterators into std::optional

* best_packing_for_ordering(_impl): switch back to generic O ordering

* example: fix whitespace

* find_best_packing: rename count_subjects to count_valid_subjects

* find_best_packing_impl: re-add scope braces

* find_best_packing_impl: remove unnecessary comment

* find_best_packing_impl: alias iterator pair as OrderType

* preserve order_type

* example: fix whitespace

* best_bin_finder: fix whitespace

* finders_interface: fix whitespace

* finders_interface: fix whitespace

* Revert "finders_interface: fix whitespace"

This reverts commit c96e235.

* find_best_packing_impl: const-ify OrderType& in for_each_order lambda

* best_bin_finder: de-format

* Apply suggestion from @geneotech

* Apply suggestion from @geneotech

* Apply suggestion from @geneotech

* comments

* comments

* adjust scope

* emulate span to simplify loops

* more readable loop

* simplify loops with ith_order lambda

* adjust

* adjust

* const

---------

Co-authored-by: geneotech <patryk.czachurski@gmail.com>
Co-authored-by: Patryk Czachurski <patryk.czachurski@softnet.pl>

Author: 3 people

.gitignore

@@ -1,6 +1,7 @@
 /cmake-build-debug/
 .DS_Store
 .idea
+.cache/
 *.iml
 example/clang_commands.sh
 example/main

example/main.cpp

@@ -118,12 +118,15 @@ int main() {
 	/*
 		The example will compile just fine if you use any of these instead:
 
+		my_rect rectangles[] = { ... };
+
 		std::vector<rect_type> rectangles;
 		std::list<rect_type> rectangles;
 
 		std::list<my_rect> rectangles;
 
-		1. The container just needs to be forward-iterable with .begin() and .end().
+		1. The container needs to be forward-iterable with std::begin() and std::end(),
+		   and have its size queryable via std::size().
 		2. The element type just needs to have get_rect() member functions defined (const and non-const).
 	*/
 

src/rectpack2D/best_bin_finder.h

@@ -1,9 +1,26 @@
 #pragma once
 #include <variant>
 #include <cassert>
+#include <algorithm>
 #include "rect_structs.h"
 
 namespace rectpack2D {
+	template <class T>
+	class span {
+		T first;
+		T second;
+	public:
+		span(T first, T second) : first(first), second(second) {}
+
+		T begin() const {
+			return first;
+		}
+
+		T end() const {
+			return second;
+		}
+	};
+
 	enum class callback_result {
 		ABORT_PACKING,
 		CONTINUE_PACKING
@@ -216,7 +233,7 @@ namespace rectpack2D {
 	rect_wh find_best_packing_impl(F for_each_order, const I input) {
 		const auto max_bin = rect_wh(input.max_bin_side, input.max_bin_side);
 
-		OrderType* best_order = nullptr;
+		std::optional<OrderType> best_order;
 
 		int best_total_inserted = -1;
 		auto best_bin = max_bin;
@@ -229,7 +246,7 @@ namespace rectpack2D {
 		thread_local empty_spaces_type root = rect_wh();
 		root.flipping_mode = input.flipping_mode;
 
-		for_each_order ([&](OrderType& current_order) {
+		for_each_order ([&](const OrderType& current_order) {
 			const auto packing = best_packing_for_ordering(
 				root,
 				current_order,
@@ -242,24 +259,24 @@ namespace rectpack2D {
 					Track which function inserts the most area in total,
 					just in case that all orders will fail to fit into the largest allowed bin.
 				*/
-				if (best_order == nullptr) {
+				if (!best_order.has_value()) {
 					if (*total_inserted > best_total_inserted) {
-						best_order = std::addressof(current_order);
+						best_order = current_order;
 						best_total_inserted = *total_inserted;
 					}
 				}
 			}
 			else if (const auto result_bin = std::get_if<rect_wh>(&packing)) {
 				/* Save the function if it performed the best. */
 				if (result_bin->area() <= best_bin.area()) {
-					best_order = std::addressof(current_order);
+					best_order = current_order;
 					best_bin = *result_bin;
 				}
 			}
 		});
 
 		{
-			assert(best_order != nullptr);
+			assert(best_order.has_value());
 
 			root.reset(best_bin);
 

src/rectpack2D/finders_interface.h

@@ -3,6 +3,7 @@
 #include <vector>
 #include <array>
 #include <variant>
+#include <memory>
 #include <algorithm>
 
 #include "insert_and_split.h"
@@ -51,10 +52,12 @@ namespace rectpack2D {
 		Subjects& subjects,
 		const finder_input<F, G>& input
 	) {
-		using order_type = std::remove_reference_t<decltype(subjects)>;
+		// Works with C arrays as well.
+		using iterator_type = decltype(std::begin(subjects));
+		using order_type = rectpack2D::span<iterator_type>;
 
 		return find_best_packing_impl<empty_spaces_type, order_type>(
-			[&subjects](auto callback) { callback(subjects); },
+			[&subjects](auto callback) { callback(order_type(std::begin(subjects), std::end(subjects))); },
 			input
 		);
 	}
@@ -77,43 +80,66 @@ namespace rectpack2D {
 		Comparators... comparators
 	) {
 		using rect_type = output_rect_t<empty_spaces_type>;
-		using order_type = std::vector<rect_type*>;
+		using order_type = rectpack2D::span<rect_type**>;
 
 		constexpr auto count_orders = 1 + sizeof...(Comparators);
-		thread_local std::array<order_type, count_orders> orders;
+		std::size_t count_valid_subjects = 0;
 
-		{
-			/* order[0] will always exist since this overload requires at least one comparator */
-			auto& initial_pointers = orders[0];
-			initial_pointers.clear();
+		// Allocate space assuming no rectangle has an area of zero.
+		// We fill orders with valid rectangles only.
+		auto orders = std::make_unique<rect_type*[]>(count_orders * std::size(subjects));
 
-			for (auto& s : subjects) {
-				auto& r = s.get_rect();
+		for (auto& s : subjects) {
+			auto& r = s.get_rect();
 
-				if (r.area() > 0) {
-					initial_pointers.emplace_back(std::addressof(r));
-				}
+			if (r.area() == 0) {
+				continue;
 			}
 
+			orders[count_valid_subjects++] = std::addressof(r);
+		}
+
+		auto ith_order = [&orders, n = count_valid_subjects](const std::size_t i) {
+			return order_type(
+				orders.get() + i       * n,
+				orders.get() + (i + 1) * n
+			);
+		};
+
+		{
+			/*
+				Zero-th order is already filled.
+				We duplicate it to all other orders.
+			*/
+			const auto first_order = ith_order(0);
+
 			for (std::size_t i = 1; i < count_orders; ++i) {
-				orders[i] = initial_pointers;
+				std::copy(
+					first_order.begin(),
+					first_order.end(),
+					ith_order(i).begin()
+				);
 			}
 		}
 
-		std::size_t f = 0;
-
-		auto& orders_ref = orders;
+		{
+			std::size_t i = 0;
 
-		auto make_order = [&f, &orders_ref](auto& predicate) {
-			std::sort(orders_ref[f].begin(), orders_ref[f].end(), predicate);
-			++f;
-		};
+			auto make_order = [&i, ith_order](auto& predicate) {
+				const auto o = ith_order(i++);
+				std::sort(o.begin(), o.end(), predicate);
+			};
 
-		make_order(comparator);
-		(make_order(comparators), ...);
+			make_order(comparator);
+			(make_order(comparators), ...);
+		}
 
 		return find_best_packing_impl<empty_spaces_type, order_type>(
-			[&orders_ref](auto callback){ for (auto& o : orders_ref) { callback(o); } },
+			[ith_order](auto callback) {
+				for (std::size_t i = 0; i < count_orders; ++i) {
+					callback(ith_order(i));
+				}
+			},
 			input
 		);
 	}