Repository navigation
Conversation
FixedCamera and other callers pass a (2,) corner into rel_to_abs; TranslationTransformation already handled that shape via broadcasting, but HomographyTransformation only accepted (n_points, 2) and crashed.
Author
|
Closing this one so it does not sit in the queue. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HomographyTransformation.rel_to_abs/abs_to_relaccept a 1D point of shape(2,), reshape it to(1, 2)for the existing matrix multiply andw=0guard, and return(2,), so the twoCoordinatesTransformationimplementations agree.(n_points, 2)behaviour is unchanged.Fixes #316.
HomographyTransformation.rel_to_abs/abs_to_relpreviously only accepted(n_points, 2). A single point of shape(2,), whichFixedCamera.adjust_framepasses as the paste origin and whichTranslationTransformationalready accepted, raised:ValueError: all the input arrays must have same number of dimensions...That is the default
MotionEstimator()path in the docs example on the issue. Reporter @Dentikka reproduced on Norfair 2.2.0 / Python 3.11; the samehstackfailure reproduces on current master withFixedCamera+ identity homography and with liveMotionEstimator().What I chose and the alternative
Translation already accepted
(2,). The ABC should be interchangeable, and the crashing caller is the default homography estimator. Returning(1, 2)from Homography without also squeezing inFixedCamerawould make the existing[::-1]a no-op (it would reverse the point axis of size 1, not x/y).Alternative: only reshape the corner to
(1, 2)inFixedCamera.adjust_frame.Can switch to the caller-only reshape if Homography stays strictly
(n, 2).FixedCamerastill only pastes a rectangle (no perspective warp). That limitation is unchanged and already documented.Test plan
pytest tests/test_camera_motion.py tests/test_drawing.py: 1D vs 2D homography equivalence, multi-point regression,FixedCamera+ identity homography,FixedCamera+ translationValueErroras FixedCamera crashes with an inappropriate point array shape #316) and pass with it restoredpytest tests/: 30 passeddemos/camera_motionwith--fixed-camera-scaleand default homography; confirm it no longer crashes (frame is still pasted as a rectangle)