Skip to content

Commit

Permalink
Make all Transform methods of OGRCoordinateTransformation and GDALTra…
Browse files Browse the repository at this point in the history 
…nsformerFunc return FALSE as soon as one point fails to transform

Fixes OSGeo#11817

Kind of a breaking change, but the current behavior was highly
inconsistent and hard to reason about.

New paragraph in MIGRATION_GUIDE.TXT:

- The following methods
  OGRCoordinateTransformation::Transform(size_t nCount, double *x, double *y,
  double *z, double *t, int *pabSuccess) and
  OGRCoordinateTransformation::TransformWithErrorCodes(size_t nCount, double *x,
  double *y, double *z, double *t, int *panErrorCodes) are modified to return
  FALSE as soon as at least one point fails to transform (to be consistent with
  the other form of Transform() that doesn't take a "t" argument), whereas
  previously they would return FALSE only if no transformation was found.

  The GDALTransformerFunc callback and its implementations (GenImgProjTransformer,
  RPCTransformer, etc.) are also modified to return FALSE as soon as at least
  one point fails to transform.
  • Loading branch information
@rouault
rouault committed Feb 7, 2025
1 parent 8d8fb57 commit 0ecf5bd
Show file tree
Hide file tree
Showing 11 changed files with 198 additions and 188 deletions.
13 changes: 13 additions & 0 deletions MIGRATION_GUIDE.TXT
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,19 @@ MIGRATION GUIDE FROM GDAL 3.10 to GDAL 3.11
- If only a specific GDAL Minor version is to be supported, this must now be
specified in the find_package call in CMake via a version range specification.

- The following methods
OGRCoordinateTransformation::Transform(size_t nCount, double *x, double *y,
double *z, double *t, int *pabSuccess) and
OGRCoordinateTransformation::TransformWithErrorCodes(size_t nCount, double *x,
double *y, double *z, double *t, int *panErrorCodes) are modified to return
FALSE as soon as at least one point fails to transform (to be consistent with
the other form of Transform() that doesn't take a "t" argument), whereas
previously they would return FALSE only if no transformation was found.

The GDALTransformerFunc callback and its implementations (GenImgProjTransformer,
RPCTransformer, etc.) are also modified to return FALSE as soon as at least
one point fails to transform.

MIGRATION GUIDE FROM GDAL 3.9 to GDAL 3.10
------------------------------------------

Expand Down
16 changes: 16 additions & 0 deletions alg/gdal_alg.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,22 @@ CPLErr CPL_DLL CPL_STDCALL GDALSieveFilter(
* Warp Related.
*/

/**
* Callback to transforms points.
*
* @param pTransformerArg return value from a GDALCreateXXXXTransformer() function
* @param bDstToSrc TRUE if transformation is from the destination
* (georeferenced) coordinates to pixel/line or FALSE when transforming
* from pixel/line to georeferenced coordinates.
* @param nPointCount the number of values in the x, y and z arrays.
* @param[in,out] x array containing the X values to be transformed. Must not be NULL.
* @param[in,out] y array containing the Y values to be transformed. Must not be NULL.
* @param[in,out] z array containing the Z values to be transformed. Must not be NULL.
* @param[out] panSuccess array in which a flag indicating success (TRUE) or
* failure (FALSE) of the transformation are placed. Must not be NULL.
*
* @return TRUE if all points have been successfully transformed.
*/
typedef int (*GDALTransformerFunc)(void *pTransformerArg, int bDstToSrc,
int nPointCount, double *x, double *y,
double *z, int *panSuccess);
Expand Down
6 changes: 4 additions & 2 deletions alg/gdal_crs.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ void GDALDestroyGCPTransformer(void *pTransformArg)
* @param panSuccess array in which a flag indicating success (TRUE) or
* failure (FALSE) of the transformation are placed.
*
* @return TRUE.
* @return TRUE if all points have been successfully transformed.
*/

int GDALGCPTransform(void *pTransformArg, int bDstToSrc, int nPointCount,
Expand All @@ -436,10 +436,12 @@ int GDALGCPTransform(void *pTransformArg, int bDstToSrc, int nPointCount,
if (psInfo->bReversed)
bDstToSrc = !bDstToSrc;

int bRet = TRUE;
for (i = 0; i < nPointCount; i++)
{
if (x[i] == HUGE_VAL || y[i] == HUGE_VAL)
{
bRet = FALSE;
panSuccess[i] = FALSE;
continue;
}
Expand All @@ -459,7 +461,7 @@ int GDALGCPTransform(void *pTransformArg, int bDstToSrc, int nPointCount,
panSuccess[i] = TRUE;
}

return TRUE;
return bRet;
}

/************************************************************************/
Expand Down
25 changes: 22 additions & 3 deletions alg/gdal_rpc.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1449,10 +1449,15 @@ GDALRPCTransformWholeLineWithDEM(const GDALRPCTransformInfo *psTransform,
const int nY = static_cast<int>(dfY);
const double dfDeltaY = dfY - nY;

int bRet = TRUE;
for (int i = 0; i < nPointCount; i++)
{
if (padfX[i] == HUGE_VAL)
{
bRet = FALSE;
panSuccess[i] = FALSE;
continue;
}

double dfDEMH = 0.0;
const double dfZ_i = padfZ ? padfZ[i] : 0.0;
Expand Down Expand Up @@ -1502,6 +1507,7 @@ GDALRPCTransformWholeLineWithDEM(const GDALRPCTransformInfo *psTransform,
dfDEMH = psTransform->dfDEMMissingValue;
else
{
bRet = FALSE;
panSuccess[i] = FALSE;
continue;
}
Expand Down Expand Up @@ -1546,6 +1552,7 @@ GDALRPCTransformWholeLineWithDEM(const GDALRPCTransformInfo *psTransform,
{
if (!RPCIsValidLongLat(psTransform, padfX[i], padfY[i]))
{
bRet = FALSE;
panSuccess[i] = FALSE;
padfX[i] = HUGE_VAL;
padfY[i] = HUGE_VAL;
Expand All @@ -1565,6 +1572,7 @@ GDALRPCTransformWholeLineWithDEM(const GDALRPCTransformInfo *psTransform,
{
if (!RPCIsValidLongLat(psTransform, padfX[i], padfY[i]))
{
bRet = FALSE;
panSuccess[i] = FALSE;
padfX[i] = HUGE_VAL;
padfY[i] = HUGE_VAL;
Expand All @@ -1582,6 +1590,7 @@ GDALRPCTransformWholeLineWithDEM(const GDALRPCTransformInfo *psTransform,
}
else
{
bRet = FALSE;
panSuccess[i] = FALSE;
padfX[i] = HUGE_VAL;
padfY[i] = HUGE_VAL;
Expand Down Expand Up @@ -1613,6 +1622,7 @@ GDALRPCTransformWholeLineWithDEM(const GDALRPCTransformInfo *psTransform,
dfDEMH = psTransform->dfDEMMissingValue;
else
{
bRet = FALSE;
panSuccess[i] = FALSE;
padfX[i] = HUGE_VAL;
padfY[i] = HUGE_VAL;
Expand All @@ -1623,6 +1633,7 @@ GDALRPCTransformWholeLineWithDEM(const GDALRPCTransformInfo *psTransform,

if (!RPCIsValidLongLat(psTransform, padfX[i], padfY[i]))
{
bRet = FALSE;
panSuccess[i] = FALSE;
padfX[i] = HUGE_VAL;
padfY[i] = HUGE_VAL;
Expand All @@ -1638,7 +1649,7 @@ GDALRPCTransformWholeLineWithDEM(const GDALRPCTransformInfo *psTransform,

VSIFree(padfDEMBuffer);

return TRUE;
return bRet;
}

/************************************************************************/
Expand Down Expand Up @@ -1900,10 +1911,12 @@ int GDALRPCTransform(void *pTransformArg, int bDstToSrc, int nPointCount,
}
}

int bRet = TRUE;
for (int i = 0; i < nPointCount; i++)
{
if (!RPCIsValidLongLat(psTransform, padfX[i], padfY[i]))
{
bRet = FALSE;
panSuccess[i] = FALSE;
padfX[i] = HUGE_VAL;
padfY[i] = HUGE_VAL;
Expand All @@ -1913,6 +1926,7 @@ int GDALRPCTransform(void *pTransformArg, int bDstToSrc, int nPointCount,
if (!GDALRPCGetHeightAtLongLat(psTransform, padfX[i], padfY[i],
&dfHeight))
{
bRet = FALSE;
panSuccess[i] = FALSE;
padfX[i] = HUGE_VAL;
padfY[i] = HUGE_VAL;
Expand All @@ -1925,13 +1939,15 @@ int GDALRPCTransform(void *pTransformArg, int bDstToSrc, int nPointCount,
panSuccess[i] = TRUE;
}

return TRUE;
return bRet;
}

if (padfZ == nullptr)
{
CPLError(CE_Failure, CPLE_NotSupported,
"Z array should be provided for reverse RPC computation");
for (int i = 0; i < nPointCount; i++)
panSuccess[i] = FALSE;
return FALSE;
}

Expand All @@ -1940,6 +1956,7 @@ int GDALRPCTransform(void *pTransformArg, int bDstToSrc, int nPointCount,
/* function uses an iterative method from an initial linear */
/* approximation. */
/* -------------------------------------------------------------------- */
int bRet = TRUE;
for (int i = 0; i < nPointCount; i++)
{
double dfResultX = 0.0;
Expand All @@ -1948,13 +1965,15 @@ int GDALRPCTransform(void *pTransformArg, int bDstToSrc, int nPointCount,
if (!RPCInverseTransformPoint(psTransform, padfX[i], padfY[i], padfZ[i],
&dfResultX, &dfResultY))
{
bRet = FALSE;
panSuccess[i] = FALSE;
padfX[i] = HUGE_VAL;
padfY[i] = HUGE_VAL;
continue;
}
if (!RPCIsValidLongLat(psTransform, padfX[i], padfY[i]))
{
bRet = FALSE;
panSuccess[i] = FALSE;
padfX[i] = HUGE_VAL;
padfY[i] = HUGE_VAL;
Expand All @@ -1967,7 +1986,7 @@ int GDALRPCTransform(void *pTransformArg, int bDstToSrc, int nPointCount,
panSuccess[i] = TRUE;
}

return TRUE;
return bRet;
}

/************************************************************************/
Expand Down
2 changes: 1 addition & 1 deletion alg/gdal_tps.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,7 @@ void GDALDestroyTPSTransformer(void *pTransformArg)
* @param panSuccess array in which a flag indicating success (TRUE) or
* failure (FALSE) of the transformation are placed.
*
* @return TRUE.
* @return TRUE if all points have been successfully transformed.
*/

int GDALTPSTransform(void *pTransformArg, int bDstToSrc, int nPointCount,
Expand Down
9 changes: 8 additions & 1 deletion alg/gdalgeoloc.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,7 @@ int GDALGeoLoc<Accessors>::Transform(void *pTransformArg, int bDstToSrc,
double *padfY, double * /* padfZ */,
int *panSuccess)
{
int bSuccess = TRUE;
GDALGeoLocTransformInfo *psTransform =
static_cast<GDALGeoLocTransformInfo *>(pTransformArg);

Expand All @@ -607,6 +608,7 @@ int GDALGeoLoc<Accessors>::Transform(void *pTransformArg, int bDstToSrc,
{
if (padfX[i] == HUGE_VAL || padfY[i] == HUGE_VAL)
{
bSuccess = FALSE;
panSuccess[i] = FALSE;
continue;
}
Expand All @@ -623,6 +625,7 @@ int GDALGeoLoc<Accessors>::Transform(void *pTransformArg, int bDstToSrc,
if (!PixelLineToXY(psTransform, dfGeoLocPixel, dfGeoLocLine,
padfX[i], padfY[i]))
{
bSuccess = FALSE;
panSuccess[i] = FALSE;
padfX[i] = HUGE_VAL;
padfY[i] = HUGE_VAL;
Expand Down Expand Up @@ -665,6 +668,7 @@ int GDALGeoLoc<Accessors>::Transform(void *pTransformArg, int bDstToSrc,
{
if (padfX[i] == HUGE_VAL || padfY[i] == HUGE_VAL)
{
bSuccess = FALSE;
panSuccess[i] = FALSE;
continue;
}
Expand All @@ -688,6 +692,7 @@ int GDALGeoLoc<Accessors>::Transform(void *pTransformArg, int bDstToSrc,
dfBMX + 1 < psTransform->nBackMapWidth &&
dfBMY + 1 < psTransform->nBackMapHeight))
{
bSuccess = FALSE;
panSuccess[i] = FALSE;
padfX[i] = HUGE_VAL;
padfY[i] = HUGE_VAL;
Expand All @@ -701,6 +706,7 @@ int GDALGeoLoc<Accessors>::Transform(void *pTransformArg, int bDstToSrc,
const auto fBMY_0_0 = pAccessors->backMapYAccessor.Get(iBMX, iBMY);
if (fBMX_0_0 == INVALID_BMXY)
{
bSuccess = FALSE;
panSuccess[i] = FALSE;
padfX[i] = HUGE_VAL;
padfY[i] = HUGE_VAL;
Expand Down Expand Up @@ -914,6 +920,7 @@ int GDALGeoLoc<Accessors>::Transform(void *pTransformArg, int bDstToSrc,
}
if (!bDone)
{
bSuccess = FALSE;
panSuccess[i] = FALSE;
padfX[i] = HUGE_VAL;
padfY[i] = HUGE_VAL;
Expand All @@ -924,7 +931,7 @@ int GDALGeoLoc<Accessors>::Transform(void *pTransformArg, int bDstToSrc,
}
}

return TRUE;
return bSuccess;
}

/*! @endcond */
Expand Down
Loading

0 comments on commit 0ecf5bd

Please sign in to comment.