-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New Transform type: Homography #11949
Conversation
Add new transform type, Homography. Add functions to compute homography from a list of GCPs. Add functions to serialize and deserialize a homography Automatically select homography transfrom when there are 4 or 5 GCPs present.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like an excellent work, although I'm not familiar with the maths involved
alg/gdal_homography.cpp
Outdated
return FALSE; | ||
} | ||
|
||
GDALMatrix LtL(9, 9); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are there references giving context for all the below maths that could be included as code comments ? / how did you derive that ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LtL ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a link to https://www.cs.unc.edu/~ronisen/teaching/fall_2023/pdf_slides/lecture9_transformation.pdf, and changed LtL
to AtA
to match the notation in that document. Is there a preferred alternative to this kind of link? I can't guarantee how long the link will be available.
alg/gdal_homography.cpp
Outdated
return FALSE; | ||
} | ||
|
||
// "Hour-glass" like shape of GCPs. Cf https://github.com/OSGeo/gdal/issues/11618 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand that comment. Hour-glass like shape is one possibility, not necesarily all the cases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hopefully the new comments clarify things.
} | ||
double cross12 = x[1] * y[2] - x[2] * y[1]; | ||
double cross23 = x[2] * y[3] - x[3] * y[2]; | ||
if (cross12 * cross23 <= 0.0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does this check do ? Should we emit an error when it fails ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check verifies that the normalized transform maps the unit square to a convex quadrilateral.
Regarding emitting an error, I followed the precedent of GDALGCPsToGeoTransform()
, which returns FALSE
without emitting an error in all cases that fail to compute a valid GeoTransform. I can change it to emit errors in those cases if needed.
/* Compose the resulting transformation with the normalization */ | ||
/* homographies. */ | ||
/* -------------------------------------------------------------------- */ | ||
double h1p2[9] = {0.0}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is it named h1p2 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Following the precedent set by gt1p2
in GDALGCPsToGeoTransform()
(gt = geotransform, h = homography).
This is an intermediate homography, which maps between pixels and normalized geo coordinates.
Co-authored-by: Even Rouault <even.rouault@spatialys.com>
Co-authored-by: Even Rouault <even.rouault@spatialys.com>
Co-authored-by: Even Rouault <even.rouault@spatialys.com>
Co-authored-by: Even Rouault <even.rouault@spatialys.com>
Co-authored-by: Even Rouault <even.rouault@spatialys.com>
What does this PR do?
Adds new transform type, Homography.
Adds functions to compute homography from a list of GCPs.
Adds functions to serialize and deserialize a homography
Automatically selects homography transform when there are 4 or 5 GCPs present. Homography provides an exact fit for 4 GCPs, and a better fit for 5 GCPs than the existing method (affine transformation).
What are related issues/pull requests?
#11940
Tasklist
- [ ] AI (Copilot or something similar) supported my development of this PR- [ ] Updated Python API documentation (swig/include/python/docs/)Environment
Provide environment details, if relevant: