ip additions
Added a resizeCopy version for Channels. concerning Flip: Added flipHorizontal for channels and also added code for flipping surfaces and channels diagonally (both horizontally and vertically), it’s simply called flipDiagonal() but could change to flip() if Cinder’s maintainers think it sounds better
Just noticed that I haven't followed the style guide about spaces and braces. Also need to sumbit a test project, let me do that and comeback with an updated PR. thanks!
I think this is good to go now. with these additions ci::ip::flip now fully corresponds to all the modes cv::flip has.
Thanks a lot for these contributions - they all look useful and we could probably fast-track their merge. A couple of high-level thoughts ahead of that:
- I think as written
flipVertical()would only work with planar channels. I'd keep the current implementation for the planar case, and then we'll need a manual iteration for non-planar. - My vote would be
rotate180()rather thanflipDiagonal() - We've deprecated Boost, so let's keep the manual template instantiation rather than using Boost.Preprocessor
- Rather than a visual test, I'd request additions to
test/Unit, which we're doing our best to add to going forward. Would love to see correctness verified programmatically - to my mind the size of the images doesn't matter too much, but testing a few cases (e.g. one pixel, even & odd on both axes) would be really useful for code like this. Specifically, I'd just algorithmically generateSurface8uwith sequential values which can be verified with a for-loop.
Thanks again for your work on this - if you're willing to make those modifications I'll do my best to get this tested and merged.
Hi Andrew! Thanks for your quick reply.
We've deprecated Boost, so let's keep the manual template instantiation rather than using Boost.Preprocessor
Sorry about that, I had read the discourse post about depreciation of Boost but it somehow slipped my mind. Also it seems somehow the original file I worked on didn't have lithium-snepo's latest commit. Will do as you said here.
My vote would be rotate180() rather than flipDiagonal()
I like rotate180, the only problem I see with it is its difference in choice of words from the other flip methods. I mean as a user if I had seen flipVertical() in Cinder before I would never think that a rotate would also exist, I'd be looking for everything Flip. I'm also up for flip(): if it's both flipHorizontal and flipVertical then it's flip :) In the end any decision is fine with me.
Rather than a visual test, I'd request additions to test/Unit, which we're doing our best to add to going forward. Would love to see correctness verified programmatically - to my mind the size of the images doesn't matter too much, but testing a few cases (e.g. one pixel, even & odd on both axes) would be really useful for code like this. Specifically, I'd just algorithmically generate Surface8u with sequential values which can be verified with a for-loop.
That sounds great and more accurate. Actually I already had problems with odd numbers when it came to dimensions and to be honest it was out of luck that I found out about that, so a programmatic verification is definitely the way to go. Also good idea about generating them in code.
I think as written flipVertical() would only work with planar channels. I'd keep the current implementation for the planar case, and then we'll need a manual iteration for non-planar.
Again in my visual test that I added to the tests folder, this code worked for both planar and non-planar, but I'd have to check again with the test/unit method.
A project at our studio got forwarded out of the blue and I suddenly got very busy for the next couple of days (forgetting the hurry it's always fun to work on a Cinder project ;) ) so I think I'll take a stab at this in a week or so. Hopefully we can wrap it then :) Cheers!