search

microsoft / psi

Platform for Situated Intelligence
https://github.com/microsoft/psi/wiki
Other
538 stars 96 forks source link

add additional image drawing methods and operators #130

Closed tteichmeister closed 3 years ago

tteichmeister commented 3 years ago

Hi,

It would be helpful to provide additional drawing functions for the image. Especially to fill a rectangle or a circle. Currently it is only possible to draw a rectangle/circle but not to fill it.

Furthermore, an additional function to draw a text with a background color would be good. Especially if you want to label objects and other stuff, with a background color as a contrast, it can help to improve the readability of the text.

(Here I marked the object with DrawRectangle and using the new DrawText method to draw the text below) image

Therefore I provided 3 methods:

I also including the operators which are also included for the existing DrawText, DrawRectangle...

ghost commented 3 years ago

CLA assistant check
All CLA requirements met.

tteichmeister commented 3 years ago

Thank you for submitting this PR - just a couple of tiny changes but otherwise everything looks great! Thanks also for including unit tests :-). For some reason there is one test (Image_GrayDrawTestWithBackground) that fails the CI build due to too many different pixels vs the reference image. I wonder if that test passes for you at all? We've sometimes noticed in the past that some of these imaging tests can fail on some machines but not others, possibly due to differences in GDI+ versions. There are tolerance and outlier parameters in the AssertAreImagesEqual method for that reason, but normally the default tolerances have worked for us.

@chitsaw It's strange, because before I submitted it, the Test was working on my machine. When I was doing it now again, this test didn't passed also on my machine. So I provided a new verification image for this test. I think I uploaded the wrong one. Since the checks passed now :-)

Thank you for your review and to let me contribute this to your project :-).

chitsaw commented 3 years ago

@tteichmeister thanks for making those changes. Yes, the tests pass now with the new image. Thanks so much for your contributions!