Skip to content

Overhaul elliptical distortion correction#2787

Merged
dagewa merged 12 commits intomainfrom
better-elliptical-distortion-test
Nov 6, 2024
Merged

Overhaul elliptical distortion correction#2787
dagewa merged 12 commits intomainfrom
better-elliptical-distortion-test

Conversation

@dagewa
Copy link
Copy Markdown
Member

@dagewa dagewa commented Oct 31, 2024

Following #2745 some bugs were found with the generation of elliptical distortion correction maps. This PR replaces the ellipse_matrix_form function with a pair of functions ellipse_to_circle_transform and circle_to_ellipse_transform, which produce the correct $2 \times 2$ matrices to transform points from an ellipse to a circle and vice versa. Better tests are added, which check the operation of these functions, as well as a "real case" test of correction of an elliptical distortion.

In the course of this, the class CreateEllipticalDistortionMaps was renamed PlaneLinearTransformationMaps, reflecting the fact that it can be used to produce correction maps for any distortion represented by a $2 \times 2$ matrix, not just elliptical distortion.

@dagewa dagewa merged commit 3eb01cd into main Nov 6, 2024
@dagewa dagewa deleted the better-elliptical-distortion-test branch November 6, 2024 13:36
graeme-winter pushed a commit that referenced this pull request Nov 19, 2024
* Add ellipse_to_circle_transform and circle_to_ellipse_transform

* Rename CreateEllipticalDistortionMaps --> PlaneLinearTransformationMaps
to accurately reflect the fact that this class calculates maps for
any linear transform represented by matrix M, not just an ellipse.

* Remove no-longer-needed ellipse_matrix_form function

* Correct the definition of the distortion matrices when l1, l2 are
linear scale factors for the lengths of ellipse axes

* Add tests for the circle<-->ellipse tranform functions and a full test
of undistorting an elliptical pattern with a 5% scale factor along one
axis. Errors < 0.5 pixels for that test.

* Improve docstrings

* Resolve an old TODO in test_translate
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants