imageproc
imageproc copied to clipboard
image-rs
Modified Template Matching / Integral Image to use Image instead of GrayImage
I decided to add a pull request to make it easier to see the changes discussed in #312.
I had to change some API to make this work, of which I don't know whether it would be approved. Specifically the sum_image_pixels in integral_image. The issue is that when changing to use GenericImageView, the compiler didn't like accessing the returned array stating it could index into the variable. I changed (at least temporarily) to return a Pixel rather than the array.
While I don't expect this specific change to be accepted, if there are any suggestions as to how best to deal with it it would be appreciated. I also still need to write tests for multi-channel images.
Do you need the ability to support any GenericImageView here, or could you get away with just implementing this for Image of T for T: Pixel + 'static? The latter is obviously less flexible, but makes the code a lot simpler. And potentially allows for faster code (several functions in this library started life operating on any generic image and were then switched to requiring a fixed image type to make them simpler and faster).
I was originally trying to work with sub-images; however, perhaps as you say it would be simpler to do with expanding GreyImage to Image/ImageBuffer. I'll get this done soon and send a new pull request.
Thanks for your advice.
I understand this isn't mergable yet, but thought it best way to ask any thoughts. I reverted integral_image and with some minor changes to turn things into Image, but I still have the same issue with ArrayData::Datatype. Do you have any thoughts on this @theotherphil ?
I didn’t get around to looking at this after all. I’m definitely interested in having this functionality in the library, but have very little time available for working on this crate at the moment so I’m afraid I’m not going to do a great job of responding to questions or PRs that require much effort from me.
I understand, thank you for responding!
I've been thinking about it as well, and I think one good way to go about this if concerned about api changes is to have sum_image_pixels return a vec of the data rather than a straight array. While this would be a slight change, I think it would be quite minimal in that I don't think it would conflict with anyone's code bases. I'll implement a version of this and push to this Pull Request, then hopefully it will be a lot less time on your part to see if you're ok with the change and merge.
I have it all implemented I think, I changed a number of traits to be based on the Num crate as it made the casting a whole lot easier. I notice out of the CI build that beta/nightly have succeeded, but not stable, although I can't see any of the issues on the logs of the stable.
I've added in tests as well, and fixed the documentation of the sum_image_pixels function for the new output. Hopefully it's in good condition for you to accept, but let me know if it isn't.
I would imagine as this PR was from over 2 years ago, that it might need to be updated. I'll try and have a look over the next few days to see.
Since this PR is quite old now and when I attempted to rebase it I got quite a few conflicts, would it be a good idea to close it? I think closing is the better alternative to simply leaving it open for many more years which makes it harder to focus on new PRs due to buildup.
