find_best_packing: store orders side-by-side in a single vector

[lyorig]

This is a simple edit to use a single std::vector for storing all orders right next to each other (which works well because all orders have the same known size), and manipulate individual orders in various algorithm functions with std::span. Hopefully, this'll both reduce heap allocations and benefit cache usage. The example provided with the library compiles and produces the same results as the current master branch.

The only drawback is that the usage of std::span imposes a C++20 requirement on library consumers—which I fully understand if that's a no-go.

[geneotech]

Hey, cool idea with reducing the number of allocations!

It does seem like an overkill to bump the C++ requirement for just std::span. Could we just pass templated It from, It to iterators instead or a It from, std::size_t count (we can then save only from to keep track of the best one)?

Sorry for the late response - I was overloaded with work.

[lyorig]

Yeah, that's a good idea. Working on that right now—just gotta read through a few pages' worth of template errors :D

[lyorig]

Should be all right now.

[lyorig]

Not sure whether you're notified of new commits, so I'll leave this comment here just in case to say that I've got another proposed fix for the issues you've mentioned. No rush though! 😎

[geneotech]

Not sure whether you're notified of new commits, so I'll leave this comment here just in case to say that I've got another proposed fix for the issues you've mentioned. No rush though! 😎

The segmentation fault is still unresolved - as explained earlier, the problem is with this line:

for (auto it = orders.begin() + count_subjects; it != orders.end(); it += count_subjects) {

Don't get me wrong, but... do you perhaps run your code before announcing it as ready? 😅 If there is a zero-sized rectangle in example.cpp, then even if it does not segfault, it will enter an infinite loop every time - it's not a matter of luck.

[lyorig]

Not sure whether you're notified of new commits, so I'll leave this comment here just in case to say that I've got another proposed fix for the issues you've mentioned. No rush though! 😎

The segmentation fault is still unresolved - as explained earlier, the problem is with this line:

for (auto it = orders.begin() + count_subjects; it != orders.end(); it += count_subjects) {

Don't get me wrong, but... do you perhaps run your code before announcing it as ready? 😅 If there is a zero-sized rectangle in example.cpp, then even if it does not segfault, it will enter an infinite loop every time - it's not a matter of luck.

Sorry, accidentally skipped over this earlier. And, well, you're right, I'm often too trigger happy when coding my own projects and don't have much experience with PRs. I'll write a few tests for myself and be a bit more diligent.

[lyorig]

I went ahead and looked for any other whitespace anomalies—there should be none left.

[geneotech]

I'll take a look tomorrow, take your time. Thank you!

[geneotech]

Okay, now the PR is a pleasure to look at. I made some finishing adjutments.

[lyorig]

LGTM 🫡

[geneotech]

Yes, I think your job here is done, I'll give it a test run in Hypersomnia just to make sure and merge in the evening.

Thank you for your determination. Hopefully you weren't discouraged by my strictness, as this library has been basically picked up by a few multi-billion dollar companies, hence becoming somewhat of my personal showpiece, so I wish to hold it to the highest standard (though admittedly, there's a bit of tidying up of my own code that I need to do). It is gladdening to see the younger generation accept feedback so readily. All the best!

[lyorig]

I totally understand why you'd want to keep this project "sanitized", especially given the reasons you outlined. Not that my initial YOLO mindset contributed in any way, but you live and you learn, right?

Anyways, I'd like to thank you as well for both your non-consensual mentorship during the whole review process, and the project itself; I stumbled upon it when I was burnt out with my own 2D game project, and it let me focus on other areas of development. Funnily enough, the idea for this optimization came about when rewriting the project in Rust as part of a larger rewrite I'm doing as a learning exercise, since that language has first-class support for spans (or slices, I think they're called).