Use center of pixel to determine if a pixel is in the ellipse mask #67
Conversation
|
Would it be possible to move all masking pixel functions and tests that are currently in nionswift to niondata? That way we update the two algorithms/tests separately from nionswift? Then the update sequence can go like this:
As a general comment, I try to move all processing to One other note: |
6fd51ff to
d572cad
Compare
|
@cmeyer Is there a reason that function_make_elliptical_mask is taking in tuples (albeit in an alias form) rather than Geometry.FloatPoint/Geometry.FloatSize? As far as I can tell all usages of it are currently turning the Geometry classes into tuples just to pass them in. Currently function_make_rectangular_mask uses the Geometry classes since that felt the most natural class to use, but I happy to change it if we want to. |
There were backwards compatibility issues in external code and in tests in other functions and I probably just carried that typing through to the new functions. There is a certain convenience when editing script functions where passing |
|
tl/dr: new function can have strict |
|
I didn't review the new PR yet, but I came across this separately: Unfortunately, I didn't add tests in 942c69a4547a4900eeefc74e9bc55e2f9d396e14 but I think we should add them now, i.e. tests to ensure masks work when out of bounds on all four sides/corners and perhaps even when entirely out of bounds. |
|
This looks good to me. But... it's marked as draft? Can you mark it as "Ready for Review" and I'll merge? |
Part of nion-software/nionswift#1472 - specifically the ellipse and rectangle mask. The centre of the shape has been shifted by 0.5 pixels towards the top right to ensure that the centre of the pixel is the position used to determine if it is in or out of the mask.
This is tested by the graphics tests in nionswift. Those tests have been updated in nion-software/nionswift#1548The tests for both of these are now added here and nion-software/nionswift#1548 will be updated to use the new functions.