spillerrec / Overmix

Automatic anime screenshot stitching in high quality
http://spillerrec.dk/category/software/programs/overmix/
GNU General Public License v3.0
193 stars 14 forks source link

Investigate using ranged containers and iterators for Plane data #78

Open spillerrec opened 9 years ago

spillerrec commented 9 years ago

Bjarne Stroustrup talk at CppCon 2015 made me reconsider all the pointer array code which Overmix have in a lot of places, especially the huge Plane implementation. The main points being that it is hard to ensure proper bound safety when you cannot ensure you pass the pointer and size information separately.

I have already started with modifying the interface so scan_line() returns a RowIt<T> instead of a T*, but this is far from enough, as the value is passed into other structures a lot of the time and needs to be considered in relation to other rows most of the time.

Containers/iterators:

Bound checking There really needs to be a lot more bounds-checking in the code. A lot of it depends on two input planes being compatible, but does not check if this is the case. Implementing it in the containers might make it easy to do, with very little overhead.

List of functions needing a paired row-iterator:

List of functions needing to access adjacent rows:

Data structures:

SubRanges:

Zipped SubRanges

Other

spillerrec commented 9 years ago

The following code needs to be looked at:

Any code which matches this pattern: scan_line(...).begin() needs to be reviewed and see if it is possible to avoid using the pointer access. Most of those cases should be in the list above though.

Furthermore I also observed that sometimes scan_line() is used against a std::vector instead of another scan_line(), so RowIt<T> should be made more general so it can be used with std::vector (and similar) as well. There is also quite a bit of code which accesses adjacent rows using pointer access, so some solution to this is quite important...

spillerrec commented 9 years ago

Data structures:

SubRanges:

New issue: ZipRowIt doesn't work if one of the rows need to be offset.

Ideas:

spillerrec commented 6 years ago

AContainer is also one we constantly iterate over, and we also iterate over planes. And we constantly assume a lot of the content of a container, like all images have consistent amount of planes...