Use center of pixel to determine if a pixel is in the ellipse mask

[KRLango]

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#1548

The tests for both of these are now added here and nion-software/nionswift#1548 will be updated to use the new functions.

[cmeyer]

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:

• Add function_make_rectangle_mask to Core.py, which already contains function_make_elliptical_mask
• Add tests to niondata to pixel test those two functions (these can be copied from nionswift)
• Once this PR is finished, remove the duplicated tests from nionswift and change nionswift to use the new function_make_rectangle_mask in niondata.

As a general comment, I try to move all processing to niondata. Eventually we may want to move the Mask-like objects there, e.g. Graphics.SpotMaskItem. But for now, function_make_rectangle_mask needs to go there.

One other note: function_make_elliptical_mask is named differently. Should we name new function function_make_rectangle_mask or function_make_rectangular_mask?

[cmeyer]

See comment above.

[KRLango]

@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.

[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?

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 fn((20,40)) is easier than from nion.utils import Geometry; fn(Geometry.FloatPoint(20,40)), but I could easily be convinced to switch to the more strict typing in both cases - although that will require changes in both niondata and nionswift simultaneously, so it might be better to do this as a separate PR.

[cmeyer]

tl/dr: new function can have strict Geometry.FloatXYZ typing.

[cmeyer]

I didn't review the new PR yet, but I came across this separately:

• Rectangle mask does not draw consistently when partially or entirely out of bounds nionswift#792

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.

[lisham2000]

Reviewed LGTM with the other PR for context

[cmeyer]

This looks good to me.

But... it's marked as draft? Can you mark it as "Ready for Review" and I'll merge?

[Ion-e]

This looks good to me!